diff --git a/nova/compute/manager.py b/nova/compute/manager.py index d035872689a6..1f45ac9b86e4 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4344,6 +4344,9 @@ class ComputeManager(manager.Manager): # NOTE(ndipanov): Free resources from the resource tracker self._update_resource_tracker(context, instance) + rt = self._get_resource_tracker() + rt.delete_allocation_for_shelve_offloaded_instance(instance) + # NOTE(sfinucan): RPC calls should no longer be attempted against this # instance, so ensure any calls result in errors self._nil_out_instance_obj_host_and_node(instance) diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 994f1ca82f36..0556eef549c0 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -1244,6 +1244,12 @@ class ResourceTracker(object): usage = {'memory_mb': memory_mb} self._update_usage(usage, nodename) + def delete_allocation_for_shelve_offloaded_instance(self, instance): + res = self.reportclient.delete_allocation_for_instance(instance.uuid) + if not res: + LOG.error("Failed to clean allocation of a shelve offloaded " + "instance", instance=instance) + def _verify_resources(self, resources): resource_keys = ["vcpus", "memory_mb", "local_gb", "cpu_info", "vcpus_used", "memory_mb_used", "local_gb_used", diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py index 82ac16b7d735..4cc25e06985f 100644 --- a/nova/tests/functional/test_servers.py +++ b/nova/tests/functional/test_servers.py @@ -1721,24 +1721,13 @@ class ServerMovingTests(test.TestCase, integrated_helpers.InstanceHelperMixin): self.api.post_server_action(server['id'], req) self._wait_for_state_change(self.api, server, 'SHELVED_OFFLOADED') source_usages = self._get_provider_usages(source_rp_uuid) - # NOTE(gibi): this is bug 1710249 where shelve offload doesn't free up - # the resources - self.assertFlavorMatchesAllocation(self.flavor1, source_usages) - # NOTE(gibi): after fixing bug 1710249 the following should be true - # after offload there should be no usages - # self.assertEqual({'VCPU': 0, - # 'MEMORY_MB': 0, - # 'DISK_GB': 0}, - # source_usages) - # NOTE(gibi): this is bug 1710249 where shelve offload doesn't free up - # the resources + self.assertEqual({'VCPU': 0, + 'MEMORY_MB': 0, + 'DISK_GB': 0}, + source_usages) + allocations = self._get_allocations_by_server_uuid(server['id']) - self.assertEqual(1, len(allocations)) - allocation = allocations[source_rp_uuid]['resources'] - self.assertFlavorMatchesAllocation(self.flavor1, allocation) - # NOTE(gibi): after fixing bug 1710249 the following should be true - # after offload there should be no allocations - # self.assertEqual(0, len(allocations)) + self.assertEqual(0, len(allocations)) def test_shelve_offload_unshelve_diff_host(self): source_hostname = self.compute1.host @@ -1772,19 +1761,10 @@ class ServerMovingTests(test.TestCase, integrated_helpers.InstanceHelperMixin): self.assertFlavorMatchesAllocation(self.flavor1, current_usages) allocations = self._get_allocations_by_server_uuid(server['id']) - # NOTE(gibi): this is bug 1710249 where shelve offload doesn't free up - # the resources - self.assertEqual(2, len(allocations)) - allocation = allocations[source_rp_uuid]['resources'] - self.assertFlavorMatchesAllocation(self.flavor1, allocation) + self.assertEqual(1, len(allocations)) allocation = allocations[current_rp_uuid]['resources'] self.assertFlavorMatchesAllocation(self.flavor1, allocation) - # NOTE(gibi): after fixing bug 1710249 the following should be true - # self.assertEqual(1, len(allocations)) - # allocation = allocations[current_rp_uuid]['resources'] - # self.assertFlavorMatchesAllocation(self.flavor1, allocation) - self._delete_and_check_allocations( server, source_rp_uuid, source_rp_uuid) @@ -1817,24 +1797,12 @@ class ServerMovingTests(test.TestCase, integrated_helpers.InstanceHelperMixin): # the host running the instance should have resource usage current_rp_uuid = self._get_provider_uuid_by_host(current_hostname) current_usages = self._get_provider_usages(current_rp_uuid) - # NOTE(gibi): this is bug 1710249 where shelve offload doesn't free up - # the resources so the host now have doubled allocation - self.assertFlavorsMatchAllocation( - self.flavor1, self.flavor1, current_usages) - - # NOTE(gibi): after fixing bug 1710249 the following should be true - # self.assertFlavorMatchesAllocation(self.flavor1, current_usages) + self.assertFlavorMatchesAllocation(self.flavor1, current_usages) allocations = self._get_allocations_by_server_uuid(server['id']) self.assertEqual(1, len(allocations)) allocation = allocations[current_rp_uuid]['resources'] - # NOTE(gibi): this is bug 1710249 where shelve offload doesn't free up - # the resources so the host now have doubled allocation - self.assertFlavorsMatchAllocation( - self.flavor1, self.flavor1, allocation) - - # NOTE(gibi): after fixing bug 1710249 the following should be true - # self.assertFlavorMatchesAllocation(self.flavor1, allocation) + self.assertFlavorMatchesAllocation(self.flavor1, allocation) self._delete_and_check_allocations( server, source_rp_uuid, source_rp_uuid) diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index e2c726c2bc91..485f2148d125 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -2539,6 +2539,16 @@ class TestUpdateUsageFromInstance(BaseTestCase): # instance that no longer exists. rc.delete_allocation_for_instance.assert_called_once_with(uuids.inst0) + def test_delete_allocation_for_shelve_offloaded_instance(self): + instance = _INSTANCE_FIXTURES[0].obj_clone() + instance.uuid = uuids.inst0 + + self.rt.delete_allocation_for_shelve_offloaded_instance(instance) + + rc = self.rt.reportclient + mock_remove_allocation = rc.delete_allocation_for_instance + mock_remove_allocation.assert_called_once_with(instance.uuid) + def test_update_usage_from_instances_goes_negative(self): # NOTE(danms): The resource tracker _should_ report negative resources # for things like free_ram_mb if overcommit is being used. This test diff --git a/nova/tests/unit/compute/test_shelve.py b/nova/tests/unit/compute/test_shelve.py index 1ef0e86a3b96..66928b3564bf 100644 --- a/nova/tests/unit/compute/test_shelve.py +++ b/nova/tests/unit/compute/test_shelve.py @@ -179,6 +179,8 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): instance = self._shelve_offload(clean_shutdown=False) mock_power_off.assert_called_once_with(instance, 0, 0) + @mock.patch('nova.compute.resource_tracker.ResourceTracker.' + 'delete_allocation_for_shelve_offloaded_instance') @mock.patch.object(nova.compute.manager.ComputeManager, '_update_resource_tracker') @mock.patch.object(nova.compute.manager.ComputeManager, @@ -188,7 +190,7 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): @mock.patch('nova.compute.utils.notify_about_instance_action') def _shelve_offload(self, mock_notify, mock_notify_instance_usage, mock_get_power_state, mock_update_resource_tracker, - clean_shutdown=True): + mock_delete_alloc, clean_shutdown=True): host = 'fake-mini' instance = self._create_fake_instance_obj(params={'host': host}) instance.task_state = task_states.SHELVING @@ -221,6 +223,7 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase): self.context, instance) mock_update_resource_tracker.assert_called_once_with(self.context, instance) + mock_delete_alloc.assert_called_once_with(instance) return instance