diff --git a/nova/api/openstack/placement/handlers/reshaper.py b/nova/api/openstack/placement/handlers/reshaper.py index a999a3e21544..0351ffd286f6 100644 --- a/nova/api/openstack/placement/handlers/reshaper.py +++ b/nova/api/openstack/placement/handlers/reshaper.py @@ -67,9 +67,10 @@ def reshape(req): generation = inventory_data['resource_provider_generation'] if generation != resource_provider.generation: raise webob.exc.HTTPConflict( - _('resource provider generation conflict: ' + _('resource provider generation conflict for provider %(rp)s: ' 'actual: %(actual)s, given: %(given)s') % - {'actual': resource_provider.generation, + {'rp': rp_uuid, + 'actual': resource_provider.generation, 'given': generation}, comment=errors.CONCURRENT_UPDATE) diff --git a/nova/conf/compute.py b/nova/conf/compute.py index 1ee9dbf86c6d..f25718d4c76d 100644 --- a/nova/conf/compute.py +++ b/nova/conf/compute.py @@ -717,10 +717,10 @@ Related options: # all about. Reference bug(s). Unless we're just going to remove it. help=""" Interval for updating nova-compute-side cache of the compute node resource -provider's aggregates and traits info. +provider's inventories, aggregates, and traits. This option specifies the number of seconds between attempts to update a -provider's aggregates and traits information in the local cache of the compute +provider's inventories, aggregates and traits in the local cache of the compute node. A value of zero disables cache refresh completely. diff --git a/nova/scheduler/client/report.py b/nova/scheduler/client/report.py index cef9962eb4fb..bb873c728db4 100644 --- a/nova/scheduler/client/report.py +++ b/nova/scheduler/client/report.py @@ -736,9 +736,6 @@ class SchedulerReportClient(object): # At this point, the whole tree exists in the local cache. for uuid_to_refresh in uuids_to_refresh: - # NOTE(efried): _refresh_associations doesn't refresh inventory - # (yet) - see that method's docstring for the why. - self._refresh_and_get_inventory(context, uuid_to_refresh) self._refresh_associations(context, uuid_to_refresh, force=True) return uuid @@ -816,18 +813,14 @@ class SchedulerReportClient(object): this process, CONF.compute.resource_provider_association_refresh seconds have passed, or the force arg has been set to True. - Note that we do *not* refresh inventories. The reason is largely - historical: all code paths that get us here are doing inventory refresh - themselves. - :param context: The security context :param rp_uuid: UUID of the resource provider to check for fresh - aggregates and traits + inventories, aggregates, and traits :param force: If True, force the refresh :param refresh_sharing: If True, fetch all the providers associated by aggregate with the specified provider, - including their traits and aggregates (but not - *their* sharing providers). + including their inventories, traits, and + aggregates (but not *their* sharing providers). :raise: On various placement API errors, one of: - ResourceProviderAggregateRetrievalFailed - ResourceProviderTraitRetrievalFailed @@ -836,6 +829,10 @@ class SchedulerReportClient(object): communication fails. """ if force or self._associations_stale(rp_uuid): + # Refresh inventories + msg = "Refreshing inventories for resource provider %s" + LOG.debug(msg, rp_uuid) + self._refresh_and_get_inventory(context, rp_uuid) # Refresh aggregates agg_info = self._get_provider_aggregates(context, rp_uuid) # If @safe_connect makes the above return None, this will raise @@ -872,11 +869,11 @@ class SchedulerReportClient(object): self._provider_tree.new_root( rp['name'], rp['uuid'], generation=rp['generation']) - # Now we have to (populate or) refresh that guy's traits - # and aggregates (but not *his* aggregate-associated - # providers). No need to override force=True for newly- - # added providers - the missing timestamp will always - # trigger them to refresh. + # Now we have to (populate or) refresh that guy's traits, + # aggregates, and inventories (but not *his* aggregate- + # associated providers). No need to override force=True for + # newly- added providers - the missing timestamp will + # always trigger them to refresh. self._refresh_associations(context, rp['uuid'], force=force, refresh_sharing=False) @@ -1069,9 +1066,6 @@ class SchedulerReportClient(object): self._ensure_resource_provider( context, rp_uuid, name=name, parent_provider_uuid=parent_provider_uuid) - # Ensure inventories are up to date (for *all* cached RPs) - for uuid in self._provider_tree.get_provider_uuids(): - self._refresh_and_get_inventory(context, uuid) # Return a *copy* of the tree. return copy.deepcopy(self._provider_tree) diff --git a/nova/tests/functional/test_report_client.py b/nova/tests/functional/test_report_client.py index 92adc994092a..58be3d144452 100644 --- a/nova/tests/functional/test_report_client.py +++ b/nova/tests/functional/test_report_client.py @@ -1180,12 +1180,19 @@ class SchedulerReportClientTests(SchedulerReportClientTestBase): for k, expected in pdict.items(): # For inventories, we're only validating totals if k is 'inventory': - self.assertEqual(set(expected), set(actual_data.inventory)) + self.assertEqual( + set(expected), set(actual_data.inventory), + "Mismatched inventory keys for provider %s" % uuid) for rc, totaldict in expected.items(): - self.assertEqual(totaldict['total'], - actual_data.inventory[rc]['total']) + self.assertEqual( + totaldict['total'], + actual_data.inventory[rc]['total'], + "Mismatched inventory totals for provider %s" % + uuid) else: - self.assertEqual(expected, getattr(actual_data, k)) + self.assertEqual(expected, getattr(actual_data, k), + "Mismatched %s for provider %s" % + (k, uuid)) def _set_up_provider_tree_allocs(self): """Create some allocations on our compute (with sharing). @@ -1294,6 +1301,14 @@ class SchedulerReportClientTests(SchedulerReportClientTestBase): # in gabbits and via update_from_provider_tree. self._set_up_provider_tree() self._set_up_provider_tree_allocs() + # Updating allocations bumps generations for affected providers. + # In real life, the subsequent update_from_provider_tree will + # bounce 409, the cache will be cleared, and the operation will be + # retried. We don't care about any of that retry logic in the scope + # of this test case, so just clear the cache so + # get_provider_tree_and_ensure_root repopulates it and we avoid the + # conflict exception. + self.client.clear_provider_cache() ptree = self.client.get_provider_tree_and_ensure_root( self.context, self.compute_uuid) @@ -1336,6 +1351,14 @@ class SchedulerReportClientTests(SchedulerReportClientTestBase): exp_allocs = self._set_up_provider_tree_allocs() # Save a copy of this for later orig_exp_allocs = copy.deepcopy(exp_allocs) + # Updating allocations bumps generations for affected providers. + # In real life, the subsequent update_from_provider_tree will + # bounce 409, the cache will be cleared, and the operation will be + # retried. We don't care about any of that retry logic in the scope + # of this test case, so just clear the cache so + # get_provider_tree_and_ensure_root repopulates it and we avoid the + # conflict exception. + self.client.clear_provider_cache() # Another null reshape: no inv changes, no alloc changes ptree = self.client.get_provider_tree_and_ensure_root( self.context, self.compute_uuid) diff --git a/nova/tests/unit/scheduler/client/test_report.py b/nova/tests/unit/scheduler/client/test_report.py index 78ac177cfd5c..977b6abef576 100644 --- a/nova/tests/unit/scheduler/client/test_report.py +++ b/nova/tests/unit/scheduler/client/test_report.py @@ -1959,12 +1959,10 @@ class TestProviderOperations(SchedulerReportClientTestCase): @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' '_get_providers_in_tree') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_refresh_and_get_inventory') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' '_refresh_associations') def test_ensure_resource_provider_refresh_fetch(self, mock_ref_assoc, - mock_ref_inv, mock_gpit): + mock_gpit): """Make sure refreshes are called with the appropriate UUIDs and flags when we fetch the provider tree from placement. """ @@ -1975,8 +1973,6 @@ class TestProviderOperations(SchedulerReportClientTestCase): self.client._ensure_resource_provider(self.context, uuids.root)) mock_gpit.assert_called_once_with(self.context, uuids.root) - mock_ref_inv.assert_has_calls([mock.call(self.context, uuid) - for uuid in tree_uuids]) mock_ref_assoc.assert_has_calls( [mock.call(self.context, uuid, force=True) for uuid in tree_uuids]) @@ -2845,202 +2841,175 @@ class TestTraits(SchedulerReportClientTestCase): class TestAssociations(SchedulerReportClientTestCase): - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_aggregates') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_traits') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_sharing_providers') - def test_refresh_associations_no_last(self, mock_shr_get, mock_trait_get, - mock_agg_get): + def setUp(self): + super(TestAssociations, self).setUp() + + self.mock_get_inv = self.useFixture(fixtures.MockPatch( + 'nova.scheduler.client.report.SchedulerReportClient.' + '_get_inventory')).mock + self.inv = { + 'VCPU': {'total': 16}, + 'MEMORY_MB': {'total': 1024}, + 'DISK_GB': {'total': 10}, + } + self.mock_get_inv.return_value = { + 'resource_provider_generation': 41, + 'inventories': self.inv, + } + + self.mock_get_aggs = self.useFixture(fixtures.MockPatch( + 'nova.scheduler.client.report.SchedulerReportClient.' + '_get_provider_aggregates')).mock + self.mock_get_aggs.return_value = report.AggInfo( + aggregates=set([uuids.agg1]), generation=42) + + self.mock_get_traits = self.useFixture(fixtures.MockPatch( + 'nova.scheduler.client.report.SchedulerReportClient.' + '_get_provider_traits')).mock + self.mock_get_traits.return_value = report.TraitInfo( + traits=set(['CUSTOM_GOLD']), generation=43) + + self.mock_get_sharing = self.useFixture(fixtures.MockPatch( + 'nova.scheduler.client.report.SchedulerReportClient.' + '_get_sharing_providers')).mock + + def assert_things_were_called(self, uuid, sharing=True): + self.mock_get_inv.assert_called_once_with(self.context, uuid) + self.mock_get_aggs.assert_called_once_with(self.context, uuid) + self.mock_get_traits.assert_called_once_with(self.context, uuid) + if sharing: + self.mock_get_sharing.assert_called_once_with( + self.context, self.mock_get_aggs.return_value[0]) + self.assertIn(uuid, self.client._association_refresh_time) + self.assertFalse( + self.client._provider_tree.has_inventory_changed(uuid, self.inv)) + self.assertTrue( + self.client._provider_tree.in_aggregates(uuid, [uuids.agg1])) + self.assertFalse( + self.client._provider_tree.in_aggregates(uuid, [uuids.agg2])) + self.assertTrue( + self.client._provider_tree.has_traits(uuid, ['CUSTOM_GOLD'])) + self.assertFalse( + self.client._provider_tree.has_traits(uuid, ['CUSTOM_SILVER'])) + self.assertEqual(43, self.client._provider_tree.data(uuid).generation) + + def assert_things_not_called(self, timer_entry=None): + self.mock_get_inv.assert_not_called() + self.mock_get_aggs.assert_not_called() + self.mock_get_traits.assert_not_called() + self.mock_get_sharing.assert_not_called() + if timer_entry is None: + self.assertFalse(self.client._association_refresh_time) + else: + self.assertIn(timer_entry, self.client._association_refresh_time) + + def reset_things(self): + self.mock_get_inv.reset_mock() + self.mock_get_aggs.reset_mock() + self.mock_get_traits.reset_mock() + self.mock_get_sharing.reset_mock() + + def test_refresh_associations_no_last(self): """Test that associations are refreshed when stale.""" uuid = uuids.compute_node # Seed the provider tree so _refresh_associations finds the provider self.client._provider_tree.new_root('compute', uuid, generation=1) - mock_agg_get.return_value = report.AggInfo( - aggregates=set([uuids.agg1]), generation=42) - mock_trait_get.return_value = report.TraitInfo( - traits=set(['CUSTOM_GOLD']), generation=43) self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_called_once_with(self.context, uuid) - mock_trait_get.assert_called_once_with(self.context, uuid) - mock_shr_get.assert_called_once_with( - self.context, mock_agg_get.return_value[0]) - self.assertIn(uuid, self.client._association_refresh_time) - self.assertTrue( - self.client._provider_tree.in_aggregates(uuid, [uuids.agg1])) - self.assertFalse( - self.client._provider_tree.in_aggregates(uuid, [uuids.agg2])) - self.assertTrue( - self.client._provider_tree.has_traits(uuid, ['CUSTOM_GOLD'])) - self.assertFalse( - self.client._provider_tree.has_traits(uuid, ['CUSTOM_SILVER'])) - self.assertEqual(43, self.client._provider_tree.data(uuid).generation) + self.assert_things_were_called(uuid) - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_aggregates') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_traits') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_sharing_providers') - def test_refresh_associations_no_refresh_sharing(self, mock_shr_get, - mock_trait_get, - mock_agg_get): + def test_refresh_associations_no_refresh_sharing(self): """Test refresh_sharing=False.""" uuid = uuids.compute_node # Seed the provider tree so _refresh_associations finds the provider self.client._provider_tree.new_root('compute', uuid, generation=1) - mock_agg_get.return_value = report.AggInfo( - aggregates=set([uuids.agg1]), generation=42) - mock_trait_get.return_value = report.TraitInfo( - traits=set(['CUSTOM_GOLD']), generation=43) self.client._refresh_associations(self.context, uuid, refresh_sharing=False) - mock_agg_get.assert_called_once_with(self.context, uuid) - mock_trait_get.assert_called_once_with(self.context, uuid) - mock_shr_get.assert_not_called() - self.assertIn(uuid, self.client._association_refresh_time) - self.assertTrue( - self.client._provider_tree.in_aggregates(uuid, [uuids.agg1])) - self.assertFalse( - self.client._provider_tree.in_aggregates(uuid, [uuids.agg2])) - self.assertTrue( - self.client._provider_tree.has_traits(uuid, ['CUSTOM_GOLD'])) - self.assertFalse( - self.client._provider_tree.has_traits(uuid, ['CUSTOM_SILVER'])) - self.assertEqual(43, self.client._provider_tree.data(uuid).generation) + self.assert_things_were_called(uuid, sharing=False) - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_aggregates') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_traits') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_sharing_providers') @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' '_associations_stale') - def test_refresh_associations_not_stale(self, mock_stale, mock_shr_get, - mock_trait_get, mock_agg_get): + def test_refresh_associations_not_stale(self, mock_stale): """Test that refresh associations is not called when the map is not stale. """ mock_stale.return_value = False uuid = uuids.compute_node self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_not_called() - mock_trait_get.assert_not_called() - mock_shr_get.assert_not_called() - self.assertFalse(self.client._association_refresh_time) + self.assert_things_not_called() @mock.patch.object(report.LOG, 'debug') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_aggregates') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_traits') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_sharing_providers') - def test_refresh_associations_time(self, mock_shr_get, mock_trait_get, - mock_agg_get, log_mock): + def test_refresh_associations_time(self, log_mock): """Test that refresh associations is called when the map is stale.""" uuid = uuids.compute_node # Seed the provider tree so _refresh_associations finds the provider self.client._provider_tree.new_root('compute', uuid, generation=1) - mock_agg_get.return_value = report.AggInfo(aggregates=set([]), - generation=42) - mock_trait_get.return_value = report.TraitInfo(traits=set([]), - generation=43) - mock_shr_get.return_value = [] # Called a first time because association_refresh_time is empty. now = time.time() self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_called_once_with(self.context, uuid) - mock_trait_get.assert_called_once_with(self.context, uuid) - mock_shr_get.assert_called_once_with(self.context, set()) + self.assert_things_were_called(uuid) log_mock.assert_has_calls([ + mock.call('Refreshing inventories for resource provider %s', uuid), + mock.call('Updating ProviderTree inventory for provider %s from ' + '_refresh_and_get_inventory using data: %s', + uuid, self.inv), mock.call('Refreshing aggregate associations for resource ' - 'provider %s, aggregates: %s', uuid, 'None'), + 'provider %s, aggregates: %s', uuid, uuids.agg1), mock.call('Refreshing trait associations for resource ' - 'provider %s, traits: %s', uuid, 'None') + 'provider %s, traits: %s', uuid, 'CUSTOM_GOLD') ]) - self.assertIn(uuid, self.client._association_refresh_time) # Clear call count. - mock_agg_get.reset_mock() - mock_trait_get.reset_mock() - mock_shr_get.reset_mock() + self.reset_things() with mock.patch('time.time') as mock_future: # Not called a second time because not enough time has passed. mock_future.return_value = (now + CONF.compute.resource_provider_association_refresh / 2) self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_not_called() - mock_trait_get.assert_not_called() - mock_shr_get.assert_not_called() + self.assert_things_not_called(timer_entry=uuid) # Called because time has passed. mock_future.return_value = (now + CONF.compute.resource_provider_association_refresh + 1) self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_called_once_with(self.context, uuid) - mock_trait_get.assert_called_once_with(self.context, uuid) - mock_shr_get.assert_called_once_with(self.context, set()) + self.assert_things_were_called(uuid) - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_aggregates') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_provider_traits') - @mock.patch('nova.scheduler.client.report.SchedulerReportClient.' - '_get_sharing_providers') - def test_refresh_associations_disabled(self, mock_shr_get, mock_trait_get, - mock_agg_get): + def test_refresh_associations_disabled(self): """Test that refresh associations can be disabled.""" self.flags(resource_provider_association_refresh=0, group='compute') uuid = uuids.compute_node # Seed the provider tree so _refresh_associations finds the provider self.client._provider_tree.new_root('compute', uuid, generation=1) - mock_agg_get.return_value = report.AggInfo(aggregates=set([]), - generation=42) - mock_trait_get.return_value = report.TraitInfo(traits=set([]), - generation=43) - mock_shr_get.return_value = [] # Called a first time because association_refresh_time is empty. now = time.time() self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_called_once_with(self.context, uuid) - mock_trait_get.assert_called_once_with(self.context, uuid) - mock_shr_get.assert_called_once_with(self.context, set()) - self.assertIn(uuid, self.client._association_refresh_time) + self.assert_things_were_called(uuid) # Clear call count. - mock_agg_get.reset_mock() - mock_trait_get.reset_mock() - mock_shr_get.reset_mock() + self.reset_things() with mock.patch('time.time') as mock_future: # A lot of time passes mock_future.return_value = now + 10000000000000 self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_not_called() - mock_trait_get.assert_not_called() - mock_shr_get.assert_not_called() + self.assert_things_not_called(timer_entry=uuid) + + self.reset_things() # Forever passes mock_future.return_value = float('inf') self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_not_called() - mock_trait_get.assert_not_called() - mock_shr_get.assert_not_called() + self.assert_things_not_called(timer_entry=uuid) + + self.reset_things() # Even if no time passes, clearing the counter triggers refresh mock_future.return_value = now del self.client._association_refresh_time[uuid] self.client._refresh_associations(self.context, uuid) - mock_agg_get.assert_called_once_with(self.context, uuid) - mock_trait_get.assert_called_once_with(self.context, uuid) - mock_shr_get.assert_called_once_with(self.context, set()) - self.assertIn(uuid, self.client._association_refresh_time) + self.assert_things_were_called(uuid) class TestComputeNodeToInventoryDict(test.NoDBTestCase):