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/common/constants.py b/manila_tempest_tests/common/constants.py
index bef35a5..abef181 100644
--- a/manila_tempest_tests/common/constants.py
+++ b/manila_tempest_tests/common/constants.py
@@ -34,3 +34,19 @@
RULE_STATE_ACTIVE = 'active'
RULE_STATE_OUT_OF_SYNC = 'out_of_sync'
RULE_STATE_ERROR = 'error'
+
+TASK_STATE_MIGRATION_STARTING = 'migration_starting'
+TASK_STATE_MIGRATION_IN_PROGRESS = 'migration_in_progress'
+TASK_STATE_MIGRATION_COMPLETING = 'migration_completing'
+TASK_STATE_MIGRATION_SUCCESS = 'migration_success'
+TASK_STATE_MIGRATION_ERROR = 'migration_error'
+TASK_STATE_MIGRATION_CANCELLED = 'migration_cancelled'
+TASK_STATE_MIGRATION_DRIVER_STARTING = 'migration_driver_starting'
+TASK_STATE_MIGRATION_DRIVER_IN_PROGRESS = 'migration_driver_in_progress'
+TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE = 'migration_driver_phase1_done'
+TASK_STATE_DATA_COPYING_STARTING = 'data_copying_starting'
+TASK_STATE_DATA_COPYING_IN_PROGRESS = 'data_copying_in_progress'
+TASK_STATE_DATA_COPYING_COMPLETING = 'data_copying_completing'
+TASK_STATE_DATA_COPYING_COMPLETED = 'data_copying_completed'
+TASK_STATE_DATA_COPYING_CANCELLED = 'data_copying_cancelled'
+TASK_STATE_DATA_COPYING_ERROR = 'data_copying_error'
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 46c0ce7..2887904 100755
--- a/manila_tempest_tests/services/share/v2/json/shares_client.py
+++ b/manila_tempest_tests/services/share/v2/json/shares_client.py
@@ -14,6 +14,7 @@
# under the License.
import json
+import six
import time
from six.moves.urllib import parse as urlparse
@@ -688,8 +689,11 @@
###############
- def list_share_types(self, params=None, version=LATEST_MICROVERSION):
+ def list_share_types(self, params=None, default=False,
+ version=LATEST_MICROVERSION):
uri = 'types'
+ if default:
+ uri += '/default'
if params is not None:
uri += '?%s' % urlparse.urlencode(params)
resp, body = self.get(uri, version=version)
@@ -1076,22 +1080,25 @@
headers=EXPERIMENTAL, extra_headers=True,
version=version)
- def wait_for_migration_status(self, share_id, dest_host, status,
+ def wait_for_migration_status(self, share_id, dest_host, status_to_wait,
version=LATEST_MICROVERSION):
"""Waits for a share to migrate to a certain host."""
+ statuses = ((status_to_wait,)
+ if not isinstance(status_to_wait, (tuple, list, set))
+ else status_to_wait)
share = self.get_share(share_id, version=version)
migration_timeout = CONF.share.migration_timeout
start = int(time.time())
- while share['task_state'] != status:
+ while share['task_state'] not in statuses:
time.sleep(self.build_interval)
share = self.get_share(share_id, version=version)
- if share['task_state'] == status:
- return share
+ if share['task_state'] in statuses:
+ break
elif share['task_state'] == 'migration_error':
raise share_exceptions.ShareMigrationException(
share_id=share['id'], src=share['host'], dest=dest_host)
elif int(time.time()) - start >= migration_timeout:
- message = ('Share %(share_id)s failed to reach status '
+ message = ('Share %(share_id)s failed to reach a status in'
'%(status)s when migrating from host %(src)s to '
'host %(dest)s within the required time '
'%(timeout)s.' % {
@@ -1099,9 +1106,10 @@
'dest': dest_host,
'share_id': share['id'],
'timeout': self.build_timeout,
- 'status': status,
+ 'status': six.text_type(statuses),
})
raise exceptions.TimeoutException(message)
+ return share
################
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}])
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 dbe5599..b52fe07 100644
--- a/manila_tempest_tests/tests/scenario/test_share_basic_ops.py
+++ b/manila_tempest_tests/tests/scenario/test_share_basic_ops.py
@@ -20,7 +20,9 @@
from tempest.lib.common.utils import test_utils
from tempest.lib import exceptions
from tempest import test
+import testtools
+from manila_tempest_tests.common import constants
from manila_tempest_tests.tests.api import base
from manila_tempest_tests.tests.scenario import manager_share as manager
from manila_tempest_tests import utils
@@ -244,29 +246,30 @@
@test.services('compute', 'network')
@test.attr(type=[base.TAG_POSITIVE, base.TAG_BACKEND])
+ @testtools.skipUnless(CONF.share.run_migration_tests,
+ "Share migration tests are disabled.")
def test_migration_files(self):
if self.protocol == "CIFS":
raise self.skipException("Test for CIFS protocol not supported "
- "at this moment. Skipping.")
+ "at this moment.")
- if not CONF.share.run_migration_tests:
- raise self.skipException("Migration tests disabled. Skipping.")
-
- pools = self.shares_admin_client.list_pools()['pools']
+ pools = self.shares_admin_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.")
instance = self.boot_instance(wait_until="BUILD")
self.create_share()
instance = self.wait_for_active_instance(instance["id"])
- share = self.shares_client.get_share(self.share['id'])
+ self.share = self.shares_client.get_share(self.share['id'])
- 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(
+ self.share, pools, default_type)
self.assertIsNotNone(dest_pool)
self.assertIsNotNone(dest_pool.get('name'))
@@ -307,7 +310,7 @@
self.umount_share(ssh_client)
- share = self.migrate_share(share['id'], dest_pool)
+ self.share = self.migrate_share(self.share['id'], dest_pool)
if utils.is_microversion_lt(CONF.share.max_api_microversion, "2.9"):
new_locations = self.share['export_locations']
else:
@@ -315,11 +318,12 @@
self.share['id'])
new_locations = [x['path'] for x in new_exports]
- self.assertEqual(dest_pool, share['host'])
+ self.assertEqual(dest_pool, self.share['host'])
locations.sort()
new_locations.sort()
self.assertNotEqual(locations, new_locations)
- self.assertEqual('migration_success', share['task_state'])
+ self.assertEqual(constants.TASK_STATE_MIGRATION_SUCCESS,
+ self.share['task_state'])
self.mount_share(new_locations[0], ssh_client)
diff --git a/manila_tempest_tests/utils.py b/manila_tempest_tests/utils.py
index dea51ab..277130e 100644
--- a/manila_tempest_tests/utils.py
+++ b/manila_tempest_tests/utils.py
@@ -100,3 +100,18 @@
TEST_NET_3 = '203.0.113.'
final_octet = six.text_type(random.randint(0, 255))
return TEST_NET_3 + final_octet
+
+
+def choose_matching_backend(share, pools, share_type):
+ extra_specs = {}
+ # fix extra specs with string values instead of boolean
+ for k, v in share_type['extra_specs'].items():
+ extra_specs[k] = (True if six.text_type(v).lower() == 'true'
+ else False if six.text_type(v).lower() == 'false'
+ else v)
+ selected_pool = next(
+ (x for x in pools if (x['name'] != share['host'] and all(
+ y in x['capabilities'].items() for y in extra_specs.items()))),
+ None)
+
+ return selected_pool