diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index f7f74ce8643d..9993202f948d 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -1506,11 +1506,17 @@ class ResourceTracker(object): exc_info=False) continue - if instance.deleted: + # NOTE(mriedem): A cross-cell migration will work with instance + # records across two cells once the migration is confirmed/reverted + # one of them will be deleted but the instance still exists in the + # other cell. Before the instance is destroyed from the old cell + # though it is marked hidden=True so if we find a deleted hidden + # instance with allocations against this compute node we just + # ignore it since the migration operation will handle cleaning up + # those allocations. + if instance.deleted and not instance.hidden: # The instance is gone, so we definitely want to remove # allocations associated with it. - # NOTE(jaypipes): This will not be true if/when we support - # cross-cell migrations... LOG.debug("Instance %s has been deleted (perhaps locally). " "Deleting allocations that remained for this " "instance against this compute host: %s.", diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 7de59728e8fe..78d5772fae6e 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -3355,7 +3355,7 @@ class TestUpdateUsageFromInstance(BaseTestCase): return_value=allocs) rc.delete_allocation_for_instance = mock.MagicMock() mock_inst_get.return_value = objects.Instance( - uuid=uuids.deleted, deleted=True) + uuid=uuids.deleted, deleted=True, hidden=False) cn = self.rt.compute_nodes[_NODENAME] ctx = mock.MagicMock() # Call the method. @@ -3370,6 +3370,35 @@ class TestUpdateUsageFromInstance(BaseTestCase): expected_attrs=[]) ctx.elevated.assert_called_once_with(read_deleted='yes') + @mock.patch('nova.objects.Instance.get_by_uuid') + def test_remove_deleted_instances_allocations_deleted_hidden_instance(self, + mock_inst_get): + """Tests the scenario where there are allocations against the local + compute node held by a deleted instance but it is hidden=True so the + ResourceTracker does not delete the allocations because it assumes + the cross-cell resize flow will handle the allocations. + """ + rc = self.rt.reportclient + allocs = report.ProviderAllocInfo( + allocations={uuids.deleted: "fake_deleted_instance"}) + rc.get_allocations_for_resource_provider = mock.MagicMock( + return_value=allocs) + rc.delete_allocation_for_instance = mock.MagicMock() + cn = self.rt.compute_nodes[_NODENAME] + mock_inst_get.return_value = objects.Instance( + uuid=uuids.deleted, deleted=True, hidden=True, + host=cn.host, node=cn.hypervisor_hostname, + task_state=task_states.RESIZE_MIGRATING) + ctx = mock.MagicMock() + # Call the method. + self.rt._remove_deleted_instances_allocations(ctx, cn, [], {}) + # Only one call should be made to delete allocations, and that should + # be for the first instance created above + rc.delete_allocation_for_instance.assert_not_called() + mock_inst_get.assert_called_once_with( + ctx.elevated.return_value, uuids.deleted, expected_attrs=[]) + ctx.elevated.assert_called_once_with(read_deleted='yes') + @mock.patch('nova.objects.Instance.get_by_uuid') def test_remove_deleted_instances_allocations_building_instance(self, mock_inst_get): @@ -3401,7 +3430,7 @@ class TestUpdateUsageFromInstance(BaseTestCase): return_value=allocs) rc.delete_allocation_for_instance = mock.MagicMock() mock_inst_get.return_value = objects.Instance( - uuid=uuids.deleted, deleted=True) + uuid=uuids.deleted, deleted=True, hidden=False) cn = self.rt.compute_nodes[_NODENAME] ctx = mock.MagicMock() # Call the method.