From 0ee1ea83b90de8d342fa242e311f1680fef66a92 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Mon, 9 Mar 2015 19:39:11 +0100 Subject: [PATCH] Preserve usage and reservations on quota deletion Current API deletes quota usage and reservations on quota limit deletion. According to API documentation what should only happen is that quotas limits revert to default values by deleting tenant/user limits. This patch fixes this issue. APIImpact: Delete on os-quota-sets will no longer remove usage and reservation quotas. Those quotas are handled by Cinder service. UpgradeImpact: There is no upgrade impact afaik. Closes-Bug: #1410034 Change-Id: I9340b6f78623cfa5b505886ad75b8e4d3cd6131b --- cinder/api/contrib/quotas.py | 2 +- cinder/db/api.py | 4 +-- cinder/db/sqlalchemy/api.py | 22 +++++++++++- cinder/quota.py | 16 ++++----- cinder/tests/unit/test_db_api.py | 59 +++++++++++++++++++++++++++----- cinder/tests/unit/test_quota.py | 34 +++++++++--------- 6 files changed, 99 insertions(+), 38 deletions(-) diff --git a/cinder/api/contrib/quotas.py b/cinder/api/contrib/quotas.py index 32667cad3..5c3c5fd5a 100644 --- a/cinder/api/contrib/quotas.py +++ b/cinder/api/contrib/quotas.py @@ -155,7 +155,7 @@ class QuotaSetsController(wsgi.Controller): authorize_delete(context) try: - db.quota_destroy_all_by_project(context, id) + db.quota_destroy_by_project(context, id) except exception.AdminRequired: raise webob.exc.HTTPForbidden() diff --git a/cinder/db/api.py b/cinder/db/api.py index f0726fea9..c82689bc5 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -793,9 +793,9 @@ def reservation_rollback(context, reservations, project_id=None): project_id=project_id) -def quota_destroy_all_by_project(context, project_id): +def quota_destroy_by_project(context, project_id): """Destroy all quotas associated with a given project.""" - return IMPL.quota_destroy_all_by_project(context, project_id) + return IMPL.quota_destroy_by_project(context, project_id) def reservation_expire(context): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index a728dc501..b2dd5c8cd 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -928,9 +928,26 @@ def reservation_rollback(context, reservations, project_id=None): reservation.delete(session=session) +def quota_destroy_by_project(*args, **kwargs): + """Destroy all limit quotas associated with a project. + + Leaves usage and reservation quotas intact. + """ + quota_destroy_all_by_project(only_quotas=True, *args, **kwargs) + + @require_admin_context @_retry_on_deadlock -def quota_destroy_all_by_project(context, project_id): +def quota_destroy_all_by_project(context, project_id, only_quotas=False): + """Destroy all quotas associated with a project. + + This includes limit quotas, usage quotas and reservation quotas. + Optionally can only remove limit quotas and leave other types as they are. + + :param context: The request context, for access checks. + :param project_id: The ID of the project being deleted. + :param only_quotas: Only delete limit quotas, leave other types intact. + """ session = get_session() with session.begin(): quotas = model_query(context, models.Quota, session=session, @@ -941,6 +958,9 @@ def quota_destroy_all_by_project(context, project_id): for quota_ref in quotas: quota_ref.delete(session=session) + if only_quotas: + return + quota_usages = model_query(context, models.QuotaUsage, session=session, read_deleted="no").\ filter_by(project_id=project_id).\ diff --git a/cinder/quota.py b/cinder/quota.py index eefedbed4..717955d68 100644 --- a/cinder/quota.py +++ b/cinder/quota.py @@ -402,16 +402,15 @@ class DbQuotaDriver(object): db.reservation_rollback(context, reservations, project_id=project_id) - def destroy_all_by_project(self, context, project_id): - """Destroy all that is associated with a project. + def destroy_by_project(self, context, project_id): + """Destroy all limit quotas associated with a project. - This includes quotas, usages and reservations. + Leave usage and reservation quotas intact. :param context: The request context, for access checks. :param project_id: The ID of the project being deleted. """ - - db.quota_destroy_all_by_project(context, project_id) + db.quota_destroy_by_project(context, project_id) def expire(self, context): """Expire reservations. @@ -806,15 +805,14 @@ class QuotaEngine(object): LOG.exception(_LE("Failed to roll back reservations " "%s"), reservations) - def destroy_all_by_project(self, context, project_id): - """Destroy all quotas, usages, and reservations associated with a - project. + def destroy_by_project(self, context, project_id): + """Destroy all quota limits associated with a project. :param context: The request context, for access checks. :param project_id: The ID of the project being deleted. """ - self._driver.destroy_all_by_project(context, project_id) + self._driver.destroy_by_project(context, project_id) def expire(self, context): """Expire reservations. diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index c7056dc27..85147ae9e 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -1361,14 +1361,57 @@ class DBAPIQuotaTestCase(BaseTest): self.assertRaises(exception.ProjectQuotaNotFound, db.quota_get, self.ctxt, 'project1', 'resource1') - def test_quota_destroy_all_by_project(self): - _quota_reserve(self.ctxt, 'project1') - db.quota_destroy_all_by_project(self.ctxt, 'project1') - self.assertEqual(db.quota_get_all_by_project(self.ctxt, 'project1'), - {'project_id': 'project1'}) - self.assertEqual(db.quota_usage_get_all_by_project(self.ctxt, - 'project1'), - {'project_id': 'project1'}) + def test_quota_destroy_by_project(self): + # Create limits, reservations and usage for project + project = 'project1' + _quota_reserve(self.ctxt, project) + expected_usage = {'project_id': project, + 'volumes': {'reserved': 1, 'in_use': 0}, + 'gigabytes': {'reserved': 2, 'in_use': 0}} + expected = {'project_id': project, 'gigabytes': 2, 'volumes': 1} + + # Check that quotas are there + self.assertEqual(expected, + db.quota_get_all_by_project(self.ctxt, project)) + self.assertEqual(expected_usage, + db.quota_usage_get_all_by_project(self.ctxt, project)) + + # Destroy only the limits + db.quota_destroy_by_project(self.ctxt, project) + + # Confirm that limits have been removed + self.assertEqual({'project_id': project}, + db.quota_get_all_by_project(self.ctxt, project)) + + # But that usage and reservations are the same + self.assertEqual(expected_usage, + db.quota_usage_get_all_by_project(self.ctxt, project)) + + def test_quota_destroy_sqlalchemy_all_by_project_(self): + # Create limits, reservations and usage for project + project = 'project1' + _quota_reserve(self.ctxt, project) + expected_usage = {'project_id': project, + 'volumes': {'reserved': 1, 'in_use': 0}, + 'gigabytes': {'reserved': 2, 'in_use': 0}} + expected = {'project_id': project, 'gigabytes': 2, 'volumes': 1} + expected_result = {'project_id': project} + + # Check that quotas are there + self.assertEqual(expected, + db.quota_get_all_by_project(self.ctxt, project)) + self.assertEqual(expected_usage, + db.quota_usage_get_all_by_project(self.ctxt, project)) + + # Destroy all quotas using SQLAlchemy Implementation + sqlalchemy_api.quota_destroy_all_by_project(self.ctxt, project, + only_quotas=False) + + # Check that all quotas have been deleted + self.assertEqual(expected_result, + db.quota_get_all_by_project(self.ctxt, project)) + self.assertEqual(expected_result, + db.quota_usage_get_all_by_project(self.ctxt, project)) def test_quota_usage_get_nonexistent(self): self.assertRaises(exception.QuotaUsageNotFound, diff --git a/cinder/tests/unit/test_quota.py b/cinder/tests/unit/test_quota.py index 90c098135..0d543e02e 100644 --- a/cinder/tests/unit/test_quota.py +++ b/cinder/tests/unit/test_quota.py @@ -344,8 +344,8 @@ class FakeDriver(object): def rollback(self, context, reservations, project_id=None): self.called.append(('rollback', context, reservations, project_id)) - def destroy_all_by_project(self, context, project_id): - self.called.append(('destroy_all_by_project', context, project_id)) + def destroy_by_project(self, context, project_id): + self.called.append(('destroy_by_project', context, project_id)) def expire(self, context): self.called.append(('expire', context)) @@ -730,14 +730,14 @@ class QuotaEngineTestCase(test.TestCase): 'resv-03'], None), ]) - def test_destroy_all_by_project(self): + def test_destroy_by_project(self): context = FakeContext(None, None) driver = FakeDriver() quota_obj = self._make_quota_obj(driver) - quota_obj.destroy_all_by_project(context, 'test_project') + quota_obj.destroy_by_project(context, 'test_project') self.assertEqual(driver.called, - [('destroy_all_by_project', + [('destroy_by_project', context, 'test_project'), ]) @@ -1187,19 +1187,19 @@ class DbQuotaDriverTestCase(test.TestCase): ('quota_reserve', expire, 0, 86400), ]) self.assertEqual(result, ['resv-1', 'resv-2', 'resv-3']) - def _stub_quota_destroy_all_by_project(self): - def fake_quota_destroy_all_by_project(context, project_id): - self.calls.append(('quota_destroy_all_by_project', project_id)) + def _stub_quota_destroy_by_project(self): + def fake_quota_destroy_by_project(context, project_id): + self.calls.append(('quota_destroy_by_project', project_id)) return None - self.stubs.Set(sqa_api, 'quota_destroy_all_by_project', - fake_quota_destroy_all_by_project) - - def test_destroy_by_project(self): - self._stub_quota_destroy_all_by_project() - self.driver.destroy_all_by_project(FakeContext('test_project', - 'test_class'), - 'test_project') - self.assertEqual(self.calls, [('quota_destroy_all_by_project', + self.stubs.Set(sqa_api, 'quota_destroy_by_project', + fake_quota_destroy_by_project) + + def test_destroy_quota_by_project(self): + self._stub_quota_destroy_by_project() + self.driver.destroy_by_project(FakeContext('test_project', + 'test_class'), + 'test_project') + self.assertEqual(self.calls, [('quota_destroy_by_project', ('test_project')), ]) -- 2.45.2