Raise InstanceFaultRollback for UnableToMigrateToSelf from _prep_resize

It is possible to cold migrate a stopped server. If, however, the
cold migrate is scheduled to the instance's current host and the
compute driver does not support cold migrating to the same host,
then UnableToMigrateToSelf was being raised from _prep_resize. If
_reschedule_resize_or_reraise re-raises that exception, then
_error_out_instance_on_exception in prep_resize handles it and
sets the instance vm_state to ACTIVE. This is wrong since the
instance power state is unchanged at this point and the instance
is actually stopped.

This fixes the problem by wrapping UnableToMigrateToSelf in
InstanceFaultRollback and raises that from _prep_resize, and
_error_out_instance_on_exception is called with the initial
vm_state (STOPPED in this case) so when _error_out_instance_on_exception
handles the InstanceFaultRollback exception it sets the instance
vm_state to STOPPED (what it already was) rather than ACTIVE.

There were no existing unit tests for the UnableToMigrateToSelf
case in _prep_resize so those are added here.

Conflicts:
      nova/tests/unit/compute/test_compute_mgr.py

NOTE(mriedem): The conflict is due to some unit tests from change
I734cc01dce13f9e75a16639faf890ddb1661b7eb not being in Stein.

Change-Id: I17543ecb572934ecc7d0bbc7a4ad2f537fa499bc
Closes-Bug: #1811235
(cherry picked from commit d1931ac)

Author: mriedem

nova/compute/manager.py

@@ -4364,8 +4364,12 @@ def _prep_resize(self, context, image, instance, instance_type,
             # check driver whether support migrate to same host
             if not self.driver.capabilities.get(
                     'supports_migrate_to_same_host', False):
-                raise exception.UnableToMigrateToSelf(
-                    instance_id=instance.uuid, host=self.host)
+                # Raise InstanceFaultRollback so that the
+                # _error_out_instance_on_exception context manager in
+                # prep_resize will set the instance.vm_state properly.
+                raise exception.InstanceFaultRollback(
+                    inner_exception=exception.UnableToMigrateToSelf(
+                        instance_id=instance.uuid, host=self.host))
 
         # NOTE(danms): Stash the new instance_type to avoid having to
         # look it up in the database later
@@ -4439,8 +4443,13 @@ def prep_resize(self, context, image, instance, instance_type,
         if node is None:
             node = self._get_nodename(instance, refresh=True)
 
-        with self._error_out_instance_on_exception(context, instance), \
-                 errors_out_migration_ctxt(migration):
+        # Pass instance_state=instance.vm_state because we can resize
+        # a STOPPED server and we don't want to set it back to ACTIVE
+        # in case _prep_resize fails.
+        instance_state = instance.vm_state
+        with self._error_out_instance_on_exception(
+                context, instance, instance_state=instance_state),\
+                errors_out_migration_ctxt(migration):
             self._send_prep_resize_notifications(
                 context, instance, fields.NotificationPhase.START,
                 instance_type)

nova/tests/unit/compute/test_compute_mgr.py

@@ -8817,6 +8817,68 @@ def doit(mock_pr, mock_r):
 
         doit()
 
+    def test_prep_resize_fails_unable_to_migrate_to_self(self):
+        """Asserts that _prep_resize handles UnableToMigrateToSelf when
+        _prep_resize is called on the host on which the instance lives and
+        the flavor is not changing.
+        """
+        instance = fake_instance.fake_instance_obj(
+            self.context, host=self.compute.host,
+            expected_attrs=['system_metadata', 'flavor'])
+        migration = mock.MagicMock(spec='nova.objects.Migration')
+        with mock.patch.dict(self.compute.driver.capabilities,
+                             {'supports_migrate_to_same_host': False}):
+            ex = self.assertRaises(
+                exception.InstanceFaultRollback, self.compute._prep_resize,
+                self.context, instance.image_meta, instance, instance.flavor,
+                filter_properties={}, node=instance.node, migration=migration)
+        self.assertIsInstance(
+            ex.inner_exception, exception.UnableToMigrateToSelf)
+
+    @mock.patch('nova.compute.utils.notify_usage_exists')
+    @mock.patch('nova.compute.manager.ComputeManager.'
+                '_notify_about_instance_usage')
+    @mock.patch('nova.compute.utils.notify_about_resize_prep_instance')
+    @mock.patch('nova.objects.Instance.save')
+    @mock.patch('nova.compute.manager.ComputeManager._revert_allocation')
+    @mock.patch('nova.compute.manager.ComputeManager.'
+                '_reschedule_resize_or_reraise')
+    @mock.patch('nova.compute.utils.add_instance_fault_from_exc')
+    def test_prep_resize_fails_rollback(
+            self, add_instance_fault_from_exc, _reschedule_resize_or_reraise,
+            _revert_allocation, mock_instance_save,
+            notify_about_resize_prep_instance, _notify_about_instance_usage,
+            notify_usage_exists):
+        """Tests that if _prep_resize raises InstanceFaultRollback, the
+        instance.vm_state is reset properly in _error_out_instance_on_exception
+        """
+        instance = fake_instance.fake_instance_obj(
+            self.context, host=self.compute.host, vm_state=vm_states.STOPPED,
+            expected_attrs=['system_metadata', 'flavor'])
+        migration = mock.MagicMock(spec='nova.objects.Migration')
+        request_spec = mock.MagicMock(spec='nova.objects.RequestSpec')
+        ex = exception.InstanceFaultRollback(
+            inner_exception=exception.UnableToMigrateToSelf(
+                instance_id=instance.uuid, host=instance.host))
+
+        def fake_reschedule_resize_or_reraise(*args, **kwargs):
+            raise ex
+
+        _reschedule_resize_or_reraise.side_effect = (
+            fake_reschedule_resize_or_reraise)
+
+        with mock.patch.object(self.compute, '_prep_resize', side_effect=ex):
+            self.assertRaises(
+                # _error_out_instance_on_exception should reraise the
+                # UnableToMigrateToSelf inside InstanceFaultRollback.
+                exception.UnableToMigrateToSelf, self.compute.prep_resize,
+                self.context, instance.image_meta, instance, instance.flavor,
+                request_spec, filter_properties={}, node=instance.node,
+                clean_shutdown=True, migration=migration, host_list=[])
+        # The instance.vm_state should remain unchanged
+        # (_error_out_instance_on_exception will set to ACTIVE by default).
+        self.assertEqual(vm_states.STOPPED, instance.vm_state)
+
     def test_get_updated_nw_info_with_pci_mapping(self):
         old_dev = objects.PciDevice(address='0000:04:00.2')
         new_dev = objects.PciDevice(address='0000:05:00.3')

nova/tests/unit/fake_instance.py

@@ -132,6 +132,9 @@ def fake_instance_obj(context, obj_instance_class=None, **updates):
             inst.vcpus = flavor.vcpus
         if 'memory_mb' in flavor and 'memory_mb' not in updates:
             inst.memory_mb = flavor.memory_mb
+        if ('instance_type_id' not in inst or inst.instance_type_id is None
+                and 'id' in flavor):
+            inst.instance_type_id = flavor.id
     inst.old_flavor = None
     inst.new_flavor = None
     inst.obj_reset_changes()