Move resource usage sync functions to db backend

Resource usage sync functions was declared in cinder/quota.py, and
using db.api public methods. This functions was moved to database
backend implementation, so now sync functions can use private
methods of database backend, and session attribute can be removed
from this public methods.

Blueprint: db-session-cleanup

Change-Id: If5386e3dc1e0d6e3127732aeb5b35bbd96bc93f0

diff --git a/cinder/db/api.py b/cinder/db/api.py
index 12357d69f95e609529cf82277f7ad9f53c363694..cc1536adb4e3198332dcf8f4b6246f93e6e6112f 100644 (file)
--- a/cinder/db/api.py
+++ b/cinder/db/api.py
@@ -188,13 +188,9 @@ def volume_data_get_for_host(context, host):
                                          host)
 
 
-def volume_data_get_for_project(context, project_id, volume_type_id=None,
-                                session=None):
+def volume_data_get_for_project(context, project_id):
     """Get (volume_count, gigabytes) for project."""
-    return IMPL.volume_data_get_for_project(context,
-                                            project_id,
-                                            volume_type_id,
-                                            session)
+    return IMPL.volume_data_get_for_project(context, project_id)
 
 
 def finish_volume_migration(context, src_vol_id, dest_vol_id):
@@ -295,13 +291,11 @@ def snapshot_update(context, snapshot_id, values):
     return IMPL.snapshot_update(context, snapshot_id, values)
 
 
-def snapshot_data_get_for_project(context, project_id, volume_type_id=None,
-                                  session=None):
+def snapshot_data_get_for_project(context, project_id, volume_type_id=None):
     """Get count and gigabytes used for snapshots for specified project."""
     return IMPL.snapshot_data_get_for_project(context,
                                               project_id,
-                                              volume_type_id,
-                                              session)
+                                              volume_type_id)
 
 
 def snapshot_get_active_by_window(context, begin, end=None, project_id=None):

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 2a2bed9e9a2ce95ebc84ee2910e8ad289968f46f..a8149807171e97d3b6a3079f426ca58269719aa0 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -241,6 +241,47 @@ def exact_filter(query, model, filters, legal_keys):
     return query
 
 
+def _sync_volumes(context, project_id, session, volume_type_id=None,
+                  volume_type_name=None):
+    (volumes, gigs) = _volume_data_get_for_project(
+        context, project_id, volume_type_id=volume_type_id, session=session)
+    key = 'volumes'
+    if volume_type_name:
+        key += '_' + volume_type_name
+    return {key: volumes}
+
+
+def _sync_snapshots(context, project_id, session, volume_type_id=None,
+                    volume_type_name=None):
+    (snapshots, gigs) = _snapshot_data_get_for_project(
+        context, project_id, volume_type_id=volume_type_id, session=session)
+    key = 'snapshots'
+    if volume_type_name:
+        key += '_' + volume_type_name
+    return {key: snapshots}
+
+
+def _sync_gigabytes(context, project_id, session, volume_type_id=None,
+                    volume_type_name=None):
+    (_junk, vol_gigs) = _volume_data_get_for_project(
+        context, project_id, volume_type_id=volume_type_id, session=session)
+    key = 'gigabytes'
+    if volume_type_name:
+        key += '_' + volume_type_name
+    if CONF.no_snapshot_gb_quota:
+        return {key: vol_gigs}
+    (_junk, snap_gigs) = _snapshot_data_get_for_project(
+        context, project_id, volume_type_id=volume_type_id, session=session)
+    return {key: vol_gigs + snap_gigs}
+
+
+QUOTA_SYNC_FUNCTIONS = {
+    '_sync_volumes': _sync_volumes,
+    '_sync_snapshots': _sync_snapshots,
+    '_sync_gigabytes': _sync_gigabytes,
+}
+
+
 ###################
 
 
@@ -763,9 +804,15 @@ def quota_reserve(context, resources, quotas, deltas, expire,
             # OK, refresh the usage
             if refresh:
                 # Grab the sync routine
-                sync = resources[resource].sync
-
-                updates = sync(elevated, project_id, session)
+                sync = QUOTA_SYNC_FUNCTIONS[resources[resource].sync]
+                volume_type_id = getattr(resources[resource],
+                                         'volume_type_id', None)
+                volume_type_name = getattr(resources[resource],
+                                           'volume_type_name', None)
+                updates = sync(elevated, project_id,
+                               volume_type_id=volume_type_id,
+                               volume_type_name=volume_type_name,
+                               session=session)
                 for res, in_use in updates.items():
                     # Make sure we have a destination for the usage!
                     if res not in usages:
@@ -1042,10 +1089,8 @@ def _volume_data_get_for_project(context, project_id, volume_type_id=None,
 
 
 @require_admin_context
-def volume_data_get_for_project(context, project_id, volume_type_id=None,
-                                session=None):
-    return _volume_data_get_for_project(context, project_id, volume_type_id,
-                                        session)
+def volume_data_get_for_project(context, project_id, volume_type_id=None):
+    return _volume_data_get_for_project(context, project_id, volume_type_id)
 
 
 @require_admin_context
@@ -1416,10 +1461,8 @@ def _snapshot_data_get_for_project(context, project_id, volume_type_id=None,
 
 
 @require_context
-def snapshot_data_get_for_project(context, project_id, volume_type_id=None,
-                                  session=None):
-    return _snapshot_data_get_for_project(context, project_id, volume_type_id,
-                                          session)
+def snapshot_data_get_for_project(context, project_id, volume_type_id=None):
+    return _snapshot_data_get_for_project(context, project_id, volume_type_id)
 
 
 @require_context

diff --git a/cinder/quota.py b/cinder/quota.py
index 72e3ad27da9c98121680f55465d037a30311d3b7..30589a6c346980936a10532e503a5b01cc4ac9a3 100644 (file)
--- a/cinder/quota.py
+++ b/cinder/quota.py
@@ -89,6 +89,7 @@ class DbQuotaDriver(object):
 
     def get_defaults(self, context, resources):
         """Given a list of resources, retrieve the default quotas.
+
         Use the class quotas named `_DEFAULT_QUOTA_NAME` as default quotas,
         if it exists.
 
@@ -487,8 +488,7 @@ class ReservableResource(BaseResource):
     """Describe a reservable resource."""
 
     def __init__(self, name, sync, flag=None):
-        """
-        Initializes a ReservableResource.
+        """Initializes a ReservableResource.
 
         Reservable resources are those resources which directly
         correspond to objects in the database, i.e., volumes, gigabytes,
@@ -507,8 +507,8 @@ class ReservableResource(BaseResource):
         ReservableResource.
 
         :param name: The name of the resource, i.e., "volumes".
-        :param sync: A callable which returns a dictionary to
-                     resynchronize the in_use count for one or more
+        :param sync: A dbapi methods name which returns a dictionary
+                     to resynchronize the in_use count for one or more
                      resources, as described above.
         :param flag: The name of the flag or configuration option
                      which specifies the default value of the quota
@@ -532,8 +532,7 @@ class CountableResource(AbsoluteResource):
     """
 
     def __init__(self, name, count, flag=None):
-        """
-        Initializes a CountableResource.
+        """Initializes a CountableResource.
 
         Countable resources are those resources which directly
         correspond to objects in the database, i.e., volumes, gigabytes,
@@ -576,51 +575,10 @@ class VolumeTypeResource(ReservableResource):
         :param volume_type: The volume type for this resource.
         """
 
-        try:
-            method = getattr(self, '_sync_%s' % part_name)
-        except AttributeError:
-            raise ValueError('Invalid resource: %s' % part_name)
-
         self.volume_type_name = volume_type['name']
         self.volume_type_id = volume_type['id']
         name = "%s_%s" % (part_name, self.volume_type_name)
-        super(VolumeTypeResource, self).__init__(name, method)
-
-    def _sync_snapshots(self, context, project_id, session):
-        """Sync snapshots for this specific volume type."""
-        (snapshots, gigs) = db.snapshot_data_get_for_project(
-            context,
-            project_id,
-            volume_type_id=self.volume_type_id,
-            session=session)
-        return {'snapshots_%s' % self.volume_type_name: snapshots}
-
-    def _sync_volumes(self, context, project_id, session):
-        """Sync volumes for this specific volume type."""
-        (volumes, gigs) = db.volume_data_get_for_project(
-            context,
-            project_id,
-            volume_type_id=self.volume_type_id,
-            session=session)
-        return {'volumes_%s' % self.volume_type_name: volumes}
-
-    def _sync_gigabytes(self, context, project_id, session):
-        """Sync gigabytes for this specific volume type."""
-        key = 'gigabytes_%s' % self.volume_type_name
-        (_junk, vol_gigs) = db.volume_data_get_for_project(
-            context,
-            project_id,
-            volume_type_id=self.volume_type_id,
-            session=session)
-        if CONF.no_snapshot_gb_quota:
-            return {key: vol_gigs}
-
-        (_junk, snap_gigs) = db.snapshot_data_get_for_project(
-            context,
-            project_id,
-            volume_type_id=self.volume_type_id,
-            session=session)
-        return {key: vol_gigs + snap_gigs}
+        super(VolumeTypeResource, self).__init__(name, "_sync_%s" % part_name)
 
 
 class QuotaEngine(object):
@@ -908,9 +866,9 @@ class VolumeTypeQuotaEngine(QuotaEngine):
 
         result = {}
         # Global quotas.
-        argses = [('volumes', _sync_volumes, 'quota_volumes'),
-                  ('snapshots', _sync_snapshots, 'quota_snapshots'),
-                  ('gigabytes', _sync_gigabytes, 'quota_gigabytes'), ]
+        argses = [('volumes', '_sync_volumes', 'quota_volumes'),
+                  ('snapshots', '_sync_snapshots', 'quota_snapshots'),
+                  ('gigabytes', '_sync_gigabytes', 'quota_gigabytes'), ]
         for args in argses:
             resource = ReservableResource(*args)
             result[resource.name] = resource
@@ -932,32 +890,4 @@ class VolumeTypeQuotaEngine(QuotaEngine):
     def register_resources(self, resources):
         raise NotImplementedError(_("Cannot register resources"))
 
-
-def _sync_volumes(context, project_id, session):
-    (volumes, gigs) = db.volume_data_get_for_project(context,
-                                                     project_id,
-                                                     session=session)
-    return {'volumes': volumes}
-
-
-def _sync_snapshots(context, project_id, session):
-    (snapshots, gigs) = db.snapshot_data_get_for_project(context,
-                                                         project_id,
-                                                         session=session)
-    return {'snapshots': snapshots}
-
-
-def _sync_gigabytes(context, project_id, session):
-    (_junk, vol_gigs) = db.volume_data_get_for_project(context,
-                                                       project_id,
-                                                       session=session)
-    if CONF.no_snapshot_gb_quota:
-        return {'gigabytes': vol_gigs}
-
-    (_junk, snap_gigs) = db.snapshot_data_get_for_project(context,
-                                                          project_id,
-                                                          session=session)
-    return {'gigabytes': vol_gigs + snap_gigs}
-
-
 QUOTAS = VolumeTypeQuotaEngine()

diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py
index 0f2bced9076364a1457f0f307d534201b301baf2..891063672f66bb1bbde3dca16ca747021ce84d6c 100644 (file)
--- a/cinder/tests/test_db_api.py
+++ b/cinder/tests/test_db_api.py
@@ -44,13 +44,12 @@ def _quota_reserve(context, project_id):
     quotas = {}
     resources = {}
     deltas = {}
-    for i in range(3):
-        resource = 'res%d' % i
-        quotas[resource] = db.quota_create(context, project_id, resource, i)
-        resources[resource] = ReservableResource(
-            resource,
-            get_sync(resource, i), 'quota_res_%d' % i)
-        deltas[resource] = i
+    for i, resource in enumerate(('volumes', 'gigabytes')):
+        quotas[resource] = db.quota_create(context, project_id,
+                                           resource, i + 1)
+        resources[resource] = ReservableResource(resource,
+                                                 '_sync_%s' % resource)
+        deltas[resource] = i + 1
     return db.quota_reserve(
         context, resources, quotas, deltas,
         datetime.datetime.utcnow(), datetime.datetime.utcnow(),
@@ -534,9 +533,9 @@ class DBAPIReservationTestCase(BaseTest):
     def test_reservation_commit(self):
         reservations = _quota_reserve(self.ctxt, 'project1')
         expected = {'project_id': 'project1',
-                    'res0': {'reserved': 0, 'in_use': 0},
-                    'res1': {'reserved': 1, 'in_use': 1},
-                    'res2': {'reserved': 2, 'in_use': 2}}
+                    'volumes': {'reserved': 1, 'in_use': 0},
+                    'gigabytes': {'reserved': 2, 'in_use': 0},
+                    }
         self.assertEqual(expected,
                          db.quota_usage_get_all_by_project(
                              self.ctxt, 'project1'))
@@ -547,9 +546,9 @@ class DBAPIReservationTestCase(BaseTest):
                           self.ctxt,
                           reservations[0])
         expected = {'project_id': 'project1',
-                    'res0': {'reserved': 0, 'in_use': 0},
-                    'res1': {'reserved': 0, 'in_use': 2},
-                    'res2': {'reserved': 0, 'in_use': 4}}
+                    'volumes': {'reserved': 0, 'in_use': 1},
+                    'gigabytes': {'reserved': 0, 'in_use': 2},
+                    }
         self.assertEqual(expected,
                          db.quota_usage_get_all_by_project(
                              self.ctxt,
@@ -558,9 +557,9 @@ class DBAPIReservationTestCase(BaseTest):
     def test_reservation_rollback(self):
         reservations = _quota_reserve(self.ctxt, 'project1')
         expected = {'project_id': 'project1',
-                    'res0': {'reserved': 0, 'in_use': 0},
-                    'res1': {'reserved': 1, 'in_use': 1},
-                    'res2': {'reserved': 2, 'in_use': 2}}
+                    'volumes': {'reserved': 1, 'in_use': 0},
+                    'gigabytes': {'reserved': 2, 'in_use': 0},
+                    }
         self.assertEqual(expected,
                          db.quota_usage_get_all_by_project(
                              self.ctxt,
@@ -572,9 +571,9 @@ class DBAPIReservationTestCase(BaseTest):
                           self.ctxt,
                           reservations[0])
         expected = {'project_id': 'project1',
-                    'res0': {'reserved': 0, 'in_use': 0},
-                    'res1': {'reserved': 0, 'in_use': 1},
-                    'res2': {'reserved': 0, 'in_use': 2}}
+                    'volumes': {'reserved': 0, 'in_use': 0},
+                    'gigabytes': {'reserved': 0, 'in_use': 0},
+                    }
         self.assertEqual(expected,
                          db.quota_usage_get_all_by_project(
                              self.ctxt,
@@ -587,9 +586,8 @@ class DBAPIReservationTestCase(BaseTest):
         db.reservation_expire(self.ctxt)
 
         expected = {'project_id': 'project1',
-                    'res0': {'reserved': 0, 'in_use': 0},
-                    'res1': {'reserved': 0, 'in_use': 1},
-                    'res2': {'reserved': 0, 'in_use': 2}}
+                    'gigabytes': {'reserved': 0, 'in_use': 0},
+                    'volumes': {'reserved': 0, 'in_use': 0}}
         self.assertEqual(expected,
                          db.quota_usage_get_all_by_project(
                              self.ctxt,
@@ -647,8 +645,8 @@ class DBAPIQuotaTestCase(BaseTest):
 
     def test_quota_reserve(self):
         reservations = _quota_reserve(self.ctxt, 'project1')
-        self.assertEqual(len(reservations), 3)
-        res_names = ['res0', 'res1', 'res2']
+        self.assertEqual(len(reservations), 2)
+        res_names = ['gigabytes', 'volumes']
         for uuid in reservations:
             reservation = db.reservation_get(self.ctxt, uuid)
             self.assertTrue(reservation.resource in res_names)
@@ -677,18 +675,17 @@ class DBAPIQuotaTestCase(BaseTest):
 
     def test_quota_usage_get(self):
         reservations = _quota_reserve(self.ctxt, 'p1')
-        quota_usage = db.quota_usage_get(self.ctxt, 'p1', 'res0')
-        expected = {'resource': 'res0', 'project_id': 'p1',
-                    'in_use': 0, 'reserved': 0, 'total': 0}
+        quota_usage = db.quota_usage_get(self.ctxt, 'p1', 'gigabytes')
+        expected = {'resource': 'gigabytes', 'project_id': 'p1',
+                    'in_use': 0, 'reserved': 2, 'total': 2}
         for key, value in expected.iteritems():
-            self.assertEqual(value, quota_usage[key])
+            self.assertEqual(value, quota_usage[key], key)
 
     def test_quota_usage_get_all_by_project(self):
         reservations = _quota_reserve(self.ctxt, 'p1')
         expected = {'project_id': 'p1',
-                    'res0': {'in_use': 0, 'reserved': 0},
-                    'res1': {'in_use': 1, 'reserved': 1},
-                    'res2': {'in_use': 2, 'reserved': 2}}
+                    'volumes': {'in_use': 0, 'reserved': 1},
+                    'gigabytes': {'in_use': 0, 'reserved': 2}}
         self.assertEqual(expected, db.quota_usage_get_all_by_project(
                          self.ctxt, 'p1'))
 

diff --git a/cinder/tests/test_quota.py b/cinder/tests/test_quota.py
index 8913dfc3c98141dbd1a4e7f486ec46624d47acc0..acacadd5ec36eb9b27c85eed6d780c0be715d8d6 100644 (file)
--- a/cinder/tests/test_quota.py
+++ b/cinder/tests/test_quota.py
@@ -415,35 +415,6 @@ class QuotaEngineTestCase(test.TestCase):
                               test_resource2=resources[1],
                               test_resource3=resources[2], ))
 
-    def test_sync_predeclared(self):
-        quota_obj = quota.QuotaEngine()
-
-        def spam(*args, **kwargs):
-            pass
-
-        resource = quota.ReservableResource('test_resource', spam)
-        quota_obj.register_resource(resource)
-
-        self.assertEqual(resource.sync, spam)
-
-    def test_sync_multi(self):
-        quota_obj = quota.QuotaEngine()
-
-        def spam(*args, **kwargs):
-            pass
-
-        resources = [
-            quota.ReservableResource('test_resource1', spam),
-            quota.ReservableResource('test_resource2', spam),
-            quota.ReservableResource('test_resource3', spam),
-            quota.ReservableResource('test_resource4', spam), ]
-        quota_obj.register_resources(resources[:2])
-
-        self.assertEqual(resources[0].sync, spam)
-        self.assertEqual(resources[1].sync, spam)
-        self.assertEqual(resources[2].sync, spam)
-        self.assertEqual(resources[3].sync, spam)
-
     def test_get_by_project(self):
         context = FakeContext('test_project', 'test_class')
         driver = FakeDriver(
@@ -1099,7 +1070,8 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
         self.sync_called = set()
 
         def make_sync(res_name):
-            def sync(context, project_id, session):
+            def fake_sync(context, project_id, volume_type_id=None,
+                          volume_type_name=None, session=None):
                 self.sync_called.add(res_name)
                 if res_name in self.usages:
                     if self.usages[res_name].in_use < 0:
@@ -1107,13 +1079,16 @@ class QuotaReserveSqlAlchemyTestCase(test.TestCase):
                     else:
                         return {res_name: self.usages[res_name].in_use - 1}
                 return {res_name: 0}
-            return sync
+            return fake_sync
 
         self.resources = {}
+        QUOTA_SYNC_FUNCTIONS = {}
         for res_name in ('volumes', 'gigabytes'):
-            res = quota.ReservableResource(res_name, make_sync(res_name))
+            res = quota.ReservableResource(res_name, '_sync_%s' % res_name)
+            QUOTA_SYNC_FUNCTIONS['_sync_%s' % res_name] = make_sync(res_name)
             self.resources[res_name] = res
 
+        self.stubs.Set(sqa_api, 'QUOTA_SYNC_FUNCTIONS', QUOTA_SYNC_FUNCTIONS)
         self.expire = timeutils.utcnow() + datetime.timedelta(seconds=3600)
 
         self.usages = {}