openstack/cinder
Code Issues Proposed changes

Preserve usage and reservations on quota deletion

Browse Source 
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
This commit is contained in:
Gorka Eguileor 2015-03-09 19:39:11 +01:00
parent 5df1f21904
commit 0ee1ea83b9
6 changed files with 98 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cinder/api/contrib/quotas.py
View File

@ -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()

4
cinder/db/api.py
View File

@ -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):

22
cinder/db/sqlalchemy/api.py
View File

@ -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).\

16
cinder/quota.py
View File

@ -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.

59
cinder/tests/unit/test_db_api.py
View File

@ -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,

32
cinder/tests/unit/test_quota.py
View File

@ -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)
self.stubs.Set(sqa_api, 'quota_destroy_by_project',
fake_quota_destroy_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',
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')), ])