rt: only map compute node if we created it
If ComputeNode.create() fails, the update_available_resource periodic will not try to create it again because it will be mapped in the compute_nodes dict and _init_compute_node will return early but trying to save changes to that ComputeNode object later will fail because there is no id on the object, since we failed to create it in the DB. This simply reverses the logic such that we only map the compute node if we successfully created it. Some existing _init_compute_node testing had to be changed since it relied on the order of when the ComputeNode object is created and put into the compute_nodes dict in order to pass the object along to some much lower-level PCI tracker code, which was arguably mocking too deep for a unit test. That is changed to avoid the low-level mocking and assertions and just assert that _setup_pci_tracker is called as expected. Conflicts: nova/compute/resource_tracker.py nova/tests/unit/compute/test_resource_tracker.py NOTE(mriedem): The resource_tracker.py conflict is due to not having I14a310b20bd9892e7b34464e6baad49bf5928ece in Rocky. The test conflicts are due to not having change I37e8ad5b14262d801702411c2c87e73550adda70 in Rocky. Change-Id: I9fa1d509a3de405d6246fb8670612c65c10cc93b Closes-Bug: #1839674 (cherry picked from commitf578146f37
) (cherry picked from commit648770bd68
)
This commit is contained in:
parent
72f9aa720f
commit
35273a844a
|
@ -580,8 +580,12 @@ class ResourceTracker(object):
|
|||
cn = objects.ComputeNode(context)
|
||||
cn.host = self.host
|
||||
self._copy_resources(cn, resources)
|
||||
self.compute_nodes[nodename] = cn
|
||||
cn.create()
|
||||
# Only map the ComputeNode into compute_nodes if create() was OK
|
||||
# because if create() fails, on the next run through here nodename
|
||||
# would be in compute_nodes and we won't try to create again (because
|
||||
# of the logic above).
|
||||
self.compute_nodes[nodename] = cn
|
||||
LOG.info('Compute node record created for '
|
||||
'%(host)s:%(node)s with uuid: %(uuid)s',
|
||||
{'host': self.host, 'node': nodename, 'uuid': cn.uuid})
|
||||
|
|
|
@ -1074,23 +1074,18 @@ class TestInitComputeNode(BaseTestCase):
|
|||
self.assertEqual(_HOSTNAME, self.rt.compute_nodes[_NODENAME].host)
|
||||
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||
return_value=objects.PciDeviceList(objects=[]))
|
||||
@mock.patch('nova.objects.ComputeNode.create')
|
||||
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
'_update')
|
||||
def test_compute_node_created_on_empty(self, update_mock, get_mock,
|
||||
create_mock, pci_tracker_mock,
|
||||
create_mock,
|
||||
get_by_hypervisor_mock):
|
||||
get_by_hypervisor_mock.return_value = []
|
||||
self._test_compute_node_created(update_mock, get_mock, create_mock,
|
||||
pci_tracker_mock,
|
||||
get_by_hypervisor_mock)
|
||||
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||
return_value=objects.PciDeviceList(objects=[]))
|
||||
@mock.patch('nova.objects.ComputeNode.create')
|
||||
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
|
@ -1098,32 +1093,26 @@ class TestInitComputeNode(BaseTestCase):
|
|||
def test_compute_node_created_on_empty_rebalance(self, update_mock,
|
||||
get_mock,
|
||||
create_mock,
|
||||
pci_tracker_mock,
|
||||
get_by_hypervisor_mock):
|
||||
get_by_hypervisor_mock.return_value = []
|
||||
self._test_compute_node_created(update_mock, get_mock, create_mock,
|
||||
pci_tracker_mock,
|
||||
get_by_hypervisor_mock,
|
||||
rebalances_nodes=True)
|
||||
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||
return_value=objects.PciDeviceList(objects=[]))
|
||||
@mock.patch('nova.objects.ComputeNode.create')
|
||||
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
'_update')
|
||||
def test_compute_node_created_too_many(self, update_mock, get_mock,
|
||||
create_mock, pci_tracker_mock,
|
||||
create_mock,
|
||||
get_by_hypervisor_mock):
|
||||
get_by_hypervisor_mock.return_value = ["fake_node_1", "fake_node_2"]
|
||||
self._test_compute_node_created(update_mock, get_mock, create_mock,
|
||||
pci_tracker_mock,
|
||||
get_by_hypervisor_mock,
|
||||
rebalances_nodes=True)
|
||||
|
||||
def _test_compute_node_created(self, update_mock, get_mock,
|
||||
create_mock, pci_tracker_mock,
|
||||
def _test_compute_node_created(self, update_mock, get_mock, create_mock,
|
||||
get_by_hypervisor_mock,
|
||||
rebalances_nodes=False):
|
||||
self.flags(cpu_allocation_ratio=1.0, ram_allocation_ratio=1.0,
|
||||
|
@ -1152,12 +1141,12 @@ class TestInitComputeNode(BaseTestCase):
|
|||
'current_workload': 0,
|
||||
'vcpus': 4,
|
||||
'running_vms': 0,
|
||||
'pci_passthrough_devices': '[]'
|
||||
'pci_passthrough_devices': '[]',
|
||||
'uuid': uuids.compute_node_uuid
|
||||
}
|
||||
# The expected compute represents the initial values used
|
||||
# when creating a compute node.
|
||||
expected_compute = objects.ComputeNode(
|
||||
id=42,
|
||||
host_ip=resources['host_ip'],
|
||||
vcpus=resources['vcpus'],
|
||||
memory_mb=resources['memory_mb'],
|
||||
|
@ -1177,20 +1166,11 @@ class TestInitComputeNode(BaseTestCase):
|
|||
cpu_allocation_ratio=1.0,
|
||||
disk_allocation_ratio=1.0,
|
||||
stats={'failed_builds': 0},
|
||||
pci_device_pools=objects.PciDevicePoolList(objects=[]),
|
||||
uuid=uuids.compute_node_uuid
|
||||
)
|
||||
|
||||
def set_cn_id():
|
||||
# The PCI tracker needs the compute node's ID when starting up, so
|
||||
# make sure that we set the ID value so we don't get a Cannot load
|
||||
# 'id' in base class error
|
||||
cn = self.rt.compute_nodes[_NODENAME]
|
||||
cn.id = 42 # Has to be a number, not a mock
|
||||
cn.uuid = uuids.compute_node_uuid
|
||||
|
||||
create_mock.side_effect = set_cn_id
|
||||
self.rt._init_compute_node(mock.sentinel.ctx, resources)
|
||||
with mock.patch.object(self.rt, '_setup_pci_tracker') as setup_pci:
|
||||
self.rt._init_compute_node(mock.sentinel.ctx, resources)
|
||||
|
||||
cn = self.rt.compute_nodes[_NODENAME]
|
||||
get_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
|
||||
|
@ -1202,22 +1182,47 @@ class TestInitComputeNode(BaseTestCase):
|
|||
get_by_hypervisor_mock.assert_not_called()
|
||||
create_mock.assert_called_once_with()
|
||||
self.assertTrue(obj_base.obj_equal_prims(expected_compute, cn))
|
||||
pci_tracker_mock.assert_called_once_with(mock.sentinel.ctx,
|
||||
42)
|
||||
setup_pci.assert_called_once_with(mock.sentinel.ctx, cn, resources)
|
||||
self.assertFalse(update_mock.called)
|
||||
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
'_setup_pci_tracker')
|
||||
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename',
|
||||
side_effect=exc.ComputeHostNotFound(host=_HOSTNAME))
|
||||
@mock.patch('nova.objects.ComputeNode.create',
|
||||
side_effect=(test.TestingException, None))
|
||||
def test_compute_node_create_fail_retry_works(self, mock_create, mock_get,
|
||||
mock_setup_pci):
|
||||
"""Tests that _init_compute_node will not save the ComputeNode object
|
||||
in the compute_nodes dict if create() fails.
|
||||
"""
|
||||
self._setup_rt()
|
||||
self.assertEqual({}, self.rt.compute_nodes)
|
||||
ctxt = context.get_context()
|
||||
# The first ComputeNode.create fails so rt.compute_nodes should
|
||||
# remain empty.
|
||||
resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)
|
||||
resources['uuid'] = uuids.cn_uuid # for the LOG.info message
|
||||
self.assertRaises(test.TestingException,
|
||||
self.rt._init_compute_node, ctxt, resources)
|
||||
self.assertEqual({}, self.rt.compute_nodes)
|
||||
# Second create works so compute_nodes should have a mapping.
|
||||
self.rt._init_compute_node(ctxt, resources)
|
||||
self.assertIn(_NODENAME, self.rt.compute_nodes)
|
||||
mock_get.assert_has_calls([mock.call(
|
||||
ctxt, _HOSTNAME, _NODENAME)] * 2)
|
||||
self.assertEqual(2, mock_create.call_count)
|
||||
mock_setup_pci.assert_called_once_with(
|
||||
ctxt, test.MatchType(objects.ComputeNode), resources)
|
||||
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_by_hypervisor')
|
||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||
return_value=objects.PciDeviceList(objects=[]))
|
||||
@mock.patch('nova.objects.ComputeNode.create')
|
||||
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
'_update')
|
||||
def test_node_removed(self, update_mock, get_mock,
|
||||
create_mock, pci_tracker_mock,
|
||||
get_by_hypervisor_mock):
|
||||
create_mock, get_by_hypervisor_mock):
|
||||
self._test_compute_node_created(update_mock, get_mock, create_mock,
|
||||
pci_tracker_mock,
|
||||
get_by_hypervisor_mock)
|
||||
self.rt.old_resources[_NODENAME] = mock.sentinel.foo
|
||||
self.assertIn(_NODENAME, self.rt.compute_nodes)
|
||||
|
|
Loading…
Reference in New Issue