openstack
/
manila
Code Issues Proposed changes

Fix Share Migration access rule mapping 

Share Migration must transfer access rules from source instance
to destination instance, but creating new ones instead of creating
new mapping between existing and destination instance. This
patch addresses this by copying the mapping.

Closes-bug: #1555677
Change-Id: Ie5a50e4117c14f822c22cace91bb8b0b95d0799d
This commit is contained in:
Rodrigo Barbieri 2016-03-11 08:24:27 -03:00
parent e9fc395802
commit d2bebca142
4 changed files with 25 additions and 66 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
manila/share/manager.py
View File

@ -824,7 +824,7 @@ class ShareManager(manager.SchedulerDependentManager):
raise exception.ShareMigrationFailed(reason=msg)
try:
helper.apply_new_access_rules(share_instance, new_share_instance)
helper.apply_new_access_rules(new_share_instance)
except Exception:
msg = _("Failed to apply new access rules during migration "
"of share %s.") % share_ref['id']

34
manila/share/migration.py
View File

@ -120,27 +120,6 @@ class ShareMigrationHelper(object):
return new_share_instance
def _add_rules_and_wait(self, share_instance, access_rules):
for access in access_rules:
values = {
'share_id': self.share['id'],
'access_type': access['access_type'],
'access_level': access['access_level'],
'access_to': access['access_to']
}
# NOTE(ganso): Instance Access Mapping is created only on
# db.share_access_create.
self.db.share_access_create(self.context, values)
self.api.allow_access_to_instance(self.context, share_instance,
access_rules)
utils.wait_for_access_update(
self.context, self.db, share_instance,
self.migration_wait_access_rules_timeout)
# NOTE(ganso): Cleanup methods do not throw exceptions, since the
# exceptions that should be thrown are the ones that call the cleanup
@ -203,13 +182,20 @@ class ShareMigrationHelper(object):
add_rules=[], delete_rules=[],
share_server=share_server)
def apply_new_access_rules(self, share_instance, new_share_instance):
def apply_new_access_rules(self, new_share_instance):
self.db.share_instance_access_copy(self.context, self.share['id'],
new_share_instance['id'])
rules = self.db.share_access_get_all_for_instance(
self.context, share_instance['id'])
self.context, new_share_instance['id'])
if len(rules) > 0:
LOG.debug("Restoring all of share %s access rules according to "
"DB.", self.share['id'])
self._add_rules_and_wait(new_share_instance, rules)
self.api.allow_access_to_instance(self.context, new_share_instance,
rules)
utils.wait_for_access_update(
self.context, self.db, new_share_instance,
self.migration_wait_access_rules_timeout)

4
manila/tests/share/test_manager.py
View File

@ -3605,7 +3605,7 @@ class ShareManagerTestCase(test.TestCase):
{'task_state': constants.TASK_STATE_MIGRATION_CANCELLED})
if status == constants.TASK_STATE_DATA_COPYING_COMPLETED:
migration_api.ShareMigrationHelper.apply_new_access_rules.\
assert_called_once_with(instance, new_instance)
assert_called_once_with(new_instance)
self.assertTrue(manager.LOG.exception.called)
def test__migration_complete(self):
@ -3658,7 +3658,7 @@ class ShareManagerTestCase(test.TestCase):
{'task_state': constants.TASK_STATE_MIGRATION_SUCCESS}),
])
migration_api.ShareMigrationHelper.apply_new_access_rules.\
assert_called_once_with(instance, new_instance)
assert_called_once_with(new_instance)
migration_api.ShareMigrationHelper.delete_instance_and_wait.\
assert_called_once_with(instance)

51
manila/tests/share/test_migration.py
View File

@ -307,8 +307,6 @@ class ShareMigrationHelperTestCase(test.TestCase):
def test_apply_new_access_rules(self):
share_instance = db_utils.create_share_instance(
share_id=self.share['id'], status=constants.STATUS_AVAILABLE)
new_share_instance = db_utils.create_share_instance(
share_id=self.share['id'], status=constants.STATUS_AVAILABLE,
access_rules_status='active')
@ -318,18 +316,25 @@ class ShareMigrationHelperTestCase(test.TestCase):
access_level='rw')
# mocks
self.mock_object(db, 'share_instance_access_copy')
self.mock_object(db, 'share_access_get_all_for_instance',
mock.Mock(return_value=[access]))
self.mock_object(self.helper, '_add_rules_and_wait')
self.mock_object(share_api.API, 'allow_access_to_instance')
self.mock_object(utils, 'wait_for_access_update')
# run
self.helper.apply_new_access_rules(share_instance, new_share_instance)
self.helper.apply_new_access_rules(new_share_instance)
# asserts
db.share_instance_access_copy(self.context, self.share['id'],
new_share_instance['id'])
db.share_access_get_all_for_instance.assert_called_once_with(
self.context, share_instance['id'])
self.helper._add_rules_and_wait.assert_called_once_with(
new_share_instance, [access])
self.context, new_share_instance['id'])
share_api.API.allow_access_to_instance.assert_called_with(
self.context, new_share_instance, [access])
utils.wait_for_access_update.assert_called_with(
self.context, db, new_share_instance,
self.helper.migration_wait_access_rules_timeout)
@ddt.data(None, Exception('fake'))
def test_cleanup_new_instance(self, exc):
@ -371,35 +376,3 @@ class ShareMigrationHelperTestCase(test.TestCase):
if exc:
migration.LOG.warning.called
def test__add_rules_and_wait(self):
access = db_utils.create_access(share_id=self.share['id'])
values = {
'share_id': self.share['id'],
'access_type': access['access_type'],
'access_level': access['access_level'],
'access_to': access['access_to']
}
self.helper.migration_wait_access_rules_timeout = 60
# mocks
self.mock_object(db, 'share_access_create')
self.mock_object(share_api.API, 'allow_access_to_instance')
self.mock_object(utils, 'wait_for_access_update')
# run
self.helper._add_rules_and_wait(self.share_instance, [access])
# asserts
db.share_access_create.assert_called_once_with(self.context, values)
share_api.API.allow_access_to_instance.assert_called_once_with(
self.context, self.share_instance, [access])
utils.wait_for_access_update.assert_called_once_with(
self.context, db, self.share_instance, 60)