From b2fbaa87679629dfb0be7a76ee7dc57980e21dcb Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Mon, 7 Sep 2020 11:27:30 +0100 Subject: [PATCH] Set 'old_flavor', 'new_flavor' on source before resize Cross-cell resize is confusing. We need to set this information ahead of time. Change-Id: I5a403c072b9f03074882b552e1925f22cb5b15b6 Signed-off-by: Stephen Finucane Partial-Bug: #1879878 --- nova/conductor/tasks/cross_cell_migrate.py | 17 ++++++++++------- .../functional/regressions/test_bug_1879878.py | 6 ++---- .../conductor/tasks/test_cross_cell_migrate.py | 12 ++++++++---- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/nova/conductor/tasks/cross_cell_migrate.py b/nova/conductor/tasks/cross_cell_migrate.py index ba0994c81a2d..2ce5380e221c 100644 --- a/nova/conductor/tasks/cross_cell_migrate.py +++ b/nova/conductor/tasks/cross_cell_migrate.py @@ -392,18 +392,23 @@ class PrepResizeAtSourceTask(base.TaskBase): ``resize_migrated`` and the migration.status will be ``post-migrating``. """ - def __init__(self, context, instance, migration, request_spec, - compute_rpcapi, image_api): + def __init__( + self, context, instance, flavor, migration, request_spec, + compute_rpcapi, image_api, + ): """Initializes this PrepResizeAtSourceTask instance. :param context: nova auth context targeted at the source cell :param instance: Instance object from the source cell + :param flavor: The new flavor if performing resize and not just a + cold migration :param migration: Migration object from the source cell :param request_spec: RequestSpec object for the resize operation :param compute_rpcapi: instance of nova.compute.rpcapi.ComputeAPI :param image_api: instance of nova.image.glance.API """ super(PrepResizeAtSourceTask, self).__init__(context, instance) + self.flavor = flavor self.migration = migration self.request_spec = request_spec self.compute_rpcapi = compute_rpcapi @@ -416,6 +421,8 @@ class PrepResizeAtSourceTask(base.TaskBase): # guest should be powered on. self.instance.system_metadata['old_vm_state'] = self.instance.vm_state self.instance.task_state = task_states.RESIZE_MIGRATING + self.instance.old_flavor = self.instance.flavor + self.instance.new_flavor = self.flavor # If the instance is not volume-backed, create a snapshot of the root # disk. @@ -782,7 +789,7 @@ class CrossCellMigrationTask(base.TaskBase): LOG.debug('Preparing source host %s for cross-cell resize.', self.source_migration.source_compute, instance=self.instance) prep_source_task = PrepResizeAtSourceTask( - self.context, self.instance, self.source_migration, + self.context, self.instance, self.flavor, self.source_migration, self.request_spec, self.compute_rpcapi, self.image_api) snapshot_id = prep_source_task.execute() self._completed_tasks['PrepResizeAtSourceTask'] = prep_source_task @@ -960,9 +967,6 @@ class ConfirmResizeTask(base.TaskBase): ctxt, self.migration.uuid) LOG.debug('Cleaning up source host %s for cross-cell resize confirm.', source_migration.source_compute, instance=source_instance) - # The instance.old_flavor field needs to be set before the source - # host drops the MoveClaim in the ResourceTracker. - source_instance.old_flavor = source_instance.flavor # Use the EventReport context manager to create the same event that # the source compute will create but in the target cell DB so we do not # have to explicitly copy it over from source to target DB. @@ -1303,7 +1307,6 @@ class RevertResizeTask(base.TaskBase): # source cell instance for doing the revert on the source compute host. instance.system_metadata['old_vm_state'] = ( self.instance.system_metadata.get('old_vm_state')) - instance.old_flavor = instance.flavor instance.task_state = task_states.RESIZE_REVERTING instance.save() diff --git a/nova/tests/functional/regressions/test_bug_1879878.py b/nova/tests/functional/regressions/test_bug_1879878.py index 99acb2a253a4..eddb12cf5a67 100644 --- a/nova/tests/functional/regressions/test_bug_1879878.py +++ b/nova/tests/functional/regressions/test_bug_1879878.py @@ -270,8 +270,7 @@ class TestCrossCell(integrated_helpers.ProviderUsageBaseTestCase): def fake_cleanup(_self, _context, _instance, *args, **kwargs): self.assertIsNotNone(_instance.old_flavor) - # FIXME(stephenfin): This should not be unset - self.assertIsNone(_instance.new_flavor) + self.assertIsNotNone(_instance.new_flavor) return orig_cleanup(_self, _context, _instance, *args, **kwargs) # TODO(stephenfin): Use a helper @@ -331,8 +330,7 @@ class TestCrossCell(integrated_helpers.ProviderUsageBaseTestCase): def fake_finish_revert_migration(_self, _context, _instance, *a, **kw): self.assertIsNotNone(_instance.old_flavor) - # FIXME(stephenfin): This should not be unset - self.assertIsNone(_instance.new_flavor) + self.assertIsNotNone(_instance.new_flavor) return orig_finish_revert(_self, _context, _instance, *a, **kw) # TODO(stephenfin): Use a helper diff --git a/nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py b/nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py index c5b71ded7b5d..08c225a29c53 100644 --- a/nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py @@ -804,7 +804,10 @@ class PrepResizeAtSourceTaskTestCase(test.NoDBTestCase): vm_state=vm_states.ACTIVE, display_name='fake-server', system_metadata={}, - host='source.host.com'), + host='source.host.com', + flavor=objects.Flavor(), + ), + objects.Flavor(), objects.Migration(), objects.RequestSpec(), compute_rpcapi=mock.Mock(), @@ -828,6 +831,8 @@ class PrepResizeAtSourceTaskTestCase(test.NoDBTestCase): # The instance should have been updated. instance_save.assert_called_once_with( expected_task_state=task_states.RESIZE_PREP) + self.assertIs(self.task.instance.old_flavor, self.task.instance.flavor) + self.assertIs(self.task.instance.new_flavor, self.task.flavor) self.assertEqual( task_states.RESIZE_MIGRATING, self.task.instance.task_state) self.assertEqual(self.task.instance.vm_state, @@ -855,6 +860,8 @@ class PrepResizeAtSourceTaskTestCase(test.NoDBTestCase): # The instance should have been updated. instance_save.assert_called_once_with( expected_task_state=task_states.RESIZE_PREP) + self.assertIs(self.task.instance.old_flavor, self.task.instance.flavor) + self.assertIs(self.task.instance.new_flavor, self.task.flavor) self.assertEqual( task_states.RESIZE_MIGRATING, self.task.instance.task_state) self.assertEqual(self.task.instance.vm_state, @@ -1182,7 +1189,6 @@ class ConfirmResizeTaskTestCase(test.NoDBTestCase): uuid=uuids.instance, flavor=objects.Flavor()) self.task._cleanup_source_host(instance) - self.assertIs(instance.old_flavor, instance.flavor) mock_action_start.assert_called_once_with( instance._context, instance.uuid, instance_actions.CONFIRM_RESIZE, want_result=False) @@ -1434,8 +1440,6 @@ class RevertResizeTaskTestCase(test.NoDBTestCase, ObjectComparatorMixin): # Fields on the source cell instance should have been updated. self.assertEqual(vm_states.ACTIVE, source_cell_instance.system_metadata['old_vm_state']) - self.assertIs(source_cell_instance.old_flavor, - source_cell_instance.flavor) self.assertEqual(task_states.RESIZE_REVERTING, source_cell_instance.task_state) mock_inst_save.assert_called_once_with()