From 1a11d5c7f35c17cfafc6b8ba57da88995c35b595 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 25 Jan 2019 10:55:00 -0500 Subject: [PATCH] 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 d1931ac0630415519f9c3d906caba9c83cc8162a) --- nova/compute/manager.py | 17 ++++-- nova/tests/unit/compute/test_compute_mgr.py | 62 +++++++++++++++++++++ nova/tests/unit/fake_instance.py | 3 + 3 files changed, 78 insertions(+), 4 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 114324b44500..afcb7518bdc7 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -4364,8 +4364,12 @@ class ComputeManager(manager.Manager): # 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 @@ class ComputeManager(manager.Manager): 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) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index c9e93feecf1b..b9792c518aa1 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -8817,6 +8817,68 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase, 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') diff --git a/nova/tests/unit/fake_instance.py b/nova/tests/unit/fake_instance.py index 39d94a029b3d..b038e9a7becf 100644 --- a/nova/tests/unit/fake_instance.py +++ b/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()