From 39a2700ffcbe05608ba2ea8a14ce13b29d95645c Mon Sep 17 00:00:00 2001 From: haixin Date: Mon, 23 Dec 2019 16:33:04 +0800 Subject: [PATCH] Extend share will go through scheduler Add an force parameter to the API layer that lets the user choose whether to go through the scheduler or not, which is boolean.and default is False,set True means extend share directly, set False means extend share will go through scheduler. Closes-Bug:#1855391 Change-Id: I6da36a687a37c78a7fb7d3f252318d03d4a05133 --- api-ref/source/parameters.yaml | 8 ++++ .../samples/share-actions-extend-request.json | 3 +- api-ref/source/share-actions.inc | 1 + manila/api/v1/shares.py | 37 ++++++++++++++++--- manila/policies/shares.py | 10 +++++ manila/scheduler/manager.py | 31 ++++++++++++++++ manila/scheduler/rpcapi.py | 17 ++++++++- manila/share/api.py | 17 +++++++-- manila/tests/api/v1/test_shares.py | 2 +- manila/tests/api/v2/test_shares.py | 2 +- manila/tests/scheduler/test_rpcapi.py | 10 +++++ manila/tests/share/test_api.py | 19 ++++++++-- ...go-through-scheduler-3a29093756dc88c1.yaml | 7 ++++ 13 files changed, 148 insertions(+), 16 deletions(-) create mode 100644 releasenotes/notes/bug-1855391-extend-share-will-go-through-scheduler-3a29093756dc88c1.yaml diff --git a/api-ref/source/parameters.yaml b/api-ref/source/parameters.yaml index 9e6a48e087..153fb3f594 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2628,6 +2628,14 @@ share_new_size: in: body required: true type: integer +share_force_extend: + description: | + (Admin only). Defines whether to go through scheduler, Set to `True` will + extend share directly. Set to `False` will go through scheduler, default + is `False`. + in: body + required: false + type: boolean share_proto: description: | The Shared File Systems protocol. A valid value diff --git a/api-ref/source/samples/share-actions-extend-request.json b/api-ref/source/samples/share-actions-extend-request.json index 0da9b36246..40804bb1fc 100644 --- a/api-ref/source/samples/share-actions-extend-request.json +++ b/api-ref/source/samples/share-actions-extend-request.json @@ -1,5 +1,6 @@ { "extend": { - "new_size": 2 + "new_size": 2, + "force": "true" } } diff --git a/api-ref/source/share-actions.inc b/api-ref/source/share-actions.inc index a0fb31e969..6a105d14b0 100644 --- a/api-ref/source/share-actions.inc +++ b/api-ref/source/share-actions.inc @@ -351,6 +351,7 @@ Request - share_id: share_id - extend: extend - new_size: share_new_size + - force: share_force_extend Request example diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index ccec9a5f14..210194206d 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -512,11 +512,11 @@ class ShareMixin(object): def _extend(self, req, id, body): """Extend size of a share.""" context = req.environ['manila.context'] - share, size = self._get_valid_resize_parameters( + share, size, force = self._get_valid_extend_parameters( context, id, body, 'os-extend') try: - self.share_api.extend(context, share, size) + self.share_api.extend(context, share, size, force=force) except (exception.InvalidInput, exception.InvalidShare) as e: raise webob.exc.HTTPBadRequest(explanation=six.text_type(e)) except exception.ShareSizeExceedsAvailableQuota as e: @@ -527,7 +527,7 @@ class ShareMixin(object): def _shrink(self, req, id, body): """Shrink size of a share.""" context = req.environ['manila.context'] - share, size = self._get_valid_resize_parameters( + share, size = self._get_valid_shrink_parameters( context, id, body, 'os-shrink') try: @@ -537,15 +537,40 @@ class ShareMixin(object): return webob.Response(status_int=http_client.ACCEPTED) - def _get_valid_resize_parameters(self, context, id, body, action): + def _get_valid_extend_parameters(self, context, id, body, action): try: share = self.share_api.get(context, id) except exception.NotFound as e: raise webob.exc.HTTPNotFound(explanation=six.text_type(e)) try: - size = int(body.get(action, - body.get(action.split('os-')[-1]))['new_size']) + size = int(body.get(action, body.get('extend'))['new_size']) + except (KeyError, ValueError, TypeError): + msg = _("New share size must be specified as an integer.") + raise webob.exc.HTTPBadRequest(explanation=msg) + + # force is True means share extend will extend directly, is False + # means will go through scheduler. Default value is False, + try: + force = strutils.bool_from_string(body.get( + action, body.get('extend'))['force'], strict=True) + except KeyError: + force = False + except (ValueError, TypeError): + msg = (_('Invalid boolean force : %(value)s') % + {'value': body.get('extend')['force']}) + raise webob.exc.HTTPBadRequest(explanation=msg) + + return share, size, force + + def _get_valid_shrink_parameters(self, context, id, body, action): + try: + share = self.share_api.get(context, id) + except exception.NotFound as e: + raise webob.exc.HTTPNotFound(explanation=six.text_type(e)) + + try: + size = int(body.get(action, body.get('shrink'))['new_size']) except (KeyError, ValueError, TypeError): msg = _("New share size must be specified as an integer.") raise webob.exc.HTTPBadRequest(explanation=msg) diff --git a/manila/policies/shares.py b/manila/policies/shares.py index e0fb2cf79f..175735dd9a 100644 --- a/manila/policies/shares.py +++ b/manila/policies/shares.py @@ -422,6 +422,16 @@ shares_policies = [ ], deprecated_rule=deprecated_share_extend ), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'force_extend', + check_str=base.RULE_ADMIN_API, + description="Force extend share.", + operations=[ + { + 'method': 'POST', + 'path': '/shares/{share_id}/action', + } + ]), policy.DocumentedRuleDefault( name=BASE_POLICY_NAME % 'shrink', check_str=base.SYSTEM_ADMIN_OR_PROJECT_MEMBER, diff --git a/manila/scheduler/manager.py b/manila/scheduler/manager.py index 8dc31dd0e2..92e6bea4a0 100644 --- a/manila/scheduler/manager.py +++ b/manila/scheduler/manager.py @@ -316,3 +316,34 @@ class SchedulerManager(manager.Manager): @coordination.synchronized('locked-clean-expired-messages') def _clean_expired_messages(self, context): self.message_api.cleanup_expired_messages(context) + + def extend_share(self, context, share, new_size, reservations, + request_spec=None, filter_properties=None): + + def _extend_share_set_error(self, context, ex, request_spec): + share_state = {'status': constants.STATUS_AVAILABLE} + self._set_share_state_and_notify('extend_share', share_state, + context, ex, request_spec) + + try: + target_host = self.driver.host_passes_filters( + context, + share['host'], + request_spec, filter_properties) + target_host.consume_from_share( + {'size': int(new_size) - share['size']}) + share_rpcapi.ShareAPI().extend_share(context, share, new_size, + reservations) + except exception.NoValidHost as ex: + quota.QUOTAS.rollback(context, reservations, + project_id=share['project_id'], + user_id=share['user_id'], + share_type_id=share['share_type_id']) + _extend_share_set_error(self, context, ex, request_spec) + self.message_api.create( + context, + message_field.Action.EXTEND, + share['project_id'], + resource_type=message_field.Resource.SHARE, + resource_id=share['id'], + exception=ex) diff --git a/manila/scheduler/rpcapi.py b/manila/scheduler/rpcapi.py index 0619e99057..8ae4378657 100644 --- a/manila/scheduler/rpcapi.py +++ b/manila/scheduler/rpcapi.py @@ -43,9 +43,10 @@ class SchedulerAPI(object): 1.8 - Rename create_consistency_group -> create_share_group method 1.9 - Add cached parameter to get_pools method 1.10 - Add timestamp to update_service_capabilities + 1.11 - Add extend_share """ - RPC_API_VERSION = '1.10' + RPC_API_VERSION = '1.11' def __init__(self): super(SchedulerAPI, self).__init__() @@ -144,3 +145,17 @@ class SchedulerAPI(object): driver_options=driver_options, request_spec=request_spec, filter_properties=filter_properties) + + def extend_share(self, context, share, new_size, reservations, + request_spec, filter_properties=None): + call_context = self.client.prepare(version='1.11') + + msg_args = { + 'share': share, + 'new_size': new_size, + 'reservations': reservations, + 'request_spec': request_spec, + 'filter_properties': filter_properties, + } + + return call_context.cast(context, 'extend_share', **msg_args) diff --git a/manila/share/api.py b/manila/share/api.py index bd3c9c4ec5..d3bcb63e8e 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -2147,8 +2147,11 @@ class API(base.Base): def get_share_network(self, context, share_net_id): return self.db.share_network_get(context, share_net_id) - def extend(self, context, share, new_size): - policy.check_policy(context, 'share', 'extend') + def extend(self, context, share, new_size, force=False): + if force: + policy.check_policy(context, 'share', 'force_extend') + else: + policy.check_policy(context, 'share', 'extend') if share['status'] != constants.STATUS_AVAILABLE: msg_params = { @@ -2242,7 +2245,15 @@ class API(base.Base): message=msg) self.update(context, share, {'status': constants.STATUS_EXTENDING}) - self.share_rpcapi.extend_share(context, share, new_size, reservations) + if force: + self.share_rpcapi.extend_share(context, share, + new_size, reservations) + else: + share_type = share_types.get_share_type( + context, share['instance']['share_type_id']) + request_spec = self._get_request_spec_dict(share, share_type) + self.scheduler_rpcapi.extend_share(context, share, new_size, + reservations, request_spec) LOG.info("Extend share request issued successfully.", resource=share) diff --git a/manila/tests/api/v1/test_shares.py b/manila/tests/api/v1/test_shares.py index 9c13d4c7dd..58c9259ddf 100644 --- a/manila/tests/api/v1/test_shares.py +++ b/manila/tests/api/v1/test_shares.py @@ -1112,7 +1112,7 @@ class ShareActionsTest(test.TestCase): share_api.API.get.assert_called_once_with(mock.ANY, id) share_api.API.extend.assert_called_once_with( - mock.ANY, share, int(size)) + mock.ANY, share, int(size), force=False) self.assertEqual(202, actual_response.status_int) @ddt.data({"os-extend": ""}, diff --git a/manila/tests/api/v2/test_shares.py b/manila/tests/api/v2/test_shares.py index d4c803302e..0756de8d78 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -2413,7 +2413,7 @@ class ShareActionsTest(test.TestCase): share_api.API.get.assert_called_once_with(mock.ANY, id) share_api.API.extend.assert_called_once_with( - mock.ANY, share, int(size)) + mock.ANY, share, int(size), force=False) self.assertEqual(202, actual_response.status_int) @ddt.data({"os-extend": ""}, diff --git a/manila/tests/scheduler/test_rpcapi.py b/manila/tests/scheduler/test_rpcapi.py index 7e102fdc27..d51ce6e51b 100644 --- a/manila/tests/scheduler/test_rpcapi.py +++ b/manila/tests/scheduler/test_rpcapi.py @@ -130,3 +130,13 @@ class SchedulerRpcAPITestCase(test.TestCase): request_spec='fake_request_spec', filter_properties='filter_properties', version='1.6') + + def test_extend_share(self): + self._test_scheduler_api('extend_share', + rpc_method='cast', + share='fake_share', + new_size='fake_size', + reservations='fake_reservations', + request_spec='fake_request_spec', + filter_properties='filter_properties', + version='1.10',) diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index c0231966ea..7c4526eae8 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -2938,9 +2938,19 @@ class ShareAPITestCase(test.TestCase): project_id='fake', is_admin=False ) + fake_type = { + 'id': 'fake_type_id', + 'extra_specs': { + 'snapshot_support': False, + 'create_share_from_snapshot_support': False, + 'driver_handles_share_servers': False, + }, + } new_size = 123 size_increase = int(new_size) - share['size'] self.mock_object(quota.QUOTAS, 'reserve') + self.mock_object(share_types, 'get_share_type', + mock.Mock(return_value=fake_type)) self.api.extend(diff_user_context, share, new_size) @@ -2972,15 +2982,18 @@ class ShareAPITestCase(test.TestCase): new_replica_size = size_increase * replica_amount expected_deltas.update({'replica_gigabytes': new_replica_size}) self.mock_object(self.api, 'update') - self.mock_object(self.api.share_rpcapi, 'extend_share') + self.mock_object(self.api.scheduler_rpcapi, 'extend_share') self.mock_object(quota.QUOTAS, 'reserve') + self.mock_object(share_types, 'get_share_type') + self.mock_object(self.api, '_get_request_spec_dict') self.api.extend(self.context, share, new_size) self.api.update.assert_called_once_with( self.context, share, {'status': constants.STATUS_EXTENDING}) - self.api.share_rpcapi.extend_share.assert_called_once_with( - self.context, share, new_size, mock.ANY + + self.api.scheduler_rpcapi.extend_share.assert_called_once_with( + self.context, share, new_size, mock.ANY, mock.ANY ) quota.QUOTAS.reserve.assert_called_once_with( self.context, **expected_deltas) diff --git a/releasenotes/notes/bug-1855391-extend-share-will-go-through-scheduler-3a29093756dc88c1.yaml b/releasenotes/notes/bug-1855391-extend-share-will-go-through-scheduler-3a29093756dc88c1.yaml new file mode 100644 index 0000000000..207122b2bf --- /dev/null +++ b/releasenotes/notes/bug-1855391-extend-share-will-go-through-scheduler-3a29093756dc88c1.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `Launchpad bug 1855391 `_ + has been fixed. The action of extend share will go through scheduler, if + there is no available share backend host, the share will rollback to + available state and create an user message about extend. \ No newline at end of file