From: Sergey Skripnick Date: Wed, 10 Jul 2013 08:55:24 +0000 (+0300) Subject: Move resource usage sync functions to db backend X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=4d4627f9c213ca5d473b88bb21e611b93975074e;p=openstack-build%2Fcinder-build.git 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 12357d69f..cc1536adb 100644 --- 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 2a2bed9e9..a81498071 100644 --- 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 72e3ad27d..30589a6c3 100644 --- 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 0f2bced90..891063672 100644 --- 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 8913dfc3c..acacadd5e 100644 --- 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 = {}