From: LisaLi Date: Fri, 15 Jan 2016 08:12:45 +0000 (+0800) Subject: Update quota when volume type renames X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=b1a322cf8fbf97a48f8fff07f70d2d32c83c9ea6;p=openstack-build%2Fcinder-build.git Update quota when volume type renames When customers rename volume type, the corresponding quota_usages should be changed. Or else quota_usage shows incorrect data. Meanwhile, it deletes quota_usages whose corresponding volume types don't exist. These invalid data are left when renaming volume types in old versions. Closes-bug: #1473183 Change-Id: I9071821f8c1a95fccef214868e5cea026fed9657 Co-Authored-By: wanghao --- diff --git a/cinder/db/api.py b/cinder/db/api.py index 2859f99ee..71674ff43 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -752,6 +752,11 @@ def quota_update(context, project_id, resource, limit): return IMPL.quota_update(context, project_id, resource, limit) +def quota_update_resource(context, old_res, new_res): + """Update resource of quotas.""" + return IMPL.quota_update_resource(context, old_res, new_res) + + def quota_destroy(context, project_id, resource): """Destroy the quota or raise if it does not exist.""" return IMPL.quota_destroy(context, project_id, resource) @@ -785,6 +790,11 @@ def quota_class_update(context, class_name, resource, limit): return IMPL.quota_class_update(context, class_name, resource, limit) +def quota_class_update_resource(context, resource, new_resource): + """Update resource name in quota_class.""" + return IMPL.quota_class_update_resource(context, resource, new_resource) + + def quota_class_destroy(context, class_name, resource): """Destroy the quota class or raise if it does not exist.""" return IMPL.quota_class_destroy(context, class_name, resource) @@ -840,6 +850,11 @@ def reservation_expire(context): return IMPL.reservation_expire(context) +def quota_usage_update_resource(context, old_res, new_res): + """Update resource field in quota_usages.""" + return IMPL.quota_usage_update_resource(context, old_res, new_res) + + ################### diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index f859bb715..1fc0cfbaf 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -535,6 +535,15 @@ def quota_allocated_get_all_by_project(context, project_id): return result +@require_context +def _quota_get_by_resource(context, resource, session=None): + rows = model_query(context, models.Quota, + session=session, + read_deleted='no').filter_by( + resource=resource).all() + return rows + + @require_admin_context def quota_create(context, project_id, resource, limit, allocated): quota_ref = models.Quota() @@ -559,6 +568,15 @@ def quota_update(context, project_id, resource, limit): return quota_ref +@require_context +def quota_update_resource(context, old_res, new_res): + session = get_session() + with session.begin(): + quotas = _quota_get_by_resource(context, old_res, session=session) + for quota in quotas: + quota.resource = new_res + + @require_admin_context def quota_allocated_update(context, project_id, resource, allocated): session = get_session() @@ -625,6 +643,17 @@ def quota_class_get_all_by_name(context, class_name): return result +@require_context +def _quota_class_get_all_by_resource(context, resource, session): + result = model_query(context, models.QuotaClass, + session=session, + read_deleted="no").\ + filter_by(resource=resource).\ + all() + + return result + + @require_admin_context def quota_class_create(context, class_name, resource, limit): quota_class_ref = models.QuotaClass() @@ -648,6 +677,16 @@ def quota_class_update(context, class_name, resource, limit): return quota_class_ref +@require_context +def quota_class_update_resource(context, old_res, new_res): + session = get_session() + with session.begin(): + quota_class_list = _quota_class_get_all_by_resource( + context, old_res, session) + for quota_class in quota_class_list: + quota_class.resource = new_res + + @require_admin_context def quota_class_destroy(context, class_name, resource): session = get_session() @@ -751,6 +790,27 @@ def _get_quota_usages(context, session, project_id): return {row.resource: row for row in rows} +def _get_quota_usages_by_resource(context, session, resource): + rows = model_query(context, models.QuotaUsage, + deleted="no", + session=session).\ + filter_by(resource=resource).\ + with_lockmode('update').\ + all() + return rows + + +@require_context +@_retry_on_deadlock +def quota_usage_update_resource(context, old_res, new_res): + session = get_session() + with session.begin(): + usages = _get_quota_usages_by_resource(context, session, old_res) + for usage in usages: + usage.resource = new_res + usage.until_refresh = 1 + + @require_context @_retry_on_deadlock def quota_reserve(context, resources, quotas, deltas, expire, @@ -903,15 +963,20 @@ def _quota_reservations(session, context, reservations): all() +def _dict_with_usage_id(usages): + return {row.id: row for row in usages.values()} + + @require_context @_retry_on_deadlock def reservation_commit(context, reservations, project_id=None): session = get_session() with session.begin(): usages = _get_quota_usages(context, session, project_id) + usages = _dict_with_usage_id(usages) for reservation in _quota_reservations(session, context, reservations): - usage = usages[reservation.resource] + usage = usages[reservation.usage_id] if reservation.delta >= 0: usage.reserved -= reservation.delta usage.in_use += reservation.delta @@ -925,9 +990,9 @@ def reservation_rollback(context, reservations, project_id=None): session = get_session() with session.begin(): usages = _get_quota_usages(context, session, project_id) - + usages = _dict_with_usage_id(usages) for reservation in _quota_reservations(session, context, reservations): - usage = usages[reservation.resource] + usage = usages[reservation.usage_id] if reservation.delta >= 0: usage.reserved -= reservation.delta diff --git a/cinder/quota.py b/cinder/quota.py index f32931db2..1226077de 100644 --- a/cinder/quota.py +++ b/cinder/quota.py @@ -942,6 +942,30 @@ class VolumeTypeQuotaEngine(QuotaEngine): def register_resources(self, resources): raise NotImplementedError(_("Cannot register resources")) + def update_quota_resource(self, context, old_type_name, new_type_name): + """Update resource in quota. + + This is to update resource in quotas, quota_classes, and + quota_usages once the name of a volume type is changed. + + :param context: The request context, for access checks. + :param old_type_name: old name of volume type. + :param new_type_name: new name of volume type. + """ + + for quota in ('volumes', 'gigabytes', 'snapshots'): + old_res = "%s_%s" % (quota, old_type_name) + new_res = "%s_%s" % (quota, new_type_name) + db.quota_usage_update_resource(context, + old_res, + new_res) + db.quota_class_update_resource(context, + old_res, + new_res) + db.quota_update_resource(context, + old_res, + new_res) + class CGQuotaEngine(QuotaEngine): """Represent the consistencygroup quotas.""" diff --git a/cinder/tests/unit/api/contrib/test_types_manage.py b/cinder/tests/unit/api/contrib/test_types_manage.py index 629b2b2bc..c39d75bec 100644 --- a/cinder/tests/unit/api/contrib/test_types_manage.py +++ b/cinder/tests/unit/api/contrib/test_types_manage.py @@ -18,6 +18,7 @@ import six import webob from cinder.api.contrib import types_manage +from cinder import context from cinder import exception from cinder import test from cinder.tests.unit.api import fakes @@ -365,19 +366,26 @@ class VolumeTypesManageApiTest(test.TestCase): self.assertRaises(webob.exc.HTTPBadRequest, self.controller._update, req, '1', body) - @mock.patch('cinder.volume.volume_types.update') @mock.patch('cinder.volume.volume_types.get_volume_type') - def test_update_only_name(self, mock_get, mock_update): + @mock.patch('cinder.db.volume_type_update') + @mock.patch('cinder.quota.VolumeTypeQuotaEngine.' + 'update_quota_resource') + def test_update_only_name(self, mock_update_quota, + mock_update, mock_get): mock_get.return_value = return_volume_types_get_volume_type_updated( '999') - body = {"volume_type": {"name": "vol_type_999_999"}} + ctxt = context.RequestContext('admin', 'fake', True) + body = {"volume_type": {"name": "vol_type_999"}} req = fakes.HTTPRequest.blank('/v2/fake/types/999') req.method = 'PUT' + req.environ['cinder.context'] = ctxt self.assertEqual(0, len(self.notifier.notifications)) res_dict = self.controller._update(req, '999', body) self.assertEqual(1, len(self.notifier.notifications)) + mock_update_quota.assert_called_once_with(ctxt, 'vol_type_999_999', + 'vol_type_999') self._check_test_results(res_dict, {'expected_name': 'vol_type_999_999', 'expected_desc': 'vol_type_desc_999'}) diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index 058d895b0..3dfa4e0d9 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -1710,6 +1710,15 @@ class DBAPIQuotaClassTestCase(BaseTest): updated = db.quota_class_get(self.ctxt, 'test_qc', 'test_resource') self.assertEqual(43, updated['hard_limit']) + def test_quota_class_update_resource(self): + old = db.quota_class_get(self.ctxt, 'test_qc', 'test_resource') + db.quota_class_update_resource(self.ctxt, + 'test_resource', + 'test_resource1') + new = db.quota_class_get(self.ctxt, 'test_qc', 'test_resource1') + self.assertEqual(old.id, new.id) + self.assertEqual('test_resource1', new.resource) + def test_quota_class_destroy_all_by_name(self): db.quota_class_create(self.ctxt, 'test2', 'res1', 43) db.quota_class_create(self.ctxt, 'test2', 'res2', 44) @@ -1752,6 +1761,13 @@ class DBAPIQuotaTestCase(BaseTest): self.assertEqual('resource1', quota.resource) self.assertEqual('project1', quota.project_id) + def test_quota_update_resource(self): + old = db.quota_create(self.ctxt, 'project1', 'resource1', 41) + db.quota_update_resource(self.ctxt, 'resource1', 'resource2') + new = db.quota_get(self.ctxt, 'project1', 'resource2') + self.assertEqual(old.id, new.id) + self.assertEqual('resource2', new.resource) + def test_quota_update_nonexistent(self): self.assertRaises(exception.ProjectQuotaNotFound, db.quota_update, diff --git a/cinder/tests/unit/test_quota.py b/cinder/tests/unit/test_quota.py index 11f792d44..22a5e4204 100644 --- a/cinder/tests/unit/test_quota.py +++ b/cinder/tests/unit/test_quota.py @@ -885,6 +885,12 @@ class VolumeTypeQuotaEngineTestCase(test.TestCase): db.volume_type_destroy(ctx, vtype['id']) db.volume_type_destroy(ctx, vtype2['id']) + def test_update_quota_resource(self): + ctx = context.RequestContext('admin', 'admin', is_admin=True) + + engine = quota.VolumeTypeQuotaEngine() + engine.update_quota_resource(ctx, 'type1', 'type2') + class DbQuotaDriverTestCase(test.TestCase): def setUp(self): diff --git a/cinder/tests/unit/test_volume_types.py b/cinder/tests/unit/test_volume_types.py index 747f4fd3e..5016bb820 100644 --- a/cinder/tests/unit/test_volume_types.py +++ b/cinder/tests/unit/test_volume_types.py @@ -15,6 +15,7 @@ import datetime +import mock import time from oslo_config import cfg @@ -84,6 +85,23 @@ class VolumeTypeTestCase(test.TestCase): new_all_vtypes, 'drive type was not deleted') + @mock.patch('cinder.quota.VolumeTypeQuotaEngine.' + 'update_quota_resource') + def test_update_volume_type_name(self, mock_update_quota): + type_ref = volume_types.create(self.ctxt, + self.vol_type1_name, + self.vol_type1_specs, + description=self.vol_type1_description) + new_type_name = self.vol_type1_name + '_updated' + volume_types.update(self.ctxt, + type_ref.id, + new_type_name, + None) + mock_update_quota.assert_called_once_with(self.ctxt, + self.vol_type1_name, + new_type_name) + volume_types.destroy(self.ctxt, type_ref.id) + def test_create_volume_type_with_invalid_params(self): """Ensure exception will be returned.""" vol_type_invalid_specs = "invalid_extra_specs" diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py index 57ef554ca..a800d1561 100644 --- a/cinder/volume/volume_types.py +++ b/cinder/volume/volume_types.py @@ -28,10 +28,11 @@ from cinder import context from cinder import db from cinder import exception from cinder.i18n import _, _LE - +from cinder import quota CONF = cfg.CONF LOG = logging.getLogger(__name__) +QUOTAS = quota.QUOTAS def create(context, @@ -62,12 +63,20 @@ def update(context, id, name, description, is_public=None): if id is None: msg = _("id cannot be None") raise exception.InvalidVolumeType(reason=msg) + old_volume_type = get_volume_type(context, id) try: type_updated = db.volume_type_update(context, id, dict(name=name, description=description, is_public=is_public)) + # Rename resource in quota if volume type name is changed. + if name: + old_type_name = old_volume_type.get('name') + if old_type_name != name: + QUOTAS.update_quota_resource(context, + old_type_name, + name) except db_exc.DBError: LOG.exception(_LE('DB error:')) raise exception.VolumeTypeUpdateFailed(id=id)