From 16b092d9cf3e7a641940543bbf5dd4937ac643c9 Mon Sep 17 00:00:00 2001 From: Eli Qiao Date: Wed, 6 Jul 2016 14:35:42 +0800 Subject: [PATCH] API: catch InstanceNotReady exception. When retrieving server diagnostics, nova should raise 409 instead of 500 in case the instance has no host yet. Closes-Bug: #1599377 Change-Id: I3748978d9faf8adc8ca7d1d1d3f02128aa22cf3f --- nova/api/openstack/compute/server_diagnostics.py | 4 ++++ nova/compute/api.py | 2 ++ .../openstack/compute/test_server_diagnostics.py | 9 +++++++++ nova/tests/unit/compute/test_compute_api.py | 14 ++++++++++++++ 4 files changed, 29 insertions(+) diff --git a/nova/api/openstack/compute/server_diagnostics.py b/nova/api/openstack/compute/server_diagnostics.py index 284f6ad462de..49bfd598ce6e 100644 --- a/nova/api/openstack/compute/server_diagnostics.py +++ b/nova/api/openstack/compute/server_diagnostics.py @@ -13,6 +13,8 @@ # License for the specific language governing permissions and limitations # under the License. +import webob + from nova.api.openstack import common from nova.api.openstack import extensions from nova.api.openstack import wsgi @@ -45,6 +47,8 @@ class ServerDiagnosticsController(wsgi.Controller): except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, 'get_diagnostics', server_id) + except exception.InstanceNotReady as e: + raise webob.exc.HTTPConflict(explanation=e.format_message()) except NotImplementedError: common.raise_feature_not_supported() diff --git a/nova/compute/api.py b/nova/compute/api.py index f00335625b03..699568a66547 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2732,10 +2732,12 @@ class API(base.Base): self._record_action_start(context, instance, instance_actions.UNPAUSE) self.compute_rpcapi.unpause_instance(context, instance) + @check_instance_host def get_diagnostics(self, context, instance): """Retrieve diagnostics for the given instance.""" return self.compute_rpcapi.get_diagnostics(context, instance=instance) + @check_instance_host def get_instance_diagnostics(self, context, instance): """Retrieve diagnostics for the given instance.""" return self.compute_rpcapi.get_instance_diagnostics(context, diff --git a/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py b/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py index 8423b96cac74..33e9c539936b 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py +++ b/nova/tests/unit/api/openstack/compute/test_server_diagnostics.py @@ -79,6 +79,15 @@ class ServerDiagnosticsTestV21(test.NoDBTestCase): res = req.get_response(self.router) self.assertEqual(409, res.status_int) + @mock.patch.object(compute_api.API, 'get_diagnostics', + side_effect=exception.InstanceNotReady('fake message')) + @mock.patch.object(compute_api.API, 'get', fake_instance_get) + def test_get_diagnostics_raise_instance_not_ready(self, + mock_get_diagnostics): + req = self._get_request() + res = req.get_response(self.router) + self.assertEqual(409, res.status_int) + @mock.patch.object(compute_api.API, 'get_diagnostics', side_effect=NotImplementedError) @mock.patch.object(compute_api.API, 'get', fake_instance_get) diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index 3fa03fbe337e..d6010d244f31 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -1942,6 +1942,20 @@ class _ComputeAPIUnitTestMixIn(object): self.assertEqual(vm_states.PAUSED, instance.vm_state) self.assertEqual(task_states.UNPAUSING, instance.task_state) + def test_get_diagnostics_none_host(self): + instance = self._create_instance_obj() + instance.host = None + self.assertRaises(exception.InstanceNotReady, + self.compute_api.get_diagnostics, + self.context, instance) + + def test_get_instance_diagnostics_none_host(self): + instance = self._create_instance_obj() + instance.host = None + self.assertRaises(exception.InstanceNotReady, + self.compute_api.get_instance_diagnostics, + self.context, instance) + def test_live_migrate_active_vm_state(self): instance = self._create_instance_obj() self._live_migrate_instance(instance)