Check host exists before evacuate

Before an evacuate is attempted, there is no checks in
place to see if the host exists. This
results in rpc call being made to non existent host.

This patch adds a simple check to both v2 and v3 extensions
to ensure that the compute service exists before we attempt
to evacuate.

Fixes bug 1208369

Change-Id: I07103ecb8310d8a891ee5fb21a1f8aa7427e0d78

nova/api/openstack/compute/contrib/evacuate.py

@@ -33,6 +33,7 @@ class Controller(wsgi.Controller):
     def __init__(self, *args, **kwargs):
         super(Controller, self).__init__(*args, **kwargs)
         self.compute_api = compute.API()
+        self.host_api = compute.HostAPI()
 
     @wsgi.action('evacuate')
     def _evacuate(self, req, id, body):
@@ -68,6 +69,12 @@ class Controller(wsgi.Controller):
             msg = _("host and onSharedStorage must be specified.")
             raise exc.HTTPBadRequest(explanation=msg)
 
+        try:
+            self.host_api.service_get_by_compute_host(context, host)
+        except exception.NotFound:
+            msg = _("Compute host %s not found.") % host
+            raise exc.HTTPNotFound(explanation=msg)
+
         try:
             instance = self.compute_api.get(context, id)
             self.compute_api.evacuate(context, instance, host,

nova/api/openstack/compute/plugins/v3/evacuate.py

@@ -34,6 +34,7 @@ class EvacuateController(wsgi.Controller):
     def __init__(self, *args, **kwargs):
         super(EvacuateController, self).__init__(*args, **kwargs)
         self.compute_api = compute.API()
+        self.host_api = compute.HostAPI()
 
     @extensions.expected_errors((400, 404, 409))
     @wsgi.action('evacuate')
@@ -70,6 +71,12 @@ class EvacuateController(wsgi.Controller):
             msg = _("host and on_shared_storage must be specified.")
             raise exc.HTTPBadRequest(explanation=msg)
 
+        try:
+            self.host_api.service_get_by_compute_host(context, host)
+        except exception.NotFound:
+            msg = _("Compute host %s not found.") % host
+            raise exc.HTTPNotFound(explanation=msg)
+
         try:
             instance = self.compute_api.get(context, id)
             self.compute_api.evacuate(context, instance, host,

nova/tests/api/openstack/compute/contrib/test_evacuate.py

@@ -20,6 +20,7 @@ import webob
 from nova.compute import api as compute_api
 from nova.compute import vm_states
 from nova import context
+from nova import exception
 from nova.openstack.common import jsonutils
 from nova import test
 from nova.tests.api.openstack import fakes
@@ -41,6 +42,17 @@ def fake_compute_api_get(self, context, instance_id):
     }
 
 
+def fake_service_get_by_compute_host(self, context, host):
+    if host == 'bad_host':
+        raise exception.ComputeHostNotFound(host=host)
+    else:
+        return {
+            'host_name': host,
+            'service': 'compute',
+            'zone': 'nova'
+            }
+
+
 class EvacuateTest(test.TestCase):
 
     _methods = ('resize', 'evacuate')
@@ -48,6 +60,8 @@ class EvacuateTest(test.TestCase):
     def setUp(self):
         super(EvacuateTest, self).setUp()
         self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(compute_api.HostAPI, 'service_get_by_compute_host',
+                       fake_service_get_by_compute_host)
         self.UUID = uuid.uuid4()
         for _method in self._methods:
             self.stubs.Set(compute_api.API, _method, fake_compute_api)
@@ -70,6 +84,25 @@ class EvacuateTest(test.TestCase):
         res = req.get_response(app)
         self.assertEqual(res.status_int, 400)
 
+    def test_evacuate_instance_with_bad_target(self):
+        ctxt = context.get_admin_context()
+        ctxt.user_id = 'fake'
+        ctxt.project_id = 'fake'
+        ctxt.is_admin = True
+        app = fakes.wsgi_app(fake_auth_context=ctxt)
+        req = webob.Request.blank('/v2/fake/servers/%s/action' % self.UUID)
+        req.method = 'POST'
+        req.body = jsonutils.dumps({
+            'evacuate': {
+                'host': 'bad_host',
+                'onSharedStorage': 'false',
+                'adminPass': 'MyNewPass'
+                }
+            })
+        req.content_type = 'application/json'
+        res = req.get_response(app)
+        self.assertEqual(res.status_int, 404)
+
     def test_evacuate_instance_with_target(self):
         ctxt = context.get_admin_context()
         ctxt.user_id = 'fake'

nova/tests/api/openstack/compute/plugins/v3/test_evacuate.py

@@ -20,6 +20,7 @@ import webob
 from nova.compute import api as compute_api
 from nova.compute import vm_states
 from nova import context
+from nova import exception
 from nova.openstack.common import jsonutils
 from nova import test
 from nova.tests.api.openstack import fakes
@@ -41,6 +42,16 @@ def fake_compute_api_get(self, context, instance_id):
     }
 
 
+def fake_service_get_by_compute_host(self, context, host):
+    if host == 'bad_host':
+        raise exception.ComputeHostNotFound(host=host)
+    else:
+        return {
+                'host_name': host,
+                'service': 'compute',
+                'zone': 'nova'}
+
+
 class EvacuateTest(test.TestCase):
 
     _methods = ('resize', 'evacuate')
@@ -48,6 +59,8 @@ class EvacuateTest(test.TestCase):
     def setUp(self):
         super(EvacuateTest, self).setUp()
         self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(compute_api.HostAPI, 'service_get_by_compute_host',
+                       fake_service_get_by_compute_host)
         self.UUID = uuid.uuid4()
         for _method in self._methods:
             self.stubs.Set(compute_api.API, _method, fake_compute_api)
@@ -76,6 +89,14 @@ class EvacuateTest(test.TestCase):
         res = req.get_response(app)
         self.assertEqual(res.status_int, 400)
 
+    def test_evacuate_instance_with_bad_host(self):
+        req, app = self._gen_request_with_app({'host': 'bad_host',
+                                               'on_shared_storage': 'False',
+                                               'admin_password': 'MyNewPass'})
+
+        res = req.get_response(app)
+        self.assertEqual(res.status_int, 404)
+
     def test_evacuate_instance_with_target(self):
         req, app = self._gen_request_with_app({'host': 'my_host',
                                                'on_shared_storage': 'False',

nova/tests/integrated/test_api_samples.py

@@ -3344,8 +3344,18 @@ class EvacuateJsonTest(ServersSampleBase):
             """Simulate validation of instance host is down."""
             return False
 
+        def fake_service_get_by_compute_host(self, context, host):
+            """Simulate that given host is a valid host."""
+            return {
+                    'host_name': host,
+                    'service': 'compute',
+                    'zone': 'nova'
+                    }
+
         self.stubs.Set(service_group_api.API, 'service_is_up',
                        fake_service_is_up)
+        self.stubs.Set(compute_api.HostAPI, 'service_get_by_compute_host',
+                       fake_service_get_by_compute_host)
 
         response = self._do_post('servers/%s/action' % uuid,
                                  'server-evacuate-req', req_subs)