Merge "Plumb graceful_exit through to EventReporter"
This commit is contained in:
+29
-7
@@ -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
|
||||
|
||||
@@ -385,7 +385,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')
|
||||
@@ -426,7 +426,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')
|
||||
@@ -516,7 +516,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)
|
||||
|
||||
@@ -540,7 +540,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):
|
||||
@@ -565,7 +565,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)
|
||||
@@ -3416,7 +3416,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,
|
||||
@@ -3426,7 +3427,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})
|
||||
@@ -6352,7 +6354,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()
|
||||
|
||||
@@ -10625,8 +10628,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):
|
||||
@@ -10715,7 +10718,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')
|
||||
@@ -11016,7 +11020,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')
|
||||
@@ -11038,7 +11043,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')
|
||||
@@ -11059,7 +11065,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')
|
||||
@@ -11082,7 +11089,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')
|
||||
|
||||
@@ -3052,7 +3052,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,
|
||||
@@ -3100,7 +3101,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'},
|
||||
@@ -3159,7 +3161,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,
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
Reference in New Issue
Block a user