Merge "Fix scope of errors_out_migration in resize_instance"

nova/compute/manager.py

@@ -3863,13 +3863,13 @@ class ComputeManager(manager.Manager):
     @wrap_exception()
     @reverts_task_state
     @wrap_instance_event(prefix='compute')
-    @errors_out_migration
     @wrap_instance_fault
     def resize_instance(self, context, instance, image,
                         reservations, migration, instance_type,
                         clean_shutdown):
         """Starts the migration of a running instance to another host."""
-        with self._error_out_instance_on_exception(context, instance):
+        with self._error_out_instance_on_exception(context, instance), \
+             errors_out_migration_ctxt(migration):
             # TODO(chaochin) Remove this until v5 RPC API
             # Code downstream may expect extra_specs to be populated since it
             # is receiving an object, so lookup the flavor to ensure this.
@@ -3927,13 +3927,13 @@ class ComputeManager(manager.Manager):
             self.compute_rpcapi.finish_resize(context, instance,
                     migration, image, disk_info, migration.dest_compute)
 
-            self._notify_about_instance_usage(context, instance, "resize.end",
+        self._notify_about_instance_usage(context, instance, "resize.end",
-                                              network_info=network_info)
+                                          network_info=network_info)
 
-            compute_utils.notify_about_instance_action(context, instance,
+        compute_utils.notify_about_instance_action(context, instance,
-                   self.host, action=fields.NotificationAction.RESIZE,
+               self.host, action=fields.NotificationAction.RESIZE,
-                   phase=fields.NotificationPhase.END)
+               phase=fields.NotificationPhase.END)
-            self.instance_events.clear_events_for_instance(instance)
+        self.instance_events.clear_events_for_instance(instance)
 
     def _terminate_volume_connections(self, context, instance, bdms):
         connector = None

nova/tests/unit/compute/test_compute_mgr.py

@@ -12,6 +12,7 @@
 
 """Unit tests for ComputeManager()."""
 
+import contextlib
 import copy
 import datetime
 import time
@@ -5451,6 +5452,9 @@ class ComputeManagerErrorsOutMigrationTestCase(test.NoDBTestCase):
 
 
 class ComputeManagerMigrationTestCase(test.NoDBTestCase):
+    class TestResizeError(Exception):
+        pass
+
     def setUp(self):
         super(ComputeManagerMigrationTestCase, self).setUp()
         self.compute = manager.ComputeManager()
@@ -5490,45 +5494,69 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase):
             migration_save.assert_called_once_with()
             migration_obj_as_admin.assert_called_once_with()
 
-    def test_resize_instance_failure(self):
+    @contextlib.contextmanager
-        self.migration.dest_host = None
+    def _mock_resize_instance(self):
         with test.nested(
             mock.patch.object(self.compute.driver,
-                              'migrate_disk_and_power_off',
+                              'migrate_disk_and_power_off'),
-                              side_effect=exception.ResizeError(reason='')),
             mock.patch.object(db, 'instance_fault_create'),
-            mock.patch.object(self.compute, '_instance_update'),
+            mock.patch.object(self.compute, '_update_resource_tracker'),
-            mock.patch.object(self.migration, 'save'),
+            mock.patch.object(self.compute, 'network_api'),
-            mock.patch.object(self.migration, 'obj_as_admin',
-                              return_value=mock.MagicMock()),
-            mock.patch.object(self.compute.network_api, 'get_instance_nw_info',
-                              return_value=None),
             mock.patch.object(self.instance, 'save'),
             mock.patch.object(self.compute, '_notify_about_instance_usage'),
             mock.patch.object(self.compute,
-                              '_get_instance_block_device_info',
+                              '_get_instance_block_device_info'),
-                              return_value=None),
             mock.patch.object(objects.BlockDeviceMappingList,
-                              'get_by_instance_uuid',
+                              'get_by_instance_uuid'),
-                              return_value=None),
+            mock.patch.object(objects.Flavor, 'get_by_id'),
-            mock.patch.object(objects.Flavor,
+            mock.patch.object(self.compute, '_terminate_volume_connections'),
-                              'get_by_id',
+            mock.patch.object(self.compute, 'compute_rpcapi'),
-                              return_value=None)
+        ) as (
-        ) as (meth, fault_create, instance_update,
+            migrate_disk_and_power_off, fault_create, instance_update,
-              migration_save, migration_obj_as_admin, nw_info, save_inst,
+            network_api, save_inst, notify, vol_block_info, bdm, flavor,
-              notify, vol_block_info, bdm, flavor):
+            terminate_volume_connections, compute_rpcapi
+        ):
             fault_create.return_value = (
                 test_instance_fault.fake_faults['fake-uuid'][0])
+            yield (migrate_disk_and_power_off, notify)
+
+    def test_resize_instance_failure(self):
+        migration = mock.NonCallableMagicMock()
+
+        with self._mock_resize_instance() as (
+                migrate_disk_and_power_off, notify):
+            migrate_disk_and_power_off.side_effect = self.TestResizeError
             self.assertRaises(
-                exception.ResizeError, self.compute.resize_instance,
+                self.TestResizeError, self.compute.resize_instance,
                 context=self.context, instance=self.instance, image=self.image,
-                reservations=[], migration=self.migration,
+                reservations=[], migration=migration,
                 instance_type='type', clean_shutdown=True)
-            self.assertEqual("error", self.migration.status)
+
-            self.assertEqual([mock.call(), mock.call()],
+        # Assert that we set the migration to an error state
-                             migration_save.mock_calls)
+        self.assertEqual("error", migration.status)
-            self.assertEqual([mock.call(), mock.call()],
+
-                             migration_obj_as_admin.mock_calls)
+    def test_resize_instance_notify_failure(self):
+        migration = mock.NonCallableMagicMock()
+        migration.dest_compute = None
+        migration.dest_node = None
+
+        # Raise an exception sending the end notification, which is after we
+        # cast the migration to the destination host
+        def fake_notify(context, instance, event, network_info=None):
+            if event == 'resize.end':
+                raise self.TestResizeError()
+
+        with self._mock_resize_instance() as (
+                migrate_disk_and_power_off, notify_about_instance_action):
+            notify_about_instance_action.side_effect = fake_notify
+            self.assertRaises(
+                self.TestResizeError, self.compute.resize_instance,
+                context=self.context, instance=self.instance, image=self.image,
+                reservations=[], migration=migration,
+                instance_type='type', clean_shutdown=True)
+
+        # Assert that we did not set the migration to an error state
+        self.assertEqual('post-migrating', migration.status)
 
     def _test_revert_resize_instance_destroy_disks(self, is_shared=False):