Improve Share Migration tempest tests

Improve coverage by adding tests that validate the share-type
change while also changing the driver mode.

Closes-bug: #1620800

Change-Id: I924c34aa69591754b437d75f43db91d77e73fb07
diff --git a/manila_tempest_tests/tests/api/admin/test_migration.py b/manila_tempest_tests/tests/api/admin/test_migration.py
index 0f55598..27ceb8b 100644
--- a/manila_tempest_tests/tests/api/admin/test_migration.py
+++ b/manila_tempest_tests/tests/api/admin/test_migration.py
@@ -13,7 +13,6 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import six
 
 import ddt
 from tempest import config
@@ -39,6 +38,8 @@
     2) Host-assisted migration: force_host_assisted_migration is True,
     nondisruptive, writable and preserve-metadata are False.
     3) 2-phase migration of both Host-assisted and Driver-assisted.
+    4) Cancelling migration past first phase.
+    5) Changing driver modes through migration.
 
     No need to test with writable, preserve-metadata and non-disruptive as
     True, values are supplied to the driver which decides what to do. Test
@@ -62,17 +63,16 @@
                 CONF.share.run_driver_assisted_migration_tests):
             raise cls.skipException("Share migration tests are disabled.")
 
-        extra_specs = {
-            'storage_protocol': CONF.share.capability_storage_protocol,
-            'driver_handles_share_servers': (
-                CONF.share.multitenancy_enabled),
-            'snapshot_support': six.text_type(
-                CONF.share.capability_snapshot_support),
-        }
         cls.new_type = cls.create_share_type(
             name=data_utils.rand_name('new_share_type_for_migration'),
             cleanup_in_class=True,
-            extra_specs=extra_specs)
+            extra_specs=utils.get_configured_extra_specs())
+
+        cls.new_type_opposite = cls.create_share_type(
+            name=data_utils.rand_name('new_share_type_for_migration_opposite'),
+            cleanup_in_class=True,
+            extra_specs=utils.get_configured_extra_specs(
+                variation='opposite_driver_modes'))
 
     @test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
     @base.skip_if_microversion_lt("2.22")
@@ -83,13 +83,6 @@
 
         share, dest_pool = self._setup_migration()
 
-        old_exports = self.shares_v2_client.list_share_export_locations(
-            share['id'])
-        self.assertNotEmpty(old_exports)
-        old_exports = [x['path'] for x in old_exports
-                       if x['is_admin_only'] is False]
-        self.assertNotEmpty(old_exports)
-
         task_state = (constants.TASK_STATE_DATA_COPYING_COMPLETED
                       if force_host_assisted
                       else constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE)
@@ -101,8 +94,19 @@
         self._validate_migration_successful(
             dest_pool, share, task_state, complete=False)
 
+        progress = self.shares_v2_client.migration_get_progress(share['id'])
+
+        self.assertEqual(task_state, progress['task_state'])
+        self.assertEqual(100, progress['total_progress'])
+
         share = self.migration_cancel(share['id'], dest_pool)
 
+        progress = self.shares_v2_client.migration_get_progress(share['id'])
+
+        self.assertEqual(
+            constants.TASK_STATE_MIGRATION_CANCELLED, progress['task_state'])
+        self.assertEqual(100, progress['total_progress'])
+
         self._validate_migration_successful(
             dest_pool, share, constants.TASK_STATE_MIGRATION_CANCELLED,
             complete=False)
@@ -110,26 +114,84 @@
     @test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
     @base.skip_if_microversion_lt("2.22")
     @ddt.data(True, False)
