diff --git a/nova/api/openstack/compute/hypervisors.py b/nova/api/openstack/compute/hypervisors.py index f20c97da8010..ce28fe2d8f66 100644 --- a/nova/api/openstack/compute/hypervisors.py +++ b/nova/api/openstack/compute/hypervisors.py @@ -15,6 +15,7 @@ """The hypervisors admin extension.""" +from oslo_log import log as logging from oslo_serialization import jsonutils import webob.exc @@ -29,6 +30,7 @@ from nova.i18n import _ from nova.policies import hypervisors as hv_policies from nova import servicegroup +LOG = logging.getLogger(__name__) ALIAS = "os-hypervisors" @@ -108,12 +110,21 @@ class HypervisorsController(wsgi.Controller): msg = _('marker [%s] not found') % marker raise webob.exc.HTTPBadRequest(explanation=msg) req.cache_db_compute_nodes(compute_nodes) - hypervisors_list = [self._view_hypervisor( - hyp, - self.host_api.service_get_by_compute_host( - context, hyp.host), - False, req) - for hyp in compute_nodes] + hypervisors_list = [] + for hyp in compute_nodes: + try: + service = self.host_api.service_get_by_compute_host( + context, hyp.host) + hypervisors_list.append( + self._view_hypervisor( + hyp, service, False, req)) + except exception.ComputeHostNotFound: + # 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. + LOG.debug('Unable to find service for compute node %s. The ' + 'service may be deleted and compute nodes need to ' + 'be manually cleaned up.', hyp.host) hypervisors_dict = dict(hypervisors=hypervisors_list) if links: @@ -145,10 +156,21 @@ class HypervisorsController(wsgi.Controller): msg = _('marker [%s] not found') % marker raise webob.exc.HTTPBadRequest(explanation=msg) req.cache_db_compute_nodes(compute_nodes) - hypervisors_list = [ - self._view_hypervisor( - hyp, self.host_api.service_get_by_compute_host(context, hyp.host), - True, req) for hyp in compute_nodes] + hypervisors_list = [] + for hyp in compute_nodes: + try: + service = self.host_api.service_get_by_compute_host( + context, hyp.host) + hypervisors_list.append( + self._view_hypervisor( + hyp, service, True, req)) + except exception.ComputeHostNotFound: + # 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. + LOG.debug('Unable to find service for compute node %s. The ' + 'service may be deleted and compute nodes need to ' + 'be manually cleaned up.', hyp.host) hypervisors_dict = dict(hypervisors=hypervisors_list) if links: hypervisors_links = self._view_builder.get_links( diff --git a/nova/tests/unit/api/openstack/compute/test_hypervisors.py b/nova/tests/unit/api/openstack/compute/test_hypervisors.py index 743c8f35e510..56478efa4cdc 100644 --- a/nova/tests/unit/api/openstack/compute/test_hypervisors.py +++ b/nova/tests/unit/api/openstack/compute/test_hypervisors.py @@ -289,6 +289,41 @@ class HypervisorsTestV21(test.NoDBTestCase): self.assertRaises(exception.PolicyNotAuthorized, self.controller.index, req) + def test_index_compute_host_not_found(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.ComputeHostNotFound(host=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.assertTrue(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) @@ -300,6 +335,49 @@ class HypervisorsTestV21(test.NoDBTestCase): self.assertRaises(exception.PolicyNotAuthorized, self.controller.detail, req) + def test_detail_compute_host_not_found(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.ComputeHostNotFound(host=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.assertTrue(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_noid(self): req = self._get_request(True) self.assertRaises(exc.HTTPNotFound, self.controller.show, req, '3')