Handle ComputeHostNotFound when listing hypervisors
Compute node resources must currently be deleted manually in the database, and as such they can reference service records which have been deleted via the services delete API. Because of this when listing hypervisors (compute nodes), we may get a ComputeHostNotFound error when trying to lookup a service record for a compute node where the service was deleted. This causes the API to fail with a 500 since it's not handled. This change handles the ComputeHostNotFound when looping over compute nodes in the hypervisors index and detail methods and simply ignores them. Change-Id: I2717274bb1bd370870acbf58c03dc59cee30cc5e Closes-Bug: #1646255
This commit is contained in:
Matt Riedemann
parent
474c2ef282
commit
f0d44c5b09
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
|
|
|
|||
Loading…
Reference in New Issue