Skip to content

Commit 2a25d1e

Browse files
committed
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 408ef8f) (cherry picked from commit 972d4e0)
1 parent f1ac518 commit 2a25d1e

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

nova/compute/manager.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3825,6 +3825,7 @@ def change_instance_metadata(self, context, diff, instance):
38253825

38263826
@wrap_exception()
38273827
@wrap_instance_event(prefix='compute')
3828+
@errors_out_migration
38283829
@wrap_instance_fault
38293830
def confirm_resize(self, context, instance, migration):
38303831
"""Confirms a migration/resize and deletes the 'old' instance.

nova/tests/unit/compute/test_compute_mgr.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6628,6 +6628,7 @@ def setUp(self):
66286628
expected_attrs=['metadata', 'system_metadata', 'info_cache'])
66296629
self.migration = objects.Migration(
66306630
context=self.context.elevated(),
6631+
id=1,
66316632
uuid=uuids.migration_uuid,
66326633
instance_uuid=self.instance.uuid,
66336634
new_instance_type_id=7,
@@ -7043,6 +7044,53 @@ def do_it(mock_rc, mock_grt):
70437044
mock_resources.return_value)
70447045
do_it()
70457046

7047+
@mock.patch('nova.compute.utils.add_instance_fault_from_exc')
7048+
@mock.patch('nova.objects.Migration.get_by_id')
7049+
@mock.patch('nova.objects.Instance.get_by_uuid')
7050+
@mock.patch('nova.compute.utils.notify_about_instance_usage')
7051+
@mock.patch('nova.compute.utils.notify_about_instance_action')
7052+
@mock.patch('nova.objects.Instance.save')
7053+
def test_confirm_resize_driver_confirm_migration_fails(
7054+
self, instance_save, notify_action, notify_usage,
7055+
instance_get_by_uuid, migration_get_by_id, add_fault):
7056+
"""Tests the scenario that driver.confirm_migration raises some error
7057+
to make sure the error is properly handled, like the instance and
7058+
migration status is set to 'error'.
7059+
"""
7060+
self.migration.status = 'confirming'
7061+
migration_get_by_id.return_value = self.migration
7062+
instance_get_by_uuid.return_value = self.instance
7063+
7064+
error = exception.HypervisorUnavailable(
7065+
host=self.migration.source_compute)
7066+
with test.nested(
7067+
mock.patch.object(self.compute, 'network_api'),
7068+
mock.patch.object(self.compute.driver, 'confirm_migration',
7069+
side_effect=error)
7070+
) as (
7071+
network_api, confirm_migration
7072+
):
7073+
self.assertRaises(exception.HypervisorUnavailable,
7074+
self.compute.confirm_resize,
7075+
self.context, self.instance, self.migration)
7076+
# Make sure the instance is in ERROR status.
7077+
self.assertEqual(vm_states.ERROR, self.instance.vm_state)
7078+
# Make sure the migration is in error status.
7079+
self.assertEqual('error', self.migration.status)
7080+
# Instance.save is called twice, once to clear the resize metadata
7081+
# and once to set the instance to ERROR status.
7082+
self.assertEqual(2, instance_save.call_count)
7083+
# The migration.status should have been saved.
7084+
self.migration.save.assert_called_once_with()
7085+
# Assert other mocks we care less about.
7086+
notify_usage.assert_called_once()
7087+
notify_action.assert_called_once()
7088+
add_fault.assert_called_once()
7089+
confirm_migration.assert_called_once()
7090+
network_api.setup_networks_on_host.assert_called_once()
7091+
instance_get_by_uuid.assert_called_once()
7092+
migration_get_by_id.assert_called_once()
7093+
70467094
@mock.patch('nova.scheduler.utils.resources_from_flavor')
70477095
def test_delete_allocation_after_move_confirm_by_migration(self, mock_rff):
70487096
mock_rff.return_value = {}

0 commit comments

Comments
 (0)