Share Migration Ocata Improvements

Implemented several improvements to share migration
according to spec [1].

Summary of changes:
- Snapshot restriction in API has been changed to return error only
when parameter force-host-assisted-migration is True
- Added preserve_snapshot to API and migration_check_compatibility
driver interface
- Changed all driver-assisted API parameters to be mandatory
- Added validation to prevent 'force_host_assisted_migration' to be
used alongside driver-assisted parameters
- Changed "same host" validation to reject only if the combination
of "host", "new_share_network" and "new_share_type" is the same as
the source
- Updated migration driver interfaces to support snapshots
- Updated zfsonlinux driver, defaulting preserve_snapshots to False
- Updated dummy driver to support preserve_snapshots

Spec update with latest changes since [1] merged
can be found in [2].

APIImpact
DocImpact

[1] I5717e902373d79ed0d55372afdedfaa98134c24e
[2] If02180ec3b5ae05c9ff18c9f5a054c33f13edcdf

Change-Id: I764b389816319ed0ac5178cadbf809cb632035b4
Partially-implements: blueprint ocata-migration-improvements
diff --git a/manila_tempest_tests/config.py b/manila_tempest_tests/config.py
index 3756e63..dcae3c0 100644
--- a/manila_tempest_tests/config.py
+++ b/manila_tempest_tests/config.py
@@ -30,7 +30,7 @@
                help="The minimum api microversion is configured to be the "
                     "value of the minimum microversion supported by Manila."),
     cfg.StrOpt("max_api_microversion",
-               default="2.28",
+               default="2.29",
                help="The maximum api microversion is configured to be the "
                     "value of the latest microversion supported by Manila."),
     cfg.StrOpt("region",
diff --git a/manila_tempest_tests/services/share/v2/json/shares_client.py b/manila_tempest_tests/services/share/v2/json/shares_client.py
index 0d88963..aa176cb 100644
--- a/manila_tempest_tests/services/share/v2/json/shares_client.py
+++ b/manila_tempest_tests/services/share/v2/json/shares_client.py
@@ -1092,8 +1092,9 @@
     def migrate_share(self, share_id, host,
                       force_host_assisted_migration=False,
                       new_share_network_id=None, writable=False,
-                      preserve_metadata=False, nondisruptive=False,
-                      new_share_type_id=None, version=LATEST_MICROVERSION):
+                      preserve_metadata=False, preserve_snapshots=False,
+                      nondisruptive=False, new_share_type_id=None,
+                      version=LATEST_MICROVERSION):
 
         body = {
             'migration_start': {
@@ -1103,6 +1104,7 @@
                 'new_share_type_id': new_share_type_id,
                 'writable': writable,
                 'preserve_metadata': preserve_metadata,
+                'preserve_snapshots': preserve_snapshots,
                 'nondisruptive': nondisruptive,
             }
         }
diff --git a/manila_tempest_tests/tests/api/admin/test_migration.py b/manila_tempest_tests/tests/api/admin/test_migration.py
index dc58a16..5a30bae 100644
--- a/manila_tempest_tests/tests/api/admin/test_migration.py
+++ b/manila_tempest_tests/tests/api/admin/test_migration.py
@@ -36,7 +36,8 @@
     1) Driver-assisted migration: force_host_assisted_migration, nondisruptive,
     writable and preserve-metadata are False.
     2) Host-assisted migration: force_host_assisted_migration is True,
-    nondisruptive, writable and preserve-metadata are False.
+    nondisruptive, writable, preserve-metadata and preserve-snapshots 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.
@@ -75,7 +76,7 @@
                 variation='opposite_driver_modes'))
 
     @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     @ddt.data(True, False)
     def test_migration_cancel(self, force_host_assisted):
 
@@ -112,7 +113,7 @@
             complete=False)
 
     @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     @ddt.data(True, False)
     def test_migration_opposite_driver_modes(self, force_host_assisted):
 
@@ -172,7 +173,7 @@
             share_type_id=new_share_type_id)
 
     @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     @ddt.data(True, False)
     def test_migration_2phase(self, force_host_assisted):
 
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 59d97f4..9d25fff 100644
--- a/manila_tempest_tests/tests/api/admin/test_migration_negative.py
+++ b/manila_tempest_tests/tests/api/admin/test_migration_negative.py
@@ -30,7 +30,7 @@
 
 
 @ddt.ddt
