From 24bf2aaa7441430a84b475efca9f4777dc243452 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 16 Dec 2019 11:07:29 -0500 Subject: [PATCH] Plumb graceful_exit through to EventReporter 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 --- nova/compute/utils.py | 36 ++++++++++++++---- nova/tests/unit/compute/test_compute.py | 38 +++++++++++-------- nova/tests/unit/compute/test_compute_api.py | 9 +++-- nova/tests/unit/compute/test_compute_mgr.py | 11 +++--- nova/tests/unit/compute/test_compute_utils.py | 34 ++++++++++++++++- nova/tests/unit/compute/test_shelve.py | 3 +- 6 files changed, 99 insertions(+), 32 deletions(-) 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 fab17e92de..07b01df927 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -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') diff --git a/nova/tests/unit/compute/test_compute_api.py b/nova/tests/unit/compute/test_compute_api.py index c04d6b6822..c689b84aca 100644 --- a/nova/tests/unit/compute/test_compute_api.py +++ b/nova/tests/unit/compute/test_compute_api.py @@ -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, diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 72aae41ae0..fb9c1f70fe 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