Fix Share Migration improper behavior for drivers
Tempest tests were not appropriate for driver-assisted migration,
so this was fixed.
Also, improved docstrings and fixed workflow for drivers when
implementing 2-phase migration to be accurate with tempest and
handle AZs, which were previously locked to the source share's
AZ.
Driver-assisted migration now creates an additional
share instance to better handle and support driver methods.
Updated allow_access and deny_access APIs to allow users to mount
migrating shares before issuing 'migration-complete'.
APIImpact
Closes-bug: #1594922
Change-Id: If4bfaf7e9d963b83c13a6fea241c2eda14f7f409
diff --git a/manila_tempest_tests/tests/api/admin/test_migration.py b/manila_tempest_tests/tests/api/admin/test_migration.py
index a5ceaa4..c5fef44 100644
--- a/manila_tempest_tests/tests/api/admin/test_migration.py
+++ b/manila_tempest_tests/tests/api/admin/test_migration.py
@@ -16,6 +16,7 @@
from tempest import config
from tempest import test
+from manila_tempest_tests.common import constants
from manila_tempest_tests.tests.api import base
from manila_tempest_tests import utils
@@ -25,7 +26,7 @@
class MigrationNFSTest(base.BaseSharesAdminTest):
"""Tests Share Migration.
- Tests migration in multi-backend environment.
+ Tests share migration in multi-backend environment.
"""
protocol = "nfs"
@@ -37,7 +38,36 @@
message = "%s tests are disabled" % cls.protocol
raise cls.skipException(message)
if not CONF.share.run_migration_tests:
- raise cls.skipException("Migration tests disabled. Skipping.")
+ raise cls.skipException("Share migration tests are disabled.")
+
+ @test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
+ @base.skip_if_microversion_lt("2.15")
+ def test_migration_cancel(self):
+
+ share, dest_pool = self._setup_migration()
+
+ old_exports = self.shares_v2_client.list_share_export_locations(
+ share['id'], version='2.15')
+ 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_states = (constants.TASK_STATE_DATA_COPYING_COMPLETED,
+ constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE)
+
+ share = self.migrate_share(
+ share['id'], dest_pool, version='2.15', notify=False,
+ wait_for_status=task_states)
+
+ self._validate_migration_successful(
+ dest_pool, share, task_states, '2.15', notify=False)
+
+ share = self.migration_cancel(share['id'], dest_pool)
+
+ self._validate_migration_successful(
+ dest_pool, share, constants.TASK_STATE_MIGRATION_CANCELLED,
+ '2.15', notify=False)
@test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
@base.skip_if_microversion_lt("2.5")
@@ -45,12 +75,11 @@
share, dest_pool = self._setup_migration()
- old_exports = share['export_locations']
-
share = self.migrate_share(share['id'], dest_pool, version='2.5')
- self._validate_migration_successful(dest_pool, share, old_exports,
- version='2.5')
+ self._validate_migration_successful(
+ dest_pool, share, constants.TASK_STATE_MIGRATION_SUCCESS,
+ version='2.5')
@test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
@base.skip_if_microversion_lt("2.15")
@@ -65,26 +94,29 @@
if x['is_admin_only'] is False]
self.assertNotEmpty(old_exports)
+ task_states = (constants.TASK_STATE_DATA_COPYING_COMPLETED,
+ constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE)
+
share = self.migrate_share(
share['id'], dest_pool, version='2.15', notify=False,
- wait_for_status='data_copying_completed')
+ wait_for_status=task_states)
- self._validate_migration_successful(dest_pool, share,
- old_exports, '2.15', notify=False)
+ self._validate_migration_successful(
+ dest_pool, share, task_states, '2.15', notify=False)
share = self.migration_complete(share['id'], dest_pool, version='2.15')
- self._validate_migration_successful(dest_pool, share, old_exports,
- version='2.15')
+ self._validate_migration_successful(
+ dest_pool, share, constants.TASK_STATE_MIGRATION_SUCCESS,
+ version='2.15')
def _setup_migration(self):
- pools = self.shares_client.list_pools()['pools']
+ pools = self.shares_v2_client.list_pools(detail=True)['pools']
if len(pools) < 2:
- raise self.skipException("At least two different pool entries "
- "are needed to run migration tests. "
- "Skipping.")
+ raise self.skipException("At least two different pool entries are "
+ "needed to run share migration tests.")
share = self.create_share(self.protocol)
share = self.shares_client.get_share(share['id'])
@@ -101,8 +133,10 @@
self.shares_v2_client.wait_for_share_status(
share['id'], 'active', status_attr='access_rules_status')
- dest_pool = next((x for x in pools if x['name'] != share['host']),
- None)
+ default_type = self.shares_v2_client.list_share_types(
+ default=True)['share_type']
+
+ dest_pool = utils.choose_matching_backend(share, pools, default_type)
self.assertIsNotNone(dest_pool)
self.assertIsNotNone(dest_pool.get('name'))
@@ -112,7 +146,12 @@
return share, dest_pool
def _validate_migration_successful(self, dest_pool, share,
- old_exports, version, notify=True):
+ status_to_wait, version, notify=True):
+
+ statuses = ((status_to_wait,)
+ if not isinstance(status_to_wait, (tuple, list, set))
+ else status_to_wait)
+
if utils.is_microversion_lt(version, '2.9'):
new_exports = share['export_locations']
self.assertNotEmpty(new_exports)
@@ -127,12 +166,7 @@
# Share migrated
if notify:
self.assertEqual(dest_pool, share['host'])
- for export in old_exports:
- self.assertFalse(export in new_exports)
- self.assertEqual('migration_success', share['task_state'])
# Share not migrated yet
else:
self.assertNotEqual(dest_pool, share['host'])
- for export in old_exports:
- self.assertTrue(export in new_exports)
- self.assertEqual('data_copying_completed', share['task_state'])
+ self.assertIn(share['task_state'], statuses)
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 908abec..3dfc31a 100644
--- a/manila_tempest_tests/tests/api/admin/test_migration_negative.py
+++ b/manila_tempest_tests/tests/api/admin/test_migration_negative.py
@@ -18,7 +18,10 @@
from tempest import test
import testtools
+from manila_tempest_tests.common import constants
+from manila_tempest_tests import share_exceptions
from manila_tempest_tests.tests.api import base
+from manila_tempest_tests import utils
CONF = config.CONF
@@ -26,7 +29,7 @@
class MigrationNFSTest(base.BaseSharesAdminTest):
"""Tests Share Migration.
- Tests migration in multi-backend environment.
+ Tests share migration in multi-backend environment.
"""
protocol = "nfs"
@@ -35,18 +38,28 @@
def resource_setup(cls):
super(MigrationNFSTest, cls).resource_setup()
if not CONF.share.run_migration_tests:
- raise cls.skipException("Migration tests disabled. Skipping.")
+ raise cls.skipException("Share migration tests are disabled.")
- cls.share = cls.create_share(cls.protocol)
- cls.share = cls.shares_client.get_share(cls.share['id'])
- pools = cls.shares_client.list_pools()['pools']
+ pools = cls.shares_client.list_pools(detail=True)['pools']
if len(pools) < 2:
raise cls.skipException("At least two different pool entries "
- "are needed to run migration tests. "
- "Skipping.")
- cls.dest_pool = next((x for x in pools
- if x['name'] != cls.share['host']), None)
+ "are needed to run share migration tests.")
+
+ 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(
+ default=True)['share_type']
+
+ dest_pool = utils.choose_matching_backend(
+ cls.share, pools, default_type)
+
+ if not dest_pool or dest_pool.get('name') is None:
+ raise share_exceptions.ShareMigrationException(
+ "No valid pool entries to run share migration tests.")
+
+ cls.dest_pool = dest_pool['name']
@test.attr(type=[base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND])
@base.skip_if_microversion_lt("2.15")
@@ -91,10 +104,14 @@
@test.attr(type=[base.TAG_NEGATIVE, base.TAG_API_WITH_BACKEND])
@base.skip_if_microversion_lt("2.5")
def test_migrate_share_not_available_v2_5(self):
- self.shares_client.reset_state(self.share['id'], 'error')
- self.shares_client.wait_for_share_status(self.share['id'], 'error')
+ self.shares_client.reset_state(
+ self.share['id'], constants.STATUS_ERROR)
+ self.shares_client.wait_for_share_status(self.share['id'],
+ constants.STATUS_ERROR)
self.assertRaises(
lib_exc.BadRequest, self.shares_v2_client.migrate_share,
self.share['id'], self.dest_pool, True, version='2.5')
- self.shares_client.reset_state(self.share['id'], 'available')
- self.shares_client.wait_for_share_status(self.share['id'], 'available')
+ self.shares_client.reset_state(self.share['id'],
+ constants.STATUS_AVAILABLE)
+ self.shares_client.wait_for_share_status(self.share['id'],
+ constants.STATUS_AVAILABLE)
diff --git a/manila_tempest_tests/tests/api/base.py b/manila_tempest_tests/tests/api/base.py
index 73fcf09..82135bf 100644
--- a/manila_tempest_tests/tests/api/base.py
+++ b/manila_tempest_tests/tests/api/base.py
@@ -420,6 +420,14 @@
return share
@classmethod
+ def migration_cancel(cls, share_id, dest_host, client=None, **kwargs):
+ client = client or cls.shares_v2_client
+ client.migration_cancel(share_id, **kwargs)
+ share = client.wait_for_migration_status(
+ share_id, dest_host, 'migration_cancelled', **kwargs)
+ return share
+
+ @classmethod
def create_share(cls, *args, **kwargs):
"""Create one share and wait for available state. Retry if allowed."""
result = cls.create_shares([{"args": args, "kwargs": kwargs}])