diff --git a/nova/api/openstack/compute/hypervisors.py b/nova/api/openstack/compute/hypervisors.py index 27e5ed75ef08..acb18a1a22d5 100644 --- a/nova/api/openstack/compute/hypervisors.py +++ b/nova/api/openstack/compute/hypervisors.py @@ -118,7 +118,8 @@ class HypervisorsController(wsgi.Controller): hypervisors_list.append( self._view_hypervisor( hyp, service, False, req)) - except exception.ComputeHostNotFound: + except (exception.ComputeHostNotFound, + exception.HostMappingNotFound): # The compute service could be deleted which doesn't delete # the compute node record, that has to be manually removed # from the database so we just ignore it when listing nodes. @@ -164,7 +165,8 @@ class HypervisorsController(wsgi.Controller): hypervisors_list.append( self._view_hypervisor( hyp, service, True, req)) - except exception.ComputeHostNotFound: + except (exception.ComputeHostNotFound, + exception.HostMappingNotFound): # The compute service could be deleted which doesn't delete # the compute node record, that has to be manually removed # from the database so we just ignore it when listing nodes. @@ -186,11 +188,12 @@ class HypervisorsController(wsgi.Controller): try: hyp = self.host_api.compute_node_get(context, id) req.cache_db_compute_node(hyp) - except (ValueError, exception.ComputeHostNotFound): + service = self.host_api.service_get_by_compute_host( + context, hyp.host) + except (ValueError, exception.ComputeHostNotFound, + exception.HostMappingNotFound): msg = _("Hypervisor with ID '%s' could not be found.") % id raise webob.exc.HTTPNotFound(explanation=msg) - service = self.host_api.service_get_by_compute_host( - context, hyp.host) return dict(hypervisor=self._view_hypervisor( hyp, service, True, req)) @@ -209,13 +212,18 @@ class HypervisorsController(wsgi.Controller): try: host = hyp.host uptime = self.host_api.get_host_uptime(context, host) + service = self.host_api.service_get_by_compute_host(context, host) except NotImplementedError: common.raise_feature_not_supported() - except (exception.ComputeServiceUnavailable, - exception.HostMappingNotFound) as e: + except exception.ComputeServiceUnavailable as e: raise webob.exc.HTTPBadRequest(explanation=e.format_message()) + except exception.HostMappingNotFound: + # NOTE(danms): This mirrors the compute_node_get() behavior + # where the node is missing, resulting in NotFound instead of + # BadRequest if we fail on the map lookup. + msg = _("Hypervisor with ID '%s' could not be found.") % id + raise webob.exc.HTTPNotFound(explanation=msg) - service = self.host_api.service_get_by_compute_host(context, host) return dict(hypervisor=self._view_hypervisor(hyp, service, False, req, uptime=uptime)) @@ -226,12 +234,17 @@ class HypervisorsController(wsgi.Controller): hypervisors = self.host_api.compute_node_search_by_hypervisor( context, id) if hypervisors: - return dict(hypervisors=[self._view_hypervisor( - hyp, - self.host_api.service_get_by_compute_host( - context, hyp.host), - False, req) - for hyp in hypervisors]) + try: + return dict(hypervisors=[ + self._view_hypervisor( + hyp, + self.host_api.service_get_by_compute_host(context, + hyp.host), + False, req) + for hyp in hypervisors]) + except exception.HostMappingNotFound: + msg = _("No hypervisor matching '%s' could be found.") % id + raise webob.exc.HTTPNotFound(explanation=msg) else: msg = _("No hypervisor matching '%s' could be found.") % id raise webob.exc.HTTPNotFound(explanation=msg) diff --git a/nova/tests/unit/api/openstack/compute/test_hypervisors.py b/nova/tests/unit/api/openstack/compute/test_hypervisors.py index ef660c28eda4..04304cd49e43 100644 --- a/nova/tests/unit/api/openstack/compute/test_hypervisors.py +++ b/nova/tests/unit/api/openstack/compute/test_hypervisors.py @@ -326,6 +326,39 @@ class HypervisorsTestV21(test.NoDBTestCase): _test(self) + def test_index_compute_host_not_mapped(self): + """Tests that we don't fail index if a host is not mapped.""" + + # two computes, a matching service only exists for the first one + compute_nodes = objects.ComputeNodeList(objects=[ + objects.ComputeNode(**TEST_HYPERS[0]), + objects.ComputeNode(**TEST_HYPERS[1]) + ]) + + def fake_service_get_by_compute_host(context, host): + if host == TEST_HYPERS[0]['host']: + return TEST_SERVICES[0] + raise exception.HostMappingNotFound(name=host) + + @mock.patch.object(self.controller.host_api, 'compute_node_get_all', + return_value=compute_nodes) + @mock.patch.object(self.controller.host_api, + 'service_get_by_compute_host', + fake_service_get_by_compute_host) + def _test(self, compute_node_get_all): + req = self._get_request(True) + result = self.controller.index(req) + self.assertEqual(1, len(result['hypervisors'])) + expected = { + 'id': compute_nodes[0].id, + 'hypervisor_hostname': compute_nodes[0].hypervisor_hostname, + 'state': 'up', + 'status': 'enabled', + } + self.assertDictEqual(expected, result['hypervisors'][0]) + + _test(self) + def test_detail(self): req = self._get_request(True) result = self.controller.detail(req) @@ -380,6 +413,76 @@ class HypervisorsTestV21(test.NoDBTestCase): _test(self) + def test_detail_compute_host_not_mapped(self): + """Tests that if a service is deleted but the compute node is not we + don't fail when listing hypervisors. + """ + + # two computes, a matching service only exists for the first one + compute_nodes = objects.ComputeNodeList(objects=[ + objects.ComputeNode(**TEST_HYPERS[0]), + objects.ComputeNode(**TEST_HYPERS[1]) + ]) + + def fake_service_get_by_compute_host(context, host): + if host == TEST_HYPERS[0]['host']: + return TEST_SERVICES[0] + raise exception.HostMappingNotFound(name=host) + + @mock.patch.object(self.controller.host_api, 'compute_node_get_all', + return_value=compute_nodes) + @mock.patch.object(self.controller.host_api, + 'service_get_by_compute_host', + fake_service_get_by_compute_host) + def _test(self, compute_node_get_all): + req = self._get_request(True) + result = self.controller.detail(req) + self.assertEqual(1, len(result['hypervisors'])) + expected = { + 'id': compute_nodes[0].id, + 'hypervisor_hostname': compute_nodes[0].hypervisor_hostname, + 'state': 'up', + 'status': 'enabled', + } + # we don't care about all of the details, just make sure we get + # the subset we care about and there are more keys than what index + # would return + hypervisor = result['hypervisors'][0] + self.assertTrue( + set(expected.keys()).issubset(set(hypervisor.keys()))) + self.assertGreater(len(hypervisor.keys()), len(expected.keys())) + self.assertEqual(compute_nodes[0].hypervisor_hostname, + hypervisor['hypervisor_hostname']) + + _test(self) + + def test_show_compute_host_not_mapped(self): + """Tests that if a service is deleted but the compute node is not we + don't fail when listing hypervisors. + """ + + # two computes, a matching service only exists for the first one + compute_nodes = objects.ComputeNodeList(objects=[ + objects.ComputeNode(**TEST_HYPERS[0]), + objects.ComputeNode(**TEST_HYPERS[1]) + ]) + + def fake_service_get_by_compute_host(context, host): + return TEST_SERVICES[0] + + @mock.patch.object(self.controller.host_api, 'compute_node_get', + fake_service_get_by_compute_host) + @mock.patch.object(self.controller.host_api, + 'service_get_by_compute_host') + def _test(self, mock_service): + req = self._get_request(True) + mock_service.side_effect = exception.HostMappingNotFound( + name='foo') + self.assertRaises(exc.HTTPNotFound, self.controller.show, + req, compute_nodes[0].id) + self.assertTrue(mock_service.called) + _test(self) + def test_show_noid(self): req = self._get_request(True) self.assertRaises(exc.HTTPNotFound, self.controller.show, req, '3') @@ -447,12 +550,28 @@ class HypervisorsTestV21(test.NoDBTestCase): mock_get_uptime.assert_called_once_with( mock.ANY, self.TEST_HYPERS_OBJ[0].host) + def test_uptime_hypervisor_not_mapped_service_get(self): + @mock.patch.object(self.controller.host_api, 'compute_node_get') + @mock.patch.object(self.controller.host_api, 'get_host_uptime') + @mock.patch.object(self.controller.host_api, + 'service_get_by_compute_host', + side_effect=exception.HostMappingNotFound( + name='dummy')) + def _test(mock_get, _, __): + req = self._get_request(True) + self.assertRaises(exc.HTTPNotFound, + self.controller.uptime, req, + self.TEST_HYPERS_OBJ[0].id) + self.assertTrue(mock_get.called) + + _test() + def test_uptime_hypervisor_not_mapped(self): with mock.patch.object(self.controller.host_api, 'get_host_uptime', side_effect=exception.HostMappingNotFound(name='dummy') ) as mock_get_uptime: req = self._get_request(True) - self.assertRaises(exc.HTTPBadRequest, + self.assertRaises(exc.HTTPNotFound, self.controller.uptime, req, self.TEST_HYPERS_OBJ[0].id) mock_get_uptime.assert_called_once_with( @@ -479,6 +598,23 @@ class HypervisorsTestV21(test.NoDBTestCase): req, 'a') self.assertEqual(1, mock_node_search.call_count) + def test_search_unmapped(self): + + @mock.patch.object(self.controller.host_api, + 'compute_node_search_by_hypervisor') + @mock.patch.object(self.controller.host_api, + 'service_get_by_compute_host') + def _test(mock_service, mock_search): + mock_search.return_value = [mock.MagicMock()] + mock_service.side_effect = exception.HostMappingNotFound( + name='foo') + req = self._get_request(True) + self.assertRaises(exc.HTTPNotFound, self.controller.search, + req, 'a') + self.assertTrue(mock_service.called) + + _test() + @mock.patch.object(objects.InstanceList, 'get_by_host', side_effect=fake_instance_get_all_by_host) def test_servers(self, mock_get):