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().

NOTE(artom) The functional test has been excised, as in stable/train
the NeutronFixture does not properly support live migration with
ports, making the test worthless. The work to support this was done as
part of bp/support-move-ops-with-qos-ports-ussuri, and starts at
commit b2734b5a9ae8b869fc9e8e229826343da3b47fcb.

NOTE(artom) The
test_post_live_migration_no_shared_storage_working_correctly and
test_post_live_migration_cinder_v3_api unit tests had to be adjusted
as part of the backport to pass with the new code.

[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)

3 files changed, 18 insertions, 5 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 6f47428adf..eaedc0238f 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -7496,7 +7496,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 d84b5be3e5..77f23c30fe 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -6482,8 +6482,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,
@@ -6495,6 +6496,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):
@@ -6537,12 +6539,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)
@@ -6565,7 +6570,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 5f01113525..67dc20f6bd 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -9287,6 +9287,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')
@@ -9305,7 +9306,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_ga,
-                  mock_clean, mock_notify_action):
+                  mock_clean, mock_notify_action, mock_get_nw_info):
             self._mock_rt()
 
             bdms = objects.BlockDeviceMappingList(objects=[vol_bdm, image_bdm])
@@ -9325,6 +9326,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()