openstack/manila
Code Issues Proposed changes

Ignore replicas in error state during allow/deny access.

Browse Source 
During replication, at least one share backed is in working and serving
the share. So update the check of instance state from (non-transitional
+ non-error) states to only non-transitional states.

Closes-bug: #1965561
Change-Id: Id23d04f8d18a0e30b511c4d501a1910f2f2b4ca6
This commit is contained in:
Kiran Pawar 2022-11-22 09:47:21 +00:00
parent 73848870a5
commit 0282f3fdb2
8 changed files with 53 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
manila/api/openstack/api_version_request.py
View File

@ -188,6 +188,8 @@ REST_API_VERSION_HISTORY = """
* 2.71 - Added 'updated_at' field in share instance show API output.
* 2.72 - Added new option ``share-network`` to share replica creare API.
* 2.73 - Added Share Snapshot Metadata to Metadata API
* 2.74 - Allow/deny share access rule even if share replicas are in
'error' state.
"""
@ -195,7 +197,7 @@ REST_API_VERSION_HISTORY = """
# The default api version request is defined to be the
# minimum version of the API supported.
_MIN_API_VERSION = "2.0"
_MAX_API_VERSION = "2.73"
_MAX_API_VERSION = "2.74"
DEFAULT_API_VERSION = _MIN_API_VERSION

4
manila/api/openstack/rest_api_version_history.rst
View File

@ -406,3 +406,7 @@ ____
---------------------
Added Metadata API methods (GET, PUT, POST, DELETE)
to Share Snapshots
2.74
----
Allow/deny share access rule even if share replicas are in 'error' state.

10
manila/api/v1/shares.py
View File

@ -450,7 +450,7 @@ class ShareMixin(object):
@wsgi.Controller.authorize('allow_access')
def _allow_access(self, req, id, body, enable_ceph=False,
allow_on_error_status=False, enable_ipv6=False,
enable_metadata=False):
enable_metadata=False, allow_on_error_state=False):
"""Add share access rule."""
context = req.environ['manila.context']
access_data = body.get('allow_access', body.get('os-allow_access'))
@ -488,7 +488,8 @@ class ShareMixin(object):
try:
access = self.share_api.allow_access(
context, share, access_type, access_to,
access_data.get('access_level'), access_data.get('metadata'))
access_data.get('access_level'), access_data.get('metadata'),
allow_on_error_state)
except exception.ShareAccessExists as e:
raise webob.exc.HTTPBadRequest(explanation=e.msg)
@ -501,7 +502,7 @@ class ShareMixin(object):
return self._access_view_builder.view(req, access)
@wsgi.Controller.authorize('deny_access')
def _deny_access(self, req, id, body):
def _deny_access(self, req, id, body, allow_on_error_state=False):
"""Remove share access rule."""
context = req.environ['manila.context']
@ -528,7 +529,8 @@ class ShareMixin(object):
share = self.share_api.get(context, id)
except exception.NotFound as error:
raise webob.exc.HTTPNotFound(explanation=error.message)
self.share_api.deny_access(context, share, access)
self.share_api.deny_access(context, share, access,
allow_on_error_state)
return webob.Response(status_int=http_client.ACCEPTED)
def _access_list(self, req, id, body):

8
manila/api/v2/shares.py
View File

@ -472,6 +472,8 @@ class ShareController(wsgi.Controller,
kwargs['enable_ipv6'] = True
if req.api_version_request >= api_version.APIVersionRequest("2.45"):
kwargs['enable_metadata'] = True
if req.api_version_request >= api_version.APIVersionRequest("2.74"):
kwargs['allow_on_error_state'] = True
return self._allow_access(*args, **kwargs)
@wsgi.Controller.api_version('2.0', '2.6')
@ -484,7 +486,11 @@ class ShareController(wsgi.Controller,
@wsgi.action('deny_access')
def deny_access(self, req, id, body):
"""Remove share access rule."""
return self._deny_access(req, id, body)
args = (req, id, body)
kwargs = {}
if req.api_version_request >= api_version.APIVersionRequest("2.74"):
kwargs['allow_on_error_state'] = True
return self._deny_access(*args, **kwargs)
@wsgi.Controller.api_version('2.0', '2.6')
@wsgi.action('os-access_list')

2
manila/common/constants.py
View File

@ -132,7 +132,7 @@ TRANSITIONAL_STATUSES = (
)
INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES = (
TRANSITIONAL_STATUSES + (STATUS_ERROR,)
TRANSITIONAL_STATUSES
)
SUPPORTED_SHARE_PROTOCOLS = (

26
manila/share/api.py
View File

@ -2198,13 +2198,20 @@ class API(base.Base):
return self.db.share_snapshot_get_latest_for_share(context, share_id)
@staticmethod
def _is_invalid_share_instance(instance):
return (instance['host'] is None
or instance['status'] in constants.
INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES)
def _any_invalid_share_instance(share, allow_on_error_state=False):
invalid_states = (
constants.INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES)
if not allow_on_error_state:
invalid_states += (constants.STATUS_ERROR,)
for instance in share.instances:
if (not instance['host'] or instance['status'] in invalid_states):
return True
return False
def allow_access(self, ctx, share, access_type, access_to,
access_level=None, metadata=None):
access_level=None, metadata=None,
allow_on_error_state=False):
"""Allow access to share."""
# Access rule validation:
@ -2220,9 +2227,7 @@ class API(base.Base):
raise exception.ShareAccessExists(access_type=access_type,
access=access_to)
# Share instance validation
if any(instance for instance in share.instances
if self._is_invalid_share_instance(instance)):
if self._any_invalid_share_instance(share, allow_on_error_state):
msg = _("New access rules cannot be applied while the share or "
"any of its replicas or migration copies lacks a valid "
"host or is in an invalid state.")
@ -2257,11 +2262,10 @@ class API(base.Base):
context, conditionally_change=conditionally_change,
share_instance_id=share_instance['id'])
def deny_access(self, ctx, share, access):
def deny_access(self, ctx, share, access, allow_on_error_state=False):
"""Deny access to share."""
if any(instance for instance in share.instances if
self._is_invalid_share_instance(instance)):
if self._any_invalid_share_instance(share, allow_on_error_state):
msg = _("Access rules cannot be denied while the share, "
"any of its replicas or migration copies lacks a valid "
"host or is in an invalid state.")

11
manila/tests/api/v2/test_shares.py
View File

@ -2451,7 +2451,7 @@ class ShareActionsTest(test.TestCase):
self.assertEqual(expected_access, access['access'])
share_api.API.allow_access.assert_called_once_with(
req.environ['manila.context'], share, 'user',
'clemsontigers', 'rw', None)
'clemsontigers', 'rw', None, False)
@ddt.data(*itertools.product(
set(['2.28', api_version._MAX_API_VERSION]),
@ -2498,10 +2498,17 @@ class ShareActionsTest(test.TestCase):
{
'metadata': {},
})
if api_version.APIVersionRequest(version) >= (
api_version.APIVersionRequest("2.74")):
allow_on_error_state = True
else:
allow_on_error_state = False
self.assertEqual(expected_access, access['access'])
share_api.API.allow_access.assert_called_once_with(
req.environ['manila.context'], share, 'user',
'clemsontigers', 'rw', None)
'clemsontigers', 'rw', None, allow_on_error_state)
def test_deny_access(self):
def _stub_deny_access(*args, **kwargs):

8
releasenotes/notes/allow-and-deny-access-rule-if-any-instance-is-valid-0e092913d30dbcdd.yaml Normal file
View File

@ -0,0 +1,8 @@
---
fixes:
- |
With replication setup at least one backend is working and serving the
shares. Starting from API version 2.74, allowing and denying access to
shares will only fail if any of the instances is in a transitional state.
Please refer to
`Launchpad bug 1965561 <https://bugs.launchpad.net/manila/+bug/1965561>`_