+    def test_migration_opposite_driver_modes(self, force_host_assisted):
+
+        self._check_migration_enabled(force_host_assisted)
+
+        share, dest_pool = self._setup_migration(opposite=True)
+
+        old_share_network_id = share['share_network_id']
+
+        # If currently configured is DHSS=False,
+        # then we need it for DHSS=True
+        if not CONF.share.multitenancy_enabled:
+
+            new_share_network_id = self.provide_share_network(
+                self.shares_v2_client, self.os_admin.networks_client,
+                isolated_creds_client=None, ignore_multitenancy_config=True)
+
+        # If currently configured is DHSS=True,
+        # then we must pass None for DHSS=False
+        else:
+            new_share_network_id = None
+
+        old_share_type_id = share['share_type']
+        new_share_type_id = self.new_type_opposite['share_type']['id']
+
+        task_state = (constants.TASK_STATE_DATA_COPYING_COMPLETED
+                      if force_host_assisted
+                      else constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE)
+
+        share = self.migrate_share(
+            share['id'], dest_pool,
+            force_host_assisted_migration=force_host_assisted,
+            wait_for_status=task_state, new_share_type_id=new_share_type_id,
+            new_share_network_id=new_share_network_id)
+
+        self._validate_migration_successful(
+            dest_pool, share, task_state, complete=False,
+            share_network_id=old_share_network_id,
+            share_type_id=old_share_type_id)
+
+        progress = self.shares_v2_client.migration_get_progress(share['id'])
+
+        self.assertEqual(task_state, progress['task_state'])
+        self.assertEqual(100, progress['total_progress'])
+
+        share = self.migration_complete(share['id'], dest_pool)
+
+        progress = self.shares_v2_client.migration_get_progress(share['id'])
+
+        self.assertEqual(
+            constants.TASK_STATE_MIGRATION_SUCCESS, progress['task_state'])
+        self.assertEqual(100, progress['total_progress'])
+
+        self._validate_migration_successful(
+            dest_pool, share, constants.TASK_STATE_MIGRATION_SUCCESS,
+            complete=True, share_network_id=new_share_network_id,
+            share_type_id=new_share_type_id)
+
+    @test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
+    @base.skip_if_microversion_lt("2.22")
+    @ddt.data(True, False)
     def test_migration_2phase(self, force_host_assisted):
 
         self._check_migration_enabled(force_host_assisted)
 
         share, dest_pool = self._setup_migration()
 
-        old_exports = self.shares_v2_client.list_share_export_locations(
-            share['id'])
-        self.assertNotEmpty(old_exports)
-        old_exports = [x['path'] for x in old_exports
-                       if x['is_admin_only'] is False]
-        self.assertNotEmpty(old_exports)
-
         task_state = (constants.TASK_STATE_DATA_COPYING_COMPLETED
                       if force_host_assisted
                       else constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE)
 
         old_share_network_id = share['share_network_id']
-        new_share_network_id = self._create_secondary_share_network(
-            old_share_network_id)
+
+        if CONF.share.multitenancy_enabled:
+            new_share_network_id = self._create_secondary_share_network(
+                old_share_network_id)
+        else:
+            new_share_network_id = None
+
         old_share_type_id = share['share_type']
         new_share_type_id = self.new_type['share_type']['id']
 
@@ -151,12 +213,18 @@
 
         share = self.migration_complete(share['id'], dest_pool)
 
+        progress = self.shares_v2_client.migration_get_progress(share['id'])
+
+        self.assertEqual(
+            constants.TASK_STATE_MIGRATION_SUCCESS, progress['task_state'])
+        self.assertEqual(100, progress['total_progress'])
+
         self._validate_migration_successful(
             dest_pool, share, constants.TASK_STATE_MIGRATION_SUCCESS,
             complete=True, share_network_id=new_share_network_id,
             share_type_id=new_share_type_id)
 
-    def _setup_migration(self):
+    def _setup_migration(self, opposite=False):
 
         pools = self.shares_v2_client.list_pools(detail=True)['pools']
 
@@ -167,25 +235,42 @@
         share = self.create_share(self.protocol)
         share = self.shares_v2_client.get_share(share['id'])
 
