From 475691a4bd5795feb50b5c9ccfe98e6487390e58 Mon Sep 17 00:00:00 2001 From: Chris Behrens Date: Wed, 11 Jan 2012 20:25:23 -0800 Subject: catch InstanceInvalidState in more places Further fixes to bug 911879 500s or 400s are returned in the OS API when actions are denied due to being in an invalid state. 409 should be returned, instead. A previous review (2846) fixed the delete case and this fixes more. When writing tests, I found a number of exceptions that are not raised anymore, and they were being incorrectly used in tests still. I fixed those up. Change-Id: I0d5b1ed52e0cc9766be8e2a7de84c8601f4bdf26 --- nova/api/openstack/common.py | 15 ++++++++++++ nova/api/openstack/v2/contrib/admin_actions.py | 27 ++++++++++++++++----- nova/api/openstack/v2/contrib/deferred_delete.py | 14 +++++++++-- nova/api/openstack/v2/servers.py | 30 ++++++++++++++++-------- nova/api/openstack/wsgi.py | 2 +- 5 files changed, 69 insertions(+), 19 deletions(-) (limited to 'nova/api') diff --git a/nova/api/openstack/common.py b/nova/api/openstack/common.py index e96c42ac9..5c2fe8b9b 100644 --- a/nova/api/openstack/common.py +++ b/nova/api/openstack/common.py @@ -334,6 +334,21 @@ def get_networks_for_instance(context, instance): return networks +def raise_http_conflict_for_instance_invalid_state(exc, action): + """Return a webob.exc.HTTPConflict instance containing a message + appropriate to return via the API based on the original + InstanceInvalidState exception. + """ + attr = exc.kwargs.get('attr') + state = exc.kwargs.get('state') + if attr and state: + msg = _("Cannot '%(action)s' while instance is in %(attr)s %(state)s") + else: + # At least give some meaningful message + msg = _("Instance is in an invalid state for '%(action)s'") + raise webob.exc.HTTPConflict(explanation=msg % locals()) + + class MetadataDeserializer(wsgi.MetadataXMLDeserializer): def deserialize(self, text): dom = minidom.parseString(text) diff --git a/nova/api/openstack/v2/contrib/admin_actions.py b/nova/api/openstack/v2/contrib/admin_actions.py index f4207354c..5479f88c4 100644 --- a/nova/api/openstack/v2/contrib/admin_actions.py +++ b/nova/api/openstack/v2/contrib/admin_actions.py @@ -56,6 +56,9 @@ class Admin_actions(extensions.ExtensionDescriptor): try: server = self.compute_api.get(ctxt, id) self.compute_api.pause(ctxt, server) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'pause') except Exception: readable = traceback.format_exc() LOG.exception(_("Compute.api::pause %s"), readable) @@ -70,6 +73,9 @@ class Admin_actions(extensions.ExtensionDescriptor): try: server = self.compute_api.get(ctxt, id) self.compute_api.unpause(ctxt, server) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'unpause') except Exception: readable = traceback.format_exc() LOG.exception(_("Compute.api::unpause %s"), readable) @@ -84,6 +90,9 @@ class Admin_actions(extensions.ExtensionDescriptor): try: server = self.compute_api.get(context, id) self.compute_api.suspend(context, server) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'suspend') except Exception: readable = traceback.format_exc() LOG.exception(_("compute.api::suspend %s"), readable) @@ -98,6 +107,9 @@ class Admin_actions(extensions.ExtensionDescriptor): try: server = self.compute_api.get(context, id) self.compute_api.resume(context, server) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'resume') except Exception: readable = traceback.format_exc() LOG.exception(_("compute.api::resume %s"), readable) @@ -112,6 +124,9 @@ class Admin_actions(extensions.ExtensionDescriptor): try: instance = self.compute_api.get(context, id) self.compute_api.resize(req.environ['nova.context'], instance) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'migrate') except Exception, e: LOG.exception(_("Error in migrate %s"), e) raise exc.HTTPBadRequest() @@ -233,12 +248,12 @@ class Admin_actions(extensions.ExtensionDescriptor): except exception.NotFound: raise exc.HTTPNotFound(_("Instance not found")) - image = self.compute_api.backup(context, - instance, - image_name, - backup_type, - rotation, - extra_properties=props) + try: + image = self.compute_api.backup(context, instance, image_name, + backup_type, rotation, extra_properties=props) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'createBackup') # build location of newly-created image entity image_id = str(image['id']) diff --git a/nova/api/openstack/v2/contrib/deferred_delete.py b/nova/api/openstack/v2/contrib/deferred_delete.py index 7ae435a8e..0b7c60073 100644 --- a/nova/api/openstack/v2/contrib/deferred_delete.py +++ b/nova/api/openstack/v2/contrib/deferred_delete.py @@ -17,9 +17,11 @@ import webob +from nova.api.openstack import common from nova.api.openstack.v2 import extensions from nova.api.openstack.v2 import servers from nova import compute +from nova import exception from nova import log as logging @@ -44,7 +46,11 @@ class Deferred_delete(extensions.ExtensionDescriptor): context = req.environ["nova.context"] instance = self.compute_api.get(context, instance_id) - self.compute_api.restore(context, instance) + try: + self.compute_api.restore(context, instance) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'restore') return webob.Response(status_int=202) def _force_delete(self, input_dict, req, instance_id): @@ -52,7 +58,11 @@ class Deferred_delete(extensions.ExtensionDescriptor): context = req.environ["nova.context"] instance = self.compute_api.get(context, instance_id) - self.compute_api.force_delete(context, instance) + try: + self.compute_api.force_delete(context, instance) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'forceDelete') return webob.Response(status_int=202) def get_actions(self): diff --git a/nova/api/openstack/v2/servers.py b/nova/api/openstack/v2/servers.py index ac8dd457a..98f9e0686 100644 --- a/nova/api/openstack/v2/servers.py +++ b/nova/api/openstack/v2/servers.py @@ -826,6 +826,9 @@ class Controller(wsgi.Controller): except exception.MigrationNotFound: msg = _("Instance has not been resized.") raise exc.HTTPBadRequest(explanation=msg) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'confirmResize') except Exception, e: LOG.exception(_("Error in confirm-resize %s"), e) raise exc.HTTPBadRequest() @@ -839,6 +842,9 @@ class Controller(wsgi.Controller): except exception.MigrationNotFound: msg = _("Instance has not been resized.") raise exc.HTTPBadRequest(explanation=msg) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'revertResize') except Exception, e: LOG.exception(_("Error in revert-resize %s"), e) raise exc.HTTPBadRequest() @@ -862,6 +868,9 @@ class Controller(wsgi.Controller): try: self.compute_api.reboot(context, instance, reboot_type) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'reboot') except Exception, e: LOG.exception(_("Error in reboot %s"), e) raise exc.HTTPUnprocessableEntity() @@ -880,6 +889,9 @@ class Controller(wsgi.Controller): except exception.CannotResizeToSameSize: msg = _("Resize requires a change in size.") raise exc.HTTPBadRequest(explanation=msg) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'resize') return webob.Response(status_int=202) @@ -893,9 +905,8 @@ class Controller(wsgi.Controller): except exception.NotFound: raise exc.HTTPNotFound() except exception.InstanceInvalidState as state_error: - state = state_error.kwargs.get("state") - msg = _("Unable to delete instance when %s") % state - raise exc.HTTPConflict(explanation=msg) + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'delete') def _get_key_name(self, req, body): if 'server' in body: @@ -1009,10 +1020,9 @@ class Controller(wsgi.Controller): image_href, password, **kwargs) - - except exception.RebuildRequiresActiveInstance: - msg = _("Instance must be active to rebuild.") - raise exc.HTTPConflict(explanation=msg) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'rebuild') except exception.InstanceNotFound: msg = _("Instance could not be found") raise exc.HTTPNotFound(explanation=msg) @@ -1064,9 +1074,9 @@ class Controller(wsgi.Controller): instance, image_name, extra_properties=props) - except exception.InstanceBusy: - msg = _("Server is currently creating an image. Please wait.") - raise webob.exc.HTTPConflict(explanation=msg) + except exception.InstanceInvalidState as state_error: + common.raise_http_conflict_for_instance_invalid_state(state_error, + 'createImage') # build location of newly-created image entity image_id = str(image['id']) diff --git a/nova/api/openstack/wsgi.py b/nova/api/openstack/wsgi.py index 2f6533dc4..2bd6fa817 100644 --- a/nova/api/openstack/wsgi.py +++ b/nova/api/openstack/wsgi.py @@ -919,7 +919,7 @@ class Fault(webob.exc.HTTPException): 403: "resizeNotAllowed", 404: "itemNotFound", 405: "badMethod", - 409: "inProgress", + 409: "inProgress", # FIXME(comstud): This doesn't seem right 413: "overLimit", 415: "badMediaType", 501: "notImplemented", -- cgit