Address review comments

- Return an error if AZ extension is not supported
- Use new testservices/errors

errors/errors.go

@@ -15,6 +15,7 @@ const (
 	DuplicateValueError = Code("DuplicateValue")
 	TimeoutError        = Code("Timeout")
 	UnauthorisedError   = Code("Unauthorised")
+	NotImplementedError = Code("NotImplemented")
 )
 
 // Error instances store an optional error cause.
@@ -95,7 +96,14 @@ func IsUnauthorised(err error) bool {
 	return false
 }
 
-// New creates a new Error instance with the specified cause.
+func IsNotImplemented(err error) bool {
+	if e, ok := err.(*gooseError); ok {
+		return e.causedBy(NotImplementedError)
+	}
+	return false
+}
+
+// makeErrorf creates a new Error instance with the specified cause.
 func makeErrorf(code Code, cause error, format string, args ...interface{}) Error {
 	return &gooseError{
 		errcode: code,
@@ -104,39 +112,47 @@ func makeErrorf(code Code, cause error, format string, args ...interface{}) Erro
 	}
 }
 
-// New creates a new Unspecified Error instance with the specified cause.
+// Newf creates a new Unspecified Error instance with the specified cause.
 func Newf(cause error, format string, args ...interface{}) Error {
 	return makeErrorf(UnspecifiedError, cause, format, args...)
 }
 
-// New creates a new NotFound Error instance with the specified cause.
+// NewNotFoundf creates a new NotFound Error instance with the specified cause.
 func NewNotFoundf(cause error, context interface{}, format string, args ...interface{}) Error {
 	if format == "" {
 		format = fmt.Sprintf("Not found: %s", context)
 	}
 	return makeErrorf(NotFoundError, cause, format, args...)
 }
 
-// New creates a new DuplicateValue Error instance with the specified cause.
+// NewDuplicateValuef creates a new DuplicateValue Error instance with the specified cause.
 func NewDuplicateValuef(cause error, context interface{}, format string, args ...interface{}) Error {
 	if format == "" {
 		format = fmt.Sprintf("Duplicate: %s", context)
 	}
 	return makeErrorf(DuplicateValueError, cause, format, args...)
 }
 
-// New creates a new Timeout Error instance with the specified cause.
+// NewTimeoutf creates a new Timeout Error instance with the specified cause.
 func NewTimeoutf(cause error, context interface{}, format string, args ...interface{}) Error {
 	if format == "" {
 		format = fmt.Sprintf("Timeout: %s", context)
 	}
 	return makeErrorf(TimeoutError, cause, format, args...)
 }
 
-// New creates a new Unauthorised Error instance with the specified cause.
+// NewUnauthorisedf creates a new Unauthorised Error instance with the specified cause.
 func NewUnauthorisedf(cause error, context interface{}, format string, args ...interface{}) Error {
 	if format == "" {
 		format = fmt.Sprintf("Unauthorised: %s", context)
 	}
 	return makeErrorf(UnauthorisedError, cause, format, args...)
 }
+
+// NewNotImplementedf creates a new NotImplemented Error instance with the specified cause.
+func NewNotImplementedf(cause error, context interface{}, format string, args ...interface{}) Error {
+	if format == "" {
+		format = fmt.Sprintf("Not implemented: %s", context)
+	}
+	return makeErrorf(NotImplementedError, cause, format, args...)
+}

errors/errors_test.go

@@ -55,6 +55,20 @@ func (s *ErrorsSuite) TestCreateUnauthorisedfError(c *C) {
 	c.Assert(err.Error(), Equals, "It was unauthorised: context")
 }
 
+func (s *ErrorsSuite) TestCreateSimpleNotImplementedfError(c *C) {
+	context := "context"
+	err := errors.NewNotImplementedf(nil, context, "")
+	c.Assert(errors.IsNotImplemented(err), Equals, true)
+	c.Assert(err.Error(), Equals, "Not implemented: context")
+}
+
+func (s *ErrorsSuite) TestCreateNotImplementedfError(c *C) {
+	context := "context"
+	err := errors.NewNotImplementedf(nil, context, "It was not implemented: %s", context)
+	c.Assert(errors.IsNotImplemented(err), Equals, true)
+	c.Assert(err.Error(), Equals, "It was not implemented: context")
+}
+
 func (s *ErrorsSuite) TestErrorCause(c *C) {
 	rootCause := errors.NewNotFoundf(nil, "some value", "")
 	// Construct a new error, based on a not found root cause.

nova/local_test.go

@@ -230,7 +230,7 @@ func (s *localLiveSuite) TestListAvailabilityZonesUnimplemented(c *C) {
 	// that error.
 	s.openstack.Nova.SetAvailabilityZones()
 	listedZones, err := s.nova.ListAvailabilityZones()
-	c.Assert(err, IsNil)
+	c.Assert(err, ErrorMatches, "the server does not support availability zones(.|\n)*")
 	c.Assert(listedZones, HasLen, 0)
 }
 

nova/nova.go

@@ -683,6 +683,10 @@ type AvailabilityZoneState struct {
 }
 
 // ListAvailabilityZones lists all availability zones.
+//
+// Availability zones are an OpenStack extension; if the server does not
+// support them, then an error satisfying errors.IsNotImplemented will be
+// returned.
 func (c *Client) ListAvailabilityZones() ([]AvailabilityZone, error) {
 	var resp struct {
 		AvailabilityZoneInfo []AvailabilityZone
@@ -692,7 +696,9 @@ func (c *Client) ListAvailabilityZones() ([]AvailabilityZone, error) {
 	if errors.IsNotFound(err) {
 		// Availability zones are an extension, so don't
 		// return an error if the API does not exist.
-		return nil, nil
+		return nil, errors.NewNotImplementedf(
+			err, nil, "the server does not support availability zones",
+		)
 	}
 	if err != nil {
 		return nil, errors.Newf(err, "failed to get list of availability zones")

testservices/errors.go

@@ -68,6 +68,10 @@ func NewRateLimitExceededError() *ServerError {
 	return serverErrorf(413, "Retry limit exceeded")
 }
 
+func NewAvailabilityZoneIsNotAvailableError() *ServerError {
+	return serverErrorf(400, "The requested availability zone is not available")
+}
+
 func NewAddFlavorError(id string) *ServerError {
 	return serverErrorf(409, "A flavor with id %q already exists", id)
 }

testservices/novaservice/service_http.go

@@ -225,14 +225,6 @@ The resource could not be found.
 		nil,
 		nil,
 	}
-	errAvailabilityZoneIsNotAvailable = &errorResponse{
-		http.StatusBadRequest,
-		`{"badRequest": {"message": "The requested availability zone is not available", "code": 400}}`,
-		"application/json; charset=UTF-8",
-		"",
-		nil,
-		nil,
-	}
 )
 
 func (e *errorResponse) Error() string {
@@ -567,7 +559,7 @@ func (n *Nova) handleRunServer(body []byte, w http.ResponseWriter, r *http.Reque
 	}
 	if az := req.Server.AvailabilityZone; az != "" {
 		if !n.availabilityZones[az].State.Available {
-			return errAvailabilityZoneIsNotAvailable
+			return testservices.AvailabilityZoneIsNotAvailable
 		}
 	}
 	n.nextServerId++

testservices/service.go

@@ -32,3 +32,7 @@ var NoMoreFloatingIPs = NewNoMoreFloatingIpsError()
 
 // IPLimitExceeded corresponds to "HTTP 413 Maximum number of floating ips exceeded"
 var IPLimitExceeded = NewIPLimitExceededError()
+
+// AvailabilityZoneIsNotAvailable corresponds to
+// "HTTP 400 The requested availability zone is not available"
+var AvailabilityZoneIsNotAvailable = NewAvailabilityZoneIsNotAvailableError()