diff --git a/nova/scheduler/client/__init__.py b/nova/scheduler/client/__init__.py index 200480c49a3e..972e9cb4e0e4 100644 --- a/nova/scheduler/client/__init__.py +++ b/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): diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index 82d732bbfb11..635384faaeb7 100644 --- a/nova/scheduler/client/report.py +++ b/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, diff --git a/nova/tests/unit/scheduler/client/test_report.py b/nova/tests/unit/scheduler/client/test_report.py index 3e4cb80e5158..c7e66011d7d6 100644 --- a/nova/tests/unit/scheduler/client/test_report.py +++ b/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.' diff --git a/nova/tests/unit/scheduler/test_client.py b/nova/tests/unit/scheduler/test_client.py index 94ad0d5da4c2..18b4e4d37918 100644 --- a/nova/tests/unit/scheduler/test_client.py +++ b/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, )