From 55868cbaf565ebf5cfb0f7634cdb2ed5d2fc6c6d Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 10 Apr 2017 18:16:01 -0400 Subject: [PATCH] Remove aggregate uuid generation on load from DB As the comments in the code suggest, we can remove this code now. There was a blocking database migration which enforced that the online data migrations to generate uuids for aggregates has been completed: 4d0915568a1011ddee7317ddfb237be0803e7790 So this code is safe to remove now. As a result, we also remove the online data migration routine which relies on the object code. Change-Id: I2050b5bdf906d2d2f8a87c79ca0804e0fc955755 --- nova/db/api.py | 4 ---- nova/db/sqlalchemy/api.py | 19 ------------------- nova/objects/aggregate.py | 18 ------------------ nova/tests/functional/db/test_aggregate.py | 3 ++- nova/tests/unit/objects/test_aggregate.py | 14 -------------- 5 files changed, 2 insertions(+), 56 deletions(-) diff --git a/nova/db/api.py b/nova/db/api.py index ce5ef5e15..fd285e53d 100644 --- a/nova/db/api.py +++ b/nova/db/api.py @@ -2044,10 +2044,6 @@ def pcidevice_online_data_migration(context, max_count): return IMPL.pcidevice_online_data_migration(context, max_count) -def aggregate_uuids_online_data_migration(context, max_count): - return IMPL.aggregate_uuids_online_data_migration(context, max_count) - - #################### diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py index 9057ea874..f88d7b024 100644 --- a/nova/db/sqlalchemy/api.py +++ b/nova/db/sqlalchemy/api.py @@ -6515,25 +6515,6 @@ def archive_deleted_rows(max_rows=None): return table_to_rows_archived -@pick_context_manager_writer -def aggregate_uuids_online_data_migration(context, max_count): - from nova.objects import aggregate - - count_all = 0 - count_hit = 0 - - results = model_query(context, models.Aggregate).filter_by( - uuid=None).limit(max_count) - for db_agg in results: - count_all += 1 - agg = aggregate.Aggregate._from_db_object(context, - aggregate.Aggregate(), - db_agg) - if 'uuid' in agg: - count_hit += 1 - return count_all, count_hit - - #################### diff --git a/nova/objects/aggregate.py b/nova/objects/aggregate.py index 943d0c713..7f34c06ce 100644 --- a/nova/objects/aggregate.py +++ b/nova/objects/aggregate.py @@ -258,20 +258,12 @@ class Aggregate(base.NovaPersistentObject, base.NovaObject): for key in aggregate.fields: if key == 'metadata': db_key = 'metadetails' - elif key == 'uuid': - continue elif key in DEPRECATED_FIELDS and key not in db_aggregate: continue else: db_key = key setattr(aggregate, key, db_aggregate[db_key]) - # NOTE(danms): Remove this conditional load (and remove uuid - # special cases above) once we're in Newton and have enforced - # that all UUIDs in the database are not NULL. - if db_aggregate.get('uuid'): - aggregate.uuid = db_aggregate['uuid'] - # NOTE: This can be removed when we remove compatibility with # the old aggregate model. if any(f not in db_aggregate for f in DEPRECATED_FIELDS): @@ -281,16 +273,6 @@ class Aggregate(base.NovaPersistentObject, base.NovaObject): aggregate._context = context aggregate.obj_reset_changes() - # NOTE(danms): This needs to come after obj_reset_changes() to make - # sure we only save the uuid, if we generate one. - # FIXME(danms): Remove this in Newton once we have enforced that - # all aggregates have uuids set in the database. - if 'uuid' not in aggregate: - aggregate.uuid = uuidutils.generate_uuid() - LOG.debug('Generating UUID %(uuid)s for aggregate %(agg)i', - dict(uuid=aggregate.uuid, agg=aggregate.id)) - aggregate.save() - return aggregate def _assert_no_hosts(self, action): diff --git a/nova/tests/functional/db/test_aggregate.py b/nova/tests/functional/db/test_aggregate.py index ab05e5367..3cfc7e10f 100644 --- a/nova/tests/functional/db/test_aggregate.py +++ b/nova/tests/functional/db/test_aggregate.py @@ -609,7 +609,8 @@ class AggregateMigrationTestCase(test.TestCase): self.context = context.get_admin_context() def test_migration(self): - db.aggregate_create(self.context, {'name': 'foo'}) + db.aggregate_create(self.context, {'name': 'foo', + 'uuid': uuidsentinel.agg_uuid}) main_aggregates_len = len(db.aggregate_get_all(self.context)) match, done = aggregate_obj.migrate_aggregates(self.context, 50) self.assertEqual(1, main_aggregates_len) diff --git a/nova/tests/unit/objects/test_aggregate.py b/nova/tests/unit/objects/test_aggregate.py index ebcd1e903..6f6d1bdd2 100644 --- a/nova/tests/unit/objects/test_aggregate.py +++ b/nova/tests/unit/objects/test_aggregate.py @@ -75,20 +75,6 @@ class _TestAggregateObject(object): mock_get_api.assert_called_once_with(self.context, 123) mock_get.assert_called_once_with(self.context, 123) - @mock.patch('nova.objects.Aggregate.save') - @mock.patch('nova.db.aggregate_get') - def test_load_allocates_uuid(self, mock_get, mock_save): - fake_agg = dict(fake_aggregate) - del fake_agg['uuid'] - mock_get.return_value = fake_agg - uuid = uuidsentinel.aggregate - with mock.patch('oslo_utils.uuidutils.generate_uuid') as mock_g: - mock_g.return_value = uuid - obj = aggregate.Aggregate.get_by_id(self.context, 123) - mock_g.assert_called_once_with() - self.assertEqual(uuid, obj.uuid) - mock_save.assert_called_once_with() - @mock.patch('nova.objects.aggregate._aggregate_get_from_db_by_uuid') @mock.patch('nova.db.aggregate_get_by_uuid') def test_get_by_uuid(self, get_by_uuid, get_by_uuid_api):