diff --git a/nova/api/openstack/compute/hypervisors.py b/nova/api/openstack/compute/hypervisors.py index 107996a18e8..8232fd9f4f1 100644 --- a/nova/api/openstack/compute/hypervisors.py +++ b/nova/api/openstack/compute/hypervisors.py @@ -172,18 +172,22 @@ def _get_hypervisors(self, req, detail=False, limit=None, marker=None, context, hyp.host) service = self.host_api.service_get_by_compute_host( context, hyp.host) - hypervisors_list.append( - self._view_hypervisor( - hyp, service, detail, req, servers=instances, - with_servers=with_servers)) - except (exception.ComputeHostNotFound, - exception.HostMappingNotFound): + 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. 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) + continue + + hypervisors_list.append( + self._view_hypervisor( + hyp, service, detail, req, servers=instances, + with_servers=with_servers)) hypervisors_dict = dict(hypervisors=hypervisors_list) if links: @@ -311,16 +315,30 @@ def _show(self, req, id, with_servers=False): try: hyp = self.host_api.compute_node_get(context, id) - instances = None - if with_servers: + except exception.ComputeHostNotFound: + # If the ComputeNode is missing, that's a straight up 404 + msg = _("Hypervisor with ID '%s' could not be found.") % id + raise webob.exc.HTTPNotFound(explanation=msg) + + instances = None + if with_servers: + try: instances = self.host_api.instance_get_all_by_host( context, hyp.host) + except exception.HostMappingNotFound: + msg = _("Hypervisor with ID '%s' could not be found.") % id + raise webob.exc.HTTPNotFound(explanation=msg) + + try: service = self.host_api.service_get_by_compute_host( context, hyp.host) - except (ValueError, exception.ComputeHostNotFound, - exception.HostMappingNotFound): + except ( + exception.ComputeHostNotFound, + exception.HostMappingNotFound, + ): msg = _("Hypervisor with ID '%s' could not be found.") % id raise webob.exc.HTTPNotFound(explanation=msg) + return dict(hypervisor=self._view_hypervisor( hyp, service, True, req, instances, with_servers)) @@ -333,18 +351,30 @@ def uptime(self, req, id): try: hyp = self.host_api.compute_node_get(context, id) - except (ValueError, exception.ComputeHostNotFound): + except exception.ComputeHostNotFound: + # If the ComputeNode is missing, that's a straight up 404 + msg = _("Hypervisor with ID '%s' could not be found.") % id + raise webob.exc.HTTPNotFound(explanation=msg) + + try: + service = self.host_api.service_get_by_compute_host( + context, hyp.host) + except ( + exception.ComputeHostNotFound, + exception.HostMappingNotFound, + ): msg = _("Hypervisor with ID '%s' could not be found.") % id raise webob.exc.HTTPNotFound(explanation=msg) # Get the uptime try: - host = hyp.host - uptime = self.host_api.get_host_uptime(context, host) - service = self.host_api.service_get_by_compute_host(context, host) + uptime = self.host_api.get_host_uptime(context, hyp.host) except NotImplementedError: common.raise_feature_not_supported() - except exception.ComputeServiceUnavailable as e: + except ( + exception.ComputeServiceUnavailable, + exception.HostNotFound, + ) as e: raise webob.exc.HTTPBadRequest(explanation=e.format_message()) except exception.HostMappingNotFound: # NOTE(danms): This mirrors the compute_node_get() behavior @@ -366,18 +396,33 @@ def search(self, req, id): """ context = req.environ['nova.context'] context.can(hv_policies.BASE_POLICY_NAME % 'search', target={}) - hypervisors = self._get_compute_nodes_by_name_pattern(context, id) - 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) + + # Get all compute nodes with a hypervisor_hostname that matches + # the given pattern. If none are found then it's a 404 error. + compute_nodes = self._get_compute_nodes_by_name_pattern(context, id) + + hypervisors = [] + for compute_node in compute_nodes: + try: + service = self.host_api.service_get_by_compute_host( + context, compute_node.host) + 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.', compute_node.host) + continue + except exception.HostMappingNotFound as e: + raise webob.exc.HTTPNotFound(explanation=e.format_message()) + + hypervisor = self._view_hypervisor( + compute_node, service, False, req) + hypervisors.append(hypervisor) + + return {'hypervisors': hypervisors} @wsgi.Controller.api_version('2.1', '2.52') @wsgi.expected_errors(404) @@ -390,20 +435,39 @@ def servers(self, req, id): """ context = req.environ['nova.context'] context.can(hv_policies.BASE_POLICY_NAME % 'servers', target={}) + + # Get all compute nodes with a hypervisor_hostname that matches + # the given pattern. If none are found then it's a 404 error. compute_nodes = self._get_compute_nodes_by_name_pattern(context, id) + hypervisors = [] for compute_node in compute_nodes: try: instances = self.host_api.instance_get_all_by_host(context, compute_node.host) + except exception.HostMappingNotFound as e: + raise webob.exc.HTTPNotFound(explanation=e.format_message()) + + try: service = self.host_api.service_get_by_compute_host( context, compute_node.host) + 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.', compute_node.host) + continue except exception.HostMappingNotFound as e: raise webob.exc.HTTPNotFound(explanation=e.format_message()) - hyp = self._view_hypervisor(compute_node, service, False, req, - instances) - hypervisors.append(hyp) - return dict(hypervisors=hypervisors) + + hypervisor = self._view_hypervisor( + compute_node, service, False, req, instances) + hypervisors.append(hypervisor) + + return {'hypervisors': hypervisors} @wsgi.expected_errors(()) def statistics(self, req): diff --git a/nova/tests/unit/api/openstack/compute/test_hypervisors.py b/nova/tests/unit/api/openstack/compute/test_hypervisors.py index 16ab33f523b..f46bca98b40 100644 --- a/nova/tests/unit/api/openstack/compute/test_hypervisors.py +++ b/nova/tests/unit/api/openstack/compute/test_hypervisors.py @@ -133,7 +133,13 @@ def fake_compute_node_search_by_hypervisor(context, hypervisor_re): def fake_compute_node_get(context, compute_id): for hyper in TEST_HYPERS_OBJ: - if hyper.uuid == compute_id or hyper.id == int(compute_id): + if hyper.uuid == compute_id: + return hyper + + if ( + (isinstance(compute_id, int) or compute_id.isdigit()) and + hyper.id == int(compute_id) + ): return hyper raise exception.ComputeHostNotFound(host=compute_id) @@ -543,33 +549,49 @@ def test_show_withid(self): self.assertEqual(dict(hypervisor=self.DETAIL_HYPERS_DICTS[0]), result) + def test_uptime(self): + with mock.patch.object( + self.controller.host_api, 'get_host_uptime', + return_value="fake uptime" + ) as mock_get_uptime: + req = self._get_request(True) + hyper_id = self._get_hyper_id() + + result = self.controller.uptime(req, hyper_id) + + expected_dict = copy.deepcopy(self.INDEX_HYPER_DICTS[0]) + expected_dict.update({'uptime': "fake uptime"}) + self.assertEqual(dict(hypervisor=expected_dict), result) + self.assertEqual(1, mock_get_uptime.call_count) + def test_uptime_noid(self): req = self._get_request(True) hyper_id = uuids.hyper3 if self.expect_uuid_for_id else '3' self.assertRaises(exc.HTTPNotFound, self.controller.uptime, req, hyper_id) - def test_uptime_notimplemented(self): - with mock.patch.object(self.controller.host_api, 'get_host_uptime', - side_effect=exc.HTTPNotImplemented() - ) as mock_get_uptime: + def test_uptime_not_implemented(self): + with mock.patch.object( + self.controller.host_api, 'get_host_uptime', + side_effect=NotImplementedError, + ) as mock_get_uptime: req = self._get_request(True) hyper_id = self._get_hyper_id() - self.assertRaises(exc.HTTPNotImplemented, - self.controller.uptime, req, hyper_id) + self.assertRaises( + exc.HTTPNotImplemented, + self.controller.uptime, req, hyper_id) self.assertEqual(1, mock_get_uptime.call_count) - def test_uptime_implemented(self): - with mock.patch.object(self.controller.host_api, 'get_host_uptime', - return_value="fake uptime" - ) as mock_get_uptime: + def test_uptime_host_not_found(self): + with mock.patch.object( + self.controller.host_api, 'get_host_uptime', + side_effect=exception.HostNotFound('foo'), + ) as mock_get_uptime: req = self._get_request(True) hyper_id = self._get_hyper_id() - result = self.controller.uptime(req, hyper_id) - - expected_dict = copy.deepcopy(self.INDEX_HYPER_DICTS[0]) - expected_dict.update({'uptime': "fake uptime"}) - self.assertEqual(dict(hypervisor=expected_dict), result) + self.assertRaises( + exc.HTTPBadRequest, + self.controller.uptime, req, hyper_id) self.assertEqual(1, mock_get_uptime.call_count) def test_uptime_non_integer_id(self): @@ -668,11 +690,31 @@ def test_servers(self, mock_get): def test_servers_not_mapped(self): req = self._get_request(True) - with mock.patch.object(self.controller.host_api, - 'instance_get_all_by_host') as m: - m.side_effect = exception.HostMappingNotFound(name='something') - self.assertRaises(exc.HTTPNotFound, - self.controller.servers, req, 'hyper') + with mock.patch.object( + self.controller.host_api, 'instance_get_all_by_host', + side_effect=exception.HostMappingNotFound(name='something'), + ): + self.assertRaises( + exc.HTTPNotFound, + self.controller.servers, req, 'hyper') + + def test_servers_compute_host_not_found(self): + req = self._get_request(True) + + with test.nested( + mock.patch.object( + self.controller.host_api, 'instance_get_all_by_host', + side_effect=fake_instance_get_all_by_host, + ), + mock.patch.object( + self.controller.host_api, 'service_get_by_compute_host', + side_effect=exception.ComputeHostNotFound(host='foo'), + ), + ): + # The result should be empty since every attempt to fetch the + # service for a hypervisor "failed" + result = self.controller.servers(req, 'hyper') + self.assertEqual({'hypervisors': []}, result) def test_servers_non_id(self): with mock.patch.object(self.controller.host_api, @@ -1021,6 +1063,26 @@ def test_servers_not_mapped(self): result = self.controller.index(req) self.assertEqual(dict(hypervisors=[]), result) + def test_servers_compute_host_not_found(self): + req = self._get_request( + use_admin_context=True, + url='/os-hypervisors?with_servers=1') + + with test.nested( + mock.patch.object( + self.controller.host_api, 'instance_get_all_by_host', + side_effect=fake_instance_get_all_by_host, + ), + mock.patch.object( + self.controller.host_api, 'service_get_by_compute_host', + side_effect=exception.ComputeHostNotFound(host='foo'), + ), + ): + # The result should be empty since every attempt to fetch the + # service for a hypervisor "failed" + result = self.controller.index(req) + self.assertEqual({'hypervisors': []}, result) + def test_list_with_servers(self): """Tests GET /os-hypervisors?with_servers=True""" instances = [