openstack/nova
Code Issues Proposed changes

Clean up resources at shelve offload

Browse Source 
Resource tracker doesn't clean up the resources of an instance after
it is offloaded since Ia93168b1560267178059284186fb2b7096c7e81f. So
this patch proposes an explicit cleanup in compute manager.

Closes-Bug: #1710249
Change-Id: Ib16e987d50162a4c11a31692704859b981915bed
This commit is contained in:
Balazs Gibizer
2017-08-15 10:24:04 +02:00
parent 13c39b1e79
commit 563d71f7cc
5 changed files with 32 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
nova/compute/manager.py
View File

@@ -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)

6
nova/compute/resource_tracker.py
View File

@@ -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",

50
nova/tests/functional/test_servers.py
View File

@@ -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)

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

@@ -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

5
nova/tests/unit/compute/test_shelve.py
View File

@@ -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