diff -Nru nova-18.1.0/debian/changelog nova-18.1.0/debian/changelog --- nova-18.1.0/debian/changelog 2019-05-16 08:58:45.000000000 +0000 +++ nova-18.1.0/debian/changelog 2019-06-10 12:36:21.000000000 +0000 @@ -1,3 +1,15 @@ +nova (2:18.1.0-0ubuntu4) cosmic; urgency=medium + + * Cherry-picked from upstream to ensure no stale + allocations are left over on failed cold + migrations (LP: #1821594). + - d/p/bug_1821594_1.patch: Fix migration record status + - d/p/bug_1821594_2.patch: Delete failed allocation part 1 + - d/p/bug_1821594_3.patch: Delete failed allocation part 2 + - d/p/bug_1821594_4.patch: New functional test + + -- Rodrigo Barbieri Mon, 10 Jun 2019 12:36:21 +0000 + nova (2:18.1.0-0ubuntu3) cosmic; urgency=medium * d/p/bug_1825882.patch: Cherry-picked from upstream to ensure diff -Nru nova-18.1.0/debian/patches/bug_1821594_1.patch nova-18.1.0/debian/patches/bug_1821594_1.patch --- nova-18.1.0/debian/patches/bug_1821594_1.patch 1970-01-01 00:00:00.000000000 +0000 +++ nova-18.1.0/debian/patches/bug_1821594_1.patch 2019-06-10 12:35:09.000000000 +0000 @@ -0,0 +1,125 @@ +From 097d8c48a9b0df100ed648f3e4e115c2543bbe1f Mon Sep 17 00:00:00 2001 +From: Matt Riedemann +Date: Mon, 25 Mar 2019 13:16:42 -0400 +Subject: [PATCH 1/4] Error out migration when confirm_resize fails + +If anything fails and raises an exception during +confirm_resize, the migration status is stuck in +"confirming" status even though the instance status +may be "ERROR". + +This change adds the errors_out_migration decorator +to the confirm_resize method to make sure the migration +status is "error" if an error is raised. + +In bug 1821594 it was the driver.confirm_migration +method that raised some exception, so a unit test is +added here which simulates a similar scenario. + +This only partially closes the bug because we are still +leaking allocations on the source node resource provider +since _delete_allocation_after_move is not called. That +will be dealt with in a separate patch. + +Conflicts: + nova/tests/functional/test_servers.py + nova/tests/unit/compute/test_compute_mgr.py + +NOTE(mriedem): The functional test conflict is due to not +having change I99427a52676826990d2a2ffc82cf30ad945b939c +in Rocky. The unit test conflict is due to not having +change I0851e2d54a1fdc82fe3291fb7e286e790f121e92 in Rocky. +The source_node attribute on the fake Migration object in +the unit test is added here because change +I312d61383345ea0ac1ab0c277b4c468e6aa94656 is not in Rocky. + +Change-Id: Ic7d78ad43a2bad7f932c22c98944accbbed9e9e2 +Partial-Bug: #1821594 +(cherry picked from commit 408ef8f84a698f764aa5d769d6d01fd9340de2e5) +(cherry picked from commit 972d4e0eb391e83fe8d3020ff95db0e6a840a224) +--- + nova/compute/manager.py | 1 + + nova/tests/unit/compute/test_compute_mgr.py | 48 +++++++++++++++++++++ + 2 files changed, 49 insertions(+) + +diff --git a/nova/compute/manager.py b/nova/compute/manager.py +index 85427d62c5..38931b8592 100644 +--- a/nova/compute/manager.py ++++ b/nova/compute/manager.py +@@ -3825,6 +3825,7 @@ class ComputeManager(manager.Manager): + + @wrap_exception() + @wrap_instance_event(prefix='compute') ++ @errors_out_migration + @wrap_instance_fault + def confirm_resize(self, context, instance, migration): + """Confirms a migration/resize and deletes the 'old' instance. +diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py +index 2b9364adae..852d1e2496 100644 +--- a/nova/tests/unit/compute/test_compute_mgr.py ++++ b/nova/tests/unit/compute/test_compute_mgr.py +@@ -6628,6 +6628,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + expected_attrs=['metadata', 'system_metadata', 'info_cache']) + self.migration = objects.Migration( + context=self.context.elevated(), ++ id=1, + uuid=uuids.migration_uuid, + instance_uuid=self.instance.uuid, + new_instance_type_id=7, +@@ -7036,6 +7037,53 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + mock_resources.return_value) + do_it() + ++ @mock.patch('nova.compute.utils.add_instance_fault_from_exc') ++ @mock.patch('nova.objects.Migration.get_by_id') ++ @mock.patch('nova.objects.Instance.get_by_uuid') ++ @mock.patch('nova.compute.utils.notify_about_instance_usage') ++ @mock.patch('nova.compute.utils.notify_about_instance_action') ++ @mock.patch('nova.objects.Instance.save') ++ def test_confirm_resize_driver_confirm_migration_fails( ++ self, instance_save, notify_action, notify_usage, ++ instance_get_by_uuid, migration_get_by_id, add_fault): ++ """Tests the scenario that driver.confirm_migration raises some error ++ to make sure the error is properly handled, like the instance and ++ migration status is set to 'error'. ++ """ ++ self.migration.status = 'confirming' ++ migration_get_by_id.return_value = self.migration ++ instance_get_by_uuid.return_value = self.instance ++ ++ error = exception.HypervisorUnavailable( ++ host=self.migration.source_compute) ++ with test.nested( ++ mock.patch.object(self.compute, 'network_api'), ++ mock.patch.object(self.compute.driver, 'confirm_migration', ++ side_effect=error) ++ ) as ( ++ network_api, confirm_migration ++ ): ++ self.assertRaises(exception.HypervisorUnavailable, ++ self.compute.confirm_resize, ++ self.context, self.instance, self.migration) ++ # Make sure the instance is in ERROR status. ++ self.assertEqual(vm_states.ERROR, self.instance.vm_state) ++ # Make sure the migration is in error status. ++ self.assertEqual('error', self.migration.status) ++ # Instance.save is called twice, once to clear the resize metadata ++ # and once to set the instance to ERROR status. ++ self.assertEqual(2, instance_save.call_count) ++ # The migration.status should have been saved. ++ self.migration.save.assert_called_once_with() ++ # Assert other mocks we care less about. ++ notify_usage.assert_called_once() ++ notify_action.assert_called_once() ++ add_fault.assert_called_once() ++ confirm_migration.assert_called_once() ++ network_api.setup_networks_on_host.assert_called_once() ++ instance_get_by_uuid.assert_called_once() ++ migration_get_by_id.assert_called_once() ++ + @mock.patch('nova.scheduler.utils.resources_from_flavor') + def test_delete_allocation_after_move_confirm_by_migration(self, mock_rff): + mock_rff.return_value = {} +-- +2.17.1 + diff -Nru nova-18.1.0/debian/patches/bug_1821594_2.patch nova-18.1.0/debian/patches/bug_1821594_2.patch --- nova-18.1.0/debian/patches/bug_1821594_2.patch 1970-01-01 00:00:00.000000000 +0000 +++ nova-18.1.0/debian/patches/bug_1821594_2.patch 2019-06-10 12:35:27.000000000 +0000 @@ -0,0 +1,267 @@ +From 198543f9ce06660a655e02240d305c86963b0ab1 Mon Sep 17 00:00:00 2001 +From: Matt Riedemann +Date: Mon, 25 Mar 2019 14:02:17 -0400 +Subject: [PATCH 2/4] Delete allocations even if _confirm_resize raises + +When we are confirming a resize, the guest is on the dest +host and the instance host/node values in the database +are pointing at the dest host, so the _confirm_resize method +on the source is really best effort. If something fails, we +should not leak allocations in placement for the source compute +node resource provider since the instance is not actually +consuming the source node provider resources. + +This change refactors the error handling around the _confirm_resize +call so the big nesting for _error_out_instance_on_exception is +moved to confirm_resize and then a try/finally is added around +_confirm_resize so we can be sure to try and cleanup the allocations +even if _confirm_resize fails in some obscure way. If _confirm_resize +does fail, the error gets re-raised along with logging a traceback +and hint about how to correct the instance state in the DB by hard +rebooting the server on the dest host. + +Conflicts: + nova/compute/manager.py + nova/tests/unit/compute/test_compute_mgr.py + +NOTE(mriedem): The manager.py conflict is due to not having change +Ibb8c12fb2799bb5ceb9e3d72a2b86dbb4f14451e in Rocky. In addition, +since change I0851e2d54a1fdc82fe3291fb7e286e790f121e92 is not in +Rocky the _delete_allocation_after_move signature needs to be +handled a bit different in this backport. The test_compute_mgr.py +conflict and source_node addition in the setUp is due to the same +two changes. + +Change-Id: I29c5f491ec20a71283190a1599e7732541de736f +Closes-Bug: #1821594 +(cherry picked from commit 03a6d26691c1f182224d59190b79f48df278099e) +(cherry picked from commit 5f515060f6c79f113f7b8107596e41056445c79f) + +Conflicts: + nova/tests/unit/compute/test_compute_mgr.py + +NOTE(ganso): An additional test_compute_mgr.py conflict was due to +not having I312d61383345ea0ac1ab0c277b4c468e6aa94656 when performing +this cherry-pick. +--- + nova/compute/manager.py | 124 ++++++++++++-------- + nova/tests/unit/compute/test_compute_mgr.py | 23 +++- + 2 files changed, 90 insertions(+), 57 deletions(-) + +diff --git a/nova/compute/manager.py b/nova/compute/manager.py +index 38931b8592..d6421b1e77 100644 +--- a/nova/compute/manager.py ++++ b/nova/compute/manager.py +@@ -3875,7 +3875,31 @@ class ComputeManager(manager.Manager): + instance=instance) + return + +- self._confirm_resize(context, instance, migration=migration) ++ with self._error_out_instance_on_exception(context, instance): ++ old_instance_type = instance.old_flavor ++ try: ++ self._confirm_resize( ++ context, instance, migration=migration) ++ except Exception: ++ # Something failed when cleaning up the source host so ++ # log a traceback and leave a hint about hard rebooting ++ # the server to correct its state in the DB. ++ with excutils.save_and_reraise_exception(logger=LOG): ++ LOG.exception( ++ 'Confirm resize failed on source host %s. ' ++ 'Resource allocations in the placement service ' ++ 'will be removed regardless because the instance ' ++ 'is now on the destination host %s. You can try ' ++ 'hard rebooting the instance to correct its ' ++ 'state.', self.host, migration.dest_compute, ++ instance=instance) ++ finally: ++ # Whether an error occurred or not, at this point the ++ # instance is on the dest host so to avoid leaking ++ # allocations in placement, delete them here. ++ self._delete_allocation_after_move( ++ context, instance, migration, old_instance_type, ++ migration.source_node) + + do_confirm_resize(context, instance, migration.id) + +@@ -3887,63 +3911,59 @@ class ComputeManager(manager.Manager): + self.host, action=fields.NotificationAction.RESIZE_CONFIRM, + phase=fields.NotificationPhase.START) + +- with self._error_out_instance_on_exception(context, instance): +- # NOTE(danms): delete stashed migration information +- old_instance_type = instance.old_flavor +- instance.old_flavor = None +- instance.new_flavor = None +- instance.system_metadata.pop('old_vm_state', None) +- instance.save() +- +- # NOTE(tr3buchet): tear down networks on source host +- self.network_api.setup_networks_on_host(context, instance, +- migration.source_compute, teardown=True) ++ # NOTE(danms): delete stashed migration information ++ old_instance_type = instance.old_flavor ++ instance.old_flavor = None ++ instance.new_flavor = None ++ instance.system_metadata.pop('old_vm_state', None) ++ instance.save() + +- network_info = self.network_api.get_instance_nw_info(context, +- instance) +- # TODO(mriedem): Get BDMs here and pass them to the driver. +- self.driver.confirm_migration(context, migration, instance, +- network_info) ++ # NOTE(tr3buchet): tear down networks on source host ++ self.network_api.setup_networks_on_host(context, instance, ++ migration.source_compute, teardown=True) + +- migration.status = 'confirmed' +- with migration.obj_as_admin(): +- migration.save() ++ network_info = self.network_api.get_instance_nw_info(context, ++ instance) ++ # TODO(mriedem): Get BDMs here and pass them to the driver. ++ self.driver.confirm_migration(context, migration, instance, ++ network_info) + +- rt = self._get_resource_tracker() +- rt.drop_move_claim(context, instance, migration.source_node, +- old_instance_type, prefix='old_') +- self._delete_allocation_after_move(context, instance, migration, +- old_instance_type, +- migration.source_node) +- instance.drop_migration_context() ++ migration.status = 'confirmed' ++ with migration.obj_as_admin(): ++ migration.save() + +- # NOTE(mriedem): The old_vm_state could be STOPPED but the user +- # might have manually powered up the instance to confirm the +- # resize/migrate, so we need to check the current power state +- # on the instance and set the vm_state appropriately. We default +- # to ACTIVE because if the power state is not SHUTDOWN, we +- # assume _sync_instance_power_state will clean it up. +- p_state = instance.power_state +- vm_state = None +- if p_state == power_state.SHUTDOWN: +- vm_state = vm_states.STOPPED +- LOG.debug("Resized/migrated instance is powered off. " +- "Setting vm_state to '%s'.", vm_state, +- instance=instance) +- else: +- vm_state = vm_states.ACTIVE ++ rt = self._get_resource_tracker() ++ rt.drop_move_claim(context, instance, migration.source_node, ++ old_instance_type, prefix='old_') ++ instance.drop_migration_context() ++ ++ # NOTE(mriedem): The old_vm_state could be STOPPED but the user ++ # might have manually powered up the instance to confirm the ++ # resize/migrate, so we need to check the current power state ++ # on the instance and set the vm_state appropriately. We default ++ # to ACTIVE because if the power state is not SHUTDOWN, we ++ # assume _sync_instance_power_state will clean it up. ++ p_state = instance.power_state ++ vm_state = None ++ if p_state == power_state.SHUTDOWN: ++ vm_state = vm_states.STOPPED ++ LOG.debug("Resized/migrated instance is powered off. " ++ "Setting vm_state to '%s'.", vm_state, ++ instance=instance) ++ else: ++ vm_state = vm_states.ACTIVE + +- instance.vm_state = vm_state +- instance.task_state = None +- instance.save(expected_task_state=[None, task_states.DELETING, +- task_states.SOFT_DELETING]) ++ instance.vm_state = vm_state ++ instance.task_state = None ++ instance.save(expected_task_state=[None, task_states.DELETING, ++ task_states.SOFT_DELETING]) + +- self._notify_about_instance_usage( +- context, instance, "resize.confirm.end", +- network_info=network_info) +- compute_utils.notify_about_instance_action(context, instance, +- self.host, action=fields.NotificationAction.RESIZE_CONFIRM, +- phase=fields.NotificationPhase.END) ++ self._notify_about_instance_usage( ++ context, instance, "resize.confirm.end", ++ network_info=network_info) ++ compute_utils.notify_about_instance_action(context, instance, ++ self.host, action=fields.NotificationAction.RESIZE_CONFIRM, ++ phase=fields.NotificationPhase.END) + + def _delete_allocation_after_move(self, context, instance, migration, + flavor, nodename): +diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py +index 852d1e2496..b7d33858bf 100644 +--- a/nova/tests/unit/compute/test_compute_mgr.py ++++ b/nova/tests/unit/compute/test_compute_mgr.py +@@ -6635,6 +6635,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + dest_compute=None, + dest_node=None, + dest_host=None, ++ source_compute='source_compute', ++ source_node='source_node', + status='migrating') + self.migration.save = mock.MagicMock() + self.useFixture(fixtures.SpawnIsSynchronousFixture()) +@@ -6988,6 +6990,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + do_finish_revert_resize() + + def test_confirm_resize_deletes_allocations(self): ++ @mock.patch('nova.objects.Instance.get_by_uuid') ++ @mock.patch('nova.objects.Migration.get_by_id') + @mock.patch.object(self.migration, 'save') + @mock.patch.object(self.compute, '_notify_about_instance_usage') + @mock.patch.object(self.compute, 'network_api') +@@ -6998,12 +7002,15 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + @mock.patch.object(self.instance, 'save') + def do_confirm_resize(mock_save, mock_drop, mock_delete, mock_get_rt, + mock_confirm, mock_nwapi, mock_notify, +- mock_mig_save): ++ mock_mig_save, mock_mig_get, mock_inst_get): + self.instance.migration_context = objects.MigrationContext() + self.migration.source_compute = self.instance['host'] + self.migration.source_node = self.instance['node'] +- self.compute._confirm_resize(self.context, self.instance, +- self.migration) ++ self.migration.status = 'confirming' ++ mock_mig_get.return_value = self.migration ++ mock_inst_get.return_value = self.instance ++ self.compute.confirm_resize(self.context, self.instance, ++ self.migration) + mock_delete.assert_called_once_with(self.context, self.instance, + self.migration, + self.instance.old_flavor, +@@ -7059,9 +7066,10 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + with test.nested( + mock.patch.object(self.compute, 'network_api'), + mock.patch.object(self.compute.driver, 'confirm_migration', +- side_effect=error) ++ side_effect=error), ++ mock.patch.object(self.compute, '_delete_allocation_after_move') + ) as ( +- network_api, confirm_migration ++ network_api, confirm_migration, delete_allocation + ): + self.assertRaises(exception.HypervisorUnavailable, + self.compute.confirm_resize, +@@ -7075,6 +7083,11 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + self.assertEqual(2, instance_save.call_count) + # The migration.status should have been saved. + self.migration.save.assert_called_once_with() ++ # Allocations should always be cleaned up even if cleaning up the ++ # source host fails. ++ delete_allocation.assert_called_once_with( ++ self.context, self.instance, self.migration, ++ self.instance.old_flavor, self.migration.source_node) + # Assert other mocks we care less about. + notify_usage.assert_called_once() + notify_action.assert_called_once() +-- +2.17.1 + diff -Nru nova-18.1.0/debian/patches/bug_1821594_3.patch nova-18.1.0/debian/patches/bug_1821594_3.patch --- nova-18.1.0/debian/patches/bug_1821594_3.patch 1970-01-01 00:00:00.000000000 +0000 +++ nova-18.1.0/debian/patches/bug_1821594_3.patch 2019-06-10 12:35:39.000000000 +0000 @@ -0,0 +1,93 @@ +From 1fbd6d3c9b864308aac2f33945046871ee601c1f Mon Sep 17 00:00:00 2001 +From: Matt Riedemann +Date: Wed, 15 May 2019 12:27:17 -0400 +Subject: [PATCH 3/4] [stable-only] Delete allocations even if _confirm_resize + raises (part 2) + +The backport 37ac54a42ec91821d62864d63c486c002608491b to fix +bug 1821594 did not account for how the _delete_allocation_after_move +method before Stein is tightly coupled to the migration status +being set to "confirmed" which is what the _confirm_resize method +does after self.driver.confirm_migration returns. + +However, if self.driver.confirm_migration raises an exception +we still want to cleanup the allocations held on the source node +and for that we call _delete_allocation_after_move. But because +of that tight coupling before Stein, we need to temporarily +mutate the migration status to "confirmed" to get the cleanup +method to do what we want. + +This isn't a problem starting in Stein because change +I0851e2d54a1fdc82fe3291fb7e286e790f121e92 removed that +tight coupling on the migration status, so this is a stable +branch only change. + +Note that we don't call self.reportclient.delete_allocation_for_instance +directly since before Stein we still need to account for a +migration that does not move the source node allocations to the +migration record, and that logic is in _delete_allocation_after_move. + +A simple unit test assertion is added here but the functional +test added in change I9d6478f492351b58aa87b8f56e907ee633d6d1c6 +will assert the bug is fixed properly before Stein. + +Change-Id: I933687891abef4878de09481937d576ce5899511 +Closes-Bug: #1821594 +--- + nova/compute/manager.py | 13 ++++++++++--- + nova/tests/unit/compute/test_compute_mgr.py | 9 ++++++++- + 2 files changed, 18 insertions(+), 4 deletions(-) + +diff --git a/nova/compute/manager.py b/nova/compute/manager.py +index d6421b1e77..b88ea82072 100644 +--- a/nova/compute/manager.py ++++ b/nova/compute/manager.py +@@ -3897,9 +3897,16 @@ class ComputeManager(manager.Manager): + # Whether an error occurred or not, at this point the + # instance is on the dest host so to avoid leaking + # allocations in placement, delete them here. +- self._delete_allocation_after_move( +- context, instance, migration, old_instance_type, +- migration.source_node) ++ # NOTE(mriedem): _delete_allocation_after_move is tightly ++ # coupled to the migration status on the confirm step so ++ # we unfortunately have to mutate the migration status to ++ # have _delete_allocation_after_move cleanup the allocation ++ # held by the migration consumer. ++ with utils.temporary_mutation( ++ migration, status='confirmed'): ++ self._delete_allocation_after_move( ++ context, instance, migration, old_instance_type, ++ migration.source_node) + + do_confirm_resize(context, instance, migration.id) + +diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py +index b7d33858bf..85e4726ea5 100644 +--- a/nova/tests/unit/compute/test_compute_mgr.py ++++ b/nova/tests/unit/compute/test_compute_mgr.py +@@ -7061,13 +7061,20 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): + migration_get_by_id.return_value = self.migration + instance_get_by_uuid.return_value = self.instance + ++ def fake_delete_allocation_after_move(_context, instance, migration, ++ flavor, nodename): ++ # The migration.status must be 'confirmed' for the method to ++ # properly cleanup the allocation held by the migration. ++ self.assertEqual('confirmed', migration.status) ++ + error = exception.HypervisorUnavailable( + host=self.migration.source_compute) + with test.nested( + mock.patch.object(self.compute, 'network_api'), + mock.patch.object(self.compute.driver, 'confirm_migration', + side_effect=error), +- mock.patch.object(self.compute, '_delete_allocation_after_move') ++ mock.patch.object(self.compute, '_delete_allocation_after_move', ++ side_effect=fake_delete_allocation_after_move) + ) as ( + network_api, confirm_migration, delete_allocation + ): +-- +2.17.1 + diff -Nru nova-18.1.0/debian/patches/bug_1821594_4.patch nova-18.1.0/debian/patches/bug_1821594_4.patch --- nova-18.1.0/debian/patches/bug_1821594_4.patch 1970-01-01 00:00:00.000000000 +0000 +++ nova-18.1.0/debian/patches/bug_1821594_4.patch 2019-06-10 12:35:47.000000000 +0000 @@ -0,0 +1,112 @@ +From 4afea924fef3318774c941f9aa6298f6795e1d8d Mon Sep 17 00:00:00 2001 +From: Rodrigo Barbieri +Date: Wed, 8 May 2019 16:01:25 -0300 +Subject: [PATCH 4/4] Add functional confirm_migration_error test + +This test checks if allocations have been +successfully cleaned up upon the driver failing +during "confirm_migration". + +This backport is not clean due to change +If6aa37d9b6b48791e070799ab026c816fda4441c, which +refactored the testing framework. Within the refactor, +new assertion methods were added and method +"assertFlavorMatchesAllocation" was modified. +This backport needed to be adapted in order to +be compatible with the testing framework prior to +If6aa37d9b6b48791e070799ab026c816fda4441c. + +Change-Id: I9d6478f492351b58aa87b8f56e907ee633d6d1c6 +Related-bug: #1821594 +(cherry picked from commit 873ac499c50125adc2fb49728d936922f9acf4a9) +(cherry picked from commit d7d7f115430c7ffeb88ec9dcd155ac69b29d7513) +--- + nova/tests/functional/test_servers.py | 72 +++++++++++++++++++++++++++ + 1 file changed, 72 insertions(+) + +diff --git a/nova/tests/functional/test_servers.py b/nova/tests/functional/test_servers.py +index a9b0898daa..94462d2d13 100644 +--- a/nova/tests/functional/test_servers.py ++++ b/nova/tests/functional/test_servers.py +@@ -1838,6 +1838,78 @@ class ServerMovingTests(integrated_helpers.ProviderUsageBaseTestCase): + new_flavor=new_flavor, source_rp_uuid=source_rp_uuid, + dest_rp_uuid=dest_rp_uuid) + ++ def test_migration_confirm_resize_error(self): ++ source_hostname = self.compute1.host ++ dest_hostname = self.compute2.host ++ ++ source_rp_uuid = self._get_provider_uuid_by_host(source_hostname) ++ dest_rp_uuid = self._get_provider_uuid_by_host(dest_hostname) ++ ++ server = self._boot_and_check_allocations(self.flavor1, ++ source_hostname) ++ ++ self._move_and_check_allocations( ++ server, request={'migrate': None}, old_flavor=self.flavor1, ++ new_flavor=self.flavor1, source_rp_uuid=source_rp_uuid, ++ dest_rp_uuid=dest_rp_uuid) ++ ++ # Mock failure ++ def fake_confirm_migration(context, migration, instance, network_info): ++ raise exception.MigrationPreCheckError( ++ reason='test_migration_confirm_resize_error') ++ ++ with mock.patch('nova.virt.fake.FakeDriver.' ++ 'confirm_migration', ++ side_effect=fake_confirm_migration): ++ ++ # Confirm the migration/resize and check the usages ++ post = {'confirmResize': None} ++ self.api.post_server_action( ++ server['id'], post, check_response_status=[204]) ++ server = self._wait_for_state_change(self.api, server, 'ERROR') ++ ++ # After confirming and error, we should have an allocation only on the ++ # destination host ++ source_usages = self._get_provider_usages(source_rp_uuid) ++ self.assertEqual({'VCPU': 0, ++ 'MEMORY_MB': 0, ++ 'DISK_GB': 0}, source_usages, ++ 'Source host %s still has usage after the failed ' ++ 'migration_confirm' % source_hostname) ++ ++ # Check that the server only allocates resource from the original host ++ allocations = self._get_allocations_by_server_uuid(server['id']) ++ self.assertEqual(1, len(allocations)) ++ ++ dest_allocation = allocations[dest_rp_uuid]['resources'] ++ self.assertFlavorMatchesAllocation(self.flavor1, dest_allocation) ++ ++ dest_usages = self._get_provider_usages(dest_rp_uuid) ++ self.assertFlavorMatchesAllocation(self.flavor1, dest_usages) ++ ++ self._run_periodics() ++ ++ # After confirming and error, we should have an allocation only on the ++ # destination host ++ source_usages = self._get_provider_usages(source_rp_uuid) ++ self.assertEqual({'VCPU': 0, ++ 'MEMORY_MB': 0, ++ 'DISK_GB': 0}, source_usages, ++ 'Source host %s still has usage after the failed ' ++ 'migration_confirm' % source_hostname) ++ ++ # Check that the server only allocates resource from the original host ++ allocations = self._get_allocations_by_server_uuid(server['id']) ++ self.assertEqual(1, len(allocations)) ++ ++ dest_allocation = allocations[dest_rp_uuid]['resources'] ++ self.assertFlavorMatchesAllocation(self.flavor1, dest_allocation) ++ ++ dest_usages = self._get_provider_usages(dest_rp_uuid) ++ self.assertFlavorMatchesAllocation(self.flavor1, dest_usages) ++ ++ self._delete_and_check_allocations(server) ++ + def _test_resize_revert(self, dest_hostname): + source_hostname = self._other_hostname(dest_hostname) + source_rp_uuid = self._get_provider_uuid_by_host(source_hostname) +-- +2.17.1 + diff -Nru nova-18.1.0/debian/patches/series nova-18.1.0/debian/patches/series --- nova-18.1.0/debian/patches/series 2019-05-16 08:58:45.000000000 +0000 +++ nova-18.1.0/debian/patches/series 2019-06-10 12:35:47.000000000 +0000 @@ -10,3 +10,7 @@ xenapi-agent-change-openssl-error-handling.patch bug_1825882.patch bug_1826523.patch +bug_1821594_1.patch +bug_1821594_2.patch +bug_1821594_3.patch +bug_1821594_4.patch