-class MigrationTest(base.BaseSharesAdminTest):
+class MigrationNegativeTest(base.BaseSharesAdminTest):
     """Tests Share Migration.
 
     Tests share migration in multi-backend environment.
@@ -40,7 +40,7 @@
 
     @classmethod
     def resource_setup(cls):
-        super(MigrationTest, cls).resource_setup()
+        super(MigrationNegativeTest, cls).resource_setup()
         if cls.protocol not in CONF.share.enable_protocols:
             message = "%s tests are disabled." % cls.protocol
             raise cls.skipException(message)
@@ -57,11 +57,11 @@
         cls.share = cls.create_share(cls.protocol)
         cls.share = cls.shares_client.get_share(cls.share['id'])
 
-        default_type = cls.shares_v2_client.list_share_types(
+        cls.default_type = cls.shares_v2_client.list_share_types(
             default=True)['share_type']
 
         dest_pool = utils.choose_matching_backend(
-            cls.share, pools, default_type)
+            cls.share, pools, cls.default_type)
 
         if not dest_pool or dest_pool.get('name') is None:
             raise share_exceptions.ShareMigrationException(
@@ -121,44 +121,65 @@
             'invalid_share_id')
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     @testtools.skipUnless(CONF.share.run_snapshot_tests,
                           "Snapshot tests are disabled.")
     def test_migrate_share_with_snapshot(self):
         snap = self.create_snapshot_wait_for_active(self.share['id'])
         self.assertRaises(
-            lib_exc.BadRequest, self.shares_v2_client.migrate_share,
-            self.share['id'], self.dest_pool)
+            lib_exc.Conflict, self.shares_v2_client.migrate_share,
+            self.share['id'], self.dest_pool,
+            force_host_assisted_migration=True)
         self.shares_client.delete_snapshot(snap['id'])
         self.shares_client.wait_for_resource_deletion(snapshot_id=snap["id"])
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
-    def test_migrate_share_same_host(self):
-        self.assertRaises(
-            lib_exc.BadRequest, self.shares_v2_client.migrate_share,
-            self.share['id'], self.share['host'])
+    @base.skip_if_microversion_lt("2.29")
+    @ddt.data(True, False)
+    def test_migrate_share_same_host(self, specified):
+        new_share_type_id = None
+        new_share_network_id = None
+        if specified:
+            new_share_type_id = self.default_type['id']
+            new_share_network_id = self.share['share_network_id']
+        self.migrate_share(
+            self.share['id'], self.share['host'],
+            wait_for_status=constants.TASK_STATE_MIGRATION_SUCCESS,
+            new_share_type_id=new_share_type_id,
+            new_share_network_id=new_share_network_id)
+        # NOTE(ganso): No need to assert, it is already waiting for correct
+        # status (migration_success).
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     def test_migrate_share_host_invalid(self):
         self.assertRaises(
             lib_exc.NotFound, self.shares_v2_client.migrate_share,
             self.share['id'], 'invalid_host')
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
-    def test_migrate_share_host_assisted_not_allowed(self):
-        self.shares_v2_client.migrate_share(
+    @base.skip_if_microversion_lt("2.29")
+    @ddt.data({'writable': False, 'preserve_metadata': False,
+               'preserve_snapshots': False, 'nondisruptive': True},
+              {'writable': False, 'preserve_metadata': False,
+               'preserve_snapshots': True, 'nondisruptive': False},
+              {'writable': False, 'preserve_metadata': True,
+               'preserve_snapshots': False, 'nondisruptive': False},
+              {'writable': True, 'preserve_metadata': False,
+               'preserve_snapshots': False, 'nondisruptive': False})
+    @ddt.unpack
+    def test_migrate_share_host_assisted_not_allowed_API(
+            self, writable, preserve_metadata, preserve_snapshots,
+            nondisruptive):
+        self.assertRaises(
+            lib_exc.BadRequest, self.shares_v2_client.migrate_share,
             self.share['id'], self.dest_pool,
-            force_host_assisted_migration=True, writable=True,
-            preserve_metadata=True)
-        self.shares_v2_client.wait_for_migration_status(
-            self.share['id'], self.dest_pool,
-            constants.TASK_STATE_MIGRATION_ERROR)
+            force_host_assisted_migration=True, writable=writable,
+            preserve_metadata=preserve_metadata, nondisruptive=nondisruptive,
+            preserve_snapshots=preserve_snapshots)
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     def test_migrate_share_change_type_no_valid_host(self):
         if not CONF.share.multitenancy_enabled:
             new_share_network_id = self.create_share_network(
@@ -176,14 +197,14 @@
             constants.TASK_STATE_MIGRATION_ERROR)
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     def test_migrate_share_not_found(self):
         self.assertRaises(
             lib_exc.NotFound, self.shares_v2_client.migrate_share,
             'invalid_share_id', self.dest_pool)
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     def test_migrate_share_not_available(self):
         self.shares_client.reset_state(self.share['id'],
                                        constants.STATUS_ERROR)
@@ -198,7 +219,7 @@
                                                  constants.STATUS_AVAILABLE)
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     def test_migrate_share_invalid_share_network(self):
         self.assertRaises(
             lib_exc.BadRequest, self.shares_v2_client.migrate_share,
@@ -206,7 +227,7 @@
             new_share_network_id='invalid_net_id')
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     def test_migrate_share_invalid_share_type(self):
         self.assertRaises(
             lib_exc.BadRequest, self.shares_v2_client.migrate_share,
@@ -214,7 +235,7 @@
             new_share_type_id='invalid_type_id')
 
     @tc.attr(base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND)
-    @base.skip_if_microversion_lt("2.22")
+    @base.skip_if_microversion_lt("2.29")
     def test_migrate_share_opposite_type_share_network_invalid(self):
 
         extra_specs = utils.get_configured_extra_specs(
diff --git a/manila_tempest_tests/tests/api/base.py b/manila_tempest_tests/tests/api/base.py
index 398108d..48cd3ed 100644
--- a/manila_tempest_tests/tests/api/base.py
+++ b/manila_tempest_tests/tests/api/base.py
@@ -424,14 +424,17 @@
     @classmethod
     def migrate_share(
             cls, share_id, dest_host, wait_for_status, client=None,
-            force_host_assisted_migration=False, new_share_network_id=None,
+            force_host_assisted_migration=False, writable=False,
+            nondisruptive=False, preserve_metadata=False,
+            preserve_snapshots=False, new_share_network_id=None,
             new_share_type_id=None, **kwargs):
         client = client or cls.shares_v2_client
         client.migrate_share(
             share_id, dest_host,
             force_host_assisted_migration=force_host_assisted_migration,
+            writable=writable, preserve_metadata=preserve_metadata,
+            nondisruptive=nondisruptive, preserve_snapshots=preserve_snapshots,
             new_share_network_id=new_share_network_id,
-            writable=False, preserve_metadata=False, nondisruptive=False,
             new_share_type_id=new_share_type_id, **kwargs)
         share = client.wait_for_migration_status(
             share_id, dest_host, wait_for_status, **kwargs)
diff --git a/manila_tempest_tests/tests/scenario/manager_share.py b/manila_tempest_tests/tests/scenario/manager_share.py
index e019733..279a924 100644
--- a/manila_tempest_tests/tests/scenario/manager_share.py
+++ b/manila_tempest_tests/tests/scenario/manager_share.py
@@ -212,10 +212,13 @@
 
         return linux_client
 
-    def _migrate_share(self, share_id, dest_host, status, client=None):
+    def _migrate_share(self, share_id, dest_host, status, force_host_assisted,
+                       client=None):
         client = client or self.shares_admin_v2_client
-        client.migrate_share(share_id, dest_host, writable=False,
-                             preserve_metadata=False, nondisruptive=False)
+        client.migrate_share(
+            share_id, dest_host, writable=False, preserve_metadata=False,
+            nondisruptive=False, preserve_snapshots=False,
+            force_host_assisted_migration=force_host_assisted)
         share = client.wait_for_migration_status(share_id, dest_host, status)
         return share
 
diff --git a/manila_tempest_tests/tests/scenario/test_share_basic_ops.py b/manila_tempest_tests/tests/scenario/test_share_basic_ops.py
index 71ed0e3..ef9a3d8 100644
--- a/manila_tempest_tests/tests/scenario/test_share_basic_ops.py
+++ b/manila_tempest_tests/tests/scenario/test_share_basic_ops.py
@@ -13,6 +13,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import ddt
+
 from oslo_log import log as logging
 from tempest.common import waiters
 from tempest import config
@@ -32,6 +34,7 @@
 LOG = logging.getLogger(__name__)
 
 
+@ddt.ddt
 class ShareBasicOpsBase(manager.ShareScenarioTest):
 
     """This smoke test case follows this basic set of operations:
