From 01e699152b99eacf61df72cac344f7276bd9c00a Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Wed, 12 Jun 2019 12:01:02 +0100 Subject: [PATCH] Remove incomplete consumer inline migrations Remove _create_incomplete_consumers_for_provider and _create_incomplete_consumer from placement.objects.allocation as we now have a blocker migration which means there should no longer be any incomplete consumers. Tests are updated, with one test that was testing both the online and inline migrations modified to simply confirm the online migration. Change-Id: I5a29685d1e22c66e4d09f04198c011e71340a5d0 --- placement/objects/allocation.py | 82 ------------------- .../tests/functional/db/test_consumer.py | 52 +----------- .../tests/unit/objects/test_allocation.py | 14 +--- 3 files changed, 5 insertions(+), 143 deletions(-) diff --git a/placement/objects/allocation.py b/placement/objects/allocation.py index a9945448e..82f92ac1d 100644 --- a/placement/objects/allocation.py +++ b/placement/objects/allocation.py @@ -319,86 +319,6 @@ def _get_allocations_by_consumer_uuid(ctx, consumer_uuid): return [dict(r) for r in ctx.session.execute(sel)] -@db_api.placement_context_manager.writer.independent -def _create_incomplete_consumers_for_provider(ctx, rp_id): - # TODO(jaypipes): Remove in Stein after a blocker migration is added. - """Creates consumer record if consumer relationship between allocations -> - consumers table is missing for any allocation on the supplied provider - internal ID, using the "incomplete consumer" project and user CONF options. - """ - alloc_to_consumer = sa.outerjoin( - _ALLOC_TBL, consumer_obj.CONSUMER_TBL, - _ALLOC_TBL.c.consumer_id == consumer_obj.CONSUMER_TBL.c.uuid) - sel = sa.select([_ALLOC_TBL.c.consumer_id]) - sel = sel.select_from(alloc_to_consumer) - sel = sel.where( - sa.and_( - _ALLOC_TBL.c.resource_provider_id == rp_id, - consumer_obj.CONSUMER_TBL.c.id.is_(None))) - missing = ctx.session.execute(sel).fetchall() - if missing: - # Do a single INSERT for all missing consumer relationships for the - # provider - incomplete_proj_id = project_obj.ensure_incomplete_project(ctx) - incomplete_user_id = user_obj.ensure_incomplete_user(ctx) - - cols = [ - _ALLOC_TBL.c.consumer_id, - incomplete_proj_id, - incomplete_user_id, - ] - sel = sa.select(cols) - sel = sel.select_from(alloc_to_consumer) - sel = sel.where( - sa.and_( - _ALLOC_TBL.c.resource_provider_id == rp_id, - consumer_obj.CONSUMER_TBL.c.id.is_(None))) - # NOTE(mnaser): It is possible to have multiple consumers having many - # allocations to the same resource provider, which would - # make the INSERT FROM SELECT fail due to duplicates. - sel = sel.group_by(_ALLOC_TBL.c.consumer_id) - target_cols = ['uuid', 'project_id', 'user_id'] - ins_stmt = consumer_obj.CONSUMER_TBL.insert().from_select( - target_cols, sel) - res = ctx.session.execute(ins_stmt) - if res.rowcount > 0: - LOG.info("Online data migration to fix incomplete consumers " - "for resource provider %s has been run. Migrated %d " - "incomplete consumer records on the fly.", rp_id, - res.rowcount) - - -@db_api.placement_context_manager.writer.independent -def _create_incomplete_consumer(ctx, consumer_id): - # TODO(jaypipes): Remove in Stein after a blocker migration is added. - """Creates consumer record if consumer relationship between allocations -> - consumers table is missing for the supplied consumer UUID, using the - "incomplete consumer" project and user CONF options. - """ - alloc_to_consumer = sa.outerjoin( - _ALLOC_TBL, consumer_obj.CONSUMER_TBL, - _ALLOC_TBL.c.consumer_id == consumer_obj.CONSUMER_TBL.c.uuid) - sel = sa.select([_ALLOC_TBL.c.consumer_id]) - sel = sel.select_from(alloc_to_consumer) - sel = sel.where( - sa.and_( - _ALLOC_TBL.c.consumer_id == consumer_id, - consumer_obj.CONSUMER_TBL.c.id.is_(None))) - missing = ctx.session.execute(sel).fetchall() - if missing: - incomplete_proj_id = project_obj.ensure_incomplete_project(ctx) - incomplete_user_id = user_obj.ensure_incomplete_user(ctx) - - ins_stmt = consumer_obj.CONSUMER_TBL.insert().values( - uuid=consumer_id, project_id=incomplete_proj_id, - user_id=incomplete_user_id) - res = ctx.session.execute(ins_stmt) - if res.rowcount > 0: - LOG.info("Online data migration to fix incomplete consumers " - "for consumer %s has been run. Migrated %d incomplete " - "consumer records on the fly.", consumer_id, res.rowcount) - - @oslo_db_api.wrap_db_retry(max_retries=5, retry_on_deadlock=True) @db_api.placement_context_manager.writer def _set_allocations(context, allocs): @@ -487,7 +407,6 @@ def _set_allocations(context, allocs): def get_all_by_resource_provider(context, rp): - _create_incomplete_consumers_for_provider(context, rp.id) db_allocs = _get_allocations_by_provider_id(context, rp.id) # Build up a list of Allocation objects, setting the Allocation object # fields to the same-named database record field we got from @@ -519,7 +438,6 @@ def get_all_by_resource_provider(context, rp): def get_all_by_consumer_id(context, consumer_id): - _create_incomplete_consumer(context, consumer_id) db_allocs = _get_allocations_by_consumer_uuid(context, consumer_id) if not db_allocs: diff --git a/placement/tests/functional/db/test_consumer.py b/placement/tests/functional/db/test_consumer.py index 702b66427..3a6c8abe2 100644 --- a/placement/tests/functional/db/test_consumer.py +++ b/placement/tests/functional/db/test_consumer.py @@ -192,60 +192,14 @@ class CreateIncompleteConsumersTestCase( # consumer_obj.create_incomplete_consumers() with a batch size of 1. # This should mean there will be two allocation records still remaining # with a missing consumer record (since we create 3 total to begin - # with). We then query the allocations table directly to grab that - # consumer UUID in the allocations table that doesn't refer to a - # consumer table record and call - # alloc_obj.get_all_by_consumer_id() with that consumer UUID. This - # should create the remaining missing consumer record "inline" in the - # alloc_obj.get_all_by_consumer_id() method. - # After that happens, there should still be a single allocation record - # that is missing a relation to the consumers table. We call the - # alloc_obj.get_all_by_resource_provider() method and verify that - # method cleans up the remaining incomplete consumers relationship. + # with). res = consumer_obj.create_incomplete_consumers(self.ctx, 1) self.assertEqual((1, 1), res) - # Grab the consumer UUID for the allocation record with a - # still-incomplete consumer record. + # Confirm there are still 2 incomplete allocations after one + # interation of the migration. res = _get_allocs_with_no_consumer_relationship(self.ctx) self.assertEqual(2, len(res)) - still_missing = res[0][0] - alloc_obj.get_all_by_consumer_id(self.ctx, still_missing) - - # There should still be a single missing consumer relationship. Let's - # grab that and call alloc_obj.get_all_by_resource_provider() - # which should clean that last one up for us. - res = _get_allocs_with_no_consumer_relationship(self.ctx) - self.assertEqual(1, len(res)) - still_missing = res[0][0] - rp1 = rp_obj.ResourceProvider(self.ctx, id=1) - alloc_obj.get_all_by_resource_provider(self.ctx, rp1) - - # get_all_by_resource_provider() should have auto-completed the still - # missing consumer record and _check_incomplete_consumers() should - # assert correctly that there are no more incomplete consumer records. - self._check_incomplete_consumers(self.ctx) - res = consumer_obj.create_incomplete_consumers(self.ctx, 10) - self.assertEqual((0, 0), res) - - def test_create_incomplete_consumers_multiple_allocs_per_consumer(self): - """Tests that missing consumer records are created when listing - allocations against a resource provider or running the online data - migration routine when the consumers have multiple allocations on the - same provider. - """ - self._create_incomplete_allocations(self.ctx, num_of_consumer_allocs=2) - # Run the online data migration to migrate one consumer. The batch size - # needs to be large enough to hit more than one consumer for this test - # where each consumer has two allocations. - res = consumer_obj.create_incomplete_consumers(self.ctx, 2) - self.assertEqual((2, 2), res) - # Migrate the rest by listing allocations on the resource provider. - rp1 = rp_obj.ResourceProvider(self.ctx, id=1) - alloc_obj.get_all_by_resource_provider(self.ctx, rp1) - self._check_incomplete_consumers(self.ctx) - res = consumer_obj.create_incomplete_consumers(self.ctx, 10) - self.assertEqual((0, 0), res) class DeleteConsumerIfNoAllocsTestCase(tb.PlacementDbBaseTestCase): diff --git a/placement/tests/unit/objects/test_allocation.py b/placement/tests/unit/objects/test_allocation.py index b34732c7d..7e915378f 100644 --- a/placement/tests/unit/objects/test_allocation.py +++ b/placement/tests/unit/objects/test_allocation.py @@ -68,13 +68,10 @@ class TestAllocationListNoDB(base.TestCase): super(TestAllocationListNoDB, self).setUp() base.fake_ensure_cache(self.context) - @mock.patch('placement.objects.allocation.' - '_create_incomplete_consumers_for_provider') @mock.patch('placement.objects.allocation.' '_get_allocations_by_provider_id', return_value=[_ALLOCATION_DB]) - def test_get_all_by_resource_provider( - self, mock_get_allocations_from_db, mock_create_consumers): + def test_get_all_by_resource_provider(self, mock_get_allocations_from_db): rp = rp_obj.ResourceProvider(self.context, id=_RESOURCE_PROVIDER_ID, uuid=uuids.resource_provider) @@ -88,22 +85,15 @@ class TestAllocationListNoDB(base.TestCase): allocations[0].created_at) self.assertEqual(_ALLOCATION_DB['updated_at'], allocations[0].updated_at) - mock_create_consumers.assert_called_once_with( - self.context, _RESOURCE_PROVIDER_ID) - @mock.patch('placement.objects.allocation.' - '_create_incomplete_consumer') @mock.patch('placement.objects.allocation.' '_get_allocations_by_consumer_uuid', return_value=[_ALLOCATION_BY_CONSUMER_DB]) - def test_get_all_by_consumer_id(self, mock_get_allocations_from_db, - mock_create_consumer): + def test_get_all_by_consumer_id(self, mock_get_allocations_from_db): allocations = alloc_obj.get_all_by_consumer_id( self.context, uuids.consumer) self.assertEqual(1, len(allocations)) - mock_create_consumer.assert_called_once_with(self.context, - uuids.consumer) mock_get_allocations_from_db.assert_called_once_with(self.context, uuids.consumer) self.assertEqual(_ALLOCATION_BY_CONSUMER_DB['used'],