Merge "Fix deletion of error state access rules"
This commit is contained in:
commit
4ad103f5af
@ -428,6 +428,11 @@ def share_access_get_all_for_share(context, share_id):
|
||||
return IMPL.share_access_get_all_for_share(context, share_id)
|
||||
|
||||
|
||||
def share_instance_access_get_all(context, access_id, session=None):
|
||||
"""Get access rules to all share instances."""
|
||||
return IMPL.share_instance_access_get_all(context, access_id, session=None)
|
||||
|
||||
|
||||
def share_access_get_all_by_type_and_access(context, share_id, access_type,
|
||||
access):
|
||||
"""Returns share access by given type and access."""
|
||||
|
@ -1600,7 +1600,10 @@ def share_access_get_all_for_share(context, share_id):
|
||||
{'share_id': share_id}).all()
|
||||
|
||||
|
||||
def _share_instance_access_get_all(context, access_id, session):
|
||||
@require_context
|
||||
def share_instance_access_get_all(context, access_id, session=None):
|
||||
if not session:
|
||||
session = get_session()
|
||||
return _share_instance_access_query(context, session, access_id).all()
|
||||
|
||||
|
||||
@ -1619,7 +1622,7 @@ def share_access_delete(context, access_id):
|
||||
session = get_session()
|
||||
|
||||
with session.begin():
|
||||
mappings = _share_instance_access_get_all(context, access_id, session)
|
||||
mappings = share_instance_access_get_all(context, access_id, session)
|
||||
|
||||
if len(mappings) > 0:
|
||||
msg = (_("Access rule %s has mappings"
|
||||
@ -1654,7 +1657,7 @@ def share_instance_access_delete(context, mapping_id):
|
||||
mapping.soft_delete(session, update_status=True,
|
||||
status_field_name='state')
|
||||
|
||||
other_mappings = _share_instance_access_get_all(
|
||||
other_mappings = share_instance_access_get_all(
|
||||
context, mapping['access_id'], session)
|
||||
|
||||
# NOTE(u_glide): Remove access rule if all mappings were removed.
|
||||
|
@ -816,9 +816,12 @@ class API(base.Base):
|
||||
raise exception.InvalidShare(reason=msg)
|
||||
|
||||
# Then check state of the access rule
|
||||
if access['state'] == constants.STATUS_ERROR:
|
||||
if (access['state'] == constants.STATUS_ERROR and not
|
||||
self.db.share_instance_access_get_all(ctx, access['id'])):
|
||||
self.db.share_access_delete(ctx, access["id"])
|
||||
elif access['state'] == constants.STATUS_ACTIVE:
|
||||
|
||||
elif access['state'] in [constants.STATUS_ACTIVE,
|
||||
constants.STATUS_ERROR]:
|
||||
for share_instance in share.instances:
|
||||
try:
|
||||
self.deny_access_to_instance(ctx, share_instance, access)
|
||||
|
@ -1317,16 +1317,42 @@ class ShareAPITestCase(test.TestCase):
|
||||
self.context, share.instance, 'fake'
|
||||
)
|
||||
|
||||
@mock.patch.object(db_api, 'share_access_delete', mock.Mock())
|
||||
@mock.patch.object(db_api, 'share_get', mock.Mock())
|
||||
@mock.patch.object(share_api.API, 'deny_access_to_instance', mock.Mock())
|
||||
@mock.patch.object(db_api, 'share_instance_access_get_all', mock.Mock())
|
||||
def test_deny_access_error(self):
|
||||
share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
|
||||
db_api.share_get.return_value = share
|
||||
access = db_utils.create_access(state=constants.STATUS_ERROR,
|
||||
share_id=share['id'])
|
||||
share_instance = share.instances[0]
|
||||
db_api.share_instance_access_get_all.return_value = [share_instance, ]
|
||||
self.api.deny_access(self.context, share, access)
|
||||
db_api.share_get.assert_called_once_with(self.context, share['id'])
|
||||
share_api.policy.check_policy.assert_called_once_with(
|
||||
self.context, 'share', 'deny_access')
|
||||
share_api.API.deny_access_to_instance.assert_called_once_with(
|
||||
self.context, share_instance, access)
|
||||
db_api.share_instance_access_get_all.assert_called_once_with(
|
||||
self.context, access['id'])
|
||||
|
||||
@mock.patch.object(db_api, 'share_get', mock.Mock())
|
||||
@mock.patch.object(db_api, 'share_instance_access_get_all', mock.Mock())
|
||||
@mock.patch.object(db_api, 'share_access_delete', mock.Mock())
|
||||
def test_deny_access_error_no_share_instance_mapping(self):
|
||||
share = db_utils.create_share(status=constants.STATUS_AVAILABLE)
|
||||
db_api.share_get.return_value = share
|
||||
access = db_utils.create_access(state=constants.STATUS_ERROR,
|
||||
share_id=share['id'])
|
||||
db_api.share_instance_access_get_all.return_value = []
|
||||
self.api.deny_access(self.context, share, access)
|
||||
db_api.share_get.assert_called_once_with(self.context, share['id'])
|
||||
share_api.policy.check_policy.assert_called_once_with(
|
||||
self.context, 'share', 'deny_access')
|
||||
db_api.share_access_delete.assert_called_once_with(
|
||||
self.context, access['id'])
|
||||
db_api.share_instance_access_get_all.assert_called_once_with(
|
||||
self.context, access['id'])
|
||||
|
||||
@mock.patch.object(db_api, 'share_instance_access_update_state',
|
||||
mock.Mock())
|
||||
|
Loading…
x
Reference in New Issue
Block a user