From cfb283cdd0fd016bc8151a5122b7b1f581203c34 Mon Sep 17 00:00:00 2001
From: Artom Lifshitz <alifshit@redhat.com>
Date: Fri, 20 Sep 2019 13:06:56 -0400
Subject: [PATCH] Move pre-3.44 Cinder post live migration test to
 test_compute_mgr

The previous patch (Id0e8b1c32600d53382e5ac938e403258c80221a0) created
_post_live_migration_remove_source_vol_connections() in an attempt to
split out _post_live_migration() into smaller more self-contained
methods. The test for the pre-3.44 Cinder API behavior was kept in
test_compute. test_compute runs API and compute services and
uses the database, and therefore should not receive new tests. Compute
API tests should go in test_compute_api, compute manager tests in
test_compute_mgr and anything that needs a running service and/or
database should go under functional/. While there will be no concerted
effort to "convert" test_compute (like for mox -> mock), it's still
good to do it when the opportunity arises. In addition, having unit
tests for two branches of the same method in different files is just
confusing. This patch removes the pre-3.44 Cinder API unit test from
test_compute and adds a new one right above the existing test for
post-3.44 Cinder API behavior in test_compute_mgr.

Change-Id: I2b07fd03fd3ba7662695384940ceda479fcf9f0a
---
 nova/tests/unit/compute/test_compute.py     | 32 ------------------
 nova/tests/unit/compute/test_compute_mgr.py | 36 +++++++++++++++++++++
 2 files changed, 36 insertions(+), 32 deletions(-)

diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py
index bb2321387bab..8b74ffea6616 100644
--- a/nova/tests/unit/compute/test_compute.py
+++ b/nova/tests/unit/compute/test_compute.py
@@ -6635,38 +6635,6 @@ class ComputeTestCase(BaseTestCase,
                 'Migrating instance to desthost finished successfully.',
                 self.stdlog.logger.output)
 
-    def test_post_live_migration_terminate_volume_connections(self):
-        c = context.get_admin_context()
-        instance = self._create_fake_instance_obj({
-                                        'host': self.compute.host,
-                                        'state_description': 'migrating',
-                                        'state': power_state.PAUSED},
-                                        ctxt=c)
-
-        bdms = block_device_obj.block_device_make_list(c,
-                [fake_block_device.FakeDbBlockDeviceDict({
-                    'source_type': 'blank', 'guest_format': None,
-                    'destination_type': 'local'}),
-                 fake_block_device.FakeDbBlockDeviceDict({
-                    'source_type': 'volume', 'destination_type': 'volume',
-                    'volume_id': uuids.volume_id}),
-                 ])
-
-        with test.nested(
-            mock.patch.object(self.compute.driver, 'get_volume_connector'),
-            mock.patch.object(cinder.API, 'terminate_connection'),
-        ) as (
-            get_volume_connector,
-            terminate_connection,
-        ):
-            get_volume_connector.return_value = 'fake-connector'
-
-            self.compute._post_live_migration_remove_source_vol_connections(
-                c, instance, bdms)
-
-            terminate_connection.assert_called_once_with(
-                    c, uuids.volume_id, 'fake-connector')
-
     @mock.patch.object(objects.ComputeNode,
                        'get_by_host_and_nodename')
     @mock.patch('nova.objects.BlockDeviceMappingList.get_by_instance_uuid')
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index d23eb0b3004e..7c9b499fb915 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -8883,6 +8883,42 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
                                                 self.instance,
                                                 migration)
 
+    def test_post_live_migration_cinder_pre_344_api(self):
+        # Because live migration has
+        # succeeded,_post_live_migration_remove_source_vol_connections()
+        # should call terminate_connection() with the volume UUID.
+        dest_host = 'test_dest_host'
+        instance = fake_instance.fake_instance_obj(self.context,
+                                                   node='dest',
+                                                   uuid=uuids.instance)
+
+        vol_bdm = fake_block_device.fake_bdm_object(
+            self.context,
+            {'source_type': 'volume', 'destination_type': 'volume',
+             'volume_id': uuids.volume, 'device_name': '/dev/vdb',
+             'instance_uuid': instance.uuid,
+             'id': 42,
+             'connection_info':
+             '{"connector": {"host": "%s"}}' % dest_host})
+        image_bdm = fake_block_device.fake_bdm_object(
+            self.context,
+            {'source_type': 'image', 'destination_type': 'local',
+             'volume_id': uuids.image_volume, 'device_name': '/dev/vdb',
+             'instance_uuid': instance.uuid})
+
+        @mock.patch.object(self.compute.driver, 'get_volume_connector')
+        @mock.patch.object(self.compute.volume_api, 'terminate_connection')
+        def _test(mock_term_conn, mock_get_vol_conn):
+            bdms = objects.BlockDeviceMappingList(objects=[vol_bdm, image_bdm])
+
+            self.compute._post_live_migration_remove_source_vol_connections(
+                self.context, instance, bdms)
+
+            mock_term_conn.assert_called_once_with(
+                self.context, uuids.volume, mock_get_vol_conn.return_value)
+
+        _test()
+
     def test_post_live_migration_cinder_v3_api(self):
         # Because live migration has succeeded, _post_live_migration
         # should call attachment_delete with the original/old attachment_id