Fix check instance host for instance action

When instance has no host, actions such as get_console_output,
start_stop_instance cause HTTP 500 response. Here change to
HTTPConflict when action called before host set.

Fix LP# 1116012

Change-Id: I6153a03f449d9fad8d0d8fb7295bdea4d2b2c2b1

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

@@ -65,6 +65,8 @@ def get_console_output(self, req, id, body):
                                                          length)
         except exception.NotFound:
             raise webob.exc.HTTPNotFound(_('Unable to get console'))
+        except exception.InstanceNotReady as e:
+            raise webob.exc.HTTPConflict(explanation=unicode(e))
 
         # XML output is not correctly escaped, so remove invalid characters
         remove_re = re.compile('[\x00-\x08\x0B-\x0C\x0E-\x1F]')

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

@@ -44,7 +44,10 @@ def _start_server(self, req, id, body):
         context = req.environ['nova.context']
         instance = self._get_instance(context, id)
         LOG.debug(_('start instance'), instance=instance)
-        self.compute_api.start(context, instance)
+        try:
+            self.compute_api.start(context, instance)
+        except exception.InstanceNotReady as e:
+            raise webob.exc.HTTPConflict(explanation=unicode(e))
         return webob.Response(status_int=202)
 
     @wsgi.action('os-stop')
@@ -53,7 +56,10 @@ def _stop_server(self, req, id, body):
         context = req.environ['nova.context']
         instance = self._get_instance(context, id)
         LOG.debug(_('stop instance'), instance=instance)
-        self.compute_api.stop(context, instance)
+        try:
+            self.compute_api.stop(context, instance)
+        except exception.InstanceNotReady as e:
+            raise webob.exc.HTTPConflict(explanation=unicode(e))
         return webob.Response(status_int=202)
 
 

nova/compute/api.py

@@ -131,6 +131,15 @@ def inner(self, context, instance, *args, **kw):
     return outer
 
 
+def check_instance_host(function):
+    @functools.wraps(function)
+    def wrapped(self, context, instance, *args, **kwargs):
+        if not instance['host']:
+            raise exception.InstanceNotReady(instance_id=instance['uuid'])
+        return function(self, context, instance, *args, **kwargs)
+    return wrapped
+
+
 def check_instance_lock(function):
     @functools.wraps(function)
     def inner(self, context, instance, *args, **kwargs):
@@ -1213,6 +1222,7 @@ def force_delete(self, context, instance):
 
     @wrap_check_policy
     @check_instance_lock
+    @check_instance_host
     @check_instance_state(vm_state=[vm_states.ACTIVE, vm_states.RESCUED,
                                     vm_states.ERROR, vm_states.STOPPED],
                           task_state=[None])
@@ -1231,6 +1241,7 @@ def stop(self, context, instance, do_cast=True):
 
     @wrap_check_policy
     @check_instance_lock
+    @check_instance_host
     @check_instance_state(vm_state=[vm_states.STOPPED])
     def start(self, context, instance):
         """Start an instance."""
@@ -2135,11 +2146,9 @@ def inject_file(self, context, instance, path, file_contents):
                 file_contents=file_contents)
 
     @wrap_check_policy
+    @check_instance_host
     def get_vnc_console(self, context, instance, console_type):
         """Get a url to an instance Console."""
-        if not instance['host']:
-            raise exception.InstanceNotReady(instance_id=instance['uuid'])
-
         connect_info = self.compute_rpcapi.get_vnc_console(context,
                 instance=instance, console_type=console_type)
 
@@ -2149,20 +2158,17 @@ def get_vnc_console(self, context, instance, console_type):
 
         return {'url': connect_info['access_url']}
 
+    @check_instance_host
     def get_vnc_connect_info(self, context, instance, console_type):
         """Used in a child cell to get console info."""
-        if not instance['host']:
-            raise exception.InstanceNotReady(instance_id=instance['uuid'])
         connect_info = self.compute_rpcapi.get_vnc_console(context,
                 instance=instance, console_type=console_type)
         return connect_info
 
     @wrap_check_policy
+    @check_instance_host
     def get_spice_console(self, context, instance, console_type):
         """Get a url to an instance Console."""
-        if not instance['host']:
-            raise exception.InstanceNotReady(instance_id=instance['uuid'])
-
         connect_info = self.compute_rpcapi.get_spice_console(context,
                 instance=instance, console_type=console_type)
 
@@ -2172,15 +2178,15 @@ def get_spice_console(self, context, instance, console_type):
 
         return {'url': connect_info['access_url']}
 
+    @check_instance_host
     def get_spice_connect_info(self, context, instance, console_type):
         """Used in a child cell to get console info."""
-        if not instance['host']:
-            raise exception.InstanceNotReady(instance_id=instance['uuid'])
         connect_info = self.compute_rpcapi.get_spice_console(context,
                 instance=instance, console_type=console_type)
         return connect_info
 
     @wrap_check_policy
