From 0372d82c2c5ae5651b00c5ab9e1cf5c77a8f87d9 Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Fri, 16 Mar 2018 16:54:14 +0000 Subject: [PATCH] Ensure that os-traits sync is attempted only at start of process Traits sync had been tried any time a request that might involve traits was called. If the global was set no syncing was done, but lock handling was happening. This change moves the syncing into the the deploy.load_app() handling. This means that the syncing will be attempted any time a new WSGI application is created. Most of the time this will be at the start of a new process, but some WSGI servers have interesting threading models so there's a (slim) possibility that it could be in a thread. Because of this latter possibility, the locking is still in place. Functional tests are updated to explicitly do the sync in their setUp(). Some changes in fixtures are required to make sure that the database is present prior to the sync. While these changes are not strictly part of extracting placement, the consolidation and isolation of database handling code makes where to put this stuff a bit cleaner and more evident: an update_database() method in deploy uses an empty DbContext class from db_api to the call the ensure_trait_sync method in resource_provider. update_database is in deploy because it an app deployment task and because putting it in db_api leads to circual import problems. blueprint placement-extract Closes-Bug: #1756151 Change-Id: Ic87518948ed5bf4ab79f9819cd94714e350ce265 --- nova/api/openstack/placement/db_api.py | 5 ++++ nova/api/openstack/placement/deploy.py | 11 +++++++++ .../placement/objects/resource_provider.py | 6 +---- nova/test.py | 5 +++- .../api/openstack/placement/db/test_base.py | 13 +++-------- .../placement/db/test_resource_provider.py | 23 ++++--------------- .../openstack/placement/fixtures/gabbits.py | 3 +++ nova/tests/functional/integrated_helpers.py | 1 + .../objects/test_resource_provider.py | 2 +- nova/tests/unit/test_fixtures.py | 2 ++ 10 files changed, 35 insertions(+), 36 deletions(-) diff --git a/nova/api/openstack/placement/db_api.py b/nova/api/openstack/placement/db_api.py index 7fb199ed1c3d..b01b8d948c6f 100644 --- a/nova/api/openstack/placement/db_api.py +++ b/nova/api/openstack/placement/db_api.py @@ -38,3 +38,8 @@ def configure(conf): def get_placement_engine(): return placement_context_manager.get_legacy_facade().get_engine() + + +@enginefacade.transaction_context_provider +class DbContext(object): + """Stub class for db session handling outside of web requests.""" diff --git a/nova/api/openstack/placement/deploy.py b/nova/api/openstack/placement/deploy.py index d4e879f1c5d4..4422411d13b3 100644 --- a/nova/api/openstack/placement/deploy.py +++ b/nova/api/openstack/placement/deploy.py @@ -16,9 +16,11 @@ import oslo_middleware from oslo_middleware import cors from nova.api.openstack.placement import auth +from nova.api.openstack.placement import db_api from nova.api.openstack.placement import fault_wrap from nova.api.openstack.placement import handler from nova.api.openstack.placement import microversion +from nova.api.openstack.placement.objects import resource_provider from nova.api.openstack.placement import requestlog from nova.api.openstack.placement import util @@ -82,6 +84,14 @@ def deploy(conf): return application +def update_database(): + """Do any database updates required at process boot time, such as + updating the traits database. + """ + ctx = db_api.DbContext() + resource_provider.ensure_trait_sync(ctx) + + # NOTE(cdent): Althought project_name is no longer used because of the # resolution of https://bugs.launchpad.net/nova/+bug/1734491, loadapp() # is considered a public interface for the creation of a placement @@ -98,4 +108,5 @@ def loadapp(config, project_name=NAME): backwards compatibility """ application = deploy(config) + update_database() return application diff --git a/nova/api/openstack/placement/objects/resource_provider.py b/nova/api/openstack/placement/objects/resource_provider.py index 97042e0b336e..a82fa81ac8fe 100644 --- a/nova/api/openstack/placement/objects/resource_provider.py +++ b/nova/api/openstack/placement/objects/resource_provider.py @@ -123,7 +123,7 @@ def _trait_sync(ctx): pass # some other process sync'd, just ignore -def _ensure_trait_sync(ctx): +def ensure_trait_sync(ctx): """Ensures that the os_traits library is synchronized to the traits db. If _TRAITS_SYNCED is False then this process has not tried to update the @@ -1442,7 +1442,6 @@ class ResourceProviderList(base.ObjectListBase, base.VersionedObject): :type filters: dict """ _ensure_rc_cache(context) - _ensure_trait_sync(context) resource_providers = cls._get_all_by_filters_from_db(context, filters) return base.obj_make_list(context, cls(context), ResourceProvider, resource_providers) @@ -2376,7 +2375,6 @@ class Trait(base.VersionedObject, base.TimestampedObject): @staticmethod @db_api.placement_context_manager.writer # trait sync can cause a write def _get_by_name_from_db(context, name): - _ensure_trait_sync(context) result = context.session.query(models.Trait).filter_by( name=name).first() if not result: @@ -2426,7 +2424,6 @@ class TraitList(base.ObjectListBase, base.VersionedObject): @staticmethod @db_api.placement_context_manager.writer # trait sync can cause a write def _get_all_from_db(context, filters): - _ensure_trait_sync(context) if not filters: filters = {} @@ -3803,7 +3800,6 @@ class AllocationCandidates(base.VersionedObject): according to `limit`. """ _ensure_rc_cache(context) - _ensure_trait_sync(context) alloc_reqs, provider_summaries = cls._get_by_requests( context, requests, limit=limit, group_policy=group_policy) return cls( diff --git a/nova/test.py b/nova/test.py index 1c93a3ae110d..63889d9b4692 100644 --- a/nova/test.py +++ b/nova/test.py @@ -312,7 +312,10 @@ class TestCase(testtools.TestCase): utils._IS_NEUTRON = None # Reset the traits sync and rc cache flags - resource_provider._TRAITS_SYNCED = False + def _reset_traits(): + resource_provider._TRAITS_SYNCED = False + _reset_traits() + self.addCleanup(_reset_traits) resource_provider._RC_CACHE = None # Reset the global QEMU version flag. images.QEMU_VERSION = None diff --git a/nova/tests/functional/api/openstack/placement/db/test_base.py b/nova/tests/functional/api/openstack/placement/db/test_base.py index bcf62b8c3466..835948e33857 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_base.py +++ b/nova/tests/functional/api/openstack/placement/db/test_base.py @@ -14,6 +14,7 @@ from oslo_utils import uuidutils +from nova.api.openstack.placement import deploy from nova.api.openstack.placement import exception from nova.api.openstack.placement.objects import consumer as consumer_obj from nova.api.openstack.placement.objects import project as project_obj @@ -55,26 +56,18 @@ class PlacementDbBaseTestCase(test.NoDBTestCase): self.useFixture(fixtures.Database()) self.placement_db = self.useFixture( fixtures.Database(database='placement')) - # Reset the _TRAITS_SYNCED global before we start and after - # we are done since other tests (notably the gabbi tests) - # may have caused it to change. - self._reset_traits_synced() - self.addCleanup(self._reset_traits_synced) self.ctx = context.RequestContext('fake-user', 'fake-project') self.user_obj = user_obj.User(self.ctx, external_id='fake-user') self.user_obj.create() self.project_obj = project_obj.Project( self.ctx, external_id='fake-project') self.project_obj.create() + # Do database syncs, such as traits sync. + deploy.update_database() # For debugging purposes, populated by _create_provider and used by # _validate_allocation_requests to make failure results more readable. self.rp_uuid_to_name = {} - @staticmethod - def _reset_traits_synced(): - """Reset the _TRAITS_SYNCED boolean to base state.""" - rp_obj._TRAITS_SYNCED = False - def _create_provider(self, name, *aggs, **kwargs): parent = kwargs.get('parent') root = kwargs.get('root') diff --git a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py index ca768b033c6b..57764c18e10b 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py +++ b/nova/tests/functional/api/openstack/placement/db/test_resource_provider.py @@ -2032,6 +2032,10 @@ class ResourceProviderTraitTestCase(tb.PlacementDbBaseTestCase): # in an exception, the sync transaction rolled back and the table # stayed empty; but _TRAITS_SYNCED got set to True, so it didn't resync # next time. + # NOTE(cdent): With change Ic87518948ed5bf4ab79f9819cd94714e350ce265 + # syncing is no longer done in the same way, so the bug fix that this + # test was testing is gone, but this test has been left in place to + # make sure we still get behavior we expect. try: rp_obj.Trait.get_by_name(self.ctx, 'CUSTOM_GOLD') except exception.TraitNotFound: @@ -2193,25 +2197,6 @@ class ResourceProviderTraitTestCase(tb.PlacementDbBaseTestCase): rp_obj.TraitList.get_all(self.ctx, filters={'associated': False})) - def test_sync_standard_traits(self): - """Tests that on a clean DB, we have zero traits in the DB but after - list all traits, os_traits have been synchronized. - """ - std_traits = os_traits.get_traits() - conn = self.placement_db.get_engine().connect() - - def _db_traits(conn): - sel = sa.select([rp_obj._TRAIT_TBL.c.name]) - return [r[0] for r in conn.execute(sel).fetchall()] - - self.assertEqual([], _db_traits(conn)) - - all_traits = [trait.name for trait in - rp_obj.TraitList.get_all(self.ctx)] - self.assertEqual(set(std_traits), set(all_traits)) - # confirm with a raw request - self.assertEqual(set(std_traits), set(_db_traits(conn))) - class SharedProviderTestCase(tb.PlacementDbBaseTestCase): """Tests that the queries used to determine placement in deployments with diff --git a/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py b/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py index b57bdefc33b3..66fb65b3680a 100644 --- a/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py +++ b/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py @@ -88,6 +88,9 @@ class APIFixture(fixture.GabbiFixture): self.placement_db_fixture.reset() self.api_db_fixture.reset() self.main_db_fixture.reset() + # Do this now instead of waiting for the WSGI app to start so that + # fixtures can have traits. + deploy.update_database() os.environ['RP_UUID'] = uuidutils.generate_uuid() os.environ['RP_NAME'] = uuidutils.generate_uuid() diff --git a/nova/tests/functional/integrated_helpers.py b/nova/tests/functional/integrated_helpers.py index 4e50ac78aeac..1259cb5f94c9 100644 --- a/nova/tests/functional/integrated_helpers.py +++ b/nova/tests/functional/integrated_helpers.py @@ -85,6 +85,7 @@ class _IntegratedTestBase(test.TestCase): nova.tests.unit.image.fake.stub_out_image_service(self) self.useFixture(cast_as_call.CastAsCall(self)) + self.useFixture(nova_fixtures.Database(database='placement')) placement = self.useFixture(nova_fixtures.PlacementFixture()) self.placement_api = placement.api diff --git a/nova/tests/unit/api/openstack/placement/objects/test_resource_provider.py b/nova/tests/unit/api/openstack/placement/objects/test_resource_provider.py index 1d4d376aa699..90a1d2ed1095 100644 --- a/nova/tests/unit/api/openstack/placement/objects/test_resource_provider.py +++ b/nova/tests/unit/api/openstack/placement/objects/test_resource_provider.py @@ -338,7 +338,7 @@ class TestTraits(_TestCase): synced = resource_provider._TRAITS_SYNCED self.assertFalse(synced) # Sync the traits - resource_provider._ensure_trait_sync(self.context) + resource_provider.ensure_trait_sync(self.context) synced = resource_provider._TRAITS_SYNCED self.assertTrue(synced) diff --git a/nova/tests/unit/test_fixtures.py b/nova/tests/unit/test_fixtures.py index 4e8b3e6f71f4..0acb3c4faa8a 100644 --- a/nova/tests/unit/test_fixtures.py +++ b/nova/tests/unit/test_fixtures.py @@ -478,6 +478,8 @@ class TestPlacementFixture(testtools.TestCase): self.useFixture(conf_fixture.ConfFixture()) # We need PlacementPolicyFixture because placement-api checks policy. self.useFixture(policy_fixture.PlacementPolicyFixture()) + # Database is needed to start placement API + self.useFixture(fixtures.Database(database='placement')) def test_responds_to_version(self): """Ensure the Placement server responds to calls sensibly."""