From 26d695876a1c16668956aee888ce9637b4fc7dc7 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 16 Dec 2019 11:09:29 -0500 Subject: [PATCH] Use graceful_exit=True in ComputeTaskManager.revert_snapshot_based_resize This passes graceful_exit=True to the wrap_instance_event decorator in ComputeTaskManager.revert_snapshot_based_resize so that upon successful completion of the RevertResizeTask, when the instance is hard destroyed from the target cell DB (used to create the action/event), a traceback is not logged for the InstanceActionNotFound exception. The same event is also finished in the source cell DB upon successful completion of the RevertResizeTask. Note that there are other ways we could have done this, e.g. moving the contents of the _execute() method to another method and then putting that in an EventReporter context with the source cell context/instance, but this was simpler. Part of blueprint cross-cell-resize Change-Id: Ibb32f7c19f5f2ec4811b165b8df748d1b7b0f9e4 --- nova/conductor/manager.py | 8 ++++---- nova/conductor/tasks/cross_cell_migrate.py | 9 +++++++++ nova/tests/functional/test_cross_cell_migrate.py | 11 +++++++---- .../unit/conductor/tasks/test_cross_cell_migrate.py | 3 +++ nova/tests/unit/conductor/test_conductor.py | 6 +++++- 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 3e8edf3e52..f582b60711 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1875,11 +1875,11 @@ class ComputeTaskManager(base.Base): task.execute() @targets_cell - # FIXME(mriedem): Upon successful completion of RevertResizeTask the + # NOTE(mriedem): Upon successful completion of RevertResizeTask the # instance is hard-deleted, along with its instance action record(s), from - # the target cell database so wrap_instance_event hits - # InstanceActionNotFound on __exit__. - @wrap_instance_event(prefix='conductor') + # the target cell database so EventReporter hits InstanceActionNotFound on + # __exit__. Pass graceful_exit=True to avoid an ugly traceback. + @wrap_instance_event(prefix='conductor', graceful_exit=True) def revert_snapshot_based_resize(self, context, instance, migration): """Executes the RevertResizeTask diff --git a/nova/conductor/tasks/cross_cell_migrate.py b/nova/conductor/tasks/cross_cell_migrate.py index 4091f80320..5b1d985b16 100644 --- a/nova/conductor/tasks/cross_cell_migrate.py +++ b/nova/conductor/tasks/cross_cell_migrate.py @@ -1413,6 +1413,15 @@ class RevertResizeTask(base.TaskBase): # instance so refresh it here so we have the latest copy. source_cell_instance.refresh() + # Finish the conductor_revert_snapshot_based_resize event in the source + # cell DB. ComputeTaskManager.revert_snapshot_based_resize uses the + # wrap_instance_event decorator to create this action/event in the + # target cell DB but now that the target cell instance is gone the + # event needs to show up in the source cell DB. + objects.InstanceActionEvent.event_finish( + source_cell_instance._context, source_cell_instance.uuid, + 'conductor_revert_snapshot_based_resize', want_result=False) + # Send the resize.revert.end notification using the instance from # the source cell since we end there. self._send_resize_revert_notification( diff --git a/nova/tests/functional/test_cross_cell_migrate.py b/nova/tests/functional/test_cross_cell_migrate.py index cd72e212c0..c788f11915 100644 --- a/nova/tests/functional/test_cross_cell_migrate.py +++ b/nova/tests/functional/test_cross_cell_migrate.py @@ -652,6 +652,10 @@ class TestMultiCellMigrate(integrated_helpers.ProviderUsageBaseTestCase): self.assertEqual('instance.resize_revert.end', end) def assert_resize_revert_actions(self, server, source_host, dest_host): + # There should not be any InstanceActionNotFound errors in the logs + # since ComputeTaskManager.revert_snapshot_based_resize passes + # graceful_exit=True to wrap_instance_event. + self.assertNotIn('InstanceActionNotFound', self.stdlog.logger.output) actions = self.api.get_instance_actions(server['id']) # The revert instance action should have been copied from the target # cell to the source cell and "completed" there, i.e. an event @@ -675,10 +679,9 @@ class TestMultiCellMigrate(integrated_helpers.ProviderUsageBaseTestCase): self.assertIn('conductor_revert_snapshot_based_resize', events_by_name) conductor_event = events_by_name[ 'conductor_revert_snapshot_based_resize'] - # The result is None because the actual update for this to set the - # result=Success is made on the action event in the target cell - # database and not reflected back in the source cell. Do we care? - self.assertIsNone(conductor_event['result']) + # The RevertResizeTask explicitly finishes this event in the source + # cell DB. + self.assertEqual('Success', conductor_event['result']) self.assertIn('compute_revert_snapshot_based_resize_at_dest', events_by_name) 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 545ce37a86..ad9519345c 100644 --- a/nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py +++ b/nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py @@ -1322,6 +1322,9 @@ class RevertResizeTaskTestCase(test.NoDBTestCase, ObjectComparatorMixin): mock_action_event.event_finish_with_failure.assert_called_once_with( source_cell_context, source_cell_instance.uuid, event_name, exc_val=None, exc_tb=None, want_result=False) + mock_action_event.event_finish.assert_called_once_with( + source_cell_context, source_cell_instance.uuid, + 'conductor_revert_snapshot_based_resize', want_result=False) # Destroy the instance in the target cell. mock_inst_destroy.assert_called_once_with(hard_delete=True) # Cleanup at source host. diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index cdc8f1ce41..3443c34e2c 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1783,9 +1783,10 @@ class _BaseTaskTestCase(object): self.context, instance=instance, migration=migration) mock_execute.assert_called_once_with() + @mock.patch('nova.compute.utils.EventReporter') @mock.patch( 'nova.conductor.tasks.cross_cell_migrate.RevertResizeTask.execute') - def test_revert_snapshot_based_resize(self, mock_execute): + def test_revert_snapshot_based_resize(self, mock_execute, mock_er): instance = self._create_fake_instance_obj(ctxt=self.context) migration = objects.Migration( context=self.context, source_compute=instance.host, @@ -1794,6 +1795,9 @@ class _BaseTaskTestCase(object): self.conductor_manager.revert_snapshot_based_resize( self.context, instance=instance, migration=migration) mock_execute.assert_called_once_with() + mock_er.assert_called_once_with( + self.context, 'conductor_revert_snapshot_based_resize', + self.conductor_manager.host, instance.uuid, graceful_exit=True) class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):