diff --git a/nova/compute/utils.py b/nova/compute/utils.py index 5a51d84ee4..c9b3130dea 100644 --- a/nova/compute/utils.py +++ b/nova/compute/utils.py @@ -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 diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 309b7f4dfc..722a40c864 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -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') diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index ba2081e990..4982cffb99 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -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, diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index d89e4fcd5c..c24e06898a 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -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() diff --git a/nova/tests/unit/compute/test_compute_utils.py b/nova/tests/unit/compute/test_compute_utils.py index 63d5e39857..e49fd33087 100644 --- a/nova/tests/unit/compute/test_compute_utils.py +++ b/nova/tests/unit/compute/test_compute_utils.py @@ -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 diff --git a/nova/tests/unit/compute/test_shelve.py b/nova/tests/unit/compute/test_shelve.py index 4cc9118564..9bdf578c2e 100644 --- a/nova/tests/unit/compute/test_shelve.py +++ b/nova/tests/unit/compute/test_shelve.py @@ -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