From cdbc428b692ca087e26fa1ab5c3d1f5fb77cdc29 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. Add an new min_version 2.64 to extend share api. force parameter only support min_version >= 2.64. Closes-Bug:#1855391 Change-Id: I6da36a687a37c78a7fb7d3f252318d03d4a05133 --- api-ref/source/parameters.yaml | 9 +++++ .../samples/share-actions-extend-request.json | 3 +- api-ref/source/share-actions.inc | 1 + .../openstack/rest_api_version_history.rst | 5 +++ manila/api/v1/shares.py | 37 ++++++++++++++++--- manila/api/v2/shares.py | 10 ++++- manila/policies/shares.py | 11 ++++++ manila/scheduler/manager.py | 34 ++++++++++++++++- 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 | 20 ++++++++-- ...go-through-scheduler-3a29093756dc88c1.yaml | 7 ++++ 15 files changed, 167 insertions(+), 18 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 22e47f7ef1..ed8fd19342 100644 --- a/api-ref/source/parameters.yaml +++ b/api-ref/source/parameters.yaml @@ -2435,6 +2435,15 @@ share_force_delete: in: body required: true type: string +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 + min_version: 2.64 share_group_host: description: | The share group host name. 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/openstack/rest_api_version_history.rst b/manila/api/openstack/rest_api_version_history.rst index 7119933387..6dad063f78 100644 --- a/manila/api/openstack/rest_api_version_history.rst +++ b/manila/api/openstack/rest_api_version_history.rst @@ -353,3 +353,8 @@ user documentation. endpoint: 'update_security_service', 'update_security_service_check' and 'add_security_service_check'. +2.64 +---- + Added 'force' field to extend share api, which can extend share directly + without go through share scheduler. + diff --git a/manila/api/v1/shares.py b/manila/api/v1/shares.py index 19e155ff0b..293c302796 100644 --- a/manila/api/v1/shares.py +++ b/manila/api/v1/shares.py @@ -515,11 +515,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: @@ -530,7 +530,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: @@ -540,15 +540,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/api/v2/shares.py b/manila/api/v2/shares.py index 0dc9e49275..14a22fa9eb 100644 --- a/manila/api/v2/shares.py +++ b/manila/api/v2/shares.py @@ -391,11 +391,19 @@ class ShareController(shares.ShareMixin, @wsgi.action('os-extend') def extend_legacy(self, req, id, body): """Extend size of a share.""" + body.get('os-extend', {}).pop('force', None) return self._extend(req, id, body) - @wsgi.Controller.api_version('2.7') + @wsgi.Controller.api_version('2.7', '2.63') @wsgi.action('extend') def extend(self, req, id, body): + """Extend size of a share.""" + body.get('extend', {}).pop('force', None) + return self._extend(req, id, body) + + @wsgi.Controller.api_version('2.64') # noqa + @wsgi.action('extend') + def extend(self, req, id, body): # pylint: disable=function-redefined # noqa F811 """Extend size of a share.""" return self._extend(req, id, body) diff --git a/manila/policies/shares.py b/manila/policies/shares.py index e0fb2cf79f..833df70eb5 100644 --- a/manila/policies/shares.py +++ b/manila/policies/shares.py @@ -422,6 +422,17 @@ shares_policies = [ ], deprecated_rule=deprecated_share_extend ), + policy.DocumentedRuleDefault( + name=BASE_POLICY_NAME % 'force_extend', + check_str=base.SYSTEM_ADMIN_OR_PROJECT_ADMIN, + scope_types=['system', 'project'], + 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 55c2bcf700..2bac36d2de 100644 --- a/manila/scheduler/manager.py +++ b/manila/scheduler/manager.py @@ -65,7 +65,7 @@ MAPPING = { class SchedulerManager(manager.Manager): """Chooses a host to create shares.""" - RPC_API_VERSION = '1.10' + RPC_API_VERSION = '1.11' def __init__(self, scheduler_driver=None, service_name=None, *args, **kwargs): @@ -316,3 +316,35 @@ 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_id, 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) + + share = db.share_get(context, share_id) + 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..f7c39f31a9 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_id, new_size, reservations, + request_spec, filter_properties=None): + call_context = self.client.prepare(version='1.11') + + msg_args = { + 'share_id': share_id, + '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 4eef754456..8af116406f 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -2127,8 +2127,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 = { @@ -2222,7 +2225,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['id'], 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 5eae4dfad8..33f4b31469 100644 --- a/manila/tests/api/v2/test_shares.py +++ b/manila/tests/api/v2/test_shares.py @@ -2418,7 +2418,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..16ad5cde05 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_id='share_id', + new_size='fake_size', + reservations='fake_reservations', + request_spec='fake_request_spec', + filter_properties='filter_properties', + version='1.11',) diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 6a191d06f5..200c3eb3e6 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -2948,9 +2948,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) @@ -2982,15 +2992,19 @@ 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(share_types, 'provision_filter_on_size') + 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['id'], 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