From 63094c288317b7624e43f4f0d9c712d6576b663d Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 11 Feb 2019 16:20:43 -0500 Subject: [PATCH] Filter duplicates from compute API get_migrations_sorted() During a cross-cell resize there are migration records with the same UUID in multiple cells and we need to filter duplicates out of the GET /os-migrations response. This adds logic to do that which picks the record with the newest "updated_at" value. As noted inline, an alternative to this would be leveraging the Migration.hidden attribute which is not currently set to True anywhere. If we used that attribute, we could handle duplicate migrations the same way we handle duplicate instances, but we would need to think about compatibility impacts to the GET /os-migrations API since the "hidden" filter parameter on that API is essentially useless today. Part of blueprint cross-cell-resize Change-Id: Ife9e5c77a02c8aac37311aac1e6d6814ff8de0e8 --- nova/compute/api.py | 49 +++++++++++++++- nova/tests/unit/compute/test_compute_api.py | 63 +++++++++++++++++++++ 2 files changed, 111 insertions(+), 1 deletion(-) diff --git a/nova/compute/api.py b/nova/compute/api.py index 5a6a7429d9d0..a65f66ba3fac 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -4921,7 +4921,54 @@ class API(base.Base): """Get all migrations for the given parameters.""" mig_objs = migration_list.get_migration_objects_sorted( context, filters, limit, marker, sort_keys, sort_dirs) - return mig_objs + # Due to cross-cell resize, we could have duplicate migration records + # while the instance is in VERIFY_RESIZE state in the destination cell + # but the original migration record still exists in the source cell. + # Filter out duplicate migration records here based on which record + # is newer (last updated). + + def _get_newer_obj(obj1, obj2): + # created_at will always be set. + created_at1 = obj1.created_at + created_at2 = obj2.created_at + # updated_at might be None + updated_at1 = obj1.updated_at + updated_at2 = obj2.updated_at + # If both have updated_at, compare using that field. + if updated_at1 and updated_at2: + if updated_at1 > updated_at2: + return obj1 + return obj2 + # Compare created_at versus updated_at. + if updated_at1: + if updated_at1 > created_at2: + return obj1 + return obj2 + if updated_at2: + if updated_at2 > created_at1: + return obj2 + return obj1 + # Compare created_at only. + if created_at1 > created_at2: + return obj1 + return obj2 + + # TODO(mriedem): This could be easier if we leveraged the "hidden" + # field on the Migration record and then just did like + # _get_unique_filter_method in the get_all() method for instances. + migrations_by_uuid = collections.OrderedDict() # maintain sort order + for migration in mig_objs: + if migration.uuid not in migrations_by_uuid: + migrations_by_uuid[migration.uuid] = migration + else: + # We have a collision, keep the newer record. + # Note that using updated_at could be wrong if changes-since or + # changes-before filters are being used but we have the same + # issue in _get_unique_filter_method for instances. + doppelganger = migrations_by_uuid[migration.uuid] + newer = _get_newer_obj(doppelganger, migration) + migrations_by_uuid[migration.uuid] = newer + return objects.MigrationList(objects=list(migrations_by_uuid.values())) def get_migrations_in_progress_by_instance(self, context, instance_uuid, migration_type=None): diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index ac57e4f1ffe5..4d4f15fb0331 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -6673,6 +6673,69 @@ class ComputeAPIUnitTestCase(_ComputeAPIUnitTestMixIn, test.NoDBTestCase): mock_target_cell.return_value.__enter__.return_value, uuids.host, use_slave=True) + def _test_get_migrations_sorted_filter_duplicates(self, migrations, + expected): + """Tests the cross-cell scenario where there are multiple migrations + with the same UUID from different cells and only one should be + returned. + """ + sort_keys = ['created_at', 'id'] + sort_dirs = ['desc', 'desc'] + filters = {'migration_type': 'resize'} + limit = 1000 + marker = None + with mock.patch( + 'nova.compute.migration_list.get_migration_objects_sorted', + return_value=objects.MigrationList( + objects=migrations)) as getter: + sorted_migrations = self.compute_api.get_migrations_sorted( + self.context, filters, sort_dirs=sort_dirs, + sort_keys=sort_keys, limit=limit, marker=marker) + self.assertEqual(1, len(sorted_migrations)) + getter.assert_called_once_with( + self.context, filters, limit, marker, sort_keys, sort_dirs) + self.assertIs(expected, sorted_migrations[0]) + + def test_get_migrations_sorted_filter_duplicates(self): + """Tests filtering duplicated Migration records where both have + created_at and updated_at set. + """ + t1 = timeutils.utcnow() + source_cell_migration = objects.Migration( + uuid=uuids.migration, created_at=t1, updated_at=t1) + t2 = t1 + datetime.timedelta(seconds=1) + target_cell_migration = objects.Migration( + uuid=uuids.migration, created_at=t2, updated_at=t2) + self._test_get_migrations_sorted_filter_duplicates( + [source_cell_migration, target_cell_migration], + target_cell_migration) + # Run it again in reverse. + self._test_get_migrations_sorted_filter_duplicates( + [target_cell_migration, source_cell_migration], + target_cell_migration) + + def test_get_migrations_sorted_filter_duplicates_using_created_at(self): + """Tests the cross-cell scenario where there are multiple migrations + with the same UUID from different cells and only one should be + returned. In this test the first Migration object to be processed has + not been updated yet but is created after the second record to process. + """ + t1 = timeutils.utcnow() + older = objects.Migration( + uuid=uuids.migration, created_at=t1, updated_at=t1) + t2 = t1 + datetime.timedelta(seconds=1) + newer = objects.Migration( + uuid=uuids.migration, created_at=t2, updated_at=None) + self._test_get_migrations_sorted_filter_duplicates( + [newer, older], newer) + # Test with just created_at. + older.updated_at = None + self._test_get_migrations_sorted_filter_duplicates( + [newer, older], newer) + # Run it again in reverse. + self._test_get_migrations_sorted_filter_duplicates( + [older, newer], newer) + class DiffDictTestCase(test.NoDBTestCase): """Unit tests for _diff_dict()."""