From a4e9a146c3993f5775501716a21632f34a63a3ad Mon Sep 17 00:00:00 2001 From: Rajesh Tailor Date: Wed, 15 Apr 2015 06:59:04 -0700 Subject: [PATCH] Fix kwargs['migration'] KeyError in @errors_out_migration decorator @errors_out_migration decorator is used in the compute manager on resize_instance and finish_resize methods of ComputeManager class. It is decorated via @utils.expects_func_args('migration') to check 'migration' is a parameter to the decorator method, however, that only ensures there is a migration argument, not that it's in args or kwargs (either is fine for what expects_func_args checks). The errors_out_migration decorator can get a KeyError when checking kwargs['migration'] and fails to set the migration status to 'error'. This fixes the KeyError in the decorator by normalizing the args/kwargs list into a single dict that we can pull the migration from. Change-Id: I774ac9b749b21085f4fbcafa4965a78d68eec9c7 Closes-Bug: 1444300 (cherry picked from commit 3add7923fc16c050d4cfaef98a87886c6b6a589c) --- nova/compute/manager.py | 5 +++- nova/tests/unit/compute/test_compute_mgr.py | 26 +++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index aa563966133e..7d66c1534e79 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -269,7 +269,10 @@ def errors_out_migration(function): return function(self, context, *args, **kwargs) except Exception: with excutils.save_and_reraise_exception(): - migration = kwargs['migration'] + wrapped_func = utils.get_wrapped_function(function) + keyed_args = safe_utils.getcallargs(wrapped_func, context, + *args, **kwargs) + migration = keyed_args['migration'] status = migration.status if status not in ['migrating', 'post-migrating']: return diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index cd30ab0880b1..11940484702e 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -3516,6 +3516,32 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): self.migration.status = 'migrating' fake_server_actions.stub_out_action_events(self.stubs) + @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 contextlib.nested( mock.patch.object(self.compute, '_finish_resize',