+        old_exports = self.shares_v2_client.list_share_export_locations(
+            share['id'])
+        self.assertNotEmpty(old_exports)
+        old_exports = [x['path'] for x in old_exports
+                       if x['is_admin_only'] is False]
+        self.assertNotEmpty(old_exports)
+
         self.shares_v2_client.create_access_rule(
             share['id'], access_to="50.50.50.50", access_level="rw")
 
         self.shares_v2_client.wait_for_share_status(
-            share['id'], 'active', status_attr='access_rules_status')
+            share['id'], constants.RULE_STATE_ACTIVE,
+            status_attr='access_rules_status')
 
         self.shares_v2_client.create_access_rule(
             share['id'], access_to="51.51.51.51", access_level="ro")
 
         self.shares_v2_client.wait_for_share_status(
-            share['id'], 'active', status_attr='access_rules_status')
+            share['id'], constants.RULE_STATE_ACTIVE,
+            status_attr='access_rules_status')
 
-        default_type = self.shares_v2_client.list_share_types(
-            default=True)['share_type']
+        if opposite:
+            dest_type = self.new_type_opposite['share_type']
+        else:
+            dest_type = self.new_type['share_type']
 
-        dest_pool = utils.choose_matching_backend(share, pools, default_type)
+        dest_pool = utils.choose_matching_backend(share, pools, dest_type)
 
-        self.assertIsNotNone(dest_pool)
-        self.assertIsNotNone(dest_pool.get('name'))
+        if opposite:
+            if not dest_pool:
+                raise self.skipException(
+                    "This test requires two pools enabled with different "
+                    "driver modes.")
+        else:
+            self.assertIsNotNone(dest_pool)
+            self.assertIsNotNone(dest_pool.get('name'))
 
         dest_pool = dest_pool['name']
 
@@ -216,9 +301,33 @@
         # Share migrated
         if complete:
             self.assertEqual(dest_pool, share['host'])
+
+            rules = self.shares_v2_client.list_access_rules(share['id'])
+            expected_rules = [{
+                'state': constants.RULE_STATE_ACTIVE,
+                'access_to': '50.50.50.50',
+                'access_type': 'ip',
+                'access_level': 'rw',
+            }, {
+                'state': constants.RULE_STATE_ACTIVE,
+                'access_to': '51.51.51.51',
+                'access_type': 'ip',
+                'access_level': 'ro',
+            }]
+            filtered_rules = [{'state': rule['state'],
+                               'access_to': rule['access_to'],
+                               'access_level': rule['access_level'],
+                               'access_type': rule['access_type']}
+                              for rule in rules]
+
+            for r in expected_rules:
+                self.assertIn(r, filtered_rules)
+            self.assertEqual(len(expected_rules), len(filtered_rules))
+
             self.shares_v2_client.delete_share(share['id'])
             self.shares_v2_client.wait_for_resource_deletion(
                 share_id=share['id'])
+
         # Share not migrated yet
         else:
             self.assertNotEqual(dest_pool, share['host'])
@@ -235,18 +344,13 @@
                     "Driver-assisted migration tests are disabled.")
 
     def _create_secondary_share_network(self, old_share_network_id):
-        if (utils.is_microversion_ge(
-                CONF.share.max_api_microversion, "2.22") and
-                CONF.share.multitenancy_enabled):
 
-            old_share_network = self.shares_v2_client.get_share_network(
-                old_share_network_id)
+        old_share_network = self.shares_v2_client.get_share_network(
+            old_share_network_id)
 
-            new_share_network = self.create_share_network(
-                cleanup_in_class=True,
-                neutron_net_id=old_share_network['neutron_net_id'],
-                neutron_subnet_id=old_share_network['neutron_subnet_id'])
+        new_share_network = self.create_share_network(
+            cleanup_in_class=True,
+            neutron_net_id=old_share_network['neutron_net_id'],
+            neutron_subnet_id=old_share_network['neutron_subnet_id'])
 
-            return new_share_network['id']
-        else:
-            return None
+        return new_share_network['id']
diff --git a/manila_tempest_tests/tests/api/admin/test_migration_negative.py b/manila_tempest_tests/tests/api/admin/test_migration_negative.py
index 5d7a578..d806ed6 100644
--- a/manila_tempest_tests/tests/api/admin/test_migration_negative.py
+++ b/manila_tempest_tests/tests/api/admin/test_migration_negative.py
@@ -13,8 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
-import six
 
