Merge "Fix allow/deny error message and race in migration"
This commit is contained in:
commit
8f2fa31d8a
|
@ -1345,20 +1345,7 @@ class API(base.Base):
|
||||||
policy.check_policy(ctx, 'share', 'allow_access')
|
policy.check_policy(ctx, 'share', 'allow_access')
|
||||||
share = self.db.share_get(ctx, share['id'])
|
share = self.db.share_get(ctx, share['id'])
|
||||||
if share['status'] != constants.STATUS_AVAILABLE:
|
if share['status'] != constants.STATUS_AVAILABLE:
|
||||||
if not (share['status'] in (constants.STATUS_MIGRATING,
|
msg = _("Share status must be %s") % constants.STATUS_AVAILABLE
|
||||||
constants.STATUS_MIGRATING_TO) and
|
|
||||||
share['task_state'] in (
|
|
||||||
constants.TASK_STATE_DATA_COPYING_ERROR,
|
|
||||||
constants.TASK_STATE_MIGRATION_ERROR,
|
|
||||||
constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE,
|
|
||||||
constants.TASK_STATE_DATA_COPYING_COMPLETED)):
|
|
||||||
msg = _("Share status must be %(available)s, or %(migrating)s "
|
|
||||||
"while first phase of migration is completed.") % {
|
|
||||||
'available': constants.STATUS_AVAILABLE,
|
|
||||||
'migrating': constants.STATUS_MIGRATING
|
|
||||||
}
|
|
||||||
else:
|
|
||||||
msg = _("Share status must be %s") % constants.STATUS_AVAILABLE
|
|
||||||
raise exception.InvalidShare(reason=msg)
|
raise exception.InvalidShare(reason=msg)
|
||||||
values = {
|
values = {
|
||||||
'share_id': share['id'],
|
'share_id': share['id'],
|
||||||
|
@ -1430,20 +1417,7 @@ class API(base.Base):
|
||||||
msg = _("Share doesn't have any instances")
|
msg = _("Share doesn't have any instances")
|
||||||
raise exception.InvalidShare(reason=msg)
|
raise exception.InvalidShare(reason=msg)
|
||||||
if share['status'] != constants.STATUS_AVAILABLE:
|
if share['status'] != constants.STATUS_AVAILABLE:
|
||||||
if not (share['status'] in (constants.STATUS_MIGRATING,
|
msg = _("Share status must be %s") % constants.STATUS_AVAILABLE
|
||||||
constants.STATUS_MIGRATING_TO) and
|
|
||||||
share['task_state'] in (
|
|
||||||
constants.TASK_STATE_DATA_COPYING_ERROR,
|
|
||||||
constants.TASK_STATE_MIGRATION_ERROR,
|
|
||||||
constants.TASK_STATE_MIGRATION_DRIVER_PHASE1_DONE,
|
|
||||||
constants.TASK_STATE_DATA_COPYING_COMPLETED)):
|
|
||||||
msg = _("Share status must be %(available)s, or %(migrating)s "
|
|
||||||
"while first phase of migration is completed.") % {
|
|
||||||
'available': constants.STATUS_AVAILABLE,
|
|
||||||
'migrating': constants.STATUS_MIGRATING
|
|
||||||
}
|
|
||||||
else:
|
|
||||||
msg = _("Share status must be %s") % constants.STATUS_AVAILABLE
|
|
||||||
raise exception.InvalidShare(reason=msg)
|
raise exception.InvalidShare(reason=msg)
|
||||||
|
|
||||||
for share_instance in share.instances:
|
for share_instance in share.instances:
|
||||||
|
|
|
@ -194,6 +194,10 @@ class ShareMigrationHelper(object):
|
||||||
LOG.debug("Restoring all of share %s access rules according to "
|
LOG.debug("Restoring all of share %s access rules according to "
|
||||||
"DB.", self.share['id'])
|
"DB.", self.share['id'])
|
||||||
|
|
||||||
|
# refresh share instance
|
||||||
|
new_share_instance = self.db.share_instance_get(
|
||||||
|
self.context, new_share_instance['id'], with_share_data=True)
|
||||||
|
|
||||||
self.api.allow_access_to_instance(self.context, new_share_instance,
|
self.api.allow_access_to_instance(self.context, new_share_instance,
|
||||||
rules)
|
rules)
|
||||||
utils.wait_for_access_update(
|
utils.wait_for_access_update(
|
||||||
|
|
|
@ -345,6 +345,8 @@ class ShareMigrationHelperTestCase(test.TestCase):
|
||||||
access_level='rw')
|
access_level='rw')
|
||||||
|
|
||||||
# mocks
|
# mocks
|
||||||
|
self.mock_object(db, 'share_instance_get',
|
||||||
|
mock.Mock(return_value=new_share_instance))
|
||||||
self.mock_object(db, 'share_instance_access_copy')
|
self.mock_object(db, 'share_instance_access_copy')
|
||||||
self.mock_object(db, 'share_access_get_all_for_instance',
|
self.mock_object(db, 'share_access_get_all_for_instance',
|
||||||
mock.Mock(return_value=[access]))
|
mock.Mock(return_value=[access]))
|
||||||
|
@ -355,6 +357,8 @@ class ShareMigrationHelperTestCase(test.TestCase):
|
||||||
self.helper.apply_new_access_rules(new_share_instance)
|
self.helper.apply_new_access_rules(new_share_instance)
|
||||||
|
|
||||||
# asserts
|
# asserts
|
||||||
|
db.share_instance_get.assert_called_once_with(
|
||||||
|
self.context, new_share_instance['id'], with_share_data=True)
|
||||||
db.share_instance_access_copy(self.context, self.share['id'],
|
db.share_instance_access_copy(self.context, self.share['id'],
|
||||||
new_share_instance['id'])
|
new_share_instance['id'])
|
||||||
db.share_access_get_all_for_instance.assert_called_once_with(
|
db.share_access_get_all_for_instance.assert_called_once_with(
|
||||||
|
|
|
@ -0,0 +1,7 @@
|
||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- Fixed access_allow and access_deny displaying incorrect error
|
||||||
|
message during migration of a share.
|
||||||
|
- Fixed access rule concurrency in migration that was preventing
|
||||||
|
new rules from being added to the migrated share.
|
||||||
|
|
Loading…
Reference in New Issue