@@ -135,12 +138,15 @@
         data = ssh_client.exec_command("sudo cat /mnt/t1")
         return data.rstrip()
 
-    def migrate_share(self, share_id, dest_host, status):
-        share = self._migrate_share(share_id, dest_host, status,
-                                    self.shares_admin_v2_client)
-        share = self._migration_complete(share['id'], dest_host)
+    def migrate_share(self, share_id, dest_host, status, force_host_assisted):
+        share = self._migrate_share(
+            share_id, dest_host, status, force_host_assisted,
+            self.shares_admin_v2_client)
         return share
 
+    def migration_complete(self, share_id, dest_host):
+        return self._migration_complete(share_id, dest_host)
+
     def create_share_network(self):
         self.net = self._create_network(namestart="manila-share")
         self.subnet = self._create_subnet(network=self.net,
@@ -266,10 +272,21 @@
         self.assertEqual(test_data, data)
 
     @tc.attr(base.TAG_POSITIVE, base.TAG_BACKEND)
+    @base.skip_if_microversion_lt("2.29")
     @testtools.skipUnless(CONF.share.run_host_assisted_migration_tests or
                           CONF.share.run_driver_assisted_migration_tests,
                           "Share migration tests are disabled.")
-    def test_migration_files(self):
+    @ddt.data(True, False)
+    def test_migration_files(self, force_host_assisted):
+
+        if (force_host_assisted and
+                not CONF.share.run_host_assisted_migration_tests):
+                raise self.skipException("Host-assisted migration tests are "
+                                         "disabled.")
+        elif (not force_host_assisted and
+              not CONF.share.run_driver_assisted_migration_tests):
+            raise self.skipException("Driver-assisted migration tests are "
+                                     "disabled.")
 
         if self.protocol != "nfs":
             raise self.skipException("Only NFS protocol supported "
@@ -300,14 +317,11 @@
         ssh_client = self.init_ssh(instance)
         self.provide_access_to_auxiliary_instance(instance)
 
-        if utils.is_microversion_lt(CONF.share.max_api_microversion, "2.9"):
-            exports = self.share['export_locations']
-        else:
-            exports = self.shares_v2_client.list_share_export_locations(
-                self.share['id'])
-            self.assertNotEmpty(exports)
-            exports = [x['path'] for x in exports]
-            self.assertNotEmpty(exports)
+        exports = self.shares_v2_client.list_share_export_locations(
+            self.share['id'])
+        self.assertNotEmpty(exports)
+        exports = [x['path'] for x in exports]
+        self.assertNotEmpty(exports)
 
         self.mount_share(exports[0], ssh_client)
 
@@ -330,23 +344,31 @@
         ssh_client.exec_command("sudo chmod -R 555 /mnt/f3")
         ssh_client.exec_command("sudo chmod -R 777 /mnt/f4")
 
-        self.umount_share(ssh_client)
-
-        task_state = (constants.TASK_STATE_DATA_COPYING_COMPLETED,
-                      constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE)
+        task_state = (constants.TASK_STATE_DATA_COPYING_COMPLETED
+                      if force_host_assisted
+                      else constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE)
 
         self.share = self.migrate_share(
-            self.share['id'], dest_pool, task_state)
+            self.share['id'], dest_pool, task_state, force_host_assisted)
 
-        if utils.is_microversion_lt(CONF.share.max_api_microversion, "2.9"):
-            new_exports = self.share['export_locations']
-            self.assertNotEmpty(new_exports)
-        else:
-            new_exports = self.shares_v2_client.list_share_export_locations(
-                self.share['id'])
-            self.assertNotEmpty(new_exports)
-            new_exports = [x['path'] for x in new_exports]
-            self.assertNotEmpty(new_exports)
+        read_only = False
+        if force_host_assisted:
+            try:
+                ssh_client.exec_command(
+                    "dd if=/dev/zero of=/mnt/f1/1m6.bin bs=1M count=1")
+            except Exception:
+                read_only = True
+            self.assertTrue(read_only)
+
+        self.umount_share(ssh_client)
+
+        self.share = self.migration_complete(self.share['id'], dest_pool)
+
+        new_exports = self.shares_v2_client.list_share_export_locations(
+            self.share['id'])
+        self.assertNotEmpty(new_exports)
+        new_exports = [x['path'] for x in new_exports]
+        self.assertNotEmpty(new_exports)
 
         self.assertEqual(dest_pool, self.share['host'])
         self.assertEqual(constants.TASK_STATE_MIGRATION_SUCCESS,