From 43ad0a2b1c7dea2c6f8888b28a61857c81067a16 Mon Sep 17 00:00:00 2001
From: Matt Riedemann <mriedem@us.ibm.com>
Date: Sat, 3 Aug 2013 13:15:12 -0700
Subject: [PATCH] Raise 404 when instance not found in admin_actions API

The pause/unpause/suspend/resume/reset_network admin action APIs do not
handle the case where the instance in the request is not found and they
raise HTTPUnprocessableEntity (code 422) instead. There are several
other operations in the same API which handle the 404 case and translate
it correctly, this patch cleans up the rest.

Closes-Bug: #1204999

Change-Id: If54d6be99db55b4f99da11fe75c14bf21685e809
---
 .../compute/contrib/admin_actions.py          | 10 +++++++
 .../compute/contrib/test_admin_actions.py     | 30 +++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/nova/api/openstack/compute/contrib/admin_actions.py b/nova/api/openstack/compute/contrib/admin_actions.py
index 3554eede82c..9f35185b375 100644
--- a/nova/api/openstack/compute/contrib/admin_actions.py
+++ b/nova/api/openstack/compute/contrib/admin_actions.py
@@ -56,6 +56,8 @@ def _pause(self, req, id, body):
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                     'pause')
+        except exception.InstanceNotFound:
+            raise exc.HTTPNotFound(_("Server not found"))
         except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::pause %s"), readable)
@@ -73,6 +75,8 @@ def _unpause(self, req, id, body):
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                     'unpause')
+        except exception.InstanceNotFound:
+            raise exc.HTTPNotFound(_("Server not found"))
         except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::unpause %s"), readable)
@@ -90,6 +94,8 @@ def _suspend(self, req, id, body):
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                     'suspend')
+        except exception.InstanceNotFound:
+            raise exc.HTTPNotFound(_("Server not found"))
         except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("compute.api::suspend %s"), readable)
@@ -107,6 +113,8 @@ def _resume(self, req, id, body):
         except exception.InstanceInvalidState as state_error:
             common.raise_http_conflict_for_instance_invalid_state(state_error,
                     'resume')
+        except exception.InstanceNotFound:
+            raise exc.HTTPNotFound(_("Server not found"))
         except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("compute.api::resume %s"), readable)
@@ -137,6 +145,8 @@ def _reset_network(self, req, id, body):
         try:
             instance = self.compute_api.get(context, id)
             self.compute_api.reset_network(context, instance)
+        except exception.InstanceNotFound:
+            raise exc.HTTPNotFound(_("Server not found"))
         except Exception:
             readable = traceback.format_exc()
             LOG.exception(_("Compute.api::reset_network %s"), readable)
diff --git a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py
index 1d9e1094803..b3ac6fb3361 100644
--- a/nova/tests/api/openstack/compute/contrib/test_admin_actions.py
+++ b/nova/tests/api/openstack/compute/contrib/test_admin_actions.py
@@ -63,6 +63,10 @@ def fake_compute_api_raises_invalid_state(*args, **kwargs):
             instance_uuid='fake')
 
 
+def fake_compute_api_raises_instance_not_found(*args, **kwargs):
+    raise exception.InstanceNotFound(instance_id='fake_id')
+
+
 def fake_compute_api_get(self, context, instance_uuid, want_objects=False):
     instance = fake_instance.fake_db_instance(
             id=1, uuid=instance_uuid, vm_state=vm_states.ACTIVE,
@@ -89,6 +93,17 @@ class AdminActionsTest(test.TestCase):
             ('resume', 'resume'),
             ('migrate', 'resize'))
 
+    _actions_that_check_for_instance = (
+            # action, method
+            ('pause', 'pause'),
+            ('unpause', 'unpause'),
+            ('suspend', 'suspend'),
+            ('resume', 'resume'),
+            ('resetNetwork', 'reset_network'),
+            ('injectNetworkInfo', 'inject_network_info'),
+            ('lock', 'lock'),
+            ('unlock', 'unlock'))
+
     def setUp(self):
         super(AdminActionsTest, self).setUp()
         self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
@@ -128,6 +143,21 @@ def test_admin_api_actions_raise_conflict_on_invalid_state(self):
             self.assertIn("Cannot \'%(_action)s\' while instance" % locals(),
                     res.body)
 
+    def test_admin_api_actions_raise_not_found(self):
+        app = fakes.wsgi_app(init_only=('servers',))
+
+        for _action, _method in self._actions_that_check_for_instance:
+            self.stubs.Set(compute_api.API, _method,
+                fake_compute_api_raises_instance_not_found)
+
+            req = webob.Request.blank('/v2/fake/servers/%s/action' % self.UUID)
+            req.method = 'POST'
+            req.body = jsonutils.dumps({_action: None})
+            req.content_type = 'application/json'
+            res = req.get_response(app)
+            self.assertEqual(res.status_int, 404)
+            self.assertIn("could not be found", res.body)
+
     def test_migrate_live_enabled(self):
         ctxt = context.get_admin_context()
         ctxt.user_id = 'fake'