commit a1d3ae3876eb394bf594625e3c3cd1f0892d9ea6 Author: Artom Lifshitz Date: Fri Aug 21 13:06:58 2020 -0400 post live migration: don't call Neutron needlessly In bug 1879787, the call to network_api.get_instance_nw_info() in _post_live_migration() on the source compute manager eventually calls out to the Neutron REST API. If this fails, the exception is unhandled, and the migrating instance - which is fully running on the destination at this point - will never be updated in the database. This update normally happens later in post_live_migration_at_destination(). The network_info variable obtained from get_instance_nw_info() is used for two things: notifications - which aren't critical - and unplugging the instance's vifs on the source - which is very important! It turns out that at the time of the get_instance_nw_info() call, the network info in the instance info cache is still valid for unplugging the source vifs. The port bindings on the destination are only activated by the network_api.migrate_instance_start() [1] call that happens shortly *after* the problematic get_instance_nw_info() call. In other words, get_instance_nw_info() will always return the source ports. Because of that, we can replace it with a call to instance.get_network_info(). Small conflict in nova/tests/unit/compute/test_compute_mgr.py due to different mocks in the context. [1] https://opendev.org/openstack/nova/src/commit/d9e04c4ff0b1a9c3383f1848dc846e93030d83cb/nova/network/neutronv2/api.py#L2493-L2522 Change-Id: If0fbae33ce2af198188c91638afef939256c2556 Closes-bug: 1879787 (cherry picked from commit 6488a5dfb293831a448596e2084f484dd0bfa916) (cherry picked from commit 2c949cb3eea9cd9282060da12d32771582953aa2) (cherry picked from commit 7ace26e4bcd69a0a3d780cfae9f6745e4177b6c2) (cherry picked from commit 703c8ef4e6d282ce26c3527ecbbe6dc3f47b363f) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index e7243d4..d62a416 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -6987,7 +6987,13 @@ class ComputeManager(manager.Manager): # Releasing vlan. # (not necessary in current implementation?) - network_info = self.network_api.get_instance_nw_info(ctxt, instance) + # NOTE(artom) At this point in time we have not bound the ports to the + # destination host yet (this happens in migrate_instance_start() + # below). Therefore, the "old" source network info that's still in the + # instance info cache is safe to use here, since it'll be used below + # during driver.post_live_migration_at_source() to unplug the VIFs on + # the source. + network_info = instance.get_network_info() self._notify_about_instance_usage(ctxt, instance, "live_migration._post.start", diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 6039ae9..a94045c 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -6546,8 +6546,9 @@ class ComputeTestCase(BaseTestCase, mock.patch.object( self.compute.network_api, 'setup_networks_on_host'), mock.patch.object(migration_obj, 'save'), + mock.patch.object(instance, 'get_network_info', return_value=[]), ) as ( - mock_migrate, mock_setup, mock_mig_save + mock_migrate, mock_setup, mock_mig_save, mock_get_nw_info ): self.compute._post_live_migration(c, instance, dest, migrate_data=migrate_data, @@ -6559,6 +6560,7 @@ class ComputeTestCase(BaseTestCase, mock_migrate.assert_called_once_with(c, instance, migration) mock_post.assert_called_once_with(c, instance, False, dest) mock_clear.assert_called_once_with(mock.ANY) + mock_get_nw_info.assert_called() @mock.patch('nova.compute.utils.notify_about_instance_action') def test_post_live_migration_working_correctly(self, mock_notify): @@ -6601,12 +6603,15 @@ class ComputeTestCase(BaseTestCase, 'clear_events_for_instance'), mock.patch.object(self.compute, 'update_available_resource'), mock.patch.object(migration_obj, 'save'), + mock.patch.object(instance, 'get_network_info'), ) as ( post_live_migration, unfilter_instance, migrate_instance_start, post_live_migration_at_destination, post_live_migration_at_source, setup_networks_on_host, - clear_events, update_available_resource, mig_save + clear_events, update_available_resource, mig_save, get_nw_info, ): + nw_info = network_model.NetworkInfo.hydrate([]) + get_nw_info.return_value = nw_info self.compute._post_live_migration(c, instance, dest, migrate_data=migrate_data, source_bdms=bdms) @@ -6629,7 +6634,7 @@ class ComputeTestCase(BaseTestCase, post_live_migration_at_destination.assert_has_calls([ mock.call(c, instance, False, dest)]) post_live_migration_at_source.assert_has_calls( - [mock.call(c, instance, [])]) + [mock.call(c, instance, nw_info)]) clear_events.assert_called_once_with(instance) update_available_resource.assert_has_calls([mock.call(c)]) self.assertEqual('completed', migration_obj.status) diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index d14d8c0..7abe5b2 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -8718,6 +8718,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): source_node='src') image_bdm.attachment_id = uuids.attachment3 + @mock.patch.object(instance, 'get_network_info') @mock.patch('nova.compute.utils.notify_about_instance_action') @mock.patch('nova.objects.ConsoleAuthToken.' 'clean_console_auths_for_instance') @@ -8737,7 +8738,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): def _test(mock_net_api, mock_notify, mock_driver, mock_rpc, mock_get_bdm_info, mock_attach_delete, mock_update_resource, mock_bdm_save, mock_rt, mock_ga, - mock_clean, mock_notify_action): + mock_clean, mock_notify_action, mock_get_nw_info): mock_rt.return_value = mock.Mock() bdms = objects.BlockDeviceMappingList(objects=[vol_bdm, image_bdm]) @@ -8754,6 +8755,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase): mock.call(self.context, instance, 'fake-mini', action='live_migration_post', phase='end')]) self.assertEqual(2, mock_notify_action.call_count) + mock_get_nw_info.assert_called() _test()