Merge pull request #754 from kaccardi/topic/fix-delete-response

ciao-controller: send ErrInstanceNotAssigned when needed

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)

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) {

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) {

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")
 )

openstack/compute/api.go

@@ -72,25 +72,24 @@ 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.
 // this helper function can help functions avoid having to switch
 // 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}
 	}