+import ddt
 from tempest import config
 from tempest.lib.common.utils import data_utils
 from tempest.lib import exceptions as lib_exc
@@ -29,6 +29,7 @@
 CONF = config.CONF
 
 
+@ddt.ddt
 class MigrationTest(base.BaseSharesAdminTest):
     """Tests Share Migration.
 
@@ -68,17 +69,11 @@
 
         cls.dest_pool = dest_pool['name']
 
-        extra_specs = {
-            'storage_protocol': CONF.share.capability_storage_protocol,
-            'driver_handles_share_servers': CONF.share.multitenancy_enabled,
-            'snapshot_support': six.text_type(
-                not CONF.share.capability_snapshot_support),
-        }
-        cls.new_type = cls.create_share_type(
+        cls.new_type_invalid = cls.create_share_type(
             name=data_utils.rand_name(
                 'new_invalid_share_type_for_migration'),
             cleanup_in_class=True,
-            extra_specs=extra_specs)
+            extra_specs=utils.get_configured_extra_specs(variation='invalid'))
 
     @test.attr(type=[base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND])
     @base.skip_if_microversion_lt("2.22")
@@ -165,9 +160,17 @@
     @test.attr(type=[base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND])
     @base.skip_if_microversion_lt("2.22")
     def test_migrate_share_change_type_no_valid_host(self):
+        if not CONF.share.multitenancy_enabled:
+            new_share_network_id = self.create_share_network(
+                neutron_net_id='fake_net_id',
+                neutron_subnet_id='fake_subnet_id')['id']
+        else:
+            new_share_network_id = None
+
         self.shares_v2_client.migrate_share(
             self.share['id'], self.dest_pool,
-            new_share_type_id=self.new_type['share_type']['id'])
+            new_share_type_id=self.new_type_invalid['share_type']['id'],
+            new_share_network_id=new_share_network_id)
         self.shares_v2_client.wait_for_migration_status(
             self.share['id'], self.dest_pool,
             constants.TASK_STATE_MIGRATION_ERROR)
@@ -207,5 +210,30 @@
     def test_migrate_share_invalid_share_type(self):
         self.assertRaises(
             lib_exc.BadRequest, self.shares_v2_client.migrate_share,
-            self.share['id'], self.dest_pool, True,
+            self.share['id'], self.dest_pool,
             new_share_type_id='invalid_type_id')
+
+    @test.attr(type=[base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND])
+    @base.skip_if_microversion_lt("2.22")
+    def test_migrate_share_opposite_type_share_network_invalid(self):
+
+        extra_specs = utils.get_configured_extra_specs(
+            variation='opposite_driver_modes')
+
+        new_type_opposite = self.create_share_type(
+            name=data_utils.rand_name('share_type_migration_negative'),
+            extra_specs=extra_specs)
+
+        new_share_network_id = None
+
+        if CONF.share.multitenancy_enabled:
+
+            new_share_network_id = self.create_share_network(
+                neutron_net_id='fake_net_id',
+                neutron_subnet_id='fake_subnet_id')['id']
+
+        self.assertRaises(
+            lib_exc.BadRequest, self.shares_v2_client.migrate_share,
+            self.share['id'], self.dest_pool,
+            new_share_type_id=new_type_opposite['share_type']['id'],
+            new_share_network_id=new_share_network_id)
diff --git a/manila_tempest_tests/tests/api/base.py b/manila_tempest_tests/tests/api/base.py
index a36f391..6b48348 100644
--- a/manila_tempest_tests/tests/api/base.py
+++ b/manila_tempest_tests/tests/api/base.py
@@ -268,7 +268,8 @@
     @classmethod
     @network_synchronized
     def provide_share_network(cls, shares_client, networks_client,
-                              isolated_creds_client=None):
+                              isolated_creds_client=None,
+                              ignore_multitenancy_config=False):
         """Used for finding/creating share network for multitenant driver.
 
         This method creates/gets entity share-network for one tenant. This
