From dda3334d8327d884873922d582e7d39d47645718 Mon Sep 17 00:00:00 2001 From: Kristen Carlson Accardi Date: Thu, 27 Oct 2016 08:42:58 -0700 Subject: [PATCH] ciao-controller: send ErrInstanceNotAssigned when needed When an operation is requested on an instance that is not yet assigned to a node, send an explicit error message for this condition from ciao-controller. In the openstack compute api, detect this error message and send a 403 Forbidden error rather than a 500 internal server error. Signed-off-by: Kristen Carlson Accardi --- ciao-controller/command.go | 6 +++--- ciao-controller/compute_test.go | 2 +- ciao-controller/openstack_compute.go | 20 +++++++++++++++----- ciao-controller/types/types.go | 3 +++ openstack/compute/api.go | 19 +++++++++---------- 5 files changed, 31 insertions(+), 19 deletions(-) diff --git a/ciao-controller/command.go b/ciao-controller/command.go index bd59d20fa..6c26ba219 100644 --- a/ciao-controller/command.go +++ b/ciao-controller/command.go @@ -41,7 +41,7 @@ func (c *controller) restartInstance(instanceID string) error { } if i.NodeID == "" { - return errors.New("Cannot restart instance not assigned to Node") + return types.ErrInstanceNotAssigned } if i.State != "exited" { @@ -60,7 +60,7 @@ func (c *controller) stopInstance(instanceID string) error { } if i.NodeID == "" { - return errors.New("Cannot stop instance not assigned to Node") + return types.ErrInstanceNotAssigned } if i.State == payloads.ComputeStatusPending { @@ -79,7 +79,7 @@ func (c *controller) deleteInstance(instanceID string) error { } if i.NodeID == "" { - return errors.New("Cannot delete instance not assigned to Node") + return types.ErrInstanceNotAssigned } go c.client.DeleteInstance(instanceID, i.NodeID) diff --git a/ciao-controller/compute_test.go b/ciao-controller/compute_test.go index 547dd8708..63d3da69d 100644 --- a/ciao-controller/compute_test.go +++ b/ciao-controller/compute_test.go @@ -322,7 +322,7 @@ func testDeleteServer(t *testing.T, httpExpectedStatus int, httpExpectedErrorSta } func TestDeleteServer(t *testing.T) { - testDeleteServer(t, http.StatusNoContent, http.StatusInternalServerError, true) + testDeleteServer(t, http.StatusNoContent, http.StatusForbidden, true) } func TestDeleteServerInvalidToken(t *testing.T) { diff --git a/ciao-controller/openstack_compute.go b/ciao-controller/openstack_compute.go index e4386c492..310a67f6e 100644 --- a/ciao-controller/openstack_compute.go +++ b/ciao-controller/openstack_compute.go @@ -192,11 +192,11 @@ func (c *controller) DeleteServer(tenant string, server string) error { } err = c.deleteInstance(server) - if err != nil { - return err + if err == types.ErrInstanceNotAssigned { + return compute.ErrInstanceNotAvailable } - return nil + return err } func (c *controller) StartServer(tenant string, ID string) error { @@ -209,7 +209,12 @@ func (c *controller) StartServer(tenant string, ID string) error { return compute.ErrServerOwner } - return c.restartInstance(ID) + err = c.restartInstance(ID) + if err == types.ErrInstanceNotAssigned { + return compute.ErrInstanceNotAvailable + } + + return err } func (c *controller) StopServer(tenant string, ID string) error { @@ -222,7 +227,12 @@ func (c *controller) StopServer(tenant string, ID string) error { return compute.ErrServerOwner } - return c.stopInstance(ID) + err = c.stopInstance(ID) + if err == types.ErrInstanceNotAssigned { + return compute.ErrInstanceNotAvailable + } + + return err } func (c *controller) ListFlavors(tenant string) (compute.Flavors, error) { diff --git a/ciao-controller/types/types.go b/ciao-controller/types/types.go index 83ea00661..41ef83f8f 100644 --- a/ciao-controller/types/types.go +++ b/ciao-controller/types/types.go @@ -508,4 +508,7 @@ var ( // ErrInstanceNotFound is returned when an instance is not found. ErrInstanceNotFound = errors.New("Instance not found") + + // ErrInstanceNotAssigned is returned when an instance is not assigned to a node. + ErrInstanceNotAssigned = errors.New("Cannot perform operation: instance not assigned to Node") ) diff --git a/openstack/compute/api.go b/openstack/compute/api.go index 974cfe05f..2d99845e9 100644 --- a/openstack/compute/api.go +++ b/openstack/compute/api.go @@ -72,10 +72,11 @@ type SecurityGroup struct { // These errors can be returned by the Service interface var ( - ErrQuota = errors.New("Tenant over quota") - ErrTenantNotFound = errors.New("Tenant not found") - ErrServerNotFound = errors.New("Server not found") - ErrServerOwner = errors.New("You are not server owner") + ErrQuota = errors.New("Tenant over quota") + ErrTenantNotFound = errors.New("Tenant not found") + ErrServerNotFound = errors.New("Server not found") + ErrServerOwner = errors.New("You are not server owner") + ErrInstanceNotAvailable = errors.New("Instance not currently available for this operation") ) // errorResponse maps service error responses to http responses. @@ -83,14 +84,12 @@ var ( // on return values all the time. func errorResponse(err error) APIResponse { switch err { - case ErrQuota: - return APIResponse{http.StatusForbidden, nil} - case ErrTenantNotFound: - return APIResponse{http.StatusNotFound, nil} - case ErrServerNotFound: + case ErrTenantNotFound, ErrServerNotFound: return APIResponse{http.StatusNotFound, nil} - case ErrServerOwner: + + case ErrQuota, ErrServerOwner, ErrInstanceNotAvailable: return APIResponse{http.StatusForbidden, nil} + default: return APIResponse{http.StatusInternalServerError, nil} }