openstack/nova
Code Issues Proposed changes

Use graceful_exit=True in ComputeTaskManager.revert_snapshot_based_resize

Browse Source 
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
This commit is contained in:
Matt Riedemann 2019-12-16 11:09:29 -05:00
parent 24bf2aaa74
commit 26d695876a
5 changed files with 28 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
nova/conductor/manager.py
View File

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

9
nova/conductor/tasks/cross_cell_migrate.py
View File

@ -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(

11
nova/tests/functional/test_cross_cell_migrate.py
View File

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

3
nova/tests/unit/conductor/tasks/test_cross_cell_migrate.py
View File

@ -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.

6
nova/tests/unit/conductor/test_conductor.py
View File

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