diff --git a/manila/db/api.py b/manila/db/api.py index 364988ff26..aedf0fc4ca 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -311,9 +311,12 @@ def share_instance_create(context, share_id, values): return IMPL.share_instance_create(context, share_id, values) -def share_instance_delete(context, instance_id): +def share_instance_delete(context, instance_id, session=None, + need_to_update_usages=False): """Delete share instance.""" - return IMPL.share_instance_delete(context, instance_id) + return IMPL.share_instance_delete( + context, instance_id, session=session, + need_to_update_usages=need_to_update_usages) def share_instance_update(context, instance_id, values, with_share_data=False): diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 06ffea5f61..9368c3131b 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -49,10 +49,12 @@ from manila.common import constants from manila.db.sqlalchemy import models from manila import exception from manila.i18n import _ +from manila import quota CONF = cfg.CONF LOG = log.getLogger(__name__) +QUOTAS = quota.QUOTAS _DEFAULT_QUOTA_NAME = 'default' PER_PROJECT_QUOTAS = [] @@ -493,8 +495,8 @@ def quota_get_all_by_project_and_user(context, project_id, user_id): ).all() result = {'project_id': project_id, 'user_id': user_id} - for quota in user_quotas: - result[quota.resource] = quota.hard_limit + for u_quota in user_quotas: + result[u_quota.resource] = u_quota.hard_limit return result @@ -516,8 +518,8 @@ def quota_get_all_by_project_and_share_type(context, project_id, 'project_id': project_id, 'share_type_id': share_type_id, } - for quota in share_type_quotas: - result[quota.resource] = quota.hard_limit + for st_quota in share_type_quotas: + result[st_quota.resource] = st_quota.hard_limit return result @@ -531,8 +533,8 @@ def quota_get_all_by_project(context, project_id): ).all() result = {'project_id': project_id} - for quota in project_quotas: - result[quota.resource] = quota.hard_limit + for p_quota in project_quotas: + result[p_quota.resource] = p_quota.hard_limit return result @@ -1416,7 +1418,8 @@ def share_instances_get_all(context, filters=None): @require_context -def share_instance_delete(context, instance_id, session=None): +def share_instance_delete(context, instance_id, session=None, + need_to_update_usages=False): if session is None: session = get_session() @@ -1432,6 +1435,31 @@ def share_instance_delete(context, instance_id, session=None): share_id=share['id']).soft_delete() share.soft_delete(session=session) + if need_to_update_usages: + reservations = None + try: + # we give the user_id of the share, to update + # the quota usage for the user, who created the share + reservations = QUOTAS.reserve( + context, + project_id=share['project_id'], + shares=-1, + gigabytes=-share['size'], + user_id=share['user_id'], + share_type_id=instance_ref['share_type_id']) + QUOTAS.commit( + context, reservations, project_id=share['project_id'], + user_id=share['user_id'], + share_type_id=instance_ref['share_type_id']) + except Exception: + LOG.exception( + "Failed to update usages deleting share '%s'.", + share["id"]) + if reservations: + QUOTAS.rollback( + context, reservations, + share_type_id=instance_ref['share_type_id']) + def _set_instances_share_data(context, instances, session): if instances and not isinstance(instances, list): diff --git a/manila/share/api.py b/manila/share/api.py index 21d3531719..d0ce4948ce 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -886,13 +886,7 @@ class API(base.Base): def delete(self, context, share, force=False): """Delete share.""" share = self.db.share_get(context, share['id']) - if context.is_admin and context.project_id != share['project_id']: - project_id = share['project_id'] - else: - project_id = context.project_id - share_id = share['id'] - statuses = (constants.STATUS_AVAILABLE, constants.STATUS_ERROR, constants.STATUS_INACTIVE) if not (force or share['status'] in statuses): @@ -922,36 +916,12 @@ class API(base.Base): raise exception.InvalidShare(reason=msg) self._check_is_share_busy(share) - - try: - # we give the user_id of the share, to update the quota usage - # for the user, who created the share - reservations = QUOTAS.reserve( - context, - project_id=project_id, - shares=-1, - gigabytes=-share['size'], - user_id=share['user_id'], - share_type_id=share['instance']['share_type_id']) - except Exception as e: - reservations = None - LOG.exception( - ("Failed to update quota for deleting share: %s"), e) - for share_instance in share.instances: if share_instance['host']: self.delete_instance(context, share_instance, force=force) else: - self.db.share_instance_delete(context, share_instance['id']) - - if reservations: - # we give the user_id of the share, to update the quota usage - # for the user, who created the share - QUOTAS.commit( - context, reservations, project_id=project_id, - user_id=share['user_id'], - share_type_id=share['instance']['share_type_id'], - ) + self.db.share_instance_delete( + context, share_instance['id'], need_to_update_usages=True) def delete_instance(self, context, share_instance, force=False): policy.check_policy(context, 'share', 'delete') diff --git a/manila/share/manager.py b/manila/share/manager.py index 8d3c0ba551..8c353f22b2 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -2754,7 +2754,9 @@ class ShareManager(manager.SchedulerDependentManager): resource_id=share_instance_id, exception=excep) - self.db.share_instance_delete(context, share_instance_id) + self.db.share_instance_delete( + context, share_instance_id, need_to_update_usages=True) + LOG.info("Share instance %s: deleted successfully.", share_instance_id) diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 6549fe624a..b810a29741 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -33,9 +33,12 @@ from manila import context from manila.db.sqlalchemy import api as db_api from manila.db.sqlalchemy import models from manila import exception +from manila import quota from manila import test from manila.tests import db_utils +QUOTAS = quota.QUOTAS + security_service_dict = { 'id': 'fake id', 'project_id': 'fake project', @@ -289,6 +292,39 @@ class ShareDatabaseAPITestCase(test.TestCase): self.assertRaises(exception.NotFound, db_api.share_metadata_get, self.ctxt, share['id']) + def test_share_instance_delete_with_share_need_to_update_usages(self): + share = db_utils.create_share() + + self.assertIsNotNone(db_api.share_get(self.ctxt, share['id'])) + self.assertIsNotNone(db_api.share_metadata_get(self.ctxt, share['id'])) + + self.mock_object(quota.QUOTAS, 'reserve', + mock.Mock(return_value='reservation')) + self.mock_object(quota.QUOTAS, 'commit') + + db_api.share_instance_delete( + self.ctxt, share.instance['id'], need_to_update_usages=True) + + self.assertRaises(exception.NotFound, db_api.share_get, + self.ctxt, share['id']) + self.assertRaises(exception.NotFound, db_api.share_metadata_get, + self.ctxt, share['id']) + quota.QUOTAS.reserve.assert_called_once_with( + self.ctxt, + project_id=share['project_id'], + shares=-1, + gigabytes=-share['size'], + share_type_id=None, + user_id=share['user_id'] + ) + quota.QUOTAS.commit.assert_called_once_with( + self.ctxt, + mock.ANY, + project_id=share['project_id'], + share_type_id=None, + user_id=share['user_id'] + ) + def test_share_instance_get(self): share = db_utils.create_share() diff --git a/manila/tests/share/test_api.py b/manila/tests/share/test_api.py index 6975e7ddba..3f36506737 100644 --- a/manila/tests/share/test_api.py +++ b/manila/tests/share/test_api.py @@ -212,9 +212,6 @@ class ShareAPITestCase(test.TestCase): self.mock_object(db_api, 'share_snapshot_get_all_for_share', mock.Mock(return_value=snapshots)) self.mock_object(self.api, 'delete_instance') - self.mock_object(quota.QUOTAS, 'reserve', - mock.Mock(return_value='reservation')) - self.mock_object(quota.QUOTAS, 'commit') return share def _setup_delete_share_instance_mocks(self, **kwargs): @@ -1758,22 +1755,6 @@ class ShareAPITestCase(test.TestCase): self.api.delete(diff_user_context, share) - quota.QUOTAS.reserve.assert_called_once_with( - diff_user_context, - project_id=share['project_id'], - shares=-1, - gigabytes=-share['size'], - share_type_id=None, - user_id=share['user_id'] - ) - quota.QUOTAS.commit.assert_called_once_with( - diff_user_context, - mock.ANY, - project_id=share['project_id'], - share_type_id=None, - user_id=share['user_id'] - ) - def test_delete_wrong_status(self): share = fakes.fake_share(status='wrongstatus') self.mock_object(db_api, 'share_get', mock.Mock(return_value=share)) @@ -1807,7 +1788,8 @@ class ShareAPITestCase(test.TestCase): self.api.delete(self.context, share) db_api.share_instance_delete.assert_called_once_with( - utils.IsAMatcher(context.RequestContext), share.instance['id']) + utils.IsAMatcher(context.RequestContext), share.instance['id'], + need_to_update_usages=True) def test_delete_share_with_snapshots(self): share = self._setup_delete_mocks(constants.STATUS_AVAILABLE, @@ -1836,16 +1818,6 @@ class ShareAPITestCase(test.TestCase): self.api.delete(self.context, share) - quota.QUOTAS.reserve.assert_called_once_with( - self.context, - project_id=share['project_id'], - shares=-1, - gigabytes=-share['size'], - share_type_id=None, - user_id=share['user_id'] - ) - self.assertFalse(quota.QUOTAS.commit.called) - @ddt.data({'status': constants.STATUS_AVAILABLE, 'force': False}, {'status': constants.STATUS_ERROR, 'force': True}) @ddt.unpack