From 26c5ee100fb7d75721355878ade9167ff88a70e6 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Wed, 24 Oct 2018 13:48:06 -0400 Subject: [PATCH] Provide allocation_ratio/reserved amounts from update_provider_tree() The purpose of the RT._normalize_inventory_from_cn_obj method is to set allocation_ratio and reserved amounts on standard resource class inventory records that get sent to placement if the virt driver did not specifically set a ratio or reserved value (which none but the ironic driver do). If the allocation_ratio or reserved amount is in the inventory data dict from the virt driver, then the normalize method ignores it and lets the virt driver take priority. However, with change I6a706ec5966cdc85f97223617662fe15d3e6dc08, any virt driver that implements the update_provider_tree() interface is storing the inventory data on the ProviderTree object which gets cached and re-used, meaning once allocation_ratio/reserved is set from RT._normalize_inventory_from_cn_obj, it doesn't get unset and the normalize method always assumes the driver provided a value which should not be changed, even if the configuration value changes. We can make the config option changes take effect by changing the semantics between _normalize_inventory_from_cn_obj and drivers that implement the update_provider_tree interface, like for the libvirt driver. Effectively with this change, when a driver implements update_provider_tree(), they now control setting the allocation_ratio and reserved resource amounts for inventory they report. The libvirt driver will use the same configuration option values that _normalize_inventory_from_cn_obj used. The only difference is in update_provider_tree we don't have the ComputeNode facade to get the "real" default values when the allocation_ratio is 0.0, so we handle that like "CONF.cpu_allocation_ratio or 16.0". Eventually that will get cleaned up with blueprint initial-allocation-ratios. Conflicts: nova/virt/driver.py NOTE(mriedem): The conflict is due to not having change Ic062446e5c620c89aec3065b34bcdc6bf5966275 in Rocky which changed the update_provider_tree() signature and docstring. Change-Id: I72c83a95dabd581998470edb9543079acb6536a5 Closes-Bug: #1799727 (cherry picked from commit ca279c68a54b054cb54d45ee9d2eed8ade9a6db5) --- doc/source/reference/update-provider-tree.rst | 7 +++++ nova/compute/resource_tracker.py | 4 +++ .../compute/test_resource_tracker.py | 31 ++++++++----------- nova/tests/unit/virt/libvirt/test_driver.py | 6 ++++ nova/virt/driver.py | 11 +++++++ nova/virt/fake.py | 10 ++++++ nova/virt/libvirt/driver.py | 14 ++++++--- 7 files changed, 61 insertions(+), 22 deletions(-) diff --git a/doc/source/reference/update-provider-tree.rst b/doc/source/reference/update-provider-tree.rst index e165f61104d5..bdaa29d20b7b 100644 --- a/doc/source/reference/update-provider-tree.rst +++ b/doc/source/reference/update-provider-tree.rst @@ -151,6 +151,13 @@ would become: } provider_tree.update_inventory(nodename, inv_data) +When reporting inventory for the standard resource classes ``VCPU``, +``MEMORY_MB`` and ``DISK_GB``, implementors of ``update_provider_tree`` may +need to set the ``allocation_ratio`` and ``reserved`` values in the +``inv_data`` dict based on configuration to reflect changes on the compute +for allocation ratios and reserved resource amounts back to the placement +service. + Porting from get_traits ~~~~~~~~~~~~~~~~~~~~~~~ To replace ``get_traits``, developers should use the diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index 3a21689e2d9c..8a8a25cf7d95 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -922,6 +922,10 @@ class ResourceTracker(object): # compute_node record if not set by the virt driver) because the # virt driver does not and will not have access to the compute_node inv_data = prov_tree.data(nodename).inventory + # TODO(mriedem): Stop calling _normalize_inventory_from_cn_obj when + # a virt driver implements update_provider_tree() since we expect + # the driver to manage the allocation ratios and reserved resource + # amounts. _normalize_inventory_from_cn_obj(inv_data, compute_node) prov_tree.update_inventory(nodename, inv_data) # Flush any changes. diff --git a/nova/tests/functional/compute/test_resource_tracker.py b/nova/tests/functional/compute/test_resource_tracker.py index e12824210859..c4500c54612d 100644 --- a/nova/tests/functional/compute/test_resource_tracker.py +++ b/nova/tests/functional/compute/test_resource_tracker.py @@ -498,6 +498,14 @@ class TestUpdateComputeNodeReservedAndAllocationRatio( CONF.reserved_host_disk_mb) } + def _assert_reserved_inventory(self, inventories): + reserved = self._get_reserved_host_values_from_config() + for rc, res in reserved.items(): + self.assertIn('reserved', inventories[rc]) + self.assertEqual(res, inventories[rc]['reserved'], + 'Unexpected resource provider inventory ' + 'reserved value for %s' % rc) + def test_update_inventory_reserved_and_allocation_ratio_from_conf(self): # Start a compute service which should create a corresponding resource # provider in the placement service. @@ -524,11 +532,7 @@ class TestUpdateComputeNodeReservedAndAllocationRatio( self.assertIn('allocation_ratio', inventories[rc]) self.assertEqual(ratio, inventories[rc]['allocation_ratio'], 'Unexpected allocation ratio for %s' % rc) - reserved = self._get_reserved_host_values_from_config() - for rc, res in reserved.items(): - self.assertIn('reserved', inventories[rc]) - self.assertEqual(res, inventories[rc]['reserved'], - 'Unexpected reserved value for %s' % rc) + self._assert_reserved_inventory(inventories) # Now change the configuration values, restart the compute service, # and ensure the changes are reflected in the resource provider @@ -566,18 +570,9 @@ class TestUpdateComputeNodeReservedAndAllocationRatio( ratio, getattr(cn, '%s_allocation_ratio' % attr_map[rc]), 'Unexpected ComputeNode allocation ratio for %s' % rc) # Make sure the values in placement are updated. - # FIXME(mriedem): The config-driven allocation ratio overrides - # are not reflected in placement until bug 1799727 is fixed. - self.assertNotEqual(ratio, inventories[rc]['allocation_ratio'], - 'Unexpected resource provider inventory ' - 'allocation ratio for %s' % rc) + self.assertEqual(ratio, inventories[rc]['allocation_ratio'], + 'Unexpected resource provider inventory ' + 'allocation ratio for %s' % rc) # The reserved host values should also come from config. - reserved = self._get_reserved_host_values_from_config() - for rc, res in reserved.items(): - self.assertIn('reserved', inventories[rc]) - # FIXME(mriedem): The config-driven reserved overrides - # are not reflected in placement until bug 1799727 is fixed. - self.assertNotEqual(res, inventories[rc]['reserved'], - 'Unexpected resource provider inventory ' - 'reserved value for %s' % rc) + self._assert_reserved_inventory(inventories) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index d86c1490e1ca..29abedd4b942 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -17982,18 +17982,24 @@ class TestUpdateProviderTree(test.NoDBTestCase): 'min_unit': 1, 'max_unit': self.vcpus, 'step_size': 1, + 'allocation_ratio': 16.0, + 'reserved': 0, }, rc_fields.ResourceClass.MEMORY_MB: { 'total': self.memory_mb, 'min_unit': 1, 'max_unit': self.memory_mb, 'step_size': 1, + 'allocation_ratio': 1.5, + 'reserved': 512, }, rc_fields.ResourceClass.DISK_GB: { 'total': self.disk_gb, 'min_unit': 1, 'max_unit': self.disk_gb, 'step_size': 1, + 'allocation_ratio': 1.0, + 'reserved': 0, }, } diff --git a/nova/virt/driver.py b/nova/virt/driver.py index a20b3cb3c362..d441416edb25 100644 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -859,6 +859,11 @@ class ComputeDriver(object): """ raise NotImplementedError() + @staticmethod + def _get_reserved_host_disk_gb_from_config(): + import nova.compute.utils as compute_utils # avoid circular import + return compute_utils.convert_mb_to_ceil_gb(CONF.reserved_host_disk_mb) + def update_provider_tree(self, provider_tree, nodename): """Update a ProviderTree object with current resource provider and inventory information. @@ -871,6 +876,12 @@ class ComputeDriver(object): This method supersedes get_inventory(): if this method is implemented, get_inventory() is not used. + Implementors of this interface are expected to set ``allocation_ratio`` + and ``reserved`` values for inventory records, which may be based on + configuration options, e.g. ``[DEFAULT]/cpu_allocation_ratio``, + depending on the driver and resource class. If not provided, allocation + ratio defaults to 1.0 and reserved defaults to 0 in placement. + :note: Renaming a provider (by deleting it from provider_tree and re-adding it with a different name) is not supported at this time. diff --git a/nova/virt/fake.py b/nova/virt/fake.py index c8ef69422dbc..af7fcedf7ce1 100644 --- a/nova/virt/fake.py +++ b/nova/virt/fake.py @@ -491,24 +491,34 @@ class FakeDriver(driver.ComputeDriver): return host_status def update_provider_tree(self, provider_tree, nodename): + # TODO(mriedem): The allocation_ratio config usage will change with + # blueprint initial-allocation-ratios. For now, the allocation ratio + # config values all default to 0.0 and the ComputeNode provides a + # facade for giving the real defaults, so we have to mimic that here. inventory = { 'VCPU': { 'total': self.vcpus, 'min_unit': 1, 'max_unit': self.vcpus, 'step_size': 1, + 'allocation_ratio': CONF.cpu_allocation_ratio or 16.0, + 'reserved': CONF.reserved_host_cpus, }, 'MEMORY_MB': { 'total': self.memory_mb, 'min_unit': 1, 'max_unit': self.memory_mb, 'step_size': 1, + 'allocation_ratio': CONF.ram_allocation_ratio or 1.5, + 'reserved': CONF.reserved_host_memory_mb, }, 'DISK_GB': { 'total': self.local_gb, 'min_unit': 1, 'max_unit': self.local_gb, 'step_size': 1, + 'allocation_ratio': CONF.disk_allocation_ratio or 1.0, + 'reserved': self._get_reserved_host_disk_gb_from_config(), }, } provider_tree.update_inventory(nodename, inventory) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 8183704a9599..6b2f3ae70294 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -6443,22 +6443,26 @@ class LibvirtDriver(driver.ComputeDriver): # TODO(sbauza): Use traits to make a better world. vgpus = self._get_vgpu_total() - # NOTE(jaypipes): We leave some fields like allocation_ratio and - # reserved out of the returned dicts here because, for now at least, - # the RT injects those values into the inventory dict based on the - # compute_nodes record values. + # TODO(mriedem): The allocation_ratio config usage will change with + # blueprint initial-allocation-ratios. For now, the allocation ratio + # config values all default to 0.0 and the ComputeNode provides a + # facade for giving the real defaults, so we have to mimic that here. result = { rc_fields.ResourceClass.VCPU: { 'total': vcpus, 'min_unit': 1, 'max_unit': vcpus, 'step_size': 1, + 'allocation_ratio': CONF.cpu_allocation_ratio or 16.0, + 'reserved': CONF.reserved_host_cpus, }, rc_fields.ResourceClass.MEMORY_MB: { 'total': memory_mb, 'min_unit': 1, 'max_unit': memory_mb, 'step_size': 1, + 'allocation_ratio': CONF.ram_allocation_ratio or 1.5, + 'reserved': CONF.reserved_host_memory_mb, }, } @@ -6474,6 +6478,8 @@ class LibvirtDriver(driver.ComputeDriver): 'min_unit': 1, 'max_unit': disk_gb, 'step_size': 1, + 'allocation_ratio': CONF.disk_allocation_ratio or 1.0, + 'reserved': self._get_reserved_host_disk_gb_from_config(), } if vgpus > 0: