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 commitb2734b5a9a
. 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]d9e04c4ff0/nova/network/neutronv2/api.py (L2493-L2522)
Change-Id: If0fbae33ce2af198188c91638afef939256c2556 Closes-bug: 1879787 (cherry picked from commit6488a5dfb2
) (cherry picked from commit2c949cb3ee
)
This commit is contained in:
parent
5cebdb7b71
commit
7ace26e4bc
|
@ -7496,7 +7496,13 @@ class ComputeManager(manager.Manager):
|
||||||
# Releasing vlan.
|
# Releasing vlan.
|
||||||
# (not necessary in current implementation?)
|
# (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,
|
self._notify_about_instance_usage(ctxt, instance,
|
||||||
"live_migration._post.start",
|
"live_migration._post.start",
|
||||||
|
|
|
@ -6482,8 +6482,9 @@ class ComputeTestCase(BaseTestCase,
|
||||||
mock.patch.object(
|
mock.patch.object(
|
||||||
self.compute.network_api, 'setup_networks_on_host'),
|
self.compute.network_api, 'setup_networks_on_host'),
|
||||||
mock.patch.object(migration_obj, 'save'),
|
mock.patch.object(migration_obj, 'save'),
|
||||||
|
mock.patch.object(instance, 'get_network_info', return_value=[]),
|
||||||
) as (
|
) 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,
|
self.compute._post_live_migration(c, instance, dest,
|
||||||
migrate_data=migrate_data,
|
migrate_data=migrate_data,
|
||||||
|
@ -6495,6 +6496,7 @@ class ComputeTestCase(BaseTestCase,
|
||||||
mock_migrate.assert_called_once_with(c, instance, migration)
|
mock_migrate.assert_called_once_with(c, instance, migration)
|
||||||
mock_post.assert_called_once_with(c, instance, False, dest)
|
mock_post.assert_called_once_with(c, instance, False, dest)
|
||||||
mock_clear.assert_called_once_with(mock.ANY)
|
mock_clear.assert_called_once_with(mock.ANY)
|
||||||
|
mock_get_nw_info.assert_called()
|
||||||
|
|
||||||
@mock.patch('nova.compute.utils.notify_about_instance_action')
|
@mock.patch('nova.compute.utils.notify_about_instance_action')
|
||||||
def test_post_live_migration_working_correctly(self, mock_notify):
|
def test_post_live_migration_working_correctly(self, mock_notify):
|
||||||
|
@ -6537,12 +6539,15 @@ class ComputeTestCase(BaseTestCase,
|
||||||
'clear_events_for_instance'),
|
'clear_events_for_instance'),
|
||||||
mock.patch.object(self.compute, 'update_available_resource'),
|
mock.patch.object(self.compute, 'update_available_resource'),
|
||||||
mock.patch.object(migration_obj, 'save'),
|
mock.patch.object(migration_obj, 'save'),
|
||||||
|
mock.patch.object(instance, 'get_network_info'),
|
||||||
) as (
|
) as (
|
||||||
post_live_migration, unfilter_instance,
|
post_live_migration, unfilter_instance,
|
||||||
migrate_instance_start, post_live_migration_at_destination,
|
migrate_instance_start, post_live_migration_at_destination,
|
||||||
post_live_migration_at_source, setup_networks_on_host,
|
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,
|
self.compute._post_live_migration(c, instance, dest,
|
||||||
migrate_data=migrate_data,
|
migrate_data=migrate_data,
|
||||||
source_bdms=bdms)
|
source_bdms=bdms)
|
||||||
|
@ -6565,7 +6570,7 @@ class ComputeTestCase(BaseTestCase,
|
||||||
post_live_migration_at_destination.assert_has_calls([
|
post_live_migration_at_destination.assert_has_calls([
|
||||||
mock.call(c, instance, False, dest)])
|
mock.call(c, instance, False, dest)])
|
||||||
post_live_migration_at_source.assert_has_calls(
|
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)
|
clear_events.assert_called_once_with(instance)
|
||||||
update_available_resource.assert_has_calls([mock.call(c)])
|
update_available_resource.assert_has_calls([mock.call(c)])
|
||||||
self.assertEqual('completed', migration_obj.status)
|
self.assertEqual('completed', migration_obj.status)
|
||||||
|
|
|
@ -9287,6 +9287,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||||
source_node='src')
|
source_node='src')
|
||||||
image_bdm.attachment_id = uuids.attachment3
|
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.compute.utils.notify_about_instance_action')
|
||||||
@mock.patch('nova.objects.ConsoleAuthToken.'
|
@mock.patch('nova.objects.ConsoleAuthToken.'
|
||||||
'clean_console_auths_for_instance')
|
'clean_console_auths_for_instance')
|
||||||
|
@ -9305,7 +9306,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||||
def _test(mock_net_api, mock_notify, mock_driver,
|
def _test(mock_net_api, mock_notify, mock_driver,
|
||||||
mock_rpc, mock_get_bdm_info, mock_attach_delete,
|
mock_rpc, mock_get_bdm_info, mock_attach_delete,
|
||||||
mock_update_resource, mock_bdm_save, mock_ga,
|
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()
|
self._mock_rt()
|
||||||
|
|
||||||
bdms = objects.BlockDeviceMappingList(objects=[vol_bdm, image_bdm])
|
bdms = objects.BlockDeviceMappingList(objects=[vol_bdm, image_bdm])
|
||||||
|
@ -9325,6 +9326,7 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
|
||||||
mock.call(self.context, instance, 'fake-mini',
|
mock.call(self.context, instance, 'fake-mini',
|
||||||
action='live_migration_post', phase='end')])
|
action='live_migration_post', phase='end')])
|
||||||
self.assertEqual(2, mock_notify_action.call_count)
|
self.assertEqual(2, mock_notify_action.call_count)
|
||||||
|
mock_get_nw_info.assert_called()
|
||||||
|
|
||||||
_test()
|
_test()
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue