openstack
/
nova
Code Issues Proposed changes

Skip _remove_deleted_instances_allocations if compute is new 

If this is the first start of the compute service and the compute node
record does not exist, the resource provider won't exist either. So when
the ResourceTracker._remove_deleted_instances_allocations method is called
it's going to log an ERROR because get_allocations_for_resource_provider
will raise an error since the resource provider doesn't yet exist (that
happens later during RT._update() on the new compute node record).

We can avoid calling _remove_deleted_instances_allocations if we know the
compute node is newly created, so this adds handling for that case.

Tests are updated and an unnecessary mock is removed along the way.

Change-Id: I37e8ad5b14262d801702411c2c87e73550adda70
Closes-Bug: #1789998
This commit is contained in:
Matt Riedemann 2018-10-10 17:37:38 -04:00
parent 6bf11e1dc1
commit 418fc93a10
2 changed files with 63 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
nova/compute/resource_tracker.py
View File

@ -554,6 +554,8 @@ class ResourceTracker(object):
:param context: security context
:param resources: initial values
:returns: True if a new compute_nodes table record was created,
False otherwise
"""
nodename = resources['hypervisor_hostname']
@ -563,7 +565,7 @@ class ResourceTracker(object):
cn = self.compute_nodes[nodename]
self._copy_resources(cn, resources)
self._setup_pci_tracker(context, cn, resources)
return
return False
# now try to get the compute node record from the
# database. If we get one we use resources to initialize
@ -572,10 +574,10 @@ class ResourceTracker(object):
self.compute_nodes[nodename] = cn
self._copy_resources(cn, resources)
self._setup_pci_tracker(context, cn, resources)
return
return False
if self._check_for_nodes_rebalance(context, resources, nodename):
return
return False
# there was no local copy and none in the database
# so we need to create a new compute node. This needs
@ -590,6 +592,7 @@ class ResourceTracker(object):
{'host': self.host, 'node': nodename, 'uuid': cn.uuid})
self._setup_pci_tracker(context, cn, resources)
return True
def _setup_pci_tracker(self, context, compute_node, resources):
if not self.pci_tracker:
@ -745,7 +748,7 @@ class ResourceTracker(object):
# initialize the compute node object, creating it
# if it does not already exist.
self._init_compute_node(context, resources)
is_new_compute_node = self._init_compute_node(context, resources)
nodename = resources['hypervisor_hostname']
@ -772,9 +775,13 @@ class ResourceTracker(object):
self._pair_instances_to_migrations(migrations, instance_by_uuid)
self._update_usage_from_migrations(context, migrations, nodename)
self._remove_deleted_instances_allocations(
context, self.compute_nodes[nodename], migrations,
instance_by_uuid)
# A new compute node means there won't be a resource provider yet since
# that would be created via the _update() call below, and if there is
# no resource provider then there are no allocations against it.
if not is_new_compute_node:
self._remove_deleted_instances_allocations(
context, self.compute_nodes[nodename], migrations,
instance_by_uuid)
# Detect and account for orphaned instances that may exist on the
# hypervisor, but are not in the DB:

59
nova/tests/unit/compute/test_resource_tracker.py
View File

@ -585,11 +585,14 @@ class TestUpdateAvailableResources(BaseTestCase):
@mock.patch('nova.objects.ComputeNode.get_by_host_and_nodename')
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
def test_startup_makes_it_through(self, get_mock, migr_mock, get_cn_mock,
pci_mock, instance_pci_mock):
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
'_remove_deleted_instances_allocations')
def test_startup_makes_it_through(self, rdia, get_mock, migr_mock,
get_cn_mock, pci_mock,
instance_pci_mock):
"""Just make sure the startup kwarg makes it from
_update_available_resource all the way down the call stack to
_update.
_update. In this case a compute node record already exists.
"""
self._setup_rt()
@ -599,6 +602,40 @@ class TestUpdateAvailableResources(BaseTestCase):
update_mock = self._update_available_resources(startup=True)
update_mock.assert_called_once_with(mock.ANY, mock.ANY, startup=True)
rdia.assert_called_once_with(
mock.ANY, get_cn_mock.return_value,
[], {})
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
return_value=objects.PciDeviceList())
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
'_init_compute_node', return_value=True)
@mock.patch('nova.objects.MigrationList.get_in_progress_by_host_and_node')
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
'_remove_deleted_instances_allocations')
def test_startup_new_compute(self, rdia, get_mock, migr_mock, init_cn_mock,
pci_mock, instance_pci_mock):
"""Just make sure the startup kwarg makes it from
_update_available_resource all the way down the call stack to
_update. In this case a new compute node record is created.
"""
self._setup_rt()
cn = _COMPUTE_NODE_FIXTURES[0]
self.rt.compute_nodes[cn.hypervisor_hostname] = cn
mock_pci_tracker = mock.MagicMock()
mock_pci_tracker.stats.to_device_pools_obj.return_value = (
objects.PciDevicePoolList())
self.rt.pci_tracker = mock_pci_tracker
get_mock.return_value = []
migr_mock.return_value = []
update_mock = self._update_available_resources(startup=True)
update_mock.assert_called_once_with(mock.ANY, mock.ANY, startup=True)
rdia.assert_not_called()
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
return_value=objects.InstancePCIRequests(requests=[]))
@ -1025,7 +1062,8 @@ class TestInitComputeNode(BaseTestCase):
compute_node = copy.deepcopy(_COMPUTE_NODE_FIXTURES[0])
self.rt.compute_nodes[_NODENAME] = compute_node
self.rt._init_compute_node(mock.sentinel.ctx, resources)
self.assertFalse(
self.rt._init_compute_node(mock.sentinel.ctx, resources))
self.assertFalse(service_mock.called)
self.assertFalse(get_mock.called)
@ -1050,7 +1088,8 @@ class TestInitComputeNode(BaseTestCase):
get_mock.side_effect = fake_get_node
resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)
self.rt._init_compute_node(mock.sentinel.ctx, resources)
self.assertFalse(
self.rt._init_compute_node(mock.sentinel.ctx, resources))
get_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
@ -1078,7 +1117,8 @@ class TestInitComputeNode(BaseTestCase):
get_by_hypervisor_mock.side_effect = fake_get_all
resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES)
self.rt._init_compute_node(mock.sentinel.ctx, resources)
self.assertFalse(
self.rt._init_compute_node(mock.sentinel.ctx, resources))
get_mock.assert_called_once_with(mock.sentinel.ctx, _HOSTNAME,
_NODENAME)
@ -1206,7 +1246,8 @@ class TestInitComputeNode(BaseTestCase):
cn.uuid = uuids.compute_node_uuid
create_mock.side_effect = set_cn_id
self.rt._init_compute_node(mock.sentinel.ctx, resources)
self.assertTrue(
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,
@ -3091,12 +3132,10 @@ class TestUpdateUsageFromInstance(BaseTestCase):
cn = objects.ComputeNode(memory_mb=1024, local_gb=10)
self.rt.compute_nodes['foo'] = cn
@mock.patch.object(self.rt,
'_remove_deleted_instances_allocations')
@mock.patch.object(self.rt, '_update_usage_from_instance')
@mock.patch('nova.objects.Service.get_minimum_version',
return_value=22)
def test(version_mock, uufi, rdia):
def test(version_mock, uufi):
self.rt._update_usage_from_instances('ctxt', [], 'foo')
test()