ironic: stop lying to the RT when ironic is down

Returning an empty list of nodes can cause all sorts of crazy behavior,
so we instead bubble up a VirtDriverNotReady exception, which the compute
manager will ignore.

Change-Id: Ib0ec1012b74e9a9e74c8879f3feed5f9332b711f
Related-Bug: #1744139
Closes-Bug: #1750450
This commit is contained in:
Jim Rollenhagen 2018-03-16 16:33:20 +00:00
parent 7cbb5764d4
commit acab8b0067
5 changed files with 39 additions and 5 deletions

View File

@ -7317,7 +7317,12 @@ class ComputeManager(manager.Manager):
compute_nodes_in_db = self._get_compute_nodes_in_db(context, compute_nodes_in_db = self._get_compute_nodes_in_db(context,
use_slave=True, use_slave=True,
startup=startup) startup=startup)
nodenames = set(self.driver.get_available_nodes()) try:
nodenames = set(self.driver.get_available_nodes())
except exception.VirtDriverNotReady:
LOG.warning("Virt driver is not ready.")
return
for nodename in nodenames: for nodename in nodenames:
self.update_available_resource_for_node(context, nodename) self.update_available_resource_for_node(context, nodename)

View File

@ -2299,3 +2299,7 @@ class CannotMigrateWithTargetHost(NovaException):
class CannotMigrateToSameHost(NovaException): class CannotMigrateToSameHost(NovaException):
msg_fmt = _("Cannot migrate to the host where the server exists.") msg_fmt = _("Cannot migrate to the host where the server exists.")
class VirtDriverNotReady(NovaException):
msg_fmt = _("Virt driver is not ready.")

View File

@ -250,6 +250,26 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
else: else:
self.assertFalse(db_node.destroy.called) self.assertFalse(db_node.destroy.called)
@mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
'delete_resource_provider')
@mock.patch.object(manager.ComputeManager,
'update_available_resource_for_node')
@mock.patch.object(fake_driver.FakeDriver, 'get_available_nodes')
@mock.patch.object(manager.ComputeManager, '_get_compute_nodes_in_db')
def test_update_available_resource_not_ready(self, get_db_nodes,
get_avail_nodes,
update_mock,
del_rp_mock):
db_nodes = [self._make_compute_node('node1', 1)]
get_db_nodes.return_value = db_nodes
get_avail_nodes.side_effect = exception.VirtDriverNotReady
self.compute.update_available_resource(self.context)
# these shouldn't get processed on VirtDriverNotReady
update_mock.assert_not_called()
del_rp_mock.assert_not_called()
@mock.patch('nova.context.get_admin_context') @mock.patch('nova.context.get_admin_context')
def test_pre_start_hook(self, get_admin_context): def test_pre_start_hook(self, get_admin_context):
"""Very simple test just to make sure update_available_resource is """Very simple test just to make sure update_available_resource is

View File

@ -534,10 +534,10 @@ class IronicDriverTestCase(test.NoDBTestCase):
@mock.patch.object(objects.Instance, 'get_by_uuid') @mock.patch.object(objects.Instance, 'get_by_uuid')
def test_list_instances_fail(self, mock_inst_by_uuid, mock_call): def test_list_instances_fail(self, mock_inst_by_uuid, mock_call):
mock_call.side_effect = exception.NovaException mock_call.side_effect = exception.NovaException
response = self.driver.list_instances() self.assertRaises(exception.VirtDriverNotReady,
self.driver.list_instances)
mock_call.assert_called_with("node.list", associated=True, limit=0) mock_call.assert_called_with("node.list", associated=True, limit=0)
self.assertFalse(mock_inst_by_uuid.called) self.assertFalse(mock_inst_by_uuid.called)
self.assertThat(response, matchers.HasLength(0))
@mock.patch.object(cw.IronicClientWrapper, 'call') @mock.patch.object(cw.IronicClientWrapper, 'call')
def test_list_instance_uuids(self, mock_call): def test_list_instance_uuids(self, mock_call):
@ -2311,9 +2311,9 @@ class IronicDriverTestCase(test.NoDBTestCase):
result = self.driver._get_node_list() result = self.driver._get_node_list()
mock_error.assert_not_called() mock_error.assert_not_called()
self.assertEqual(["node1", "node2"], result) self.assertEqual(["node1", "node2"], result)
result = self.driver._get_node_list() self.assertRaises(exception.VirtDriverNotReady,
self.driver._get_node_list)
mock_error.assert_called_once() mock_error.assert_called_once()
self.assertEqual([], result)
class IronicDriverSyncTestCase(IronicDriverTestCase): class IronicDriverSyncTestCase(IronicDriverTestCase):

View File

@ -576,6 +576,7 @@ class IronicDriver(virt_driver.ComputeDriver):
If unable to connect ironic server, an empty list is returned. If unable to connect ironic server, an empty list is returned.
:returns: a list of raw node from ironic :returns: a list of raw node from ironic
:raises: VirtDriverNotReady
""" """
node_list = [] node_list = []
@ -584,15 +585,18 @@ class IronicDriver(virt_driver.ComputeDriver):
except exception.NovaException as e: except exception.NovaException as e:
LOG.error("Failed to get the list of nodes from the Ironic " LOG.error("Failed to get the list of nodes from the Ironic "
"inventory. Error: %s", e) "inventory. Error: %s", e)
raise exception.VirtDriverNotReady()
except Exception as e: except Exception as e:
LOG.error("An unknown error has occurred when trying to get the " LOG.error("An unknown error has occurred when trying to get the "
"list of nodes from the Ironic inventory. Error: %s", e) "list of nodes from the Ironic inventory. Error: %s", e)
raise exception.VirtDriverNotReady()
return node_list return node_list
def list_instances(self): def list_instances(self):
"""Return the names of all the instances provisioned. """Return the names of all the instances provisioned.
:returns: a list of instance names. :returns: a list of instance names.
:raises: VirtDriverNotReady
""" """
# NOTE(lucasagomes): limit == 0 is an indicator to continue # NOTE(lucasagomes): limit == 0 is an indicator to continue
@ -607,6 +611,7 @@ class IronicDriver(virt_driver.ComputeDriver):
"""Return the UUIDs of all the instances provisioned. """Return the UUIDs of all the instances provisioned.
:returns: a list of instance UUIDs. :returns: a list of instance UUIDs.
:raises: VirtDriverNotReady
""" """
# NOTE(lucasagomes): limit == 0 is an indicator to continue # NOTE(lucasagomes): limit == 0 is an indicator to continue