@@ -279,6 +280,8 @@
         :param isolated_creds_client: DynamicCredentialProvider instance
             If provided, then its networking will be used if needed.
             If not provided, then common network will be used if needed.
+        :param ignore_multitenancy_config: provide a share network regardless
+            of 'multitenancy_enabled' configuration value.
         :returns: str -- share network id for shares_client tenant
         :returns: None -- if single-tenant driver used
         """
@@ -287,83 +290,87 @@
         search_word = "reusable"
         sn_name = "autogenerated_by_tempest_%s" % search_word
 
-        if not CONF.share.multitenancy_enabled:
+        if (not ignore_multitenancy_config and
+                not CONF.share.multitenancy_enabled):
             # Assumed usage of a single-tenant driver
             share_network_id = None
-        elif sc.share_network_id:
-            # Share-network already exists, use it
-            share_network_id = sc.share_network_id
-        elif not CONF.share.create_networks_when_multitenancy_enabled:
-            share_network_id = None
-
-            # Try get suitable share-network
-            share_networks = sc.list_share_networks_with_detail()
-            for sn in share_networks:
-                if (sn["neutron_net_id"] is None and
-                        sn["neutron_subnet_id"] is None and
-                        sn["name"] and search_word in sn["name"]):
-                    share_network_id = sn["id"]
-                    break
-
-            # Create new share-network if one was not found
-            if share_network_id is None:
-                sn_desc = "This share-network was created by tempest"
-                sn = sc.create_share_network(name=sn_name, description=sn_desc)
-                share_network_id = sn["id"]
         else:
-            net_id = subnet_id = share_network_id = None
-
-            if not isolated_creds_client:
-                # Search for networks, created in previous runs
-                service_net_name = "share-service"
-                networks = networks_client.list_networks()
-                if "networks" in networks.keys():
-                    networks = networks["networks"]
-                for network in networks:
-                    if (service_net_name in network["name"] and
-                            sc.tenant_id == network['tenant_id']):
-                        net_id = network["id"]
-                        if len(network["subnets"]) > 0:
-                            subnet_id = network["subnets"][0]
-                            break
-
-                # Create suitable network
-                if (net_id is None or subnet_id is None):
-                    ic = dynamic_creds.DynamicCredentialProvider(
-                        identity_version=CONF.identity.auth_version,
-                        name=service_net_name,
-                        admin_role=CONF.identity.admin_role,
-                        admin_creds=(
-                            common_creds.get_configured_admin_credentials()))
-                    net_data = ic._create_network_resources(sc.tenant_id)
-                    network, subnet, router = net_data
-                    net_id = network["id"]
-                    subnet_id = subnet["id"]
+            if sc.share_network_id:
+                # Share-network already exists, use it
+                share_network_id = sc.share_network_id
+            elif not CONF.share.create_networks_when_multitenancy_enabled:
+                share_network_id = None
 
                 # Try get suitable share-network
                 share_networks = sc.list_share_networks_with_detail()
                 for sn in share_networks:
-                    if (net_id == sn["neutron_net_id"] and
-                            subnet_id == sn["neutron_subnet_id"] and
+                    if (sn["neutron_net_id"] is None and
+                            sn["neutron_subnet_id"] is None and
                             sn["name"] and search_word in sn["name"]):
                         share_network_id = sn["id"]
                         break
-            else:
-                sn_name = "autogenerated_by_tempest_for_isolated_creds"
-                # Use precreated network and subnet from isolated creds
-                net_id = isolated_creds_client.get_credentials(
-                    isolated_creds_client.type_of_creds).network['id']
-                subnet_id = isolated_creds_client.get_credentials(
-                    isolated_creds_client.type_of_creds).subnet['id']
 
-            # Create suitable share-network
-            if share_network_id is None:
-                sn_desc = "This share-network was created by tempest"
-                sn = sc.create_share_network(name=sn_name,
-                                             description=sn_desc,
-                                             neutron_net_id=net_id,
-                                             neutron_subnet_id=subnet_id)
-                share_network_id = sn["id"]
+                # Create new share-network if one was not found
+                if share_network_id is None:
+                    sn_desc = "This share-network was created by tempest"
+                    sn = sc.create_share_network(name=sn_name,
+                                                 description=sn_desc)
+                    share_network_id = sn["id"]
+            else:
+                net_id = subnet_id = share_network_id = None
+
+                if not isolated_creds_client:
+                    # Search for networks, created in previous runs
+                    service_net_name = "share-service"
+                    networks = networks_client.list_networks()
+                    if "networks" in networks.keys():
+                        networks = networks["networks"]
+                    for network in networks:
+                        if (service_net_name in network["name"] and
+                                sc.tenant_id == network['tenant_id']):
+                            net_id = network["id"]
+                            if len(network["subnets"]) > 0:
+                                subnet_id = network["subnets"][0]
+                                break
+
+                    # Create suitable network
+                    if net_id is None or subnet_id is None:
+                        ic = dynamic_creds.DynamicCredentialProvider(
+                            identity_version=CONF.identity.auth_version,
+                            name=service_net_name,
+                            admin_role=CONF.identity.admin_role,
+                            admin_creds=(
+                                common_creds.
+                                get_configured_admin_credentials()))
+                        net_data = ic._create_network_resources(sc.tenant_id)
+                        network, subnet, router = net_data
+                        net_id = network["id"]
+                        subnet_id = subnet["id"]
+
+                    # Try get suitable share-network
+                    share_networks = sc.list_share_networks_with_detail()
+                    for sn in share_networks:
+                        if (net_id == sn["neutron_net_id"] and
+                                subnet_id == sn["neutron_subnet_id"] and
+                                sn["name"] and search_word in sn["name"]):
+                            share_network_id = sn["id"]
+                            break
+                else:
+                    sn_name = "autogenerated_by_tempest_for_isolated_creds"
+                    # Use precreated network and subnet from isolated creds
+                    net_id = isolated_creds_client.get_credentials(
+                        isolated_creds_client.type_of_creds).network['id']
+                    subnet_id = isolated_creds_client.get_credentials(
+                        isolated_creds_client.type_of_creds).subnet['id']
+
+                # Create suitable share-network
+                if share_network_id is None:
+                    sn_desc = "This share-network was created by tempest"
+                    sn = sc.create_share_network(name=sn_name,
+                                                 description=sn_desc,
+                                                 neutron_net_id=net_id,
+                                                 neutron_subnet_id=subnet_id)
+                    share_network_id = sn["id"]
 
         return share_network_id
 
diff --git a/manila_tempest_tests/utils.py b/manila_tempest_tests/utils.py
index 277130e..5c93443 100644
--- a/manila_tempest_tests/utils.py
+++ b/manila_tempest_tests/utils.py
@@ -115,3 +115,36 @@
         None)
 
     return selected_pool
+
+
+def get_configured_extra_specs(variation=None):
+    """Retrieve essential extra specs according to configuration in tempest.
+
+    :param variation: can assume possible values: None to be as configured in
+        tempest; 'opposite_driver_modes' for as configured in tempest but
+        inverse driver mode; 'invalid' for inverse as configured in tempest,
+        ideal for negative tests.
+    :return: dict containing essential extra specs.
+    """
+
+    extra_specs = {'storage_protocol': CONF.share.capability_storage_protocol}
+
+    if variation == 'invalid':
+        extra_specs['driver_handles_share_servers'] = (
+            not CONF.share.multitenancy_enabled)
+        extra_specs['snapshot_support'] = (
+            not CONF.share.capability_snapshot_support)
+
+    elif variation == 'opposite_driver_modes':
+        extra_specs['driver_handles_share_servers'] = (
+            not CONF.share.multitenancy_enabled)
+        extra_specs['snapshot_support'] = (
+            CONF.share.capability_snapshot_support)
+
+    else:
+        extra_specs['driver_handles_share_servers'] = (
+            CONF.share.multitenancy_enabled)
+        extra_specs['snapshot_support'] = (
+            CONF.share.capability_snapshot_support)
+
+    return extra_specs