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
(cherry picked from commit a99722bb85
)
This commit is contained in:
parent
d4ec96611c
commit
980013d700
|
@ -457,7 +457,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)
|
||||
|
||||
|
@ -1066,7 +1066,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)
|
||||
|
@ -1176,8 +1176,8 @@ class ResourceTracker(object):
|
|||
self.reportclient.update_instance_allocation(context, cn,
|
||||
instance, 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
|
||||
|
@ -1486,13 +1486,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.
|
||||
|
@ -1500,15 +1503,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,
|
||||
|
@ -1518,6 +1526,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
|
||||
|
|
|
@ -726,6 +726,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
|||
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
|
||||
actual_resources))
|
||||
|
||||
@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',
|
||||
|
@ -736,7 +738,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
|
||||
|
@ -788,6 +791,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
|||
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
|
||||
actual_resources))
|
||||
|
||||
@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',
|
||||
|
@ -798,7 +803,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
|
||||
|
@ -847,6 +853,8 @@ class TestUpdateAvailableResources(BaseTestCase):
|
|||
self.assertTrue(obj_base.obj_equal_prims(expected_resources,
|
||||
actual_resources))
|
||||
|
||||
@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',
|
||||
|
@ -857,7 +865,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
|
||||
|
@ -2232,6 +2241,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',
|
||||
|
@ -2246,7 +2257,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.
|
||||
|
@ -2373,6 +2385,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',
|
||||
|
@ -2384,7 +2398,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.
|
||||
|
@ -2740,7 +2755,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)
|
||||
|
@ -2750,7 +2765,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()
|
||||
|
||||
|
@ -2760,7 +2775,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')
|
||||
|
@ -2773,7 +2789,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.'
|
||||
|
@ -2792,7 +2809,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.'
|
||||
|
@ -2804,7 +2822,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.'
|
||||
|
@ -2819,7 +2838,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…
Reference in New Issue