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

diff --git a/cinder/api/contrib/quotas.py b/cinder/api/contrib/quotas.py
index 32667cad345b3d060416c63533aa055703039030..5c3c5fd5a0a7898a5c7bb088d10302b762686422 100644 (file)
--- 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 f0726fea9d360c18d3ef7c8241e125e20f7b01ec..c82689bc54e8f1da1fff2cf4bc649d88132346d6 100644 (file)
--- 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 a728dc501ca669114abb0fe328a9e9ced5d2452a..b2dd5c8cdcd0dd1e2f7ef12b312d628a6572e5c9 100644 (file)
--- 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 eefedbed4b60ff25be2444bb49b1a83ee21d12af..717955d68c7b25cb71e4668005b4ac4137ab95da 100644 (file)
--- 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 c7056dc27b4d7e309e0c977099c361a806f1b684..85147ae9e8c391f3194d00b98d89a03f3792d684 100644 (file)
--- 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 90c098135a943a1d433ec4f81f2e9d500d73aadb..0d543e02edd0dca26bd7a08d5869ef755dc0f891 100644 (file)
--- 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')), ])