diff --git a/nova/compute/manager.py b/nova/compute/manager.py index b558f620f8..a5ead22a1a 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -108,37 +108,43 @@ wrap_exception = functools.partial(exception_wrapper.wrap_exception, binary='nova-compute') +@contextlib.contextmanager +def errors_out_migration_ctxt(migration): + """Context manager to error out migration on failure.""" + + try: + yield + except Exception as ex: + with excutils.save_and_reraise_exception(): + # NOTE(rajesht): If InstanceNotFound error is thrown from + # decorated function, migration status should be set to + # 'error', without checking current migration status. + if not isinstance(ex, exception.InstanceNotFound): + status = migration.status + if status not in ['migrating', 'post-migrating']: + return + + migration.status = 'error' + try: + with migration.obj_as_admin(): + migration.save() + except Exception: + LOG.debug('Error setting migration status for instance %s.', + migration.instance_uuid, exc_info=True) + + @utils.expects_func_args('migration') def errors_out_migration(function): """Decorator to error out migration on failure.""" @functools.wraps(function) def decorated_function(self, context, *args, **kwargs): - try: + wrapped_func = safe_utils.get_wrapped_function(function) + keyed_args = inspect.getcallargs(wrapped_func, self, context, + *args, **kwargs) + migration = keyed_args['migration'] + with errors_out_migration_ctxt(migration): return function(self, context, *args, **kwargs) - except Exception as ex: - with excutils.save_and_reraise_exception(): - wrapped_func = safe_utils.get_wrapped_function(function) - keyed_args = inspect.getcallargs(wrapped_func, self, context, - *args, **kwargs) - migration = keyed_args['migration'] - - # NOTE(rajesht): If InstanceNotFound error is thrown from - # decorated function, migration status should be set to - # 'error', without checking current migration status. - if not isinstance(ex, exception.InstanceNotFound): - status = migration.status - if status not in ['migrating', 'post-migrating']: - return - - migration.status = 'error' - try: - with migration.obj_as_admin(): - migration.save() - except Exception: - LOG.debug('Error setting migration status ' - 'for instance %s.', - migration.instance_uuid, exc_info=True) return decorated_function diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 35b485aa6e..6c79400979 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -18,6 +18,7 @@ import time from cinderclient import exceptions as cinder_exception from cursive import exception as cursive_exception +import ddt from eventlet import event as eventlet_event import mock import netaddr @@ -5331,6 +5332,73 @@ class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase): self.assertEqual(expected_call, create_error_call) +@ddt.ddt +class ComputeManagerErrorsOutMigrationTestCase(test.NoDBTestCase): + def setUp(self): + super(ComputeManagerErrorsOutMigrationTestCase, self).setUp() + self.context = context.RequestContext(fakes.FAKE_USER_ID, + fakes.FAKE_PROJECT_ID) + + self.instance = fake_instance.fake_instance_obj(self.context) + + self.migration = objects.Migration() + self.migration.instance_uuid = self.instance.uuid + self.migration.status = 'migrating' + self.migration.id = 0 + + @mock.patch.object(objects.Migration, 'save') + @mock.patch.object(objects.Migration, 'obj_as_admin') + def test_decorator(self, mock_save, mock_obj_as_admin): + # Tests that errors_out_migration decorator in compute manager sets + # migration status to 'error' when an exception is raised from + # decorated method + + @manager.errors_out_migration + def fake_function(self, context, instance, migration): + raise test.TestingException() + + mock_obj_as_admin.return_value = mock.MagicMock() + + self.assertRaises(test.TestingException, fake_function, + self, self.context, self.instance, self.migration) + self.assertEqual('error', self.migration.status) + mock_save.assert_called_once_with() + mock_obj_as_admin.assert_called_once_with() + + @mock.patch.object(objects.Migration, 'save') + @mock.patch.object(objects.Migration, 'obj_as_admin') + def test_contextmanager(self, mock_save, mock_obj_as_admin): + # Tests that errors_out_migration_ctxt context manager in compute + # manager sets migration status to 'error' when an exception is raised + # from decorated method + + def test_function(): + with manager.errors_out_migration_ctxt(self.migration): + raise test.TestingException() + + mock_obj_as_admin.return_value = mock.MagicMock() + + self.assertRaises(test.TestingException, test_function) + self.assertEqual('error', self.migration.status) + mock_save.assert_called_once_with() + mock_obj_as_admin.assert_called_once_with() + + @ddt.data('completed', 'finished') + @mock.patch.object(objects.Migration, 'save') + def test_status_exclusion(self, status, mock_save): + # Tests that errors_out_migration doesn't error out migration if the + # status is anything other than 'migrating' or 'post-migrating' + self.migration.status = status + + def test_function(): + with manager.errors_out_migration_ctxt(self.migration): + raise test.TestingException() + + self.assertRaises(test.TestingException, test_function) + self.assertEqual(status, self.migration.status) + mock_save.assert_not_called() + + class ComputeManagerMigrationTestCase(test.NoDBTestCase): def setUp(self): super(ComputeManagerMigrationTestCase, self).setUp() @@ -5347,32 +5415,6 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): self.useFixture(fixtures.SpawnIsSynchronousFixture()) self.useFixture(fixtures.EventReporterStub()) - @mock.patch.object(objects.Migration, 'save') - @mock.patch.object(objects.Migration, 'obj_as_admin') - def test_errors_out_migration_decorator(self, mock_save, - mock_obj_as_admin): - # Tests that errors_out_migration decorator in compute manager - # sets migration status to 'error' when an exception is raised - # from decorated method - instance = fake_instance.fake_instance_obj(self.context) - - migration = objects.Migration() - migration.instance_uuid = instance.uuid - migration.status = 'migrating' - migration.id = 0 - - @manager.errors_out_migration - def fake_function(self, context, instance, migration): - raise test.TestingException() - - mock_obj_as_admin.return_value = mock.MagicMock() - - self.assertRaises(test.TestingException, fake_function, - self, self.context, instance, migration) - self.assertEqual('error', migration.status) - mock_save.assert_called_once_with() - mock_obj_as_admin.assert_called_once_with() - def test_finish_resize_failure(self): with test.nested( mock.patch.object(self.compute, '_finish_resize',