Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(41)

Issue 103900045: Add support for Availability Zones

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 11 months ago by axw
Modified:
5 years, 11 months ago
Reviewers:
mp+222275, dfc, gz
Visibility:
Public.

Description

Add support for Availability Zones - Added ListAvailabilityZones to Nova client - Added AvailabilityZone field to RunServerOpts - Added AvailabilityZone field to ServerDetail - Updated nova test-service to support all of the above Availability zones are an OpenStack extension, so I've made it so that ListAvailabilityZones will ignore any 404 to the os-availability-zone URL. https://code.launchpad.net/~axwalk/goose/nova-availability-zones/+merge/222275 (do not edit description out of merge proposal)

Patch Set 1 #

Total comments: 6

Patch Set 2 : Add support for Availability Zones #

Total comments: 5

Patch Set 3 : Add support for Availability Zones #

Unified diffs Side-by-side diffs Delta from patch set Stats (+303 lines, -70 lines) Patch
A [revision details] View 1 2 1 chunk +2 lines, -0 lines 0 comments Download
M errors/errors.go View 1 2 6 chunks +22 lines, -6 lines 0 comments Download
M errors/errors_test.go View 1 2 1 chunk +14 lines, -0 lines 0 comments Download
M nova/live_test.go View 8 chunks +37 lines, -20 lines 0 comments Download
M nova/local_test.go View 1 2 1 chunk +45 lines, -0 lines 0 comments Download
M nova/nova.go View 1 2 4 chunks +45 lines, -6 lines 0 comments Download
M testservices/errors.go View 1 2 1 chunk +4 lines, -0 lines 0 comments Download
M testservices/novaservice/service.go View 1 5 chunks +62 lines, -20 lines 0 comments Download
M testservices/novaservice/service_http.go View 1 2 5 chunks +47 lines, -18 lines 0 comments Download
M testservices/novaservice/service_http_test.go View 1 chunk +21 lines, -0 lines 0 comments Download
M testservices/service.go View 1 2 1 chunk +4 lines, -0 lines 0 comments Download

Messages

Total messages: 8
axw
Please take a look.
5 years, 11 months ago (2014-06-06 04:14:27 UTC) #1
dfc
https://codereview.appspot.com/103900045/diff/1/nova/local_test.go File nova/local_test.go (right): https://codereview.appspot.com/103900045/diff/1/nova/local_test.go#newcode251 nova/local_test.go:251: c.Assert(err, IsNil) Are these zones guaranteed to be returned ...
5 years, 11 months ago (2014-06-06 04:33:17 UTC) #2
axw
Please take a look. https://codereview.appspot.com/103900045/diff/1/nova/local_test.go File nova/local_test.go (right): https://codereview.appspot.com/103900045/diff/1/nova/local_test.go#newcode251 nova/local_test.go:251: c.Assert(err, IsNil) On 2014/06/06 04:33:17, ...
5 years, 11 months ago (2014-06-06 05:09:32 UTC) #3
gz
Do any of our openstack clouds actually support this extension? Neither canonistack nor HP seem ...
5 years, 11 months ago (2014-06-07 14:54:10 UTC) #4
axw
On 2014/06/07 14:54:10, gz wrote: > Do any of our openstack clouds actually support this ...
5 years, 11 months ago (2014-06-07 14:55:27 UTC) #5
gz
Ah, interesting. I wasn't on their 12.12 deployment (which is being retired Monday), so must ...
5 years, 11 months ago (2014-06-07 15:54:24 UTC) #6
gz
LGTM. https://codereview.appspot.com/103900045/diff/20001/nova/local_test.go File nova/local_test.go (right): https://codereview.appspot.com/103900045/diff/20001/nova/local_test.go#newcode231 nova/local_test.go:231: s.openstack.Nova.SetAvailabilityZones() This is pretty magical test server behaviour, ...
5 years, 11 months ago (2014-06-07 17:13:03 UTC) #7
axw
5 years, 11 months ago (2014-06-09 01:37:27 UTC) #8
Please take a look.

https://codereview.appspot.com/103900045/diff/20001/nova/nova.go
File nova/nova.go (right):

https://codereview.appspot.com/103900045/diff/20001/nova/nova.go#newcode695
nova/nova.go:695: return nil, nil
On 2014/06/07 17:13:03, gz wrote:
> I'd prefer we return a specific error that can be checked, but that's not
really
> a job for this proposal, we need better goose-wide handling of extensions and
> errors.

I think you're right, and I'd prefer to get the API right to begin with. The
error type/method of checking may change later, but at least we should start out
with propagating the error.

I've created a error code in goose/errors for "not implemented".

https://codereview.appspot.com/103900045/diff/20001/testservices/novaservice/...
File testservices/novaservice/service_http.go (right):

https://codereview.appspot.com/103900045/diff/20001/testservices/novaservice/...
testservices/novaservice/service_http.go:228: errAvailabilityZoneIsNotAvailable
= &errorResponse{
On 2014/06/07 17:13:03, gz wrote:
> Rather than doing this the old way, you should use the new
> testservices/errors.go method of creating the error.

Done.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b