Merge "Refactor volume connection cleanup out of _post_live_migration" into stable/train

3 files changed, 53 insertions, 90 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 5027362c9b..0e013a3cc5 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -7540,37 +7540,16 @@ class ComputeManager(manager.Manager):
 
         return (do_cleanup, destroy_disks)
 
-    @wrap_exception()
-    @wrap_instance_fault
-    def _post_live_migration(self, ctxt, instance, dest,
-                             block_migration=False, migrate_data=None,
-                             source_bdms=None):
-        """Post operations for live migration.
-
-        This method is called from live_migration
-        and mainly updating database record.
-
-        :param ctxt: security context
-        :param instance: instance dict
-        :param dest: destination host
-        :param block_migration: if true, prepare for block migration
-        :param migrate_data: if not None, it is a dict which has data
-        :param source_bdms: BDMs prior to modification by the destination
-                            compute host. Set by _do_live_migration and not
-                            part of the callback interface, so this is never
-                            None
-        required for live migration without shared storage
+    def _post_live_migration_remove_source_vol_connections(
+            self, context, instance, source_bdms):
+        """Disconnect volume connections from the source host during
+        _post_live_migration.
 
+        :param context: nova auth RequestContext
+        :param instance: Instance object being live migrated
+        :param source_bdms: BlockDeviceMappingList representing the attached
+            volumes with connection_info set for the source host
         """
-        LOG.info('_post_live_migration() is started..',
-                 instance=instance)
-
-        # Cleanup source host post live-migration
-        block_device_info = self._get_instance_block_device_info(
-                            ctxt, instance, bdms=source_bdms)
-        self.driver.post_live_migration(ctxt, instance, block_device_info,
-                                        migrate_data)
-
         # Detaching volumes.
         connector = self.driver.get_volume_connector(instance)
         for bdm in source_bdms:
@@ -7592,27 +7571,62 @@ class ComputeManager(manager.Manager):
                         # remove the volume connection without detaching from
                         # hypervisor because the instance is not running
                         # anymore on the current host
-                        self.volume_api.terminate_connection(ctxt,
+                        self.volume_api.terminate_connection(context,
                                                              bdm.volume_id,
                                                              connector)
                     else:
                         # cinder v3.44 api flow - delete the old attachment
                         # for the source host
-                        self.volume_api.attachment_delete(ctxt,
+                        self.volume_api.attachment_delete(context,
                                                           bdm.attachment_id)
 
                 except Exception as e:
                     if bdm.attachment_id is None:
                         LOG.error('Connection for volume %s not terminated on '
                                   'source host %s during post_live_migration: '
-                                   '%s', bdm.volume_id, self.host,
-                                   six.text_type(e), instance=instance)
+                                  '%s', bdm.volume_id, self.host,
+                                  six.text_type(e), instance=instance)
                     else:
                         LOG.error('Volume attachment %s not deleted on source '
                                   'host %s during post_live_migration: %s',
                                   bdm.attachment_id, self.host,
                                   six.text_type(e), instance=instance)
 
+    @wrap_exception()
+    @wrap_instance_fault
+    def _post_live_migration(self, ctxt, instance, dest,
+                             block_migration=False, migrate_data=None,
+                             source_bdms=None):
+        """Post operations for live migration.
+
+        This method is called from live_migration
+        and mainly updating database record.
+
+        :param ctxt: security context
+        :param instance: instance dict
+        :param dest: destination host
+        :param block_migration: if true, prepare for block migration
+        :param migrate_data: if not None, it is a dict which has data
+        :param source_bdms: BDMs prior to modification by the destination
+                            compute host. Set by _do_live_migration and not
+                            part of the callback interface, so this is never
+                            None
+        required for live migration without shared storage
+
+        """
+        LOG.info('_post_live_migration() is started..',
+                 instance=instance)
+
+        # Cleanup source host post live-migration
+        block_device_info = self._get_instance_block_device_info(
+                            ctxt, instance, bdms=source_bdms)
+        self.driver.post_live_migration(ctxt, instance, block_device_info,
+                                        migrate_data)
+
+        # Disconnect volumes from this (the source) host.
+        self._post_live_migration_remove_source_vol_connections(
+            ctxt, instance, source_bdms)
+
         # Releasing vlan.
         # (not necessary in current implementation?)
 
diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py
index dc3a49e5a1..fef93b470d 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -6658,9 +6658,6 @@ class ComputeTestCase(BaseTestCase,
                                         'state_description': 'migrating',
                                         'state': power_state.PAUSED},
                                         ctxt=c)
-        instance.update({'task_state': task_states.MIGRATING,
-                         'power_state': power_state.PAUSED})
-        instance.save()
 
         bdms = block_device_obj.block_device_make_list(c,
                 [fake_block_device.FakeDbBlockDeviceDict({
@@ -6672,29 +6669,16 @@ class ComputeTestCase(BaseTestCase,
                  ])
 
         with test.nested(
-            mock.patch.object(self.compute.network_api,
-                              'migrate_instance_start'),
-            mock.patch.object(self.compute.compute_rpcapi,
-                              'post_live_migration_at_destination'),
-            mock.patch.object(self.compute.network_api,
-                              'setup_networks_on_host'),
-            mock.patch.object(self.compute.instance_events,
-                              'clear_events_for_instance'),
-            mock.patch.object(self.compute,
-                              '_get_instance_block_device_info'),
             mock.patch.object(self.compute.driver, 'get_volume_connector'),
             mock.patch.object(cinder.API, 'terminate_connection'),
         ) as (
-            migrate_instance_start, post_live_migration_at_destination,
-            setup_networks_on_host, clear_events_for_instance,
-            get_instance_volume_block_device_info, get_volume_connector,
+            get_volume_connector,
             terminate_connection,
         ):
             get_volume_connector.return_value = 'fake-connector'
 
-            self.compute._post_live_migration(c, instance, 'dest_host',
-                                              migrate_data=mock.MagicMock(),
-                                              source_bdms=bdms)
+            self.compute._post_live_migration_remove_source_vol_connections(
+                c, instance, bdms)
 
             terminate_connection.assert_called_once_with(
                     c, uuids.volume_id, 'fake-connector')
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index dfa14a4c8d..c22aa01ffd 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -9387,52 +9387,17 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
              'volume_id': volume_id, 'device_name': '/dev/vdb',
              'instance_uuid': instance.uuid})
         vol_bdm.attachment_id = uuids.attachment
-        migrate_data = migrate_data_obj.LiveMigrateData()
-        migrate_data.migration = objects.Migration(uuid=uuids.migration,
-                                                   dest_node=instance.node,
-                                                   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')
-        @mock.patch.object(migrate_data.migration, 'save',
-                           new=lambda: None)
-        @mock.patch.object(self.compute.reportclient,
-                           'get_allocations_for_consumer_by_provider')
-        @mock.patch.object(vol_bdm, 'save')
-        @mock.patch.object(self.compute, 'update_available_resource')
         @mock.patch.object(self.compute.volume_api, 'attachment_delete')
-        @mock.patch.object(self.compute, '_get_instance_block_device_info')
-        @mock.patch.object(self.compute, 'compute_rpcapi')
-        @mock.patch.object(self.compute, 'driver')
-        @mock.patch.object(self.compute, '_notify_about_instance_usage')
-        @mock.patch.object(self.compute, 'network_api')
-        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_get_nw_info):
-            self._mock_rt()
-
+        def _test(mock_attach_delete):
             bdms = objects.BlockDeviceMappingList(objects=[vol_bdm, image_bdm])
 
-            self.compute._post_live_migration(self.context,
-                                              instance,
-                                              dest_host,
-                                              migrate_data=migrate_data,
-                                              source_bdms=bdms)
+            self.compute._post_live_migration_remove_source_vol_connections(
+                self.context, instance, bdms)
 
             mock_attach_delete.assert_called_once_with(
                 self.context, uuids.attachment)
-            mock_clean.assert_called_once_with(self.context, instance.uuid)
-            mock_notify_action.assert_has_calls([
-                mock.call(self.context, instance, 'fake-mini',
-                          action='live_migration_post', phase='start'),
-                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()