Scheduler set_inventory_for_provider does nested

SchedulerReportClient.set_inventory_for_provider and its SchedulerClient
wrapper now accept a parent_provider_uuid kwarg, which must be specified
for any provider that isn't a root.  If the method winds up creating the
provider, and parent_provider_uuid is None (the default), the provider
is created as a root - this is the previous behavior.  If
parent_provider_uuid is specified, and the method winds up creating the
provider, it is created as a child of the provider indicated.

Change-Id: I7cfbc80a9a41e97623950deaab9a7b0604fa487d
blueprint: nested-resource-providers

nova/scheduler/client/__init__.py

@@ -57,11 +57,13 @@ class SchedulerClient(object):
     def delete_aggregate(self, context, aggregate):
         self.queryclient.delete_aggregate(context, aggregate)
 
-    def set_inventory_for_provider(self, rp_uuid, rp_name, inv_data):
+    def set_inventory_for_provider(self, rp_uuid, rp_name, inv_data,
+                                   parent_provider_uuid=None):
         self.reportclient.set_inventory_for_provider(
             rp_uuid,
             rp_name,
             inv_data,
+            parent_provider_uuid=parent_provider_uuid,
         )
 
     def update_compute_node(self, compute_node):

nova/scheduler/client/report.py

@@ -491,7 +491,8 @@ class SchedulerReportClient(object):
         LOG.error(msg, args)
         raise exception.ResourceProviderCreationFailed(name=name)
 
-    def _ensure_resource_provider(self, uuid, name=None):
+    def _ensure_resource_provider(self, uuid, name=None,
+                                  parent_provider_uuid=None):
         """Ensures that the placement API has a record of a resource provider
         with the supplied UUID. If not, creates the resource provider record in
         the placement API for the supplied UUID, passing in a name for the
@@ -510,7 +511,11 @@ class SchedulerReportClient(object):
         :param name: Optional name for the resource provider if the record
                      does not exist. If empty, the name is set to the UUID
                      value
+        :param parent_provider_uuid: Optional UUID of the immediate parent
         """
+        # NOTE(efried): We currently have no code path where we need to set the
+        # parent_provider_uuid on a previously-parent-less provider - so we do
+        # NOT handle that scenario here.
         if self._provider_tree.exists(uuid):
             self._refresh_aggregate_map(uuid)
             return uuid
@@ -519,13 +524,25 @@ class SchedulerReportClient(object):
         # the placement API.
         rp = self._get_resource_provider(uuid)
         if rp is None:
-            rp = self._create_resource_provider(uuid, name or uuid)
+            rp = self._create_resource_provider(
+                uuid, name or uuid, parent_provider_uuid=parent_provider_uuid)
 
         # If there had been no resource provider record, force refreshing
         # the aggregate map.
         self._refresh_aggregate_map(uuid, force=True)
 
-        return self._provider_tree.new_root(rp['name'], uuid, rp['generation'])
+        # If this is a root node (no parent), create it as such
+        if parent_provider_uuid is None:
+            return self._provider_tree.new_root(
+                rp['name'], uuid, rp['generation'])
+
+        # Not a root - we have to insert it into the proper place in the tree.
+        # NOTE(efried): We populate self._provider_tree from the top down, so
+        # we can count on the parent being in the tree - we don't have to
+        # retrieve it from placement.
+        return self._provider_tree.new_child(rp['name'], parent_provider_uuid,
+                                             uuid=uuid,
+                                             generation=rp['generation'])
 
     def _get_inventory(self, rp_uuid):
         url = '/resource_providers/%s/inventories' % rp_uuid
@@ -799,7 +816,8 @@ class SchedulerReportClient(object):
         msg_args['err'] = r.text
         LOG.error(msg, msg_args)
 
-    def set_inventory_for_provider(self, rp_uuid, rp_name, inv_data):
+    def set_inventory_for_provider(self, rp_uuid, rp_name, inv_data,
+                                   parent_provider_uuid=None):
         """Given the UUID of a provider, set the inventory records for the
         provider to the supplied dict of resources.
 
@@ -808,11 +826,16 @@ class SchedulerReportClient(object):
                         a record for it in the placement API
         :param inv_data: Dict, keyed by resource class name, of inventory data
                          to set against the provider
+        :param parent_provider_uuid:
+                If the provider is not a root, this is required, and represents
+                the UUID of the immediate parent, which is a provider for which
+                this method has already been invoked.
 
         :raises: exc.InvalidResourceClass if a supplied custom resource class
                  name does not meet the placement API's format requirements.
         """
-        self._ensure_resource_provider(rp_uuid, rp_name)
+        self._ensure_resource_provider(
+            rp_uuid, rp_name, parent_provider_uuid=parent_provider_uuid)
 
         # Auto-create custom resource classes coming from a virt driver
         list(map(self._ensure_resource_class,

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

@@ -1155,8 +1155,8 @@ class TestProviderOperations(SchedulerReportClientTestCase):
             self.client._ensure_resource_provider, uuids.compute_node)
 
         get_rp_mock.assert_called_once_with(uuids.compute_node)
-        create_rp_mock.assert_called_once_with(uuids.compute_node,
-                                               uuids.compute_node)
+        create_rp_mock.assert_called_once_with(
+            uuids.compute_node, uuids.compute_node, parent_provider_uuid=None)
         self.assertFalse(self.client._provider_tree.exists(uuids.compute_node))
         self.assertFalse(get_agg_mock.called)
         self.assertEqual({}, self.client._provider_aggregate_map)
@@ -1178,7 +1178,11 @@ class TestProviderOperations(SchedulerReportClientTestCase):
             'name': 'compute-name',
             'generation': 1,
         }
-        self.client._ensure_resource_provider(uuids.compute_node)
+        self.assertEqual(
+            uuids.compute_node,
+            self.client._ensure_resource_provider(uuids.compute_node))
+        self._validate_provider(uuids.compute_node, name='compute-name',
+                                generation=1, parent_uuid=None)
 
         get_agg_mock.assert_called_once_with(uuids.compute_node)
         self.assertIn(uuids.compute_node, self.client._provider_aggregate_map)
@@ -1190,16 +1194,77 @@ class TestProviderOperations(SchedulerReportClientTestCase):
         create_rp_mock.assert_called_once_with(
                 uuids.compute_node,
                 uuids.compute_node,  # name param defaults to UUID if None
+                parent_provider_uuid=None,
         )
         self.assertTrue(self.client._provider_tree.exists(uuids.compute_node))
 
         create_rp_mock.reset_mock()
 
-        self.client._ensure_resource_provider(uuids.compute_node,
-                                              'compute-name')
+        self.assertEqual(
+            uuids.compute_node,
+            self.client._ensure_resource_provider(uuids.compute_node))
+        self._validate_provider(uuids.compute_node, name='compute-name',
+                                generation=1, parent_uuid=None)
+
         # Shouldn't be called now that provider is in cache...
         self.assertFalse(create_rp_mock.called)
 
+        # Validate the path where we specify a name (don't default to the UUID)
+        self.client._ensure_resource_provider(uuids.cn2, 'a-name')
+        create_rp_mock.assert_called_once_with(
+                uuids.cn2, 'a-name', parent_provider_uuid=None)
+
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
+                '_refresh_aggregate_map', new=mock.Mock())
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
+                '_create_resource_provider')
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
+                '_get_resource_provider')
+    def test_ensure_resource_provider_tree(self, get_rp_mock, create_rp_mock):
+        """Test _ensure_resource_provider with a tree of providers."""
+        def _create_resource_provider(uuid, name, parent_provider_uuid=None):
+            """Mock side effect for creating the RP with the specified args."""
+            return {
+                'uuid': uuid,
+                'name': name,
+                'generation': 0,
+                'parent_provider_uuid': parent_provider_uuid
+            }
+        create_rp_mock.side_effect = _create_resource_provider
+
+        # Not initially in the placement database, so we have to create it.
+        get_rp_mock.return_value = None
+
+        # Create the root
+        root = self.client._ensure_resource_provider(uuids.root)
+        self.assertEqual(uuids.root, root)
+
+        # Now create a child
+        child1 = self.client._ensure_resource_provider(
+            uuids.child1, name='junior', parent_provider_uuid=uuids.root)
+        self.assertEqual(uuids.child1, child1)
+
+        # If we re-ensure the child, we get the object from the tree, not a
+        # newly-created one - i.e. the early .find() works like it should.
+        self.assertIs(child1,
+                      self.client._ensure_resource_provider(uuids.child1))
+
+        # Make sure we can create a grandchild
+        grandchild = self.client._ensure_resource_provider(
+            uuids.grandchild, parent_provider_uuid=uuids.child1)
+        self.assertEqual(uuids.grandchild, grandchild)
+
+        # Now create a second child of the root and make sure it doesn't wind
+        # up in some crazy wrong place like under child1 or grandchild
+        child2 = self.client._ensure_resource_provider(
+            uuids.child2, parent_provider_uuid=uuids.root)
+        self.assertEqual(uuids.child2, child2)
+
+        # At this point we should get all the providers.
+        self.assertEqual(
+            set([uuids.root, uuids.child1, uuids.child2, uuids.grandchild]),
+            self.client._provider_tree.get_provider_uuids())
+
     def test_get_allocation_candidates(self):
         resp_mock = mock.Mock(status_code=200)
         json_data = {
@@ -2455,6 +2520,7 @@ There was a conflict when trying to complete your request.
         mock_erp.assert_called_once_with(
             mock.sentinel.rp_uuid,
             mock.sentinel.rp_name,
+            parent_provider_uuid=None,
         )
         # No custom resource classes to ensure...
         self.assertFalse(mock_erc.called)
@@ -2489,6 +2555,7 @@ There was a conflict when trying to complete your request.
         mock_erp.assert_called_once_with(
             mock.sentinel.rp_uuid,
             mock.sentinel.rp_name,
+            parent_provider_uuid=None,
         )
         self.assertFalse(mock_gocr.called)
         self.assertFalse(mock_erc.called)
@@ -2554,6 +2621,7 @@ There was a conflict when trying to complete your request.
         mock_erp.assert_called_once_with(
             mock.sentinel.rp_uuid,
             mock.sentinel.rp_name,
+            parent_provider_uuid=None,
         )
         mock_erc.assert_called_once_with('CUSTOM_IRON_SILVER')
         mock_upd.assert_called_once_with(
@@ -2563,6 +2631,19 @@ There was a conflict when trying to complete your request.
         self.assertFalse(mock_gocr.called)
         self.assertFalse(mock_del.called)
 
+    @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())
+    @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
+                '_ensure_resource_provider')
+    def test_set_inventory_for_provider_with_parent(self, mock_erp):
+        """Ensure parent UUID is sent through."""
+        self.client.set_inventory_for_provider(
+            uuids.child, 'junior', {}, parent_provider_uuid=uuids.parent)
+        mock_erp.assert_called_once_with(
+            uuids.child, 'junior', parent_provider_uuid=uuids.parent)
+
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'
                 'put')
     @mock.patch('nova.scheduler.client.report.SchedulerReportClient.'

nova/tests/unit/scheduler/test_client.py

@@ -114,4 +114,19 @@ class SchedulerClientTestCase(test.NoDBTestCase):
             mock.sentinel.rp_uuid,
             mock.sentinel.rp_name,
             mock.sentinel.inv_data,
+            parent_provider_uuid=None,
+        )
+        # Pass the optional parent_provider_uuid
+        mock_set.reset_mock()
+        self.client.set_inventory_for_provider(
+            mock.sentinel.child_uuid,
+            mock.sentinel.child_name,
+            mock.sentinel.inv_data2,
+            parent_provider_uuid=mock.sentinel.rp_uuid,
+        )
+        mock_set.assert_called_once_with(
+            mock.sentinel.child_uuid,
+            mock.sentinel.child_name,
+            mock.sentinel.inv_data2,
+            parent_provider_uuid=mock.sentinel.rp_uuid,
         )