Merge lp://staging/~axwalk/goose/nova-availability-zones into lp://staging/goose

Reviewers: mp+222275_code.launchpad.net,

Message:
Please take a look.

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)

Please review this at https://codereview.appspot.com/103900045/

Affected files (+260, -64 lines):
   A [revision details]
   M nova/live_test.go
   M nova/local_test.go
   M nova/nova.go
   M testservices/novaservice/service.go
   M testservices/novaservice/service_http.go
   M testservices/novaservice/service_http_test.go

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 in order? Should this be
jc.SameContents

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

https://codereview.appspot.com/103900045/diff/1/testservices/novaservice/service.go#newcode826
testservices/novaservice/service.go:826: sort.Sort(azByName(zones))
Please document the order of az's is stored.

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

https://codereview.appspot.com/103900045/diff/1/testservices/novaservice/service_http_test.go#newcode1196
testservices/novaservice/service_http_test.go:1196:
c.Assert(expected.Zones, DeepEquals, zones)
jc.SameContents

https://codereview.appspot.com/103900045/

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, dfc wrote:
> Are these zones guaranteed to be returned in order? Should this be
> jc.SameContents

Yes, they are sorted by the server.

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

https://codereview.appspot.com/103900045/diff/1/testservices/novaservice/service.go#newcode826
testservices/novaservice/service.go:826: sort.Sort(azByName(zones))
On 2014/06/06 04:33:17, dfc wrote:
> Please document the order of az's is stored.

Added to the doc comment of this function. There's no user-facing place
to add a comment.

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

https://codereview.appspot.com/103900045/diff/1/testservices/novaservice/service_http_test.go#newcode1196
testservices/novaservice/service_http_test.go:1196:
c.Assert(expected.Zones, DeepEquals, zones)
On 2014/06/06 04:33:17, dfc wrote:
> jc.SameContents

They are sorted by the server. No need to introduce a dependency on
juju/testing/checkers here.

https://codereview.appspot.com/103900045/

Do any of our openstack clouds actually support this extension? Neither
canonistack nor HP seem to. I'm not totally up to dat on current plans,
but this seemed to be a dead end a year ago, with different mechanisms
talked about as fulfilling the role that weren't availabilty zone based.

https://codereview.appspot.com/103900045/

On 2014/06/07 14:54:10, gz wrote:
> Do any of our openstack clouds actually support this extension?
Neither
> canonistack nor HP seem to. I'm not totally up to dat on current
plans, but this
> seemed to be a dead end a year ago, with different mechanisms talked
about as
> fulfilling the role that weren't availabilty zone based.

HP's US West has 4 availability zones. I've tested this live.

https://codereview.appspot.com/103900045/

Ah, interesting. I wasn't on their 12.12 deployment (which is being
retired Monday), so must have been added for the 13.5 one.

https://codereview.appspot.com/103900045/

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, but you have a nice big
comment at least.

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
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.

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

https://codereview.appspot.com/103900045/diff/20001/testservices/novaservice/service_http.go#newcode228
testservices/novaservice/service_http.go:228:
errAvailabilityZoneIsNotAvailable = &errorResponse{
Rather than doing this the old way, you should use the new
testservices/errors.go method of creating the error.

https://codereview.appspot.com/103900045/

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/service_http.go
File testservices/novaservice/service_http.go (right):

https://codereview.appspot.com/103900045/diff/20001/testservices/novaservice/service_http.go#newcode228
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.

https://codereview.appspot.com/103900045/