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)

2 files changed, 30 insertions, 1 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index aa56396613..7d66c1534e 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 cd30ab0880..1194048470 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',