+    @check_instance_host
     def get_console_output(self, context, instance, tail_length=None):
         """Get console output for an instance."""
         return self.compute_rpcapi.get_console_output(context,

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

@@ -35,6 +35,10 @@ def fake_get_console_output(self, _context, _instance, tail_length):
     return '\n'.join(fixture)
 
 
+def fake_get_console_output_not_ready(self, _context, _instance, tail_length):
+    raise exception.InstanceNotReady(instance_id=_instance["uuid"])
+
+
 def fake_get(self, context, instance_uuid):
     return {'uuid': instance_uuid}
 
@@ -133,3 +137,15 @@ def test_get_text_console_bad_body(self):
 
         res = req.get_response(self.app)
         self.assertEqual(res.status_int, 400)
+
+    def test_get_console_output_not_ready(self):
+        self.stubs.Set(compute_api.API, 'get_console_output',
+                       fake_get_console_output_not_ready)
+        body = {'os-getConsoleOutput': {'length': 3}}
+        req = webob.Request.blank('/v2/fake/servers/1/action')
+        req.method = "POST"
+        req.body = jsonutils.dumps(body)
+        req.headers["content-type"] = "application/json"
+
+        res = req.get_response(self.app)
+        self.assertEqual(res.status_int, 409)

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

@@ -17,6 +17,7 @@
 
 from nova.api.openstack.compute.contrib import server_start_stop
 from nova.compute import api as compute_api
+from nova import exception
 from nova import test
 from nova.tests.api.openstack import fakes
 
@@ -25,6 +26,10 @@ def fake_compute_api_get(self, context, instance_id):
     return {'id': 1, 'uuid': instance_id}
 
 
+def fake_start_stop_not_ready(self, context, instance):
+    raise exception.InstanceNotReady(instance_id=instance["uuid"])
+
+
 class ServerStartStopTest(test.TestCase):
 
     def setUp(self):
@@ -41,6 +46,14 @@ def test_start(self):
         body = dict(start="")
         self.controller._start_server(req, 'test_inst', body)
 
+    def test_start_not_ready(self):
+        self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(compute_api.API, 'start', fake_start_stop_not_ready)
+        req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
+        body = dict(start="")
+        self.assertRaises(webob.exc.HTTPConflict,
+            self.controller._start_server, req, 'test_inst', body)
+
     def test_stop(self):
         self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
         self.mox.StubOutWithMock(compute_api.API, 'stop')
@@ -51,6 +64,14 @@ def test_stop(self):
         body = dict(stop="")
         self.controller._stop_server(req, 'test_inst', body)
 
+    def test_stop_not_ready(self):
+        self.stubs.Set(compute_api.API, 'get', fake_compute_api_get)
+        self.stubs.Set(compute_api.API, 'stop', fake_start_stop_not_ready)
+        req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
+        body = dict(start="")
+        self.assertRaises(webob.exc.HTTPConflict,
+            self.controller._stop_server, req, 'test_inst', body)
+
     def test_start_with_bogus_id(self):
         req = fakes.HTTPRequest.blank('/v2/fake/servers/test_inst/action')
         body = dict(start="")

nova/tests/compute/test_compute.py

@@ -3783,6 +3783,15 @@ def test_start(self):
 
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_start_no_host(self):
+        instance = self._create_fake_instance(params={'host': ''})
+
+        self.assertRaises(exception.InstanceNotReady,
+                          self.compute_api.start,
+                          self.context, instance)
+
+        db.instance_destroy(self.context, instance['uuid'])
+
     def test_stop(self):
         instance = jsonutils.to_primitive(self._create_fake_instance())
         instance_uuid = instance['uuid']
@@ -3798,6 +3807,15 @@ def test_stop(self):
 
         db.instance_destroy(self.context, instance['uuid'])
 
+    def test_stop_no_host(self):
+        instance = self._create_fake_instance(params={'host': ''})
+
+        self.assertRaises(exception.InstanceNotReady,
+                          self.compute_api.stop,
+                          self.context, instance)
+
+        db.instance_destroy(self.context, instance['uuid'])
+
     def test_start_shutdown(self):
         def check_state(instance_uuid, power_state_, vm_state_, task_state_):
             instance = db.instance_get_by_uuid(self.context, instance_uuid)
@@ -5636,6 +5654,15 @@ def test_console_output(self):
                 fake_instance, tail_length=fake_tail_length)
         self.assertEqual(output, fake_console_output)
 
+    def test_console_output_no_host(self):
+        instance = self._create_fake_instance(params={'host': ''})
+
+        self.assertRaises(exception.InstanceNotReady,
+                          self.compute_api.get_console_output,
+                          self.context, instance)
+
+        db.instance_destroy(self.context, instance['uuid'])
+
     def test_attach_volume(self):
         # Ensure instance can be soft rebooted.