openstack/nova
Code Issues Proposed changes

Simplify FinishResizeAtDestTask event handling

Browse Source 
Rather than copy the instance action event from the target
cell DB to the source cell DB when
_finish_snapshot_based_resize_at_dest fails on the dest host,
we can simply use the EventReporter context in conductor with
the source cell context and get the same event.

Part of blueprint cross-cell-resize

Change-Id: I8053765797f097859bf585459f4a00d31844708e
This commit is contained in:
Matt Riedemann
2019-11-20 19:02:23 -05:00
parent 6aafb29820
commit 166df25325
2 changed files with 29 additions and 124 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -509,10 +509,15 @@ class FinishResizeAtDestTask(base.TaskBase):
# the destination compute.
self.instance.task_state = task_states.RESIZE_MIGRATED
self.instance.save()
event_name = 'compute_finish_snapshot_based_resize_at_dest'
source_cell_context = self.source_cell_instance._context
try:
self.compute_rpcapi.finish_snapshot_based_resize_at_dest(
self.context, self.instance, self.migration, self.snapshot_id,
self.request_spec)
with compute_utils.EventReporter(
source_cell_context, event_name,
self.migration.dest_compute, self.instance.uuid):
self.compute_rpcapi.finish_snapshot_based_resize_at_dest(
self.context, self.instance, self.migration,
self.snapshot_id, self.request_spec)
# finish_snapshot_based_resize_at_dest updates the target cell
# instance so we need to refresh it here to have the latest copy.
self.instance.refresh()
@@ -526,15 +531,9 @@ class FinishResizeAtDestTask(base.TaskBase):
self.source_cell_instance.task_state = None
self.source_cell_instance.vm_state = vm_states.ERROR
self.source_cell_instance.save()
source_cell_context = self.source_cell_instance._context
# wrap_instance_fault (this is best effort)
self._copy_latest_fault(source_cell_context)
# wrap_instance_event (this is best effort)
self._copy_finish_snapshot_based_resize_at_dest_event(
source_cell_context)
def _copy_latest_fault(self, source_cell_context):
"""Copies the latest instance fault from the target cell to the source
@@ -554,48 +553,6 @@ class FinishResizeAtDestTask(base.TaskBase):
'Failed to copy instance fault from target cell DB',
instance=self.instance)
def _copy_finish_snapshot_based_resize_at_dest_event(
self, source_cell_context):
"""Copies the compute_finish_snapshot_based_resize_at_dest event from
the target cell database to the source cell database.
:param source_cell_context: nova auth request context targeted at the
source cell
"""
event_name = 'compute_finish_snapshot_based_resize_at_dest'
try:
# TODO(mriedem): Need a method on InstanceActionEventList to
# lookup an event by action request_id and event name.
# Get the single action for this request in the target cell DB.
action = objects.InstanceAction.get_by_request_id(
self.context, self.instance.uuid,
self.context.request_id)
if action:
# Get the events for this action in the target cell DB.
events = objects.InstanceActionEventList.get_by_action(
self.context, action.id)
# Find the finish_snapshot_based_resize_at_dest event and
# create it in the source cell DB.
for event in events:
if event.event == event_name:
event_clone = clone_creatable_object(
source_cell_context, event)
event_clone.create(action.instance_uuid,
action.request_id)
break
else:
LOG.warning('Failed to find InstanceActionEvent with '
'name %s in target cell DB', event_name,
instance=self.instance)
else:
LOG.warning(
'Failed to find InstanceAction by request_id %s',
self.context.request_id, instance=self.instance)
except Exception:
LOG.exception(
'Failed to copy %s instance action event from target cell DB',
event_name, instance=self.instance)
def _update_instance_mapping(self):
"""Swaps the hidden field value on the source and target cell instance
and updates the instance mapping to point at the target cell.

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

@@ -935,6 +935,11 @@ class FinishResizeAtDestTaskTestCase(test.TestCase):
source_instance = self._create_instance(
self.source_context, create_instance_mapping=True,
hidden=False)
# Create the instance action record in the source cell which is needed
# by the EventReporter.
objects.InstanceAction.action_start(
self.source_context, source_instance.uuid,
instance_actions.RESIZE, want_result=False)
# Create the target cell instance which would normally be a clone of
# the source cell instance but the only thing these tests care about
# is that the UUID matches. The target cell instance is also hidden.
@@ -989,12 +994,10 @@ class FinishResizeAtDestTaskTestCase(test.TestCase):
with test.nested(
mock.patch.object(self.task.compute_rpcapi,
'finish_snapshot_based_resize_at_dest',
side_effect=test.TestingException),
side_effect=test.TestingException('oops')),
mock.patch.object(self.task, '_copy_latest_fault'),
mock.patch.object(
self.task, '_copy_finish_snapshot_based_resize_at_dest_event'),
) as (
finish_resize, copy_fault, copy_event
finish_resize, copy_fault
):
self.assertRaises(test.TestingException,
self.task._finish_snapshot_based_resize_at_dest)
@@ -1006,7 +1009,20 @@ class FinishResizeAtDestTaskTestCase(test.TestCase):
# And the latest fault and instance action event should have been
# copied from the target cell DB to the source cell DB.
copy_fault.assert_called_once_with(self.source_context)
copy_event.assert_called_once_with(self.source_context)
# Assert the event was recorded in the source cell DB.
event_name = 'compute_finish_snapshot_based_resize_at_dest'
action = objects.InstanceAction.get_by_request_id(
source_instance._context, source_instance.uuid,
source_instance._context.request_id)
self.assertIsNotNone(action, 'InstanceAction not found.')
events = objects.InstanceActionEventList.get_by_action(
source_instance._context, action.id)
self.assertEqual(1, len(events), events)
self.assertEqual(event_name, events[0].event)
self.assertEqual('Error', events[0].result)
self.assertIn('_finish_snapshot_based_resize_at_dest',
events[0].traceback)
self.assertEqual(self.task.migration.dest_compute, events[0].host)
# Assert the instance mapping was never updated.
mock_im_save.assert_not_called()
@@ -1039,74 +1055,6 @@ class FinishResizeAtDestTaskTestCase(test.TestCase):
self.assertIn('Failed to copy instance fault from target cell DB',
mock_log.call_args[0][0])
@mock.patch('nova.conductor.tasks.cross_cell_migrate.LOG.warning')
def test_copy_finish_snapshot_based_resize_at_dest_event(self, mock_warn):
"""Tests _copy_finish_snapshot_based_resize_at_dest_event working
without errors (but also warning cases).
"""
# First run it without any action record created and we should get a
# warning logged that the action could not be found.
self.task._copy_finish_snapshot_based_resize_at_dest_event(
self.source_context)
mock_warn.assert_called_once()
self.assertIn('Failed to find InstanceAction by request_id',
mock_warn.call_args[0][0])
# The source and target context must have the same request_id for this
# to work.
self.assertEqual(self.source_context.request_id,
self.target_context.request_id)
# Create an action record in the source cell database. This is needed
# to find the action for the events when they get copied over.
src_action = objects.InstanceAction.action_start(
self.source_context, self.task.instance.uuid, 'resize')
# Create the same action in the target cell database.
objects.InstanceAction.action_start(
self.target_context, self.task.instance.uuid, 'resize')
# Now run it again without creating the underlying event record and
# we should log a warning that no event was found.
mock_warn.reset_mock()
self.task._copy_finish_snapshot_based_resize_at_dest_event(
self.source_context)
mock_warn.assert_called_once()
self.assertIn('Failed to find InstanceActionEvent',
mock_warn.call_args[0][0])
# Generate the event in the target cell database.
@compute_utils.wrap_instance_event(prefix='compute')
def finish_snapshot_based_resize_at_dest(_self, context, instance):
raise test.TestingException('oops')
self.assertRaises(test.TestingException,
finish_snapshot_based_resize_at_dest,
mock.Mock(host='dest-host'),
self.target_context, self.task.instance)
self.task._copy_finish_snapshot_based_resize_at_dest_event(
self.source_context)
# There should now be one InstanceActionEvent in the source cell DB.
src_events = objects.InstanceActionEventList.get_by_action(
self.source_context, src_action.id)
self.assertEqual(1, len(src_events))
self.assertEqual('compute_finish_snapshot_based_resize_at_dest',
src_events[0].event)
self.assertEqual('Error', src_events[0].result)
@mock.patch('nova.conductor.tasks.cross_cell_migrate.LOG.exception')
@mock.patch('nova.objects.InstanceAction.get_by_request_id',
side_effect=test.TestingException)
def test_copy_finish_snapshot_based_resize_at_dest_event_error(
self, get_by_request_id, mock_log):
"""Tests that _copy_finish_snapshot_based_resize_at_dest_event errors
are swallowed.
"""
self.task._copy_finish_snapshot_based_resize_at_dest_event(
self.source_context)
mock_log.assert_called_once()
self.assertIn('Failed to copy %s instance action event from target',
mock_log.call_args[0][0])
class UtilityTestCase(test.NoDBTestCase):
"""Tests utility methods in the cross_cell_migrate module."""