From 36fae29ea59ba70a9a05e2fcc312c0c391cd22d0 Mon Sep 17 00:00:00 2001 From: Igor Malinovskiy Date: Thu, 4 Jun 2015 09:46:24 +0300 Subject: [PATCH] Add share shrink API Implement optional API for shrinking shares: "manila shrink " - Implement tenant-facing API for shrinking shares - Add appropriate unit tests Partially implements bp share-shrink-api Change-Id: I48f8e05793a992b11fb180b0b4dbf43bcffb4229 --- etc/manila/policy.json | 1 + manila/api/contrib/share_actions.py | 40 ++++++--- manila/common/constants.py | 7 +- manila/exception.py | 10 +++ manila/share/api.py | 32 +++++++ manila/share/driver.py | 15 ++++ manila/share/manager.py | 56 +++++++++++- manila/share/rpcapi.py | 9 +- .../tests/api/contrib/test_share_actions.py | 41 +++++++++ manila/tests/policy.json | 1 + manila/tests/share/test_api.py | 30 +++++++ manila/tests/share/test_manager.py | 87 +++++++++++++++++++ manila/tests/share/test_rpcapi.py | 7 ++ 13 files changed, 323 insertions(+), 13 deletions(-) diff --git a/etc/manila/policy.json b/etc/manila/policy.json index 173de2851b..7061726889 100644 --- a/etc/manila/policy.json +++ b/etc/manila/policy.json @@ -21,6 +21,7 @@ "share:allow_access": "rule:default", "share:deny_access": "rule:default", "share:extend": "rule:default", + "share:shrink": "rule:default", "share:get_share_metadata": "rule:default", "share:delete_share_metadata": "rule:default", "share:update_share_metadata": "rule:default", diff --git a/manila/api/contrib/share_actions.py b/manila/api/contrib/share_actions.py index dfb25131b8..2f08599bb7 100644 --- a/manila/api/contrib/share_actions.py +++ b/manila/api/contrib/share_actions.py @@ -139,16 +139,8 @@ class ShareActionsController(wsgi.Controller): def _extend(self, req, id, body): """Extend size of share.""" context = req.environ['manila.context'] - 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['os-extend']['new_size']) - except (KeyError, ValueError, TypeError): - msg = _("New share size must be specified as an integer.") - raise webob.exc.HTTPBadRequest(explanation=msg) + share, size = self._get_valid_resize_parameters( + context, id, body, 'os-extend') try: self.share_api.extend(context, share, size) @@ -159,6 +151,34 @@ class ShareActionsController(wsgi.Controller): return webob.Response(status_int=202) + @wsgi.action('os-shrink') + def _shrink(self, req, id, body): + """Shrink size of share.""" + context = req.environ['manila.context'] + share, size = self._get_valid_resize_parameters( + context, id, body, 'os-shrink') + + try: + self.share_api.shrink(context, share, size) + except (exception.InvalidInput, exception.InvalidShare) as e: + raise webob.exc.HTTPBadRequest(explanation=six.text_type(e)) + + return webob.Response(status_int=202) + + def _get_valid_resize_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[action]['new_size']) + except (KeyError, ValueError, TypeError): + msg = _("New share size must be specified as an integer.") + raise webob.exc.HTTPBadRequest(explanation=msg) + + return share, size + # def create_resource(): # return wsgi.Resource(ShareActionsController()) diff --git a/manila/common/constants.py b/manila/common/constants.py index 96ad205f1b..294cdd8387 100644 --- a/manila/common/constants.py +++ b/manila/common/constants.py @@ -29,11 +29,16 @@ STATUS_UNMANAGE_ERROR = 'unmanage_error' STATUS_UNMANAGED = 'unmanaged' STATUS_EXTENDING = 'extending' STATUS_EXTENDING_ERROR = 'extending_error' +STATUS_SHRINKING = 'shrinking' +STATUS_SHRINKING_ERROR = 'shrinking_error' +STATUS_SHRINKING_POSSIBLE_DATA_LOSS_ERROR = ( + 'shrinking_possible_data_loss_error' +) TRANSITIONAL_STATUSES = ( STATUS_CREATING, STATUS_DELETING, STATUS_MANAGING, STATUS_UNMANAGING, - STATUS_EXTENDING, + STATUS_EXTENDING, STATUS_SHRINKING, ) SUPPORTED_SHARE_PROTOCOLS = ( diff --git a/manila/exception.py b/manila/exception.py index 6ec78c4b76..3894d43e98 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -487,6 +487,16 @@ class ShareExtendingError(ManilaException): "in the driver: %(reason)s") +class ShareShrinkingError(ManilaException): + message = _("Share %(share_id)s could not be shrunk due to error " + "in the driver: %(reason)s") + + +class ShareShrinkingPossibleDataLoss(ManilaException): + message = _("Share %(share_id)s could not be shrunk due to " + "possible data loss") + + class InstanceNotFound(NotFound): message = _("Instance %(instance_id)s could not be found.") diff --git a/manila/share/api.py b/manila/share/api.py index 53deff0f85..078da3003a 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -733,3 +733,35 @@ class API(base.Base): self.share_rpcapi.extend_share(context, share, new_size, reservations) LOG.info(_LI("Extend share request issued successfully."), resource=share) + + def shrink(self, context, share, new_size): + policy.check_policy(context, 'share', 'shrink') + + status = six.text_type(share['status']).lower() + valid_statuses = (constants.STATUS_AVAILABLE, + constants.STATUS_SHRINKING_POSSIBLE_DATA_LOSS_ERROR) + + if status not in valid_statuses: + msg_params = { + 'valid_status': ", ".join(valid_statuses), + 'share_id': share['id'], + 'status': status, + } + msg = _("Share %(share_id)s status must in (%(valid_status)s) " + "to shrink, but current status is: " + "%(status)s.") % msg_params + raise exception.InvalidShare(reason=msg) + + size_decrease = int(share['size']) - int(new_size) + if size_decrease <= 0 or new_size <= 0: + msg = (_("New size for shrink must be less " + "than current size and greater than 0 (current: %(size)s," + " new: %(new_size)s)") % {'new_size': new_size, + 'size': share['size']}) + raise exception.InvalidInput(reason=msg) + + self.update(context, share, {'status': constants.STATUS_SHRINKING}) + self.share_rpcapi.shrink_share(context, share, new_size) + LOG.info(_LI("Shrink share (id=%(id)s) request issued successfully." + " New size: %(size)s") % {'id': share['id'], + 'size': new_size}) diff --git a/manila/share/driver.py b/manila/share/driver.py index e3345d66a1..2bafce80a8 100644 --- a/manila/share/driver.py +++ b/manila/share/driver.py @@ -384,6 +384,21 @@ class ShareDriver(object): """ raise NotImplementedError() + def shrink_share(self, share, new_size, share_server=None): + """Shrinks size of existing share. + + If consumed space on share larger than new_size driver should raise + ShareShrinkingPossibleDataLoss exception: + raise ShareShrinkingPossibleDataLoss(share_id=share['id']) + + :param share: Share model + :param new_size: New size of share (new_size < share['size']) + :param share_server: Optional -- Share server model + + :raises ShareShrinkingPossibleDataLoss, NotImplementedError + """ + raise NotImplementedError() + def teardown_server(self, *args, **kwargs): if self.driver_handles_share_servers: return self._teardown_server(*args, **kwargs) diff --git a/manila/share/manager.py b/manila/share/manager.py index 115ec2f4b8..854032087e 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -93,7 +93,7 @@ QUOTAS = quota.QUOTAS class ShareManager(manager.SchedulerDependentManager): """Manages NAS storages.""" - RPC_API_VERSION = '1.2' + RPC_API_VERSION = '1.3' def __init__(self, share_driver=None, service_name=None, *args, **kwargs): """Load the driver from args, or from flags.""" @@ -885,3 +885,57 @@ class ShareManager(manager.SchedulerDependentManager): share = self.db.share_update(context, share['id'], share_update) LOG.info(_LI("Extend share completed successfully."), resource=share) + + def shrink_share(self, context, share_id, new_size): + context = context.elevated() + share = self.db.share_get(context, share_id) + share_server = self._get_share_server(context, share) + project_id = share['project_id'] + new_size = int(new_size) + + def error_occurred(exc, msg, status=constants.STATUS_SHRINKING_ERROR): + LOG.exception(msg, resource=share) + self.db.share_update(context, share['id'], {'status': status}) + + raise exception.ShareShrinkingError( + reason=six.text_type(exc), share_id=share_id) + + reservations = None + + try: + size_decrease = int(share['size']) - new_size + reservations = QUOTAS.reserve(context, + project_id=share['project_id'], + gigabytes=-size_decrease) + except Exception as e: + error_occurred( + e, _LE("Failed to update quota on share shrinking.")) + + try: + self.driver.shrink_share( + share, new_size, share_server=share_server) + # NOTE(u_glide): Replace following except block by error notification + # when Manila has such mechanism. It's possible because drivers + # shouldn't shrink share when this validation error occurs. + except Exception as e: + if isinstance(e, exception.ShareShrinkingPossibleDataLoss): + msg = _LE("Shrink share failed due to possible data loss.") + status = constants.STATUS_SHRINKING_POSSIBLE_DATA_LOSS_ERROR + error_params = {'msg': msg, 'status': status} + else: + error_params = {'msg': _LE("Shrink share failed.")} + + try: + error_occurred(e, **error_params) + finally: + QUOTAS.rollback(context, reservations, project_id=project_id) + + QUOTAS.commit(context, reservations, project_id=project_id) + + share_update = { + 'size': new_size, + 'status': constants.STATUS_AVAILABLE + } + share = self.db.share_update(context, share['id'], share_update) + + LOG.info(_LI("Shrink share completed successfully."), resource=share) diff --git a/manila/share/rpcapi.py b/manila/share/rpcapi.py index 30867466f8..2f35dcf54c 100644 --- a/manila/share/rpcapi.py +++ b/manila/share/rpcapi.py @@ -34,6 +34,7 @@ class ShareAPI(object): 1.0 - Initial version. 1.1 - Add manage_share() and unmanage_share() methods 1.2 - Add extend_share() method + 1.3 - Add shrink_share() method ''' BASE_RPC_API_VERSION = '1.0' @@ -42,7 +43,7 @@ class ShareAPI(object): super(ShareAPI, self).__init__() target = messaging.Target(topic=CONF.share_topic, version=self.BASE_RPC_API_VERSION) - self.client = rpc.get_client(target, version_cap='1.2') + self.client = rpc.get_client(target, version_cap='1.3') def create_share(self, ctxt, share, host, request_spec, filter_properties, @@ -116,3 +117,9 @@ class ShareAPI(object): cctxt = self.client.prepare(server=host, version='1.2') cctxt.cast(ctxt, 'extend_share', share_id=share['id'], new_size=new_size, reservations=reservations) + + def shrink_share(self, ctxt, share, new_size): + host = utils.extract_host(share['host']) + cctxt = self.client.prepare(server=host, version='1.3') + cctxt.cast(ctxt, 'shrink_share', share_id=share['id'], + new_size=new_size) diff --git a/manila/tests/api/contrib/test_share_actions.py b/manila/tests/api/contrib/test_share_actions.py index 158b57f899..2bd62dd988 100644 --- a/manila/tests/api/contrib/test_share_actions.py +++ b/manila/tests/api/contrib/test_share_actions.py @@ -184,3 +184,44 @@ class ShareActionsTest(test.TestCase): mock.Mock(side_effect=source('fake'))) self.assertRaises(target, self.controller._extend, req, id, body) + + def test_shrink(self): + id = 'fake_share_id' + share = stubs.stub_share_get(None, None, id) + self.mock_object(share_api.API, 'get', mock.Mock(return_value=share)) + self.mock_object(share_api.API, "shrink") + + size = '123' + body = {"os-shrink": {'new_size': size}} + req = fakes.HTTPRequest.blank('/v1/shares/%s/action' % id) + + actual_response = self.controller._shrink(req, id, body) + + share_api.API.get.assert_called_once_with(mock.ANY, id) + share_api.API.shrink.assert_called_once_with( + mock.ANY, share, int(size)) + self.assertEqual(202, actual_response.status_int) + + @ddt.data({"os-shrink": ""}, + {"os-shrink": {"new_size": "foo"}}, + {"os-shrink": {"new_size": {'foo': 'bar'}}}) + def test_shrink_invalid_body(self, body): + id = 'fake_share_id' + req = fakes.HTTPRequest.blank('/v1/shares/%s/action' % id) + + self.assertRaises(webob.exc.HTTPBadRequest, + self.controller._shrink, req, id, body) + + @ddt.data({'source': exception.InvalidInput, + 'target': webob.exc.HTTPBadRequest}, + {'source': exception.InvalidShare, + 'target': webob.exc.HTTPBadRequest}) + @ddt.unpack + def test_shrink_exception(self, source, target): + id = 'fake_share_id' + req = fakes.HTTPRequest.blank('/v1/shares/%s/action' % id) + body = {"os-shrink": {'new_size': '123'}} + self.mock_object(share_api.API, "shrink", + mock.Mock(side_effect=source('fake'))) + + self.assertRaises(target, self.controller._shrink, req, id, body) \ No newline at end of file diff --git a/manila/tests/policy.json b/manila/tests/policy.json index abe3550d50..a33e75f441 100644 --- a/manila/tests/policy.json +++ b/manila/tests/policy.json @@ -16,6 +16,7 @@ "share:get_snapshot": "", "share:get_all_snapshots": "", "share:extend": "", + "share:shrink": "", "share_network:create": "", "share_network:index": "", diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index f9febb1d38..15bf84367e 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -1412,6 +1412,36 @@ class ShareAPITestCase(test.TestCase): self.context, share, new_size, mock.ANY ) + def test_shrink_invalid_status(self): + invalid_status = 'fake' + share = fake_share('fake', status=invalid_status) + + self.assertRaises(exception.InvalidShare, + self.api.shrink, self.context, share, 123) + + @ddt.data(300, 0, -1) + def test_shrink_invalid_size(self, new_size): + share = fake_share('fake', status=constants.STATUS_AVAILABLE, size=200) + + self.assertRaises(exception.InvalidInput, + self.api.shrink, self.context, share, new_size) + + @ddt.data(constants.STATUS_AVAILABLE, + constants.STATUS_SHRINKING_POSSIBLE_DATA_LOSS_ERROR) + def test_shrink_valid(self, share_status): + share = fake_share('fake', status=share_status, size=100) + new_size = 50 + self.mock_object(self.api, 'update') + self.mock_object(self.api.share_rpcapi, 'shrink_share') + + self.api.shrink(self.context, share, new_size) + + self.api.update.assert_called_once_with( + self.context, share, {'status': constants.STATUS_SHRINKING}) + self.api.share_rpcapi.shrink_share.assert_called_once_with( + self.context, share, new_size + ) + class OtherTenantsShareActionsTestCase(test.TestCase): def setUp(self): diff --git a/manila/tests/share/test_manager.py b/manila/tests/share/test_manager.py index ea71e1e576..554bfe4915 100644 --- a/manila/tests/share/test_manager.py +++ b/manila/tests/share/test_manager.py @@ -21,6 +21,7 @@ import mock from oslo_serialization import jsonutils from oslo_utils import importutils from oslo_utils import timeutils +import six from manila.common import constants from manila import context @@ -1600,3 +1601,89 @@ class ShareManagerTestCase(test.TestCase): manager.db.share_update.assert_called_once_with( mock.ANY, share_id, shr_update ) + + def test_shrink_share_quota_error(self): + size = 5 + new_size = 1 + share = self._create_share(size=size) + share_id = share['id'] + + self.mock_object(self.share_manager.db, 'share_update') + self.mock_object(quota.QUOTAS, 'reserve', + mock.Mock(side_effect=Exception('fake'))) + + self.assertRaises( + exception.ShareShrinkingError, + self.share_manager.shrink_share, self.context, share_id, new_size) + + quota.QUOTAS.reserve.assert_called_with( + mock.ANY, + project_id=six.text_type(share['project_id']), + gigabytes=new_size - size + ) + self.assertTrue(self.share_manager.db.share_update.called) + + @ddt.data({'exc': exception.InvalidShare('fake'), + 'status': constants.STATUS_SHRINKING_ERROR}, + {'exc': exception.ShareShrinkingPossibleDataLoss("fake"), + 'status': constants.STATUS_SHRINKING_POSSIBLE_DATA_LOSS_ERROR}) + @ddt.unpack + def test_shrink_share_invalid(self, exc, status): + share = self._create_share() + new_size = 1 + share_id = share['id'] + + self.mock_object(self.share_manager, 'driver') + self.mock_object(self.share_manager.db, 'share_update') + self.mock_object(self.share_manager.db, 'share_get', + mock.Mock(return_value=share)) + self.mock_object(quota.QUOTAS, 'reserve') + self.mock_object(quota.QUOTAS, 'rollback') + self.mock_object(self.share_manager.driver, 'shrink_share', + mock.Mock(side_effect=exc)) + + self.assertRaises( + exception.ShareShrinkingError, + self.share_manager.shrink_share, self.context, share_id, new_size) + + self.share_manager.driver.shrink_share.assert_called_once_with( + share, new_size, share_server=None) + self.share_manager.db.share_update.assert_called_once_with( + mock.ANY, share_id, {'status': status} + ) + self.assertTrue(quota.QUOTAS.reserve.called) + self.assertTrue(quota.QUOTAS.rollback.called) + self.assertTrue(self.share_manager.db.share_get.called) + + def test_shrink_share(self): + share = self._create_share() + share_id = share['id'] + new_size = 123 + shr_update = { + 'size': int(new_size), + 'status': constants.STATUS_AVAILABLE + } + fake_share_server = 'fake' + + manager = self.share_manager + self.mock_object(manager, 'driver') + self.mock_object(manager.db, 'share_get', + mock.Mock(return_value=share)) + self.mock_object(manager.db, 'share_update', + mock.Mock(return_value=share)) + self.mock_object(quota.QUOTAS, 'commit') + self.mock_object(manager.driver, 'shrink_share') + self.mock_object(manager, '_get_share_server', + mock.Mock(return_value=fake_share_server)) + + self.share_manager.shrink_share(self.context, share_id, new_size) + + self.assertTrue(manager._get_share_server.called) + manager.driver.shrink_share.assert_called_once_with( + share, new_size, share_server=fake_share_server + ) + quota.QUOTAS.commit.assert_called_once_with( + mock.ANY, mock.ANY, project_id=share['project_id']) + manager.db.share_update.assert_called_once_with( + mock.ANY, share_id, shr_update + ) diff --git a/manila/tests/share/test_rpcapi.py b/manila/tests/share/test_rpcapi.py index 0bf88bdfc2..c24f9165bb 100644 --- a/manila/tests/share/test_rpcapi.py +++ b/manila/tests/share/test_rpcapi.py @@ -170,3 +170,10 @@ class ShareRpcAPITestCase(test.TestCase): share=self.fake_share, new_size=123, reservations={'fake': 'fake'}) + + def test_shrink_share(self): + self._test_share_api('shrink_share', + rpc_method='cast', + version='1.3', + share=self.fake_share, + new_size=123)