Merge "Ensure resource classes correctly"

nova/scheduler/client/report.py

@@ -1057,9 +1057,7 @@ class SchedulerReportClient(object):
             parent_provider_uuid=parent_provider_uuid)
 
         # Auto-create custom resource classes coming from a virt driver
-        for rc_name in inv_data:
+        self._ensure_resource_classes(context, set(inv_data))
-            if rc_name not in fields.ResourceClass.STANDARD:
-                self._ensure_resource_class(context, rc_name)
 
         if inv_data:
             self._update_inventory(context, rp_uuid, inv_data)
@@ -1221,34 +1219,38 @@ class SchedulerReportClient(object):
         raise exception.ResourceProviderUpdateFailed(url=url, error=resp.text)
 
     @safe_connect
-    def _ensure_resource_class(self, context, name):
+    def _ensure_resource_classes(self, context, names):
-        """Make sure a custom resource class exists.
+        """Make sure resource classes exist.
-
-        PUT the resource class using microversion 1.7.
-
-        Returns the name of the resource class if it was successfully
-        created or already exists. Otherwise None.
 
         :param context: The security context
-        :param name: String name of the resource class to check/create.
+        :param names: Iterable of string names of the resource classes to
-        :raises: `exception.InvalidResourceClass` upon error.
+                      check/create.  Must not be None.
+        :raises: exception.InvalidResourceClass if an attempt is made to create
+                 an invalid resource class.
         """
-        # no payload on the put request
+        # Placement API version that supports PUT /resource_classes/CUSTOM_*
-        response = self.put("/resource_classes/%s" % name, None, version="1.7",
+        # to create (or validate the existence of) a consumer-specified
-                            global_request_id=context.global_id)
+        # resource class.
-        if 200 <= response.status_code < 300:
+        version = '1.7'
-            return name
+        to_ensure = set(n for n in names
-        else:
+                        if n.startswith(fields.ResourceClass.CUSTOM_NAMESPACE))
-            msg = ("Failed to ensure resource class record with placement API "
+
-                   "for resource class %(rc_name)s. Got %(status_code)d: "
+        for name in to_ensure:
-                   "%(err_text)s.")
+            # no payload on the put request
-            args = {
+            resp = self.put(
-                'rc_name': name,
+                "/resource_classes/%s" % name, None, version=version,
-                'status_code': response.status_code,
+                global_request_id=context.global_id)
-                'err_text': response.text,
+            if not resp:
-            }
+                msg = ("Failed to ensure resource class record with placement "
-            LOG.error(msg, args)
+                       "API for resource class %(rc_name)s. Got "
-            raise exception.InvalidResourceClass(resource_class=name)
+                       "%(status_code)d: %(err_text)s.")
+                args = {
+                    'rc_name': name,
+                    'status_code': resp.status_code,
+                    'err_text': resp.text,
+                }
+                LOG.error(msg, args)
+                raise exception.InvalidResourceClass(resource_class=name)
 
     def update_compute_node(self, context, compute_node):
         """Creates or updates stats for the supplied compute node.

nova/tests/functional/api/openstack/placement/test_report_client.py

@@ -207,7 +207,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,
@@ -279,18 +279,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
+            self.client.set_inventory_for_provider(
-            # standard resource classes.
+                self.context, self.compute_uuid, self.compute_name, inv)
-            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)
 
     @mock.patch('keystoneauth1.session.Session.get_endpoint',
                 return_value='http://localhost:80/placement')

nova/tests/unit/scheduler/client/test_report.py

@@ -3119,7 +3119,7 @@ There was a conflict when trying to complete your request.
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_delete_inventory')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_ensure_resource_class')
+                '_ensure_resource_classes')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_ensure_resource_provider')
     def test_set_inventory_for_provider_no_custom(self, mock_erp, mock_erc,
@@ -3166,7 +3166,8 @@ There was a conflict when trying to complete your request.
             parent_provider_uuid=None,
         )
         # No custom resource classes to ensure...
-        self.assertFalse(mock_erc.called)
+        mock_erc.assert_called_once_with(self.context,
+                                         set(['VCPU', 'MEMORY_MB', 'DISK_GB']))
         mock_upd.assert_called_once_with(
             self.context,
             mock.sentinel.rp_uuid,
@@ -3179,7 +3180,7 @@ There was a conflict when trying to complete your request.
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_delete_inventory')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_ensure_resource_class')
+                '_ensure_resource_classes')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_ensure_resource_provider')
     def test_set_inventory_for_provider_no_inv(self, mock_erp, mock_erc,
@@ -3200,7 +3201,7 @@ There was a conflict when trying to complete your request.
             mock.sentinel.rp_name,
             parent_provider_uuid=None,
         )
-        self.assertFalse(mock_erc.called)
+        mock_erc.assert_called_once_with(self.context, set())
         self.assertFalse(mock_upd.called)
         mock_del.assert_called_once_with(self.context, mock.sentinel.rp_uuid)
 
@@ -3209,7 +3210,7 @@ There was a conflict when trying to complete your request.
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_delete_inventory')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_ensure_resource_class')
+                '_ensure_resource_classes')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_ensure_resource_provider')
     def test_set_inventory_for_provider_with_custom(self, mock_erp,
@@ -3265,7 +3266,9 @@ There was a conflict when trying to complete your request.
             mock.sentinel.rp_name,
             parent_provider_uuid=None,
         )
-        mock_erc.assert_called_once_with(self.context, 'CUSTOM_IRON_SILVER')
+        mock_erc.assert_called_once_with(
+            self.context,
+            set(['VCPU', 'MEMORY_MB', 'DISK_GB', 'CUSTOM_IRON_SILVER']))
         mock_upd.assert_called_once_with(
             self.context,
             mock.sentinel.rp_uuid,
@@ -3276,7 +3279,7 @@ There was a conflict when trying to complete your request.
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_delete_inventory', new=mock.Mock())
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
-                '_ensure_resource_class', new=mock.Mock())
+                '_ensure_resource_classes', new=mock.Mock())
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 '_ensure_resource_provider')
     def test_set_inventory_for_provider_with_parent(self, mock_erp):
@@ -3588,3 +3591,39 @@ class TestAllocations(SchedulerReportClientTestCase):
         # With a 409, only the error should be called
         self.assertEqual(0, mock_log.info.call_count)
         self.assertEqual(1, mock_log.error.call_count)
+
+
+class TestResourceClass(SchedulerReportClientTestCase):
+    def setUp(self):
+        super(TestResourceClass, self).setUp()
+        _put_patch = mock.patch(
+            "nova.scheduler.client.report.SchedulerReportClient.put")
+        self.addCleanup(_put_patch.stop)
+        self.mock_put = _put_patch.start()
+
+    def test_ensure_resource_classes(self):
+        rcs = ['VCPU', 'CUSTOM_FOO', 'MEMORY_MB', 'CUSTOM_BAR']
+        self.client._ensure_resource_classes(self.context, rcs)
+        self.mock_put.assert_has_calls([
+            mock.call('/resource_classes/%s' % rc, None, version='1.7',
+                      global_request_id=self.context.global_id)
+            for rc in ('CUSTOM_FOO', 'CUSTOM_BAR')
+        ], any_order=True)
+
+    def test_ensure_resource_classes_none(self):
+        for empty in ([], (), set(), {}):
+            self.client._ensure_resource_classes(self.context, empty)
+            self.mock_put.assert_not_called()
+
+    def test_ensure_resource_classes_put_fail(self):
+        resp = requests.Response()
+        resp.status_code = 503
+        self.mock_put.return_value = resp
+        rcs = ['VCPU', 'MEMORY_MB', 'CUSTOM_BAD']
+        self.assertRaises(
+            exception.InvalidResourceClass,
+            self.client._ensure_resource_classes, self.context, rcs)
+        # Only called with the "bad" one
+        self.mock_put.assert_called_once_with(
+            '/resource_classes/CUSTOM_BAD', None, version='1.7',
+            global_request_id=self.context.global_id)