diff --git a/manila/api/openstack/api_version_request.py b/manila/api/openstack/api_version_request.py index 67d2080c94..5abecca220 100644 --- a/manila/api/openstack/api_version_request.py +++ b/manila/api/openstack/api_version_request.py @@ -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 diff --git a/manila/api/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index 96969c91d8..af38ea4005 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -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. diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index 8099b9a9fb..dfb66673b9 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -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): diff --git a/manila/api/v2/shares.py b/manila/api/v2/shares.py index f09ae90933..05976d8424 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -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') diff --git a/manila/common/constants.py b/manila/common/constants.py index f14bdd1d38..5f7e55b14e 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -132,7 +132,7 @@ TRANSITIONAL_STATUSES = ( ) INVALID_SHARE_INSTANCE_STATUSES_FOR_ACCESS_RULE_UPDATES = ( - TRANSITIONAL_STATUSES + (STATUS_ERROR,) + TRANSITIONAL_STATUSES ) SUPPORTED_SHARE_PROTOCOLS = ( diff --git a/manila/share/api.py b/manila/share/api.py index 5b0a183c14..0b395baad3 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -2199,13 +2199,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: @@ -2221,9 +2228,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.") @@ -2258,11 +2263,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.") diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index 59f6bef3f8..5c4e12bffb 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -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): diff --git a/releasenotes/notes/allow-and-deny-access-rule-if-any-instance-is-valid-0e092913d30dbcdd.yaml b/releasenotes/notes/allow-and-deny-access-rule-if-any-instance-is-valid-0e092913d30dbcdd.yaml new file mode 100644 index 0000000000..1c190a2ff9 --- /dev/null +++ b/releasenotes/notes/allow-and-deny-access-rule-if-any-instance-is-valid-0e092913d30dbcdd.yaml @@ -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 `_