Properly track local root disk usage during moves
Change I0839470c4bcfb16590a0d87b306d683b059bf8a9 fixed root disk usage tracking for volume-backed instances when performing an instance_claim which happens during the initial server create and unshelve. However, root disk reporting is still wrong for volume-backed instances during move claims (resize and evacuate) because a move claim calls _update_usage_from_migration which passes a Flavor object to ResourceTracker._get_usage_dict() and that method didn't have "is_bfv" logic in that scenario. This fixes the bug by always passing the instance object to the _get_usage_dict() method so we can determine if it's volume-backed and if so report the root_gb usage as 0. The related functional regression test is updated appropriately to show the bug is fixed for volume-backed instances. Change-Id: Ia19264ae7c88bb03ed3118795d4011ceb62ef92c Closes-Bug: #1796737
This commit is contained in:
Matt Riedemann
parent
cb36ca76df
commit
a99722bb85
nova
compute
tests
@ -459,7 +459,7 @@ class ResourceTracker(object):
|
||||
numa_topology = self._get_migration_context_resource(
|
||||
'numa_topology', instance, prefix=prefix)
|
||||
usage = self._get_usage_dict(
|
||||
instance_type, numa_topology=numa_topology)
|
||||
instance_type, instance, numa_topology=numa_topology)
|
||||
self._drop_pci_devices(instance, nodename, prefix)
|
||||
self._update_usage(usage, nodename, sign=-1)
|
||||
|
||||
@ -1084,7 +1084,7 @@ class ResourceTracker(object):
|
||||
if itype:
|
||||
cn = self.compute_nodes[nodename]
|
||||
usage = self._get_usage_dict(
|
||||
itype, numa_topology=numa_topology)
|
||||
itype, instance, numa_topology=numa_topology)
|
||||
if self.pci_tracker and sign:
|
||||
self.pci_tracker.update_pci_for_instance(
|
||||
context, instance, sign=sign)
|
||||
@ -1190,8 +1190,8 @@ class ResourceTracker(object):
|
||||
instance,
|
||||
sign=sign)
|
||||
# new instance, update compute node resource usage:
|
||||
self._update_usage(self._get_usage_dict(instance), nodename,
|
||||
sign=sign)
|
||||
self._update_usage(self._get_usage_dict(instance, instance),
|
||||
nodename, sign=sign)
|
||||
|
||||
# Stop tracking removed instances in the is_bfv cache. This needs to
|
||||
# happen *after* calling _get_usage_dict() since that relies on the
|
||||
@ -1444,13 +1444,16 @@ class ResourceTracker(object):
|
||||
# them. In that case - just get the instance flavor.
|
||||
return instance.flavor
|
||||
|
||||
def _get_usage_dict(self, object_or_dict, **updates):
|
||||
def _get_usage_dict(self, object_or_dict, instance, **updates):
|
||||
"""Make a usage dict _update methods expect.
|
||||
|
||||
Accepts a dict or an Instance or Flavor object, and a set of updates.
|
||||
Converts the object to a dict and applies the updates.
|
||||
|
||||
:param object_or_dict: instance or flavor as an object or just a dict
|
||||
:param instance: nova.objects.Instance for the related operation; this
|
||||
is needed to determine if the instance is
|
||||
volume-backed
|
||||
:param updates: key-value pairs to update the passed object.
|
||||
Currently only considers 'numa_topology', all other
|
||||
keys are ignored.
|
||||
@ -1458,15 +1461,20 @@ class ResourceTracker(object):
|
||||
:returns: a dict with all the information from object_or_dict updated
|
||||
with updates
|
||||
"""
|
||||
usage = {}
|
||||
if isinstance(object_or_dict, objects.Instance):
|
||||
|
||||
def _is_bfv():
|
||||
# Check to see if we have the is_bfv value cached.
|
||||
if object_or_dict.uuid in self.is_bfv:
|
||||
is_bfv = self.is_bfv[object_or_dict.uuid]
|
||||
if instance.uuid in self.is_bfv:
|
||||
is_bfv = self.is_bfv[instance.uuid]
|
||||
else:
|
||||
is_bfv = compute_utils.is_volume_backed_instance(
|
||||
object_or_dict._context, object_or_dict)
|
||||
self.is_bfv[object_or_dict.uuid] = is_bfv
|
||||
instance._context, instance)
|
||||
self.is_bfv[instance.uuid] = is_bfv
|
||||
return is_bfv
|
||||
|
||||
usage = {}
|
||||
if isinstance(object_or_dict, objects.Instance):
|
||||
is_bfv = _is_bfv()
|
||||
usage = {'memory_mb': object_or_dict.flavor.memory_mb,
|
||||
'swap': object_or_dict.flavor.swap,
|
||||
'vcpus': object_or_dict.flavor.vcpus,
|
||||
@ -1476,6 +1484,8 @@ class ResourceTracker(object):
|
||||
'numa_topology': object_or_dict.numa_topology}
|
||||
elif isinstance(object_or_dict, objects.Flavor):
|
||||
usage = obj_base.obj_to_primitive(object_or_dict)
|
||||
if _is_bfv():
|
||||
usage['root_gb'] = 0
|
||||
else:
|
||||
usage.update(object_or_dict)
|
||||
|
||||
|
||||
@ -23,9 +23,9 @@ class BootFromVolumeTest(integrated_helpers.InstanceHelperMixin,
|
||||
response = self.admin_api.api_get('/os-hypervisors/statistics')
|
||||
return response.body['hypervisor_statistics']
|
||||
|
||||
def _verify_zero_local_gb_used(self, expected=0):
|
||||
def _verify_zero_local_gb_used(self):
|
||||
stats = self._get_hypervisor_stats()
|
||||
self.assertEqual(expected, stats['local_gb_used'])
|
||||
self.assertEqual(0, stats['local_gb_used'])
|
||||
|
||||
def _verify_instance_flavor_not_zero(self, instance_uuid):
|
||||
# We are trying to avoid saving instance records with root_gb=0
|
||||
@ -88,8 +88,7 @@ class BootFromVolumeTest(integrated_helpers.InstanceHelperMixin,
|
||||
self._wait_for_state_change(self.api, created_server, 'VERIFY_RESIZE')
|
||||
|
||||
# Check that hypervisor local disk reporting is still 0
|
||||
# FIXME(mriedem): Expect 0 once bug 1796737 is fixed.
|
||||
self._verify_zero_local_gb_used(expected=16384)
|
||||
self._verify_zero_local_gb_used()
|
||||
# Check that instance has not been saved with 0 root_gb
|
||||
self._verify_instance_flavor_not_zero(server_id)
|
||||
# Check that request spec has not been saved with 0 root_gb
|
||||
@ -101,8 +100,7 @@ class BootFromVolumeTest(integrated_helpers.InstanceHelperMixin,
|
||||
self._wait_for_state_change(self.api, created_server, 'ACTIVE')
|
||||
|
||||
# Check that hypervisor local disk reporting is still 0
|
||||
# FIXME(mriedem): Expect 0 once bug 1796737 is fixed.
|
||||
self._verify_zero_local_gb_used(expected=8192)
|
||||
self._verify_zero_local_gb_used()
|
||||
# Check that instance has not been saved with 0 root_gb
|
||||
self._verify_instance_flavor_not_zero(server_id)
|
||||
# Check that request spec has not been saved with 0 root_gb
|
||||
@ -115,8 +113,7 @@ class BootFromVolumeTest(integrated_helpers.InstanceHelperMixin,
|
||||
'SHELVED_OFFLOADED')
|
||||
|
||||
# Check that hypervisor local disk reporting is still 0
|
||||
# FIXME(mriedem): Expect 0 once bug 1796737 is fixed.
|
||||
self._verify_zero_local_gb_used(expected=8192)
|
||||
self._verify_zero_local_gb_used()
|
||||
# Check that instance has not been saved with 0 root_gb
|
||||
self._verify_instance_flavor_not_zero(server_id)
|
||||
# Check that request spec has not been saved with 0 root_gb
|
||||
@ -128,8 +125,7 @@ class BootFromVolumeTest(integrated_helpers.InstanceHelperMixin,
|
||||
self._wait_for_state_change(self.api, created_server, 'ACTIVE')
|
||||
|
||||
# Check that hypervisor local disk reporting is still 0
|
||||
# FIXME(mriedem): Expect 0 once bug 1796737 is fixed.
|
||||
self._verify_zero_local_gb_used(expected=8192)
|
||||
self._verify_zero_local_gb_used()
|
||||
# Check that instance has not been saved with 0 root_gb
|
||||
self._verify_instance_flavor_not_zero(server_id)
|
||||
# Check that request spec has not been saved with 0 root_gb
|
||||
@ -144,8 +140,7 @@ class BootFromVolumeTest(integrated_helpers.InstanceHelperMixin,
|
||||
self._wait_for_state_change(self.api, created_server, 'ACTIVE')
|
||||
|
||||
# Check that hypervisor local disk reporting is still 0
|
||||
# FIXME(mriedem): Expect 0 once bug 1796737 is fixed.
|
||||
self._verify_zero_local_gb_used(expected=8192)
|
||||
self._verify_zero_local_gb_used()
|
||||
# Check that instance has not been saved with 0 root_gb
|
||||
self._verify_instance_flavor_not_zero(server_id)
|
||||
# Check that request spec has not been saved with 0 root_gb
|
||||
|
||||
@ -755,6 +755,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
||||
actual_resources))
|
||||
update_mock.assert_called_once()
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance',
|
||||
return_value=False)
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||
@ -765,7 +767,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
|
||||
def test_no_instances_source_migration(self, get_mock, get_inst_mock,
|
||||
migr_mock, get_cn_mock, pci_mock,
|
||||
instance_pci_mock):
|
||||
instance_pci_mock,
|
||||
mock_is_volume_backed_instance):
|
||||
# We test the behavior of update_available_resource() when
|
||||
# there is an active migration that involves this compute node
|
||||
# as the source host not the destination host, and the resource
|
||||
@ -818,6 +821,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
||||
actual_resources))
|
||||
update_mock.assert_called_once()
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance',
|
||||
return_value=False)
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||
@ -828,7 +833,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
|
||||
def test_no_instances_dest_migration(self, get_mock, get_inst_mock,
|
||||
migr_mock, get_cn_mock, pci_mock,
|
||||
instance_pci_mock):
|
||||
instance_pci_mock,
|
||||
mock_is_volume_backed_instance):
|
||||
# We test the behavior of update_available_resource() when
|
||||
# there is an active migration that involves this compute node
|
||||
# as the destination host not the source host, and the resource
|
||||
@ -878,6 +884,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
||||
actual_resources))
|
||||
update_mock.assert_called_once()
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance',
|
||||
return_value=False)
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
|
||||
return_value=objects.InstancePCIRequests(requests=[]))
|
||||
@mock.patch('nova.objects.PciDeviceList.get_by_compute_node',
|
||||
@ -888,7 +896,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
|
||||
def test_no_instances_dest_evacuation(self, get_mock, get_inst_mock,
|
||||
migr_mock, get_cn_mock, pci_mock,
|
||||
instance_pci_mock):
|
||||
instance_pci_mock,
|
||||
mock_is_volume_backed_instance):
|
||||
# We test the behavior of update_available_resource() when
|
||||
# there is an active evacuation that involves this compute node
|
||||
# as the destination host not the source host, and the resource
|
||||
@ -2266,6 +2275,8 @@ class TestResize(BaseTestCase):
|
||||
def test_instance_build_resize_confirm(self):
|
||||
self._test_instance_build_resize()
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance',
|
||||
return_value=False)
|
||||
@mock.patch('nova.objects.Service.get_minimum_version',
|
||||
return_value=22)
|
||||
@mock.patch('nova.pci.stats.PciDeviceStats.support_requests',
|
||||
@ -2280,7 +2291,8 @@ class TestResize(BaseTestCase):
|
||||
@mock.patch('nova.objects.ComputeNode.save')
|
||||
def test_resize_claim_dest_host_with_pci(self, save_mock, get_mock,
|
||||
migr_mock, get_cn_mock, pci_mock, pci_req_mock, pci_claim_mock,
|
||||
pci_dev_save_mock, pci_supports_mock, version_mock):
|
||||
pci_dev_save_mock, pci_supports_mock, version_mock,
|
||||
mock_is_volume_backed_instance):
|
||||
# Starting from an empty destination compute node, perform a resize
|
||||
# operation for an instance containing SR-IOV PCI devices on the
|
||||
# original host.
|
||||
@ -2407,6 +2419,8 @@ class TestResize(BaseTestCase):
|
||||
mock_pci_free_device.assert_called_once_with(
|
||||
pci_dev, mock.ANY)
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance',
|
||||
return_value=False)
|
||||
@mock.patch('nova.objects.Service.get_minimum_version',
|
||||
return_value=22)
|
||||
@mock.patch('nova.objects.InstancePCIRequests.get_by_instance',
|
||||
@ -2418,7 +2432,8 @@ class TestResize(BaseTestCase):
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host_and_node')
|
||||
@mock.patch('nova.objects.ComputeNode.save')
|
||||
def test_resize_claim_two_instances(self, save_mock, get_mock, migr_mock,
|
||||
get_cn_mock, pci_mock, instance_pci_mock, version_mock):
|
||||
get_cn_mock, pci_mock, instance_pci_mock, version_mock,
|
||||
mock_is_volume_backed_instance):
|
||||
# Issue two resize claims against a destination host with no prior
|
||||
# instances on it and validate that the accounting for resources is
|
||||
# correct.
|
||||
@ -2774,7 +2789,7 @@ class TestUpdateUsageFromInstance(BaseTestCase):
|
||||
mock_check_bfv.return_value = True
|
||||
# Make sure the cache is empty.
|
||||
self.assertNotIn(self.instance.uuid, self.rt.is_bfv)
|
||||
result = self.rt._get_usage_dict(self.instance)
|
||||
result = self.rt._get_usage_dict(self.instance, self.instance)
|
||||
self.assertEqual(0, result['root_gb'])
|
||||
mock_check_bfv.assert_called_once_with(
|
||||
self.instance._context, self.instance)
|
||||
@ -2784,7 +2799,7 @@ class TestUpdateUsageFromInstance(BaseTestCase):
|
||||
# Now run _get_usage_dict again to make sure we don't call
|
||||
# is_volume_backed_instance.
|
||||
mock_check_bfv.reset_mock()
|
||||
result = self.rt._get_usage_dict(self.instance)
|
||||
result = self.rt._get_usage_dict(self.instance, self.instance)
|
||||
self.assertEqual(0, result['root_gb'])
|
||||
mock_check_bfv.assert_not_called()
|
||||
|
||||
@ -2794,7 +2809,8 @@ class TestUpdateUsageFromInstance(BaseTestCase):
|
||||
mock_check_bfv.return_value = False
|
||||
instance_with_swap = self.instance.obj_clone()
|
||||
instance_with_swap.flavor.swap = 10
|
||||
result = self.rt._get_usage_dict(instance_with_swap)
|
||||
result = self.rt._get_usage_dict(
|
||||
instance_with_swap, instance_with_swap)
|
||||
self.assertEqual(10, result['swap'])
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance')
|
||||
@ -2807,7 +2823,8 @@ class TestUpdateUsageFromInstance(BaseTestCase):
|
||||
_NODENAME)
|
||||
|
||||
mock_update_usage.assert_called_once_with(
|
||||
self.rt._get_usage_dict(self.instance), _NODENAME, sign=1)
|
||||
self.rt._get_usage_dict(self.instance, self.instance),
|
||||
_NODENAME, sign=1)
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance')
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
@ -2826,7 +2843,8 @@ class TestUpdateUsageFromInstance(BaseTestCase):
|
||||
# The instance should have been removed from the is_bfv cache.
|
||||
self.assertNotIn(self.instance.uuid, self.rt.is_bfv)
|
||||
mock_update_usage.assert_called_once_with(
|
||||
self.rt._get_usage_dict(self.instance), _NODENAME, sign=-1)
|
||||
self.rt._get_usage_dict(self.instance, self.instance),
|
||||
_NODENAME, sign=-1)
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance')
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
@ -2838,7 +2856,8 @@ class TestUpdateUsageFromInstance(BaseTestCase):
|
||||
_NODENAME)
|
||||
|
||||
mock_update_usage.assert_called_once_with(
|
||||
self.rt._get_usage_dict(self.instance), _NODENAME, sign=1)
|
||||
self.rt._get_usage_dict(self.instance, self.instance),
|
||||
_NODENAME, sign=1)
|
||||
|
||||
@mock.patch('nova.compute.utils.is_volume_backed_instance')
|
||||
@mock.patch('nova.compute.resource_tracker.ResourceTracker.'
|
||||
@ -2853,7 +2872,8 @@ class TestUpdateUsageFromInstance(BaseTestCase):
|
||||
self.instance, _NODENAME, True)
|
||||
|
||||
mock_update_usage.assert_called_once_with(
|
||||
self.rt._get_usage_dict(self.instance), _NODENAME, sign=-1)
|
||||
self.rt._get_usage_dict(self.instance, self.instance),
|
||||
_NODENAME, sign=-1)
|
||||
|
||||
@mock.patch('nova.objects.Instance.get_by_uuid')
|
||||
def test_remove_deleted_instances_allocations_deleted_instance(self,
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user