From 47a28bc4c392d38b438963a4e3447d24ec8a5294 Mon Sep 17 00:00:00 2001 From: Eric Fried Date: Wed, 31 Jan 2018 18:06:17 -0600 Subject: [PATCH] Ensure resource classes correctly Previously, SchedulerReportClient.set_inventory_for_provider used this logic to pre-create custom resource classes found in the input inventory. list(map(self._ensure_resource_class, (rc_name for rc_name in inv_data if rc_name not in fields.ResourceClass.STANDARD))) ...where _ensure_resource_class would always attempt to create the resource class it was given, and raise an exception if that failed for any reason. Note in particular that attempting to create an already-extant "standard" resource class will fail just the same as attempting to create any nonexistent resource class that doesn't conform to the schema (i.e. beginning with "CUSTOM_"). The problem is that this relies on the local system's notion of the set of standard resource classes. If the placement service is running newer code, standard resource classes may have been added that the compute service doesn't know about yet. This change set solves the problem by only attempting to create resource classes with the 'CUSTOM_' prefix. self._ensure_resource_class (which worked on a single resource class) is replaced with self._ensure_resource_classes (plural) which accepts a list of *all* the desired resource classes (including the standard ones), filters down to the CUSTOM_* ones, and attempts to create the remainder. Note that if placement ever decides to start allowing creation of resource classes with prefixes other than CUSTOM_, it will have to do so under a new microversion, so we can't future-proof this by e.g. prefetching all the known resource classes from placement and attempting to create any not found in that list. In the process of doing this rework, obsolete code paths relying on pre-1.7 placement microversions are removed. Change-Id: I600ed262e1b5d11facbf461e28291193665280cf Closes-Bug: #1746615 --- .../openstack/placement/test_report_client.py | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/nova/tests/functional/api/openstack/placement/test_report_client.py b/nova/tests/functional/api/openstack/placement/test_report_client.py index 93b925c79..056ab6551 100644 --- a/nova/tests/functional/api/openstack/placement/test_report_client.py +++ b/nova/tests/functional/api/openstack/placement/test_report_client.py @@ -202,7 +202,7 @@ class SchedulerReportClientTests(test.TestCase): # Try setting some invalid inventory and make sure the report # client raises the expected error. inv_data = { - 'BAD_FOO': { + 'CUSTOM_BOGU$_CLA$$': { 'total': 100, 'reserved': 0, 'min_unit': 1, @@ -274,18 +274,8 @@ class SchedulerReportClientTests(test.TestCase): } with interceptor.RequestsInterceptor(app=self.app, url=self.url): self.client.update_compute_node(self.context, self.compute_node) - # Simulate that our locally-running code has an outdated notion of - # standard resource classes. - with mock.patch.object(fields.ResourceClass, 'STANDARD', - ('VCPU', 'MEMORY_MB', 'DISK_GB')): - # TODO(efried): Once bug #1746615 is fixed, this will no longer - # raise, and can be replaced with: - # self.client.set_inventory_for_provider( - # self.context, self.compute_uuid, self.compute_name, inv) - self.assertRaises( - exception.InvalidResourceClass, - self.client.set_inventory_for_provider, - self.context, self.compute_uuid, self.compute_name, inv) + self.client.set_inventory_for_provider( + self.context, self.compute_uuid, self.compute_name, inv) @mock.patch('keystoneauth1.session.Session.get_endpoint', return_value='http://localhost:80/placement')