Merge "Handle ComputeHostNotFound when listing hypervisors"

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(
+        hypervisors_list = []
-                            hyp,
+        for hyp in compute_nodes:
-                            self.host_api.service_get_by_compute_host(
+            try:
-                                context, hyp.host),
+                service = self.host_api.service_get_by_compute_host(
-                            False, req)
+                    context, hyp.host)
-                            for hyp in compute_nodes]
+                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 = [
+        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, self.host_api.service_get_by_compute_host(context, hyp.host),
+                        hyp, service, True, req))
-            True, req) for hyp in compute_nodes]
+            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(

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')