diff --git a/karbor/api/v1/providers.py b/karbor/api/v1/providers.py index 3549b031..c151602b 100644 --- a/karbor/api/v1/providers.py +++ b/karbor/api/v1/providers.py @@ -29,6 +29,7 @@ from karbor.i18n import _ from karbor import objects from karbor.policies import providers as provider_policy +from karbor import quota from karbor.services.protection import api as protection_api from karbor import utils @@ -44,6 +45,7 @@ query_provider_filters_opts = [ ) ), ] +QUOTAS = quota.QUOTAS query_checkpoint_filters_opts = [ cfg.ListOpt( @@ -391,20 +393,33 @@ class ProvidersController(wsgi.Controller): }, "extra_info": checkpoint_extra_info } + try: - checkpoint_id = self.protection_api.protect(context, plan, - checkpoint_properties) - except Exception as error: - msg = _("Create checkpoint failed: %s") % error - raise exc.HTTPBadRequest(explanation=msg) + reserve_opts = {'checkpoints': 1} + reservations = QUOTAS.reserve(context, **reserve_opts) + except exception.OverQuota as e: + quota.process_reserve_over_quota( + context, e, + resource='checkpoints') + else: + checkpoint_id = None + try: + checkpoint_id = self.protection_api.protect( + context, plan, checkpoint_properties) + QUOTAS.commit(context, reservations) + except Exception as error: + if not checkpoint_id: + QUOTAS.rollback(context, reservations) + msg = _("Create checkpoint failed: %s") % error + raise exc.HTTPBadRequest(explanation=msg) - checkpoint_properties['id'] = checkpoint_id + checkpoint_properties['id'] = checkpoint_id - LOG.info("Create the checkpoint successfully. checkpoint_id:%s", - checkpoint_id) - returnval = self._checkpoint_view_builder.detail( - req, checkpoint_properties) - return returnval + LOG.info("Create the checkpoint successfully. checkpoint_id:%s", + checkpoint_id) + returnval = self._checkpoint_view_builder.detail( + req, checkpoint_properties) + return returnval def checkpoints_show(self, req, provider_id, checkpoint_id): """Return data about the given checkpoint id.""" @@ -455,24 +470,33 @@ class ProvidersController(wsgi.Controller): def checkpoints_delete(self, req, provider_id, checkpoint_id): """Delete a checkpoint.""" context = req.environ['karbor.context'] + context.can(provider_policy.CHECKPOINT_DELETE_POLICY) LOG.info("Delete checkpoint with id: %s.", checkpoint_id) LOG.info("provider_id: %s.", provider_id) + try: + checkpoint = self._checkpoint_get(context, provider_id, + checkpoint_id) + except exception.CheckpointNotFound as error: + raise exc.HTTPNotFound(explanation=error.msg) + except exception.AccessCheckpointNotAllowed as error: + raise exc.HTTPForbidden(explanation=error.msg) + project_id = checkpoint.get('project_id') - if not uuidutils.is_uuid_like(provider_id): - msg = _("Invalid provider id provided.") - raise exc.HTTPBadRequest(explanation=msg) - - if not uuidutils.is_uuid_like(checkpoint_id): - msg = _("Invalid checkpoint id provided.") - raise exc.HTTPBadRequest(explanation=msg) - - context.can(provider_policy.CHECKPOINT_DELETE_POLICY) try: self.protection_api.delete(context, provider_id, checkpoint_id) except exception.DeleteCheckpointNotAllowed as error: raise exc.HTTPForbidden(explantion=error.msg) + try: + reserve_opts = {'checkpoints': -1} + reservations = QUOTAS.reserve( + context, project_id=project_id, **reserve_opts) + except Exception: + LOG.exception("Failed to update usages after deleting checkpoint.") + else: + QUOTAS.commit(context, reservations, project_id=project_id) + LOG.info("Delete checkpoint request issued successfully.") return {} diff --git a/karbor/api/v1/quota_classes.py b/karbor/api/v1/quota_classes.py index 8f14f2d6..39fb44c1 100644 --- a/karbor/api/v1/quota_classes.py +++ b/karbor/api/v1/quota_classes.py @@ -44,6 +44,7 @@ class QuotaClassesViewBuilder(common.ViewBuilder): """Detailed view of a single quota class.""" keys = ( 'plans', + 'checkpoints', ) view = {key: quota.get(key) for key in keys} if quota_class: diff --git a/karbor/api/v1/quotas.py b/karbor/api/v1/quotas.py index d644c367..5303a055 100644 --- a/karbor/api/v1/quotas.py +++ b/karbor/api/v1/quotas.py @@ -46,6 +46,7 @@ class QuotasViewBuilder(common.ViewBuilder): """Detailed view of a single quota.""" keys = ( 'plans', + 'checkpoints', ) view = {key: quota.get(key) for key in keys} if project_id: diff --git a/karbor/exception.py b/karbor/exception.py index f5fc1274..7d3e96fa 100644 --- a/karbor/exception.py +++ b/karbor/exception.py @@ -447,6 +447,10 @@ class PlanLimitExceeded(QuotaError): message = _("Maximum number of plans allowed (%(allowed)d) exceeded") +class CheckpointLimitExceeded(QuotaError): + message = _("Maximum number of checkpoints allowed (%(allowed)d) exceeded") + + class UnexpectedOverQuota(QuotaError): message = _("Unexpected over quota on %(name)s.") diff --git a/karbor/quota.py b/karbor/quota.py index 5e32659e..d18c6950 100644 --- a/karbor/quota.py +++ b/karbor/quota.py @@ -36,6 +36,9 @@ quota_opts = [ cfg.IntOpt('quota_plans', default=50, help='The number of volume backups allowed per project'), + cfg.IntOpt('quota_checkpoints', + default=-1, + help='The number of checkpoints allowed per project'), cfg.IntOpt('reservation_expire', default=86400, help='number of seconds until a reservation expires'), @@ -766,13 +769,18 @@ QUOTAS = QuotaEngine() resources = [ ReservableResource('plans', None, 'quota_plans'), + ReservableResource('checkpoints', None, + 'quota_checkpoints'), ] QUOTAS.register_resources(resources) -OVER_QUOTA_RESOURCE_EXCEPTIONS = {'plans': exception.PlanLimitExceeded} +OVER_QUOTA_RESOURCE_EXCEPTIONS = { + 'plans': exception.PlanLimitExceeded, + 'checkpoints': exception.CheckpointLimitExceeded +} def process_reserve_over_quota(context, over_quota_exception, diff --git a/karbor/tests/unit/api/v1/test_providers.py b/karbor/tests/unit/api/v1/test_providers.py index d1c3fcf1..be5d5e48 100644 --- a/karbor/tests/unit/api/v1/test_providers.py +++ b/karbor/tests/unit/api/v1/test_providers.py @@ -53,10 +53,10 @@ class ProvidersApiTest(base.TestCase): @mock.patch( 'karbor.services.protection.api.API.' 'show_checkpoint') - def test_checkpoint_show(self, moak_show_checkpoint): + def test_checkpoint_show(self, mock_show_checkpoint): req = fakes.HTTPRequest.blank('/v1/providers/' '{provider_id}/checkpoints/') - moak_show_checkpoint.return_value = { + mock_show_checkpoint.return_value = { "provider_id": "efc6a88b-9096-4bb6-8634-cda182a6e12a", "project_id": "446a04d8-6ff5-4e0e-99a4-827a6389e9ff", "id": "2220f8b1-975d-4621-a872-fa9afb43cb6c" @@ -66,7 +66,7 @@ class ProvidersApiTest(base.TestCase): '2220f8b1-975d-4621-a872-fa9afb43cb6c', '2220f8b1-975d-4621-a872-fa9afb43cb6c' ) - self.assertTrue(moak_show_checkpoint.called) + self.assertTrue(mock_show_checkpoint.called) @mock.patch( 'karbor.services.protection.api.API.' @@ -101,54 +101,40 @@ class ProvidersApiTest(base.TestCase): '2220f8b1-975d-4621-a872-fa9afb43cb6c') self.assertTrue(moak_list_checkpoints.called) - @mock.patch( - 'karbor.services.protection.api.API.' - 'delete') - def test_checkpoints_delete(self, moak_delete): + @mock.patch('karbor.quota.QuotaEngine.commit') + @mock.patch('karbor.quota.QuotaEngine.reserve') + @mock.patch('karbor.services.protection.api.API.show_checkpoint') + @mock.patch('karbor.services.protection.api.API.delete') + def test_checkpoints_delete(self, mock_delete, mock_show_checkpoint, + mock_reserve, mock_commit): req = fakes.HTTPRequest.blank('/v1/providers/' '{provider_id}/checkpoints/') + mock_show_checkpoint.return_value = { + "provider_id": "efc6a88b-9096-4bb6-8634-cda182a6e12a", + "project_id": "446a04d8-6ff5-4e0e-99a4-827a6389e9ff", + "id": "2220f8b1-975d-4621-a872-fa9afb43cb6c" + } self.controller.checkpoints_delete( req, '2220f8b1-975d-4621-a872-fa9afb43cb6c', '2220f8b1-975d-4621-a872-fa9afb43cb6c') - self.assertTrue(moak_delete.called) + self.assertTrue(mock_delete.called) + self.assertTrue(mock_reserve.called) + self.assertTrue(mock_commit.called) - def test_checkpoints_delete_with_invalid_provider_id(self): - req = fakes.HTTPRequest.blank('/v1/providers/' - '{provider_id}/checkpoints/') - invalid_provider_id = '1' - self.assertRaises(exc.HTTPBadRequest, - self.controller.checkpoints_delete, - req, - invalid_provider_id, - '2220f8b1-975d-4621-a872-fa9afb43cb6c' - ) - - def test_checkpoints_delete_with_invalid_checkpoint_id(self): - req = fakes.HTTPRequest.blank('/v1/providers/' - '{provider_id}/checkpoints/') - invalid_checkpoint_id = '1' - self.assertRaises(exc.HTTPBadRequest, - self.controller.checkpoints_delete, - req, - '2220f8b1-975d-4621-a872-fa9afb43cb6c', - invalid_checkpoint_id - ) - - @mock.patch( - 'karbor.services.protection.api.API.' - 'protect') - @mock.patch( - 'karbor.objects.plan.Plan.get_by_id') - def test_checkpoints_create(self, mock_plan_create, - mock_protect): + @mock.patch('karbor.quota.QuotaEngine.commit') + @mock.patch('karbor.quota.QuotaEngine.reserve') + @mock.patch('karbor.services.protection.api.API.protect') + @mock.patch('karbor.objects.plan.Plan.get_by_id') + def test_checkpoints_create(self, mock_plan_get, mock_protect, + mock_reserve, mock_commit): checkpoint = { "plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6" } body = {"checkpoint": checkpoint} req = fakes.HTTPRequest.blank('/v1/providers/' '{provider_id}/checkpoints/') - mock_plan_create.return_value = { + mock_plan_get.return_value = { "plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6", "provider_id": "2220f8b1-975d-4621-a872-fa9afb43cb6c" } @@ -159,78 +145,57 @@ class ProvidersApiTest(base.TestCase): req, '2220f8b1-975d-4621-a872-fa9afb43cb6c', body=body) - self.assertTrue(mock_plan_create.called) + self.assertTrue(mock_plan_get.called) + self.assertTrue(mock_reserve.called) self.assertTrue(mock_protect.called) + self.assertTrue(mock_commit.called) - @mock.patch( - 'karbor.services.protection.api.API.' - 'protect') - @mock.patch( - 'karbor.objects.plan.Plan.get_by_id') - def test_checkpoints_create_with_invalid_provider_id(self, - mock_plan_create, - mock_protect): - checkpoint = { - "plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6" - } + @mock.patch('karbor.quota.process_reserve_over_quota') + @mock.patch('karbor.quota.QuotaEngine.reserve') + @mock.patch('karbor.services.protection.api.API.protect') + @mock.patch('karbor.objects.plan.Plan.get_by_id') + def test_checkpoints_create_with_over_quota_exception( + self, mock_plan_get, mock_protect, mock_quota_reserve, + mock_process_reserve_over_quota): + checkpoint = {"plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6"} body = {"checkpoint": checkpoint} req = fakes.HTTPRequest.blank('/v1/providers/' '{provider_id}/checkpoints/') - mock_plan_create.return_value = { + mock_plan_get.return_value = { "plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6", "provider_id": "2220f8b1-975d-4621-a872-fa9afb43cb6c" } mock_protect.return_value = { "checkpoint_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6" } - invalid_provider_id = None - self.assertRaises(exception.InvalidInput, - self.controller.checkpoints_create, req, - invalid_provider_id, - body=body) + mock_quota_reserve.side_effect = exception.OverQuota + self.controller.checkpoints_create( + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body=body) + self.assertTrue(mock_process_reserve_over_quota.called) - @mock.patch( - 'karbor.services.protection.api.API.' - 'protect') - @mock.patch( - 'karbor.objects.plan.Plan.get_by_id') - def test_checkpoints_create_with_non_exist_plan(self, - mock_plan_create, - mock_protect): - checkpoint = { - "plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6" - } - body = {"checkpoint": checkpoint} - req = fakes.HTTPRequest.blank( - '/v1/providers/{provider_id}/checkpoints/') - mock_plan_create.return_value = None - mock_protect.return_value = { - "checkpoint_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6" - } - self.assertRaises(exception.PlanNotFound, - self.controller.checkpoints_create, req, - "2220f8b1-975d-4621-a872-fa9afb43cb6c", body=body) - - @mock.patch( - 'karbor.services.protection.api.API.protect') - @mock.patch( - 'karbor.objects.plan.Plan.get_by_id') - def test_checkpoints_create_with_invalid_plan(self, - mock_plan_create, - mock_protect): + @mock.patch('karbor.quota.QuotaEngine.rollback') + @mock.patch('karbor.services.protection.api.API.protect') + @mock.patch('karbor.objects.plan.Plan.get_by_id') + def test_checkpoint_create_failed_with_protection_exception( + self, mock_plan_get, mock_protect, mock_quota_rollback): checkpoint = {"plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6"} body = {"checkpoint": checkpoint} - req = fakes.HTTPRequest.blank( - '/v1/providers/{provider_id}/checkpoints/') - mock_plan_create.return_value = \ - {"plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6", - "provider_id": "2220f8b1-975d-4621-a872-fa9afb43cb6c"} - mock_protect.return_value = \ - {"checkpoint_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6"} - self.assertRaises(exception.InvalidPlan, - self.controller.checkpoints_create, - req, "2220f8b1-5ea6-4621-a872-fa9afb43cb6c", - body=body) + req = fakes.HTTPRequest.blank('/v1/providers/' + '{provider_id}/checkpoints/') + mock_plan_get.return_value = { + "plan_id": "2c3a12ee-5ea6-406a-8b64-862711ff85e6", + "provider_id": "2220f8b1-975d-4621-a872-fa9afb43cb6c" + } + mock_protect.side_effect = Exception + self.assertRaises( + exc.HTTPBadRequest, + self.controller.checkpoints_create, + req, + '2220f8b1-975d-4621-a872-fa9afb43cb6c', + body=body) + self.assertTrue(mock_quota_rollback.called) @mock.patch('karbor.services.protection.api.API.reset_state') def test_checkpoints_update_reset_state(self, mock_reset_state):