openstack
/
nova
Code Issues Proposed changes

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
This commit is contained in:
Matt Riedemann 2019-02-11 16:20:43 -05:00
parent a05ef30fb9
commit 63094c2883
2 changed files with 111 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

49
nova/compute/api.py
View File

@ -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):

63
nova/tests/unit/compute/test_compute_api.py
View File

@ -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()."""