From 2fc321aea3dc2fdd36cb3d950a09bd87d07f7ebc Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Wed, 17 Jul 2019 18:28:11 +0100 Subject: [PATCH] Move rc_cache onto RequestContext Experimentation led to the discovery (as described in the story noted below) that the global RC_CACHE is not safe in a multi-process environment, which is common for a web service designed to scale horizontally. In rare circumstances it is possible for a custom resource class to be deleted in one process but still appear to exist in another. For many situations this wouldn't really matter, but there are cases, even more rare, where it would be possible to write allocations or resource provider inventory using the wrong resource class id. On the related story, a variety of options were discussed to fix this. Reading through the code this one (which is option 2) was the only one that proved workable in a relatively straightforward fashion: Have a per request cache. To that end, when a RequestContext is created (per request) the resource class table is scanned to create a cache. Because the context is local to this request, we no longer need to do any locking around the cache, either when we create it or when we clear it: The caller is linear. The cost of this is that now every single request starts with a scan of the resource class table. This isn't horrible: if we had no cache at all we'd be reading rows from that table multiple times throughout any request (read or write). We should probably do some performance analysis to see what the impact of this might be. The perfload jobs may be able to give a limited sense of what the impact is, but profiling will be required for accuracy. It is the case that the functional tests seem a bit slower because of that additional db query. Change-Id: I409a5e819a72d64e66ee390e4528da0c503d8d05 Story: 2006232 Task: 35833 --- placement/context.py | 2 + placement/deploy.py | 2 - placement/objects/allocation.py | 13 +++--- placement/objects/allocation_candidate.py | 13 +++--- placement/objects/inventory.py | 3 +- placement/objects/research_context.py | 7 ++- placement/objects/resource_class.py | 7 ++- placement/objects/resource_provider.py | 17 ++++--- placement/objects/usage.py | 12 ++--- placement/resource_class_cache.py | 46 ++++++++----------- placement/tests/fixtures.py | 2 - placement/tests/unit/base.py | 24 ++++++++++ placement/tests/unit/cmd/test_manage.py | 3 +- .../tests/unit/handlers/test_aggregate.py | 4 +- placement/tests/unit/handlers/test_util.py | 4 +- placement/tests/unit/objects/base.py | 10 +--- .../tests/unit/objects/test_allocation.py | 1 - .../tests/unit/objects/test_inventory.py | 5 +- placement/tests/unit/test_context.py | 4 +- placement/tests/unit/test_policy.py | 4 +- placement/tests/unit/test_util.py | 5 +- 21 files changed, 93 insertions(+), 95 deletions(-) create mode 100644 placement/tests/unit/base.py diff --git a/placement/context.py b/placement/context.py index b76ef1e78..787b625c5 100644 --- a/placement/context.py +++ b/placement/context.py @@ -15,6 +15,7 @@ from oslo_db.sqlalchemy import enginefacade from placement import exception from placement import policy +from placement import resource_class_cache as rc_cache @enginefacade.transaction_context_provider @@ -22,6 +23,7 @@ class RequestContext(context.RequestContext): def __init__(self, *args, **kwargs): self.config = kwargs.pop('config', None) + self.rc_cache = rc_cache.ensure(self) super(RequestContext, self).__init__(*args, **kwargs) def can(self, action, target=None, fatal=True): diff --git a/placement/deploy.py b/placement/deploy.py index 5e3ac373f..b120490cc 100644 --- a/placement/deploy.py +++ b/placement/deploy.py @@ -27,7 +27,6 @@ from placement.objects import resource_class from placement.objects import trait from placement import policy from placement import requestlog -from placement import resource_class_cache as rc_cache from placement import util @@ -125,7 +124,6 @@ def update_database(conf): ctx = db_api.DbContext() trait.ensure_sync(ctx) resource_class.ensure_sync(ctx) - rc_cache.ensure(ctx) # NOTE(cdent): Althought project_name is no longer used because of the diff --git a/placement/objects/allocation.py b/placement/objects/allocation.py index 82f92ac1d..b370087ed 100644 --- a/placement/objects/allocation.py +++ b/placement/objects/allocation.py @@ -24,7 +24,6 @@ from placement.objects import consumer as consumer_obj from placement.objects import project as project_obj from placement.objects import resource_provider as rp_obj from placement.objects import user as user_obj -from placement import resource_class_cache as rc_cache _ALLOC_TBL = models.Allocation.__table__ @@ -120,7 +119,7 @@ def _check_capacity_exceeded(ctx, allocs): # # We then take the results of the above and determine if any of the # inventory will have its capacity exceeded. - rc_ids = set([rc_cache.RC_CACHE.id_from_string(a.resource_class) + rc_ids = set([ctx.rc_cache.id_from_string(a.resource_class) for a in allocs]) provider_uuids = set([a.resource_provider.uuid for a in allocs]) provider_ids = set([a.resource_provider.id for a in allocs]) @@ -175,7 +174,7 @@ def _check_capacity_exceeded(ctx, allocs): # Ensure that all providers have existing inventory missing_provs = provider_uuids - provs_with_inv if missing_provs: - class_str = ', '.join([rc_cache.RC_CACHE.string_from_id(rc_id) + class_str = ', '.join([ctx.rc_cache.string_from_id(rc_id) for rc_id in rc_ids]) provider_str = ', '.join(missing_provs) raise exception.InvalidInventory( @@ -185,7 +184,7 @@ def _check_capacity_exceeded(ctx, allocs): rp_resource_class_sum = collections.defaultdict( lambda: collections.defaultdict(int)) for alloc in allocs: - rc_id = rc_cache.RC_CACHE.id_from_string(alloc.resource_class) + rc_id = ctx.rc_cache.id_from_string(alloc.resource_class) rp_uuid = alloc.resource_provider.uuid if rp_uuid not in res_providers: res_providers[rp_uuid] = alloc.resource_provider @@ -377,7 +376,7 @@ def _set_allocations(context, allocs): continue consumer_id = alloc.consumer.uuid rp = alloc.resource_provider - rc_id = rc_cache.RC_CACHE.id_from_string(alloc.resource_class) + rc_id = context.rc_cache.id_from_string(alloc.resource_class) ins_stmt = _ALLOC_TBL.insert().values( resource_provider_id=rp.id, resource_class_id=rc_id, @@ -428,7 +427,7 @@ def get_all_by_resource_provider(context, rp): objs.append( Allocation( id=rec['id'], resource_provider=rp, - resource_class=rc_cache.RC_CACHE.string_from_id( + resource_class=context.rc_cache.string_from_id( rec['resource_class_id']), consumer=consumer, used=rec['used'], @@ -474,7 +473,7 @@ def get_all_by_consumer_id(context, consumer_id): uuid=rec['resource_provider_uuid'], name=rec['resource_provider_name'], generation=rec['resource_provider_generation']), - resource_class=rc_cache.RC_CACHE.string_from_id( + resource_class=context.rc_cache.string_from_id( rec['resource_class_id']), consumer=consumer, used=rec['used'], diff --git a/placement/objects/allocation_candidate.py b/placement/objects/allocation_candidate.py index 2e1280bd1..a92c0c376 100644 --- a/placement/objects/allocation_candidate.py +++ b/placement/objects/allocation_candidate.py @@ -27,7 +27,6 @@ from placement import exception from placement.objects import research_context as res_ctx from placement.objects import resource_provider as rp_obj from placement.objects import trait as trait_obj -from placement import resource_class_cache as rc_cache _ALLOC_TBL = models.Allocation.__table__ @@ -288,12 +287,13 @@ def _alloc_candidates_multiple_providers(rg_ctx, rp_candidates): # resource class internal ID, of lists of AllocationRequestResource objects tree_dict = collections.defaultdict(lambda: collections.defaultdict(list)) + rc_cache = rg_ctx.context.rc_cache for rp in rp_candidates.rps_info: rp_summary = summaries[rp.id] tree_dict[rp.root_id][rp.rc_id].append( AllocationRequestResource( resource_provider=rp_summary.resource_provider, - resource_class=rc_cache.RC_CACHE.string_from_id(rp.rc_id), + resource_class=rc_cache.string_from_id(rp.rc_id), amount=rg_ctx.resources[rp.rc_id])) # Next, build up a set of allocation requests. These allocation requests @@ -391,7 +391,7 @@ def _alloc_candidates_single_provider(rg_ctx, rw_ctx, rp_tuples): for rp_id, root_id in rp_tuples: rp_summary = summaries[rp_id] req_obj = _allocation_request_for_provider( - rg_ctx.resources, rp_summary.resource_provider, + rg_ctx.context, rg_ctx.resources, rp_summary.resource_provider, suffix=rg_ctx.suffix) # Exclude this if its anchor (which is its root) isn't in our # prefiltered list of anchors @@ -416,7 +416,8 @@ def _alloc_candidates_single_provider(rg_ctx, rw_ctx, rp_tuples): return alloc_requests, list(summaries.values()) -def _allocation_request_for_provider(requested_resources, provider, suffix): +def _allocation_request_for_provider(context, requested_resources, provider, + suffix): """Returns an AllocationRequest object containing AllocationRequestResource objects for each resource class in the supplied requested resources dict. @@ -430,7 +431,7 @@ def _allocation_request_for_provider(requested_resources, provider, suffix): resource_requests = [ AllocationRequestResource( resource_provider=provider, - resource_class=rc_cache.RC_CACHE.string_from_id(rc_id), + resource_class=context.rc_cache.string_from_id(rc_id), amount=amount ) for rc_id, amount in requested_resources.items() ] @@ -508,7 +509,7 @@ def _build_provider_summaries(context, usages, prov_traits): used = int(usage['used'] or 0) allocation_ratio = usage['allocation_ratio'] cap = int((usage['total'] - usage['reserved']) * allocation_ratio) - rc_name = rc_cache.RC_CACHE.string_from_id(rc_id) + rc_name = context.rc_cache.string_from_id(rc_id) rpsr = ProviderSummaryResource( resource_class=rc_name, capacity=cap, diff --git a/placement/objects/inventory.py b/placement/objects/inventory.py index 50960bdcc..3f3b70528 100644 --- a/placement/objects/inventory.py +++ b/placement/objects/inventory.py @@ -15,7 +15,6 @@ import sqlalchemy as sa from placement.db.sqlalchemy import models from placement import db_api -from placement import resource_class_cache as rc_cache _INV_TBL = models.Inventory.__table__ @@ -75,7 +74,7 @@ def get_all_by_resource_provider(context, rp): inv_list = [ Inventory( resource_provider=rp, - resource_class=rc_cache.RC_CACHE.string_from_id( + resource_class=context.rc_cache.string_from_id( rec['resource_class_id']), **rec) for rec in db_inv diff --git a/placement/objects/research_context.py b/placement/objects/research_context.py index c56400c05..a45abd79d 100644 --- a/placement/objects/research_context.py +++ b/placement/objects/research_context.py @@ -23,7 +23,6 @@ from placement import db_api from placement import exception from placement.objects import rp_candidates from placement.objects import trait as trait_obj -from placement import resource_class_cache as rc_cache # TODO(tetsuro): Move these public symbols in a central place. @@ -65,7 +64,7 @@ class RequestGroupSearchContext(object): # A dict, keyed by resource class internal ID, of the amounts of that # resource class being requested by the group. self.resources = { - rc_cache.RC_CACHE.id_from_string(key): value + context.rc_cache.id_from_string(key): value for key, value in group.resources.items() } @@ -526,7 +525,7 @@ def get_provider_ids_matching(rg_ctx): provs_with_resource = set() first = True for rc_id, amount in rg_ctx.resources.items(): - rc_name = rc_cache.RC_CACHE.string_from_id(rc_id) + rc_name = rg_ctx.context.rc_cache.string_from_id(rc_id) provs_with_resource = rg_ctx.get_rps_with_resource(rc_id) LOG.debug("found %d providers with available %d %s", len(provs_with_resource), amount, rc_name) @@ -620,7 +619,7 @@ def get_trees_matching_all(rg_ctx, rw_ctx): provs_with_inv = rp_candidates.RPCandidateList() for rc_id, amount in rg_ctx.resources.items(): - rc_name = rc_cache.RC_CACHE.string_from_id(rc_id) + rc_name = rg_ctx.context.rc_cache.string_from_id(rc_id) provs_with_inv_rc = rp_candidates.RPCandidateList() rc_provs_with_inv = rg_ctx.get_rps_with_resource(rc_id) diff --git a/placement/objects/resource_class.py b/placement/objects/resource_class.py index 50e470ff5..7c8ff9077 100644 --- a/placement/objects/resource_class.py +++ b/placement/objects/resource_class.py @@ -23,7 +23,6 @@ from sqlalchemy import func from placement.db.sqlalchemy import models from placement import db_api from placement import exception -from placement import resource_class_cache as rc_cache _RC_TBL = models.ResourceClass.__table__ _RESOURCE_CLASSES_LOCK = 'resource_classes_sync' @@ -70,7 +69,7 @@ class ResourceClass(object): :raises: ResourceClassNotFound if no such resource class was found """ - rc = rc_cache.RC_CACHE.all_from_string(name) + rc = context.rc_cache.all_from_string(name) obj = cls(context, id=rc['id'], name=rc['name'], updated_at=rc['updated_at'], created_at=rc['created_at']) return obj @@ -157,7 +156,7 @@ class ResourceClass(object): resource_class=self.name) self._destroy(self._context, self.id, self.name) - rc_cache.RC_CACHE.clear() + self._context.rc_cache.clear() @staticmethod @db_api.placement_context_manager.writer @@ -188,7 +187,7 @@ class ResourceClass(object): raise exception.ResourceClassCannotUpdateStandard( resource_class=self.name) self._save(self._context, self.id, self.name, updates) - rc_cache.RC_CACHE.clear() + self._context.rc_cache.clear() @staticmethod @db_api.placement_context_manager.writer diff --git a/placement/objects/resource_provider.py b/placement/objects/resource_provider.py index 827625534..663896fe8 100644 --- a/placement/objects/resource_provider.py +++ b/placement/objects/resource_provider.py @@ -32,7 +32,6 @@ from placement import exception from placement.objects import inventory as inv_obj from placement.objects import research_context as res_ctx from placement.objects import trait as trait_obj -from placement import resource_class_cache as rc_cache _TRAIT_TBL = models.Trait.__table__ _ALLOC_TBL = models.Allocation.__table__ @@ -81,7 +80,7 @@ def _delete_inventory_from_provider(ctx, rp, to_delete): allocations = ctx.session.execute(allocation_query).fetchall() if allocations: resource_classes = ', '.join( - [rc_cache.RC_CACHE.string_from_id(alloc[0]) + [ctx.rc_cache.string_from_id(alloc[0]) for alloc in allocations]) raise exception.InventoryInUse(resource_classes=resource_classes, resource_provider=rp.uuid) @@ -105,7 +104,7 @@ def _add_inventory_to_provider(ctx, rp, inv_list, to_add): adding to resource provider. """ for rc_id in to_add: - rc_str = rc_cache.RC_CACHE.string_from_id(rc_id) + rc_str = ctx.rc_cache.string_from_id(rc_id) inv_record = inv_obj.find(inv_list, rc_str) ins_stmt = _INV_TBL.insert().values( resource_provider_id=rp.id, @@ -133,7 +132,7 @@ def _update_inventory_for_provider(ctx, rp, inv_list, to_update): """ exceeded = [] for rc_id in to_update: - rc_str = rc_cache.RC_CACHE.string_from_id(rc_id) + rc_str = ctx.rc_cache.string_from_id(rc_id) inv_record = inv_obj.find(inv_list, rc_str) allocation_query = sa.select( [func.sum(_ALLOC_TBL.c.used).label('usage')]) @@ -171,7 +170,7 @@ def _add_inventory(context, rp, inventory): :raises `exception.ResourceClassNotFound` if inventory.resource_class cannot be found in the DB. """ - rc_id = rc_cache.RC_CACHE.id_from_string(inventory.resource_class) + rc_id = context.rc_cache.id_from_string(inventory.resource_class) _add_inventory_to_provider( context, rp, [inventory], set([rc_id])) rp.increment_generation() @@ -184,7 +183,7 @@ def _update_inventory(context, rp, inventory): :raises `exception.ResourceClassNotFound` if inventory.resource_class cannot be found in the DB. """ - rc_id = rc_cache.RC_CACHE.id_from_string(inventory.resource_class) + rc_id = context.rc_cache.id_from_string(inventory.resource_class) exceeded = _update_inventory_for_provider( context, rp, [inventory], set([rc_id])) rp.increment_generation() @@ -198,7 +197,7 @@ def _delete_inventory(context, rp, resource_class): :raises `exception.ResourceClassNotFound` if resource_class cannot be found in the DB. """ - rc_id = rc_cache.RC_CACHE.id_from_string(resource_class) + rc_id = context.rc_cache.id_from_string(resource_class) if not _delete_inventory_from_provider(context, rp, [rc_id]): raise exception.NotFound( 'No inventory of class %s found for delete' @@ -227,7 +226,7 @@ def _set_inventory(context, rp, inv_list): from a provider that has allocations for that resource class. """ existing_resources = _get_current_inventory_resources(context, rp) - these_resources = set([rc_cache.RC_CACHE.id_from_string(r.resource_class) + these_resources = set([context.rc_cache.id_from_string(r.resource_class) for r in inv_list]) # Determine which resources we should be adding, deleting and/or @@ -965,7 +964,7 @@ def _get_all_by_filters_from_db(context, filters): if rps_bad_aggs: query = query.where(~rp.c.id.in_(rps_bad_aggs)) for rc_name, amount in resources.items(): - rc_id = rc_cache.RC_CACHE.id_from_string(rc_name) + rc_id = context.rc_cache.id_from_string(rc_name) rps_with_resource = res_ctx.get_providers_with_resource( context, rc_id, amount) rps_with_resource = (rp[0] for rp in rps_with_resource) diff --git a/placement/objects/usage.py b/placement/objects/usage.py index e5995236e..3bdd3aeb3 100644 --- a/placement/objects/usage.py +++ b/placement/objects/usage.py @@ -15,16 +15,12 @@ from sqlalchemy import sql from placement.db.sqlalchemy import models from placement import db_api -from placement import resource_class_cache as rc_cache class Usage(object): - def __init__(self, resource_class=None, resource_class_id=None, usage=0): + def __init__(self, resource_class=None, usage=0): self.resource_class = resource_class - if resource_class_id is not None: - self.resource_class = rc_cache.RC_CACHE.string_from_id( - resource_class_id) self.usage = int(usage) @@ -55,7 +51,8 @@ def _get_all_by_resource_provider_uuid(context, rp_uuid): models.Allocation.resource_class_id)) .filter(models.ResourceProvider.uuid == rp_uuid) .group_by(models.Inventory.resource_class_id)) - result = [dict(resource_class_id=item[0], usage=item[1]) + result = [dict(resource_class=context.rc_cache.string_from_id(item[0]), + usage=item[1]) for item in query.all()] return result @@ -74,6 +71,7 @@ def _get_all_by_project_user(context, project_id, user_id=None): models.Consumer.user_id == models.User.id) query = query.filter(models.User.external_id == user_id) query = query.group_by(models.Allocation.resource_class_id) - result = [dict(resource_class_id=item[0], usage=item[1]) + result = [dict(resource_class=context.rc_cache.string_from_id(item[0]), + usage=item[1]) for item in query.all()] return result diff --git a/placement/resource_class_cache.py b/placement/resource_class_cache.py index ee1a9b36a..1bb7dac54 100644 --- a/placement/resource_class_cache.py +++ b/placement/resource_class_cache.py @@ -10,30 +10,24 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_concurrency import lockutils import sqlalchemy as sa from placement.db.sqlalchemy import models from placement import db_api from placement import exception -RC_CACHE = None _RC_TBL = models.ResourceClass.__table__ -_LOCKNAME = 'rc_cache' @db_api.placement_context_manager.reader def ensure(ctx): - """Ensures that a singleton resource class cache has been created in the - module's scope. + """Ensures that a resource class cache has been created for the provided + context. :param ctx: `placement.context.RequestContext` that may be used to grab a DB connection. """ - global RC_CACHE - if RC_CACHE is not None: - return - RC_CACHE = ResourceClassCache(ctx) + return ResourceClassCache(ctx) @db_api.placement_context_manager.reader @@ -66,10 +60,9 @@ class ResourceClassCache(object): self.all_cache = {} def clear(self): - with lockutils.lock(_LOCKNAME): - self.id_cache = {} - self.str_cache = {} - self.all_cache = {} + self.id_cache = {} + self.str_cache = {} + self.all_cache = {} def id_from_string(self, rc_str): """Given a string representation of a resource class -- e.g. "DISK_GB" @@ -89,11 +82,10 @@ class ResourceClassCache(object): return rc_id # Otherwise, check the database table - with lockutils.lock(_LOCKNAME): - _refresh_from_db(self.ctx, self) - if rc_str in self.id_cache: - return self.id_cache[rc_str] - raise exception.ResourceClassNotFound(resource_class=rc_str) + _refresh_from_db(self.ctx, self) + if rc_str in self.id_cache: + return self.id_cache[rc_str] + raise exception.ResourceClassNotFound(resource_class=rc_str) def all_from_string(self, rc_str): """Given a string representation of a resource class -- e.g. "DISK_GB" @@ -112,11 +104,10 @@ class ResourceClassCache(object): return rc_id_str # Otherwise, check the database table - with lockutils.lock(_LOCKNAME): - _refresh_from_db(self.ctx, self) - if rc_str in self.all_cache: - return self.all_cache[rc_str] - raise exception.ResourceClassNotFound(resource_class=rc_str) + _refresh_from_db(self.ctx, self) + if rc_str in self.all_cache: + return self.all_cache[rc_str] + raise exception.ResourceClassNotFound(resource_class=rc_str) def string_from_id(self, rc_id): """The reverse of the id_from_string() method. Given a supplied numeric @@ -135,8 +126,7 @@ class ResourceClassCache(object): return rc_str # Otherwise, check the database table - with lockutils.lock(_LOCKNAME): - _refresh_from_db(self.ctx, self) - if rc_id in self.str_cache: - return self.str_cache[rc_id] - raise exception.ResourceClassNotFound(resource_class=rc_id) + _refresh_from_db(self.ctx, self) + if rc_id in self.str_cache: + return self.str_cache[rc_id] + raise exception.ResourceClassNotFound(resource_class=rc_id) diff --git a/placement/tests/fixtures.py b/placement/tests/fixtures.py index be235b355..1405b1298 100644 --- a/placement/tests/fixtures.py +++ b/placement/tests/fixtures.py @@ -26,7 +26,6 @@ from placement import db_api as placement_db from placement import deploy from placement.objects import resource_class from placement.objects import trait -from placement import resource_class_cache as rc_cache class Database(test_fixtures.GeneratesSchema, test_fixtures.AdHocDbFixture): @@ -77,4 +76,3 @@ class Database(test_fixtures.GeneratesSchema, test_fixtures.AdHocDbFixture): def cleanup(self): trait._TRAITS_SYNCED = False resource_class._RESOURCE_CLASSES_SYNCED = False - rc_cache.RC_CACHE = None diff --git a/placement/tests/unit/base.py b/placement/tests/unit/base.py new file mode 100644 index 000000000..d982034c9 --- /dev/null +++ b/placement/tests/unit/base.py @@ -0,0 +1,24 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import fixtures +import testtools + + +class ContextTestCase(testtools.TestCase): + """Base class for tests that need a mocked resource_class_cache on Context. + """ + + def setUp(self): + super(ContextTestCase, self).setUp() + self.useFixture( + fixtures.MockPatch('placement.resource_class_cache.ensure')) diff --git a/placement/tests/unit/cmd/test_manage.py b/placement/tests/unit/cmd/test_manage.py index 0cc949cb8..cd972f355 100644 --- a/placement/tests/unit/cmd/test_manage.py +++ b/placement/tests/unit/cmd/test_manage.py @@ -20,6 +20,7 @@ import testtools from placement.cmd import manage from placement import conf +from placement.tests.unit import base class TestCommandParsers(testtools.TestCase): @@ -109,7 +110,7 @@ class TestCommandParsers(testtools.TestCase): self.output.stdout.read()) -class TestDBCommands(testtools.TestCase): +class TestDBCommands(base.ContextTestCase): def setUp(self): super(TestDBCommands, self).setUp() diff --git a/placement/tests/unit/handlers/test_aggregate.py b/placement/tests/unit/handlers/test_aggregate.py index 3f8849f12..c8282638e 100644 --- a/placement/tests/unit/handlers/test_aggregate.py +++ b/placement/tests/unit/handlers/test_aggregate.py @@ -13,16 +13,16 @@ import mock import six -import testtools import webob from placement import context from placement import exception from placement.handlers import aggregate from placement.objects import resource_provider +from placement.tests.unit import base -class TestAggregateHandlerErrors(testtools.TestCase): +class TestAggregateHandlerErrors(base.ContextTestCase): """Tests that make sure errors hard to trigger by gabbi result in expected exceptions. """ diff --git a/placement/tests/unit/handlers/test_util.py b/placement/tests/unit/handlers/test_util.py index 730381c4b..82bb91896 100644 --- a/placement/tests/unit/handlers/test_util.py +++ b/placement/tests/unit/handlers/test_util.py @@ -17,7 +17,6 @@ import microversion_parse from oslo_config import cfg from oslo_config import fixture as config_fixture from oslo_utils.fixture import uuidsentinel -import testtools import webob from placement import conf @@ -28,9 +27,10 @@ from placement import microversion from placement.objects import consumer as consumer_obj from placement.objects import project as project_obj from placement.objects import user as user_obj +from placement.tests.unit import base -class TestEnsureConsumer(testtools.TestCase): +class TestEnsureConsumer(base.ContextTestCase): def setUp(self): super(TestEnsureConsumer, self).setUp() self.conf = cfg.ConfigOpts() diff --git a/placement/tests/unit/objects/base.py b/placement/tests/unit/objects/base.py index 5c0c83db9..61042b089 100644 --- a/placement/tests/unit/objects/base.py +++ b/placement/tests/unit/objects/base.py @@ -10,21 +10,15 @@ # License for the specific language governing permissions and limitations # under the License. -import mock from oslo_config import cfg from oslo_config import fixture as config_fixture -import testtools from placement import conf from placement import context -from placement import resource_class_cache as rc_cache +from placement.tests.unit import base as unit_base -def fake_ensure_cache(ctxt): - rc_cache.RC_CACHE = mock.MagicMock() - - -class TestCase(testtools.TestCase): +class TestCase(unit_base.ContextTestCase): """Base class for other tests in this file. It establishes the RequestContext used as self.context in the tests. diff --git a/placement/tests/unit/objects/test_allocation.py b/placement/tests/unit/objects/test_allocation.py index 7e915378f..3ec1ebacc 100644 --- a/placement/tests/unit/objects/test_allocation.py +++ b/placement/tests/unit/objects/test_allocation.py @@ -66,7 +66,6 @@ class TestAllocationListNoDB(base.TestCase): def setUp(self): super(TestAllocationListNoDB, self).setUp() - base.fake_ensure_cache(self.context) @mock.patch('placement.objects.allocation.' '_get_allocations_by_provider_id', diff --git a/placement/tests/unit/objects/test_inventory.py b/placement/tests/unit/objects/test_inventory.py index 8cbf7ac94..02c45fef7 100644 --- a/placement/tests/unit/objects/test_inventory.py +++ b/placement/tests/unit/objects/test_inventory.py @@ -46,11 +46,8 @@ _INVENTORY_DB = { class TestInventoryNoDB(base.TestCase): - @mock.patch('placement.resource_class_cache.ensure', - side_effect=base.fake_ensure_cache) @mock.patch('placement.objects.inventory._get_inventory_by_provider_id') - def test_get_all_by_resource_provider(self, mock_get, mock_ensure_cache): - mock_ensure_cache(self.context) + def test_get_all_by_resource_provider(self, mock_get): expected = [dict(_INVENTORY_DB, resource_provider_id=_RESOURCE_PROVIDER_ID), dict(_INVENTORY_DB, diff --git a/placement/tests/unit/test_context.py b/placement/tests/unit/test_context.py index f538c8bfe..665a94f65 100644 --- a/placement/tests/unit/test_context.py +++ b/placement/tests/unit/test_context.py @@ -11,13 +11,13 @@ # under the License. import mock -import testtools from placement import context from placement import exception +from placement.tests.unit import base -class TestPlacementRequestContext(testtools.TestCase): +class TestPlacementRequestContext(base.ContextTestCase): """Test cases for PlacementRequestContext.""" def setUp(self): diff --git a/placement/tests/unit/test_policy.py b/placement/tests/unit/test_policy.py index a621049d8..6dcf30101 100644 --- a/placement/tests/unit/test_policy.py +++ b/placement/tests/unit/test_policy.py @@ -16,16 +16,16 @@ import fixtures from oslo_config import cfg from oslo_config import fixture as config_fixture from oslo_policy import policy as oslo_policy -import testtools from placement import conf from placement import context from placement import exception from placement import policy +from placement.tests.unit import base from placement.tests.unit import policy_fixture -class PlacementPolicyTestCase(testtools.TestCase): +class PlacementPolicyTestCase(base.ContextTestCase): """Tests interactions with placement policy.""" def setUp(self): super(PlacementPolicyTestCase, self).setUp() diff --git a/placement/tests/unit/test_util.py b/placement/tests/unit/test_util.py index 92fdb92b0..40360f1b8 100644 --- a/placement/tests/unit/test_util.py +++ b/placement/tests/unit/test_util.py @@ -31,6 +31,7 @@ from placement import lib as pl from placement import microversion from placement.objects import resource_class as rc_obj from placement.objects import resource_provider as rp_obj +from placement.tests.unit import base from placement import util @@ -283,7 +284,7 @@ class TestRequireContent(testtools.TestCase): self.assertTrue(self.handler(req)) -class TestPlacementURLs(testtools.TestCase): +class TestPlacementURLs(base.ContextTestCase): def setUp(self): super(TestPlacementURLs, self).setUp() @@ -946,7 +947,7 @@ class TestParseQsRequestGroups(testtools.TestCase): webob.exc.HTTPBadRequest, self.do_parse, qs, version=(1, 22)) -class TestPickLastModified(testtools.TestCase): +class TestPickLastModified(base.ContextTestCase): def setUp(self): super(TestPickLastModified, self).setUp()