From f5783d90bcb0b52469d0e0ada3933271afb84e2a Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Wed, 25 Jul 2018 15:40:37 +0100 Subject: [PATCH] [placement] Use base test in placement functional tests There is now a placement.base.TestCase for placement functional tests which assembles the necessary configuration and fixtures. This change uses that base class in the db functional tests and extends the base class as required to add all the necessary functionality. In the process issues were exposed in the fixtures.gabbits use of oslo_config (causing tests to fail based on fallout from changes elsewhere in the functional tests) so this change also fixes that and limits the gabbi tests to only caring about the placement database connection, which is more correct and how it should have been all along. This change removes the ConfPatcher fixture in fixtures.placement because it is no better than using the oslo_config provided fixture and was in the way while diagnosing difficulties with getting these changes to work correctly. The root cause of those problems was that placement changes were at cross purposes with how the nova.tests.fixtures.Database fixture expects to work: The goal of these changes was to only configure and establish those fixtures that were strictly necessary for placement in the placements tests. However, when _any_ database is requested from the Database fixture, the context managers for all of them are configured. This means, for example, that if a placement fixture, which originally was not configuring a 'connection' string for the api or main databases, ran before a later api db fixture, the api db fixture would fail with no connection string available. The quick and dirty fix is used here to fix the problem: we set reasonable configuration for all three databases in the placement tests that need the placement database fixture. In the future when placement is extracted, these problems go away so it does not seem worth the effort (at least not now) to restructure the nova Database fixture. blueprint: placement-extract Change-Id: Ice89e9a25f74caaa53b7df079bd529d172354524 --- .../api/openstack/placement/base.py | 37 ++++++++----- .../db/test_allocation_candidates.py | 7 ++- .../api/openstack/placement/db/test_base.py | 17 ++---- .../openstack/placement/db/test_consumer.py | 11 +--- .../placement/db/test_resource_class_cache.py | 9 ++- .../openstack/placement/fixtures/gabbits.py | 55 ++++++++----------- .../openstack/placement/fixtures/placement.py | 33 +---------- .../api_sample_tests/api_sample_base.py | 1 + 8 files changed, 68 insertions(+), 102 deletions(-) diff --git a/nova/tests/functional/api/openstack/placement/base.py b/nova/tests/functional/api/openstack/placement/base.py index 60b1bb89edc6..d49b1286dd6d 100644 --- a/nova/tests/functional/api/openstack/placement/base.py +++ b/nova/tests/functional/api/openstack/placement/base.py @@ -14,9 +14,9 @@ from oslo_config import cfg from oslo_config import fixture as config_fixture import testtools +from nova.api.openstack.placement import context +from nova.api.openstack.placement import deploy from nova.api.openstack.placement.objects import resource_provider -from nova import config -from nova import context from nova.tests import fixtures from nova.tests.unit import policy_fixture @@ -36,21 +36,32 @@ class TestCase(testtools.TestCase): # Manage required configuration conf_fixture = self.useFixture(config_fixture.Config(CONF)) - conf_fixture.conf.set_default( - 'connection', "sqlite://", group='placement_database') - conf_fixture.conf.set_default( - 'sqlite_synchronous', False, group='placement_database') - config.parse_args([], default_config_files=[], configure_db=False, - init_rpc=False) + # The Database fixture will get confused if only one of the databases + # is configured. + for group in ('placement_database', 'api_database', 'database'): + conf_fixture.config( + group=group, + connection='sqlite://', + sqlite_synchronous=False) + CONF([], default_config_files=[]) self.useFixture(policy_fixture.PlacementPolicyFixture()) + + self.useFixture(fixtures.StandardLogging()) + self.useFixture(fixtures.OutputStreamCapture()) + # Filter ignorable warnings during test runs. + self.useFixture(fixtures.WarningsFixture()) + self.placement_db = self.useFixture( fixtures.Database(database='placement')) - self._reset_traits_synced() - self.context = context.get_admin_context() - self.addCleanup(self._reset_traits_synced) + self._reset_database() + self.context = context.RequestContext() + # Do database syncs, such as traits sync. + deploy.update_database() + self.addCleanup(self._reset_database) @staticmethod - def _reset_traits_synced(): - """Reset the _TRAITS_SYNCED boolean to base state.""" + def _reset_database(): + """Reset database sync flags to base state.""" resource_provider._TRAITS_SYNCED = False + resource_provider._RC_CACHE = None diff --git a/nova/tests/functional/api/openstack/placement/db/test_allocation_candidates.py b/nova/tests/functional/api/openstack/placement/db/test_allocation_candidates.py index 1771c91c6b21..e78adadc6241 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_allocation_candidates.py +++ b/nova/tests/functional/api/openstack/placement/db/test_allocation_candidates.py @@ -10,6 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. import os_traits +from oslo_config import cfg import six import sqlalchemy as sa @@ -21,6 +22,9 @@ from nova.tests.functional.api.openstack.placement.db import test_base as tb from nova.tests import uuidsentinel as uuids +CONF = cfg.CONF + + class ProviderDBHelperTestCase(tb.PlacementDbBaseTestCase): def test_get_provider_ids_matching(self): @@ -554,7 +558,8 @@ class AllocationCandidatesTestCase(tb.PlacementDbBaseTestCase): # Do it again, with conf set to randomize. We can't confirm the # random-ness but we can be sure the code path doesn't explode. - self.flags(randomize_allocation_candidates=True, group='placement') + CONF.set_override('randomize_allocation_candidates', True, + group='placement') # Ask for two candidates. limit = 2 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 4c1730853ec3..e8e78bd232d2 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_base.py +++ b/nova/tests/functional/api/openstack/placement/db/test_base.py @@ -14,15 +14,12 @@ from oslo_utils import uuidutils -from nova.api.openstack.placement import context -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 from nova.api.openstack.placement.objects import resource_provider as rp_obj from nova.api.openstack.placement.objects import user as user_obj -from nova import test -from nova.tests import fixtures +from nova.tests.functional.api.openstack.placement import base from nova.tests import uuidsentinel as uuids @@ -63,22 +60,18 @@ def set_traits(rp, *traits): return tlist -class PlacementDbBaseTestCase(test.NoDBTestCase): - USES_DB_SELF = True +class PlacementDbBaseTestCase(base.TestCase): def setUp(self): super(PlacementDbBaseTestCase, self).setUp() - self.useFixture(fixtures.Database()) - self.placement_db = self.useFixture( - fixtures.Database(database='placement')) - self.ctx = context.RequestContext('fake-user', 'fake-project') + # we use context in some places and ctx in other. We should only use + # context, but let's paper over that for now. + self.ctx = self.context 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 = {} diff --git a/nova/tests/functional/api/openstack/placement/db/test_consumer.py b/nova/tests/functional/api/openstack/placement/db/test_consumer.py index 693c1cbbbbcc..9fe2f2493d19 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_consumer.py +++ b/nova/tests/functional/api/openstack/placement/db/test_consumer.py @@ -13,7 +13,6 @@ from oslo_config import cfg import sqlalchemy as sa -from nova.api.openstack.placement import context from nova.api.openstack.placement import db_api from nova.api.openstack.placement import exception from nova.api.openstack.placement.objects import consumer as consumer_obj @@ -21,8 +20,7 @@ from nova.api.openstack.placement.objects import project as project_obj from nova.api.openstack.placement.objects import resource_provider as rp_obj from nova.api.openstack.placement.objects import user as user_obj from nova import rc_fields as fields -from nova import test -from nova.tests import fixtures +from nova.tests.functional.api.openstack.placement import base from nova.tests.functional.api.openstack.placement.db import test_base as tb from nova.tests import uuidsentinel as uuids @@ -99,14 +97,11 @@ def _get_allocs_with_no_consumer_relationship(ctx): # NOTE(jaypipes): The tb.PlacementDbBaseTestCase creates a project and user # which is why we don't base off that. We want a completely bare DB for this # test. -class CreateIncompleteConsumersTestCase(test.NoDBTestCase): - USES_DB_SELF = True +class CreateIncompleteConsumersTestCase(base.TestCase): def setUp(self): super(CreateIncompleteConsumersTestCase, self).setUp() - self.useFixture(fixtures.Database()) - self.api_db = self.useFixture(fixtures.Database(database='placement')) - self.ctx = context.RequestContext('fake-user', 'fake-project') + self.ctx = self.context @db_api.placement_context_manager.writer def _create_incomplete_allocations(self, ctx): diff --git a/nova/tests/functional/api/openstack/placement/db/test_resource_class_cache.py b/nova/tests/functional/api/openstack/placement/db/test_resource_class_cache.py index 414f6cdeffc6..e3b017ed4027 100644 --- a/nova/tests/functional/api/openstack/placement/db/test_resource_class_cache.py +++ b/nova/tests/functional/api/openstack/placement/db/test_resource_class_cache.py @@ -18,18 +18,17 @@ from oslo_utils import timeutils from nova.api.openstack.placement import exception from nova.db.sqlalchemy import resource_class_cache as rc_cache from nova import rc_fields as fields -from nova import test -from nova.tests import fixtures +from nova.tests.functional.api.openstack.placement import base -class TestResourceClassCache(test.TestCase): +class TestResourceClassCache(base.TestCase): def setUp(self): super(TestResourceClassCache, self).setUp() - self.db = self.useFixture(fixtures.Database(database='placement')) + db = self.placement_db self.context = mock.Mock() sess_mock = mock.Mock() - sess_mock.connection.side_effect = self.db.get_engine().connect + sess_mock.connection.side_effect = db.get_engine().connect self.context.session = sess_mock @mock.patch('sqlalchemy.select') diff --git a/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py b/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py index 7b0d04c4c564..9e9497fa8c68 100644 --- a/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py +++ b/nova/tests/functional/api/openstack/placement/fixtures/gabbits.py @@ -13,6 +13,8 @@ import os from gabbi import fixture +from oslo_config import cfg +from oslo_config import fixture as config_fixture from oslo_middleware import cors from oslo_utils import uuidutils @@ -24,8 +26,6 @@ from nova.api.openstack.placement.objects import project as project_obj from nova.api.openstack.placement.objects import resource_provider as rp_obj from nova.api.openstack.placement.objects import user as user_obj from nova.api.openstack.placement import policies -from nova import conf -from nova import config from nova import rc_fields as fields from nova.tests import fixtures from nova.tests.functional.api.openstack.placement.db import test_base as tb @@ -33,7 +33,7 @@ from nova.tests.unit import policy_fixture from nova.tests import uuidsentinel as uuids -CONF = conf.CONF +CONF = cfg.CONF def setup_app(): @@ -43,9 +43,6 @@ def setup_app(): class APIFixture(fixture.GabbiFixture): """Setup the required backend fixtures for a basic placement service.""" - def __init__(self): - self.conf = None - def start_fixture(self): # Set up stderr and stdout captures by directly driving the # existing nova fixtures that do that. This captures the @@ -59,15 +56,17 @@ class APIFixture(fixture.GabbiFixture): self.warnings_fixture = fixtures.WarningsFixture() self.warnings_fixture.setUp() - self.conf = CONF - self.conf.set_override('auth_strategy', 'noauth2', group='api') - # Be explicit about all three database connections to avoid - # potential conflicts with config on disk. - self.conf.set_override('connection', "sqlite://", group='database') - self.conf.set_override('connection', "sqlite://", - group='api_database') - self.conf.set_override('connection', "sqlite://", - group='placement_database') + self.conf_fixture = config_fixture.Config(CONF) + self.conf_fixture.setUp() + # The Database fixture will get confused if only one of the databases + # is configured. + for group in ('placement_database', 'api_database', 'database'): + self.conf_fixture.config( + group=group, + connection='sqlite://', + sqlite_synchronous=False) + self.conf_fixture.config( + group='api', auth_strategy='noauth2') self.context = context.RequestContext() @@ -75,23 +74,15 @@ class APIFixture(fixture.GabbiFixture): # effect of exercising the "don't use cors" path in # deploy.py. Without setting some config the group will not # be present. - self.conf.register_opts(cors.CORS_OPTS, 'cors') + CONF.register_opts(cors.CORS_OPTS, 'cors') # Make sure default_config_files is an empty list, not None. # If None /etc/nova/nova.conf is read and confuses results. - config.parse_args([], default_config_files=[], configure_db=False, - init_rpc=False) + CONF([], default_config_files=[]) - # NOTE(cdent): All three database fixtures need to be - # managed for database handling to work and not cause - # conflicts with other tests in the same process. self._reset_db_flags() self.placement_db_fixture = fixtures.Database('placement') - self.api_db_fixture = fixtures.Database('api') - self.main_db_fixture = fixtures.Database('main') - self.placement_db_fixture.reset() - self.api_db_fixture.reset() - self.main_db_fixture.reset() + self.placement_db_fixture.setUp() # Do this now instead of waiting for the WSGI app to start so that # fixtures can have traits. deploy.update_database() @@ -110,9 +101,7 @@ class APIFixture(fixture.GabbiFixture): os.environ['ALT_PARENT_PROVIDER_UUID'] = uuidutils.generate_uuid() def stop_fixture(self): - self.placement_db_fixture.cleanup() - self.api_db_fixture.cleanup() - self.main_db_fixture.cleanup() + self.placement_db_fixture.cleanUp() # Since we clean up the DB, we need to reset the traits sync # flag to make sure the next run will recreate the traits and @@ -123,8 +112,7 @@ class APIFixture(fixture.GabbiFixture): self.warnings_fixture.cleanUp() self.output_stream_fixture.cleanUp() self.standard_logging_fixture.cleanUp() - if self.conf: - self.conf.reset() + self.conf_fixture.cleanUp() @staticmethod def _reset_db_flags(): @@ -411,8 +399,9 @@ class CORSFixture(APIFixture): # NOTE(cdent): If we remove this override, then the cors # group ends up not existing in the conf, so when deploy.py # wants to load the CORS middleware, it will not. - self.conf.set_override('allowed_origin', 'http://valid.example.com', - group='cors') + self.conf_fixture.config( + group='cors', + allowed_origin='http://valid.example.com') # TODO(efried): Common with test_allocation_candidates diff --git a/nova/tests/functional/api/openstack/placement/fixtures/placement.py b/nova/tests/functional/api/openstack/placement/fixtures/placement.py index b087d74e5929..f8c3948ef971 100644 --- a/nova/tests/functional/api/openstack/placement/fixtures/placement.py +++ b/nova/tests/functional/api/openstack/placement/fixtures/placement.py @@ -12,6 +12,7 @@ import fixtures from oslo_config import cfg +from oslo_config import fixture as config_fixture from oslo_utils import uuidutils from wsgi_intercept import interceptor @@ -21,35 +22,6 @@ from nova.api.openstack.placement import deploy CONF = cfg.CONF -class ConfPatcher(fixtures.Fixture): - """Fixture to patch and restore global CONF. - - This also resets overrides for everything that is patched during - its teardown. - """ - # TODO(cdent): This is intentionally duplicated from nova's fixtures. - # This comment becomes redundant once placement is extracted. - - def __init__(self, **kwargs): - """Constructor - - :params group: if specified all config options apply to that group. - - :params **kwargs: the rest of the kwargs are processed as a - set of key/value pairs to be set as configuration override. - - """ - super(ConfPatcher, self).__init__() - self.group = kwargs.pop('group', None) - self.args = kwargs - - def setUp(self): - super(ConfPatcher, self).setUp() - for k, v in self.args.items(): - self.addCleanup(CONF.clear_override, k, self.group) - CONF.set_override(k, v, self.group) - - class PlacementFixture(fixtures.Fixture): """A fixture to placement operations. @@ -67,7 +39,8 @@ class PlacementFixture(fixtures.Fixture): def setUp(self): super(PlacementFixture, self).setUp() - self.useFixture(ConfPatcher(group='api', auth_strategy='noauth2')) + conf_fixture = config_fixture.Config(CONF) + conf_fixture.config(group='api', auth_strategy='noauth2') loader = deploy.loadapp(CONF) app = lambda: loader self.endpoint = 'http://%s/placement' % uuidutils.generate_uuid() diff --git a/nova/tests/functional/api_sample_tests/api_sample_base.py b/nova/tests/functional/api_sample_tests/api_sample_base.py index 2a9e76ea247a..89d9d806aa66 100644 --- a/nova/tests/functional/api_sample_tests/api_sample_base.py +++ b/nova/tests/functional/api_sample_tests/api_sample_base.py @@ -107,6 +107,7 @@ class ApiSampleTestBaseV21(testscenarios.WithScenarios, if not self.SUPPORTS_CELLS: self.useFixture(fixtures.Database()) self.useFixture(fixtures.Database(database='api')) + # FIXME(cdent): Placement db already provided by IntegratedHelpers self.useFixture(fixtures.Database(database='placement')) self.useFixture(fixtures.DefaultFlavorsFixture()) self.useFixture(fixtures.SingleCellSimple())