openstack/nova
Code Issues Proposed changes

Plumb graceful_exit through to EventReporter

Browse Source 
This adds a kwarg to wrap_instance_event to be used in the
EventReporter to allow the caller to tell EventReporter to
gracefully handle InstanceActionNotFound on __exit__.

This will be used by ComputeTaskManager.revert_snapshot_based_resize
which starts an action in the target cell DB but upon successful
exit of the RevertResizeTask the instance in the target cell DB
will be hard destroyed resulting in an InstanceActionNotFound
traceback which should be avoided.

Part of blueprint cross-cell-resize

Change-Id: Ie48a9c0a285f77e260f675fbe9282df9f02282b1
This commit is contained in:
Matt Riedemann 2019-12-16 11:07:29 -05:00
parent 3a66b8fdc0
commit 24bf2aaa74
6 changed files with 99 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
nova/compute/utils.py
View File

@ -24,6 +24,7 @@ import traceback
import netifaces
from oslo_log import log
from oslo_serialization import jsonutils
from oslo_utils import excutils
import six
from nova import block_device
@ -1364,13 +1365,19 @@ def get_stashed_volume_connector(bdm, instance):
class EventReporter(object):
"""Context manager to report instance action events."""
"""Context manager to report instance action events.
def __init__(self, context, event_name, host, *instance_uuids):
If constructed with ``graceful_exit=True`` the __exit__ function will
handle and not re-raise on InstanceActionNotFound.
"""
def __init__(self, context, event_name, host, *instance_uuids,
graceful_exit=False):
self.context = context
self.event_name = event_name
self.instance_uuids = instance_uuids
self.host = host
self.graceful_exit = graceful_exit
def __enter__(self):
for uuid in self.instance_uuids:
@ -1382,17 +1389,31 @@ class EventReporter(object):
def __exit__(self, exc_type, exc_val, exc_tb):
for uuid in self.instance_uuids:
objects.InstanceActionEvent.event_finish_with_failure(
self.context, uuid, self.event_name, exc_val=exc_val,
exc_tb=exc_tb, want_result=False)
try:
objects.InstanceActionEvent.event_finish_with_failure(
self.context, uuid, self.event_name, exc_val=exc_val,
exc_tb=exc_tb, want_result=False)
except exception.InstanceActionNotFound:
# If the instance action was not found then determine if we
# should re-raise based on the graceful_exit attribute.
with excutils.save_and_reraise_exception(
reraise=not self.graceful_exit):
if self.graceful_exit:
return True
return False
def wrap_instance_event(prefix):
def wrap_instance_event(prefix, graceful_exit=False):
"""Wraps a method to log the event taken on the instance, and result.
This decorator wraps a method to log the start and result of an event, as
part of an action taken on an instance.
:param prefix: prefix for the event name, usually a service binary like
"compute" or "conductor" to indicate the origin of the event.
:param graceful_exit: True if the decorator should gracefully handle
InstanceActionNotFound errors, False otherwise. This should rarely be
True.
"""
@utils.expects_func_args('instance')
def helper(function):
@ -1406,7 +1427,8 @@ def wrap_instance_event(prefix):
event_name = '{0}_{1}'.format(prefix, function.__name__)
host = self.host if hasattr(self, 'host') else None
with EventReporter(context, event_name, host, instance_uuid):
with EventReporter(context, event_name, host, instance_uuid,
graceful_exit=graceful_exit):
return function(self, context, *args, **kwargs)
return decorated_function
return helper

38
nova/tests/unit/compute/test_compute.py
View File

@ -404,7 +404,7 @@ class ComputeVolumeTestCase(BaseTestCase):
self.assertEqual(self.cinfo.get('serial'), uuids.volume_id)
mock_event.assert_called_once_with(
self.context, 'compute_attach_volume', CONF.host,
instance.uuid)
instance.uuid, graceful_exit=False)
@mock.patch.object(compute_utils, 'EventReporter')
@mock.patch('nova.context.RequestContext.elevated')
@ -445,7 +445,7 @@ class ComputeVolumeTestCase(BaseTestCase):
])
mock_event.assert_called_once_with(
self.context, 'compute_attach_volume', CONF.host,
instance.uuid)
instance.uuid, graceful_exit=False)
@mock.patch('nova.context.RequestContext.elevated')
@mock.patch('nova.compute.utils.notify_about_volume_attach_detach')
@ -535,7 +535,7 @@ class ComputeVolumeTestCase(BaseTestCase):
])
mock_event.assert_called_once_with(
self.context, 'compute_attach_volume', CONF.host,
instance.uuid)
instance.uuid, graceful_exit=False)
self.assertIsInstance(mock_debug_log.call_args[0][1],
exception.VolumeAttachmentNotFound)
@ -559,7 +559,7 @@ class ComputeVolumeTestCase(BaseTestCase):
self.assertFalse(mock_destroy.called)
mock_event.assert_called_once_with(
self.context, 'compute_detach_volume', CONF.host,
instance.uuid)
instance.uuid, graceful_exit=False)
@mock.patch.object(compute_utils, 'EventReporter')
def test_detach_volume_bdm_destroyed(self, mock_event):
@ -584,7 +584,7 @@ class ComputeVolumeTestCase(BaseTestCase):
self.assertTrue(mock_destroy.called)
mock_event.assert_called_once_with(
self.context, 'compute_detach_volume', CONF.host,
instance.uuid)
instance.uuid, graceful_exit=False)
def test_await_block_device_created_too_slow(self):
self.flags(block_device_allocate_retries=2)
@ -3438,7 +3438,8 @@ class ComputeTestCase(BaseTestCase,
mock_event.assert_called_once_with(self.context,
'compute_snapshot_instance',
CONF.host,
inst_obj.uuid)
inst_obj.uuid,
graceful_exit=False)
else:
self.assertRaises(test.TestingException,
self.compute.backup_instance,
@ -3448,7 +3449,8 @@ class ComputeTestCase(BaseTestCase,
mock_event.assert_called_once_with(self.context,
'compute_backup_instance',
CONF.host,
inst_obj.uuid)
inst_obj.uuid,
graceful_exit=False)
self.assertEqual(expected_state, self.fake_image_delete_called)
self._assert_state({'task_state': None})
@ -6376,7 +6378,8 @@ class ComputeTestCase(BaseTestCase,
self.assertIsNone(ret)
mock_event.assert_called_with(
c, 'compute_live_migration', CONF.host, instance.uuid)
c, 'compute_live_migration', CONF.host, instance.uuid,
graceful_exit=False)
# cleanup
instance.destroy()
@ -10672,8 +10675,8 @@ class ComputeAPITestCase(BaseTestCase):
instance.uuid,
'/dev/vdb')
mock_event.assert_called_once_with(
self.context, 'api_attach_volume', CONF.host, instance.uuid
)
self.context, 'api_attach_volume', CONF.host, instance.uuid,
graceful_exit=False)
self.assertTrue(mock_attach.called)
def test_attach_volume_shelved_offloaded_new_flow(self):
@ -10762,7 +10765,8 @@ class ComputeAPITestCase(BaseTestCase):
mock_event.assert_called_once_with(self.context,
'api_detach_volume',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
self.assertTrue(mock_local_cleanup.called)
@mock.patch.object(nova.volume.cinder.API, 'begin_detaching')
@ -11063,7 +11067,8 @@ class ComputeAPITestCase(BaseTestCase):
mock_event.assert_called_once_with(self.context,
'api_lock',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
mock_notify.assert_called_once_with(
self.context, instance, CONF.host, action='lock',
source='nova-api')
@ -11085,7 +11090,8 @@ class ComputeAPITestCase(BaseTestCase):
mock_event.assert_called_once_with(self.context,
'api_lock',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
mock_notify.assert_called_once_with(
self.context, instance, CONF.host, action='lock',
source='nova-api')
@ -11106,7 +11112,8 @@ class ComputeAPITestCase(BaseTestCase):
mock_event.assert_called_once_with(self.context,
'api_unlock',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
mock_notify.assert_called_once_with(
self.context, instance, CONF.host, action='unlock',
source='nova-api')
@ -11129,7 +11136,8 @@ class ComputeAPITestCase(BaseTestCase):
mock_event.assert_called_once_with(self.context,
'api_unlock',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
mock_notify.assert_called_once_with(
self.context, instance, CONF.host, action='unlock',
source='nova-api')

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

@ -3054,7 +3054,8 @@ class _ComputeAPIUnitTestMixIn(object):
mock_event.assert_called_once_with(self.context,
'api_snapshot_instance',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
bdm = fake_block_device.FakeDbBlockDeviceDict(
{'no_device': False, 'volume_id': '1', 'boot_index': 0,
@ -3102,7 +3103,8 @@ class _ComputeAPIUnitTestMixIn(object):
mock_event.assert_called_once_with(self.context,
'api_snapshot_instance',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
instance.system_metadata['image_mappings'] = jsonutils.dumps(
[{'virtual': 'ami', 'device': 'vda'},
@ -3161,7 +3163,8 @@ class _ComputeAPIUnitTestMixIn(object):
mock_event.assert_called_once_with(self.context,
'api_snapshot_instance',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
def test_snapshot_volume_backed(self):
self._test_snapshot_volume_backed(quiesce_required=False,

11
nova/tests/unit/compute/test_compute_mgr.py
View File

@ -2405,7 +2405,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
mock.ANY)])
event.assert_called_once_with(
self.context, 'compute_attach_interface', CONF.host,
f_instance.uuid)
f_instance.uuid, graceful_exit=False)
with mock.patch.dict(self.compute.driver.capabilities,
supports_attach_interface=True):
@ -2432,7 +2432,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
[mock.call(self.context, f_instance, mock.ANY, mock.ANY)])
event.assert_called_once_with(
self.context, 'compute_detach_interface', CONF.host,
f_instance.uuid)
f_instance.uuid, graceful_exit=False)
do_test()
@ -2581,7 +2581,8 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
mock_event.assert_called_once_with(self.context,
'compute_swap_volume',
CONF.host,
instance1.uuid)
instance1.uuid,
graceful_exit=False)
def _assert_volume_api(self, context, volume, *args):
self.assertTrue(uuidutils.is_uuid_like(volume))
@ -3108,7 +3109,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
self.context, dest_check_data, drvr_check_result)
mock_event.assert_called_once_with(
self.context, 'compute_check_can_live_migrate_source', CONF.host,
instance.uuid)
instance.uuid, graceful_exit=False)
mock_check.assert_called_once_with(self.context, instance,
dest_check_data,
{'block_device_mapping': 'fake'})
@ -3209,7 +3210,7 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
mock_event.assert_called_once_with(
self.context, 'compute_check_can_live_migrate_destination',
CONF.host, instance.uuid)
CONF.host, instance.uuid, graceful_exit=False)
def test_check_can_live_migrate_destination_success(self):
self._test_check_can_live_migrate_destination()

34
nova/tests/unit/compute/test_compute_utils.py
View File

@ -1076,7 +1076,8 @@ class ComputeUtilsTestCase(test.NoDBTestCase):
fake_event(self.compute, self.context, instance=inst)
# if the class doesn't include a self.host, the default host is None
mock_event.assert_called_once_with(self.context, 'compute_fake_event',
None, uuids.instance)
None, uuids.instance,
graceful_exit=False)
@mock.patch.object(objects.InstanceActionEvent, 'event_start')
@mock.patch.object(objects.InstanceActionEvent,
@ -1127,6 +1128,37 @@ class ComputeUtilsTestCase(test.NoDBTestCase):
args, kwargs = mock_finish.call_args
self.assertIsInstance(kwargs['exc_val'], exception.NovaException)
@mock.patch('nova.objects.InstanceActionEvent.event_start')
@mock.patch('nova.objects.InstanceActionEvent.event_finish_with_failure')
def _test_event_reporter_graceful_exit(self, error, mock_event_finish,
mock_event_start):
with compute_utils.EventReporter(self.context, 'fake_event',
'fake.host', uuids.instance,
graceful_exit=True):
mock_event_finish.side_effect = error
mock_event_start.assert_called_once_with(
self.context, uuids.instance, 'fake_event', want_result=False,
host='fake.host')
mock_event_finish.assert_called_once_with(
self.context, uuids.instance, 'fake_event', exc_val=None,
exc_tb=None, want_result=False)
def test_event_reporter_graceful_exit_action_not_found(self):
"""Tests that when graceful_exit=True and InstanceActionNotFound is
raised it is handled and not re-raised.
"""
error = exception.InstanceActionNotFound(
request_id=self.context.request_id, instance_uuid=uuids.instance)
self._test_event_reporter_graceful_exit(error)
def test_event_reporter_graceful_exit_unexpected_error(self):
"""Tests that even if graceful_exit=True the EventReporter will
re-raise an unexpected exception.
"""
error = test.TestingException('uh oh')
self.assertRaises(test.TestingException,
self._test_event_reporter_graceful_exit, error)
@mock.patch('netifaces.interfaces')
def test_get_machine_ips_value_error(self, mock_interfaces):
# Tests that the utility method does not explode if netifaces raises

3
nova/tests/unit/compute/test_shelve.py
View File

@ -267,7 +267,8 @@ class ShelveComputeManagerTestCase(test_compute.BaseTestCase):
mock_event.assert_called_once_with(self.context,
'compute_shelve_offload_instance',
CONF.host,
instance.uuid)
instance.uuid,
graceful_exit=False)
return instance