openstack
/
manila
Code Issues Proposed changes

Merge "Destroy type quotas when a share type is deleted"

This commit is contained in:
Zuul 2019-03-18 12:52:41 +00:00 committed by Gerrit Code Review
parent 856f6a09e1 4b6cfcf690
commit b2787fded5
7 changed files with 214 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
manila/db/api.py
View File

@ -280,11 +280,10 @@ def quota_destroy_all_by_project_and_user(context, project_id, user_id):
project_id, user_id)
def quota_destroy_all_by_project_and_share_type(context, project_id,
share_type_id):
"""Destroy all quotas associated with a given project and user."""
return IMPL.quota_destroy_all_by_project_and_share_type(
context, project_id, share_type_id)
def quota_destroy_all_by_share_type(context, share_type_id, project_id=None):
"""Destroy all quotas associated with a given share type and project."""
return IMPL.quota_destroy_all_by_share_type(
context, share_type_id, project_id=project_id)
def quota_destroy_all_by_project(context, project_id):

51
manila/db/sqlalchemy/api.py
View File

@ -1206,34 +1206,42 @@ def quota_destroy_all_by_project_and_user(context, project_id, user_id):
@require_admin_context
def quota_destroy_all_by_project_and_share_type(context, project_id,
share_type_id):
def quota_destroy_all_by_share_type(context, share_type_id, project_id=None):
"""Soft deletes all quotas, usages and reservations.
:param context: request context for queries, updates and logging
:param share_type_id: ID of the share type to filter the quotas, usages
and reservations under.
:param project_id: ID of the project to filter the quotas, usages and
reservations under. If not provided, share type quotas for all
projects will be acted upon.
"""
session = get_session()
with session.begin():
model_query(
share_type_quotas = model_query(
context, models.ProjectShareTypeQuota, session=session,
read_deleted="no",
).filter_by(
project_id=project_id,
).filter_by(
share_type_id=share_type_id,
).soft_delete(synchronize_session=False)
).filter_by(share_type_id=share_type_id)
model_query(
share_type_quota_usages = model_query(
context, models.QuotaUsage, session=session, read_deleted="no",
).filter_by(
project_id=project_id,
).filter_by(
share_type_id=share_type_id,
).soft_delete(synchronize_session=False)
).filter_by(share_type_id=share_type_id)
model_query(
share_type_quota_reservations = model_query(
context, models.Reservation, session=session, read_deleted="no",
).filter_by(
project_id=project_id,
).filter_by(
share_type_id=share_type_id,
).soft_delete(synchronize_session=False)
).filter_by(share_type_id=share_type_id)
if project_id is not None:
share_type_quotas = share_type_quotas.filter_by(
project_id=project_id)
share_type_quota_usages = share_type_quota_usages.filter_by(
project_id=project_id)
share_type_quota_reservations = (
share_type_quota_reservations.filter_by(project_id=project_id))
share_type_quotas.soft_delete(synchronize_session=False)
share_type_quota_usages.soft_delete(synchronize_session=False)
share_type_quota_reservations.soft_delete(synchronize_session=False)
@require_admin_context
@ -4070,6 +4078,9 @@ def share_type_destroy(context, id):
(model_query(context, models.ShareTypes, session=session).
filter_by(id=id).soft_delete())
# Destroy any quotas, usages and reservations for the share type:
quota_destroy_all_by_share_type(context, id)
def _share_type_access_query(context, session=None):
return model_query(context, models.ShareTypeProjects, session=session,

4
manila/quota.py
View File

@ -581,8 +581,8 @@ class DbQuotaDriver(object):
:param share_type_id: The UUID of the share type.
"""
db.quota_destroy_all_by_project_and_share_type(
context, project_id, share_type_id)
db.quota_destroy_all_by_share_type(context, share_type_id,
project_id=project_id)
def expire(self, context):
"""Expire reservations.

156
manila/tests/db/sqlalchemy/test_api.py
View File

@ -3082,6 +3082,7 @@ class PurgeDeletedTest(test.TestCase):
self.assertEqual(0, s_row + type_row)
@ddt.ddt
class ShareTypeAPITestCase(test.TestCase):
def setUp(self):
@ -3089,6 +3090,161 @@ class ShareTypeAPITestCase(test.TestCase):
self.ctxt = context.RequestContext(
user_id='user_id', project_id='project_id', is_admin=True)
@ddt.data({'used_by_shares': True, 'used_by_groups': False},
{'used_by_shares': False, 'used_by_groups': True},
{'used_by_shares': True, 'used_by_groups': True})
@ddt.unpack
def test_share_type_destroy_in_use(self, used_by_shares,
used_by_groups):
share_type_1 = db_utils.create_share_type(
name='orange', extra_specs={'somekey': 'someval'})
share_type_2 = db_utils.create_share_type(name='regalia')
if used_by_shares:
share_1 = db_utils.create_share(share_type_id=share_type_1['id'])
db_utils.create_share(share_type_id=share_type_2['id'])
if used_by_groups:
group_type_1 = db_utils.create_share_group_type(
name='crimson', share_types=[share_type_1['id']])
group_type_2 = db_utils.create_share_group_type(
name='tide', share_types=[share_type_2['id']])
share_group_1 = db_utils.create_share_group(
share_group_type_id=group_type_1['id'],
share_types=[share_type_1['id']])
db_utils.create_share_group(
share_group_type_id=group_type_2['id'],
share_types=[share_type_2['id']])
self.assertRaises(exception.ShareTypeInUse,
db_api.share_type_destroy,
self.ctxt, share_type_1['id'])
self.assertRaises(exception.ShareTypeInUse,
db_api.share_type_destroy,
self.ctxt, share_type_2['id'])
# Let's cleanup share_type_1 and verify it is gone
if used_by_shares:
db_api.share_instance_delete(self.ctxt, share_1.instance.id)
if used_by_groups:
db_api.share_group_destroy(self.ctxt, share_group_1['id'])
db_api.share_group_type_destroy(self.ctxt,
group_type_1['id'])
self.assertIsNone(db_api.share_type_destroy(
self.ctxt, share_type_1['id']))
self.assertDictMatch(
{}, db_api.share_type_extra_specs_get(
self.ctxt, share_type_1['id']))
self.assertRaises(exception.ShareTypeNotFound,
db_api.share_type_get,
self.ctxt, share_type_1['id'])
# share_type_2 must still be around
self.assertEqual(
share_type_2['id'],
db_api.share_type_get(self.ctxt, share_type_2['id'])['id'])
@ddt.data({'usages': False, 'reservations': False},
{'usages': False, 'reservations': True},
{'usages': True, 'reservations': False})
@ddt.unpack
def test_share_type_destroy_quotas_and_reservations(self, usages,
reservations):
share_type = db_utils.create_share_type(name='clemsontigers')
shares_quota = db_api.quota_create(
self.ctxt, "fake-project-id", 'shares', 10,
share_type_id=share_type['id'])
snapshots_quota = db_api.quota_create(
self.ctxt, "fake-project-id", 'snapshots', 30,
share_type_id=share_type['id'])
if reservations:
resources = {
'shares': quota.ReservableResource('shares', '_sync_shares'),
'snapshots': quota.ReservableResource(
'snapshots', '_sync_snapshots'),
}
project_quotas = {
'shares': shares_quota.hard_limit,
'snapshots': snapshots_quota.hard_limit,
}
user_quotas = {
'shares': shares_quota.hard_limit,
'snapshots': snapshots_quota.hard_limit,
}
deltas = {'shares': 1, 'snapshots': 3}
expire = timeutils.utcnow() + datetime.timedelta(seconds=86400)
reservation_uuids = db_api.quota_reserve(
self.ctxt, resources, project_quotas, user_quotas,
project_quotas, deltas, expire, False, 30,
project_id='fake-project-id', share_type_id=share_type['id'])
db_session = db_api.get_session()
q_reservations = db_api._quota_reservations_query(
db_session, self.ctxt, reservation_uuids).all()
# There should be 2 "user" reservations and 2 "share-type"
# quota reservations
self.assertEqual(4, len(q_reservations))
q_share_type_reservations = [qr for qr in q_reservations
if qr['share_type_id'] is not None]
# There should be exactly two "share type" quota reservations
self.assertEqual(2, len(q_share_type_reservations))
for q_reservation in q_share_type_reservations:
self.assertEqual(q_reservation['share_type_id'],
share_type['id'])
if usages:
db_api.quota_usage_create(self.ctxt, 'fake-project-id',
'fake-user-id', 'shares', 3, 2, False,
share_type_id=share_type['id'])
db_api.quota_usage_create(self.ctxt, 'fake-project-id',
'fake-user-id', 'snapshots', 2, 2, False,
share_type_id=share_type['id'])
q_usages = db_api.quota_usage_get_all_by_project_and_share_type(
self.ctxt, 'fake-project-id', share_type['id'])
self.assertEqual(3, q_usages['shares']['in_use'])
self.assertEqual(2, q_usages['shares']['reserved'])
self.assertEqual(2, q_usages['snapshots']['in_use'])
self.assertEqual(2, q_usages['snapshots']['reserved'])
# Validate that quotas exist
share_type_quotas = db_api.quota_get_all_by_project_and_share_type(
self.ctxt, 'fake-project-id', share_type['id'])
expected_quotas = {
'project_id': 'fake-project-id',
'share_type_id': share_type['id'],
'shares': 10,
'snapshots': 30,
}
self.assertDictMatch(expected_quotas, share_type_quotas)
db_api.share_type_destroy(self.ctxt, share_type['id'])
self.assertRaises(exception.ShareTypeNotFound,
db_api.share_type_get,
self.ctxt, share_type['id'])
# Quotas must be gone
share_type_quotas = db_api.quota_get_all_by_project_and_share_type(
self.ctxt, 'fake-project-id', share_type['id'])
self.assertEqual({'project_id': 'fake-project-id',
'share_type_id': share_type['id']},
share_type_quotas)
# Check usages and reservations
if usages:
q_usages = db_api.quota_usage_get_all_by_project_and_share_type(
self.ctxt, 'fake-project-id', share_type['id'])
expected_q_usages = {'project_id': 'fake-project-id',
'share_type_id': share_type['id']}
self.assertDictMatch(expected_q_usages, q_usages)
if reservations:
q_reservations = db_api._quota_reservations_query(
db_session, self.ctxt, reservation_uuids).all()
# just "user" quota reservations should be left, since we didn't
# clean them up.
self.assertEqual(2, len(q_reservations))
for q_reservation in q_reservations:
self.assertIsNone(q_reservation['share_type_id'])
def test_share_type_get_by_name_or_id_found_by_id(self):
share_type = db_utils.create_share_type()

12
manila/tests/db_utils.py
View File

@ -234,6 +234,18 @@ def create_share_type(**kwargs):
return _create_db_row(db.share_type_create, share_type, kwargs)
def create_share_group_type(**kwargs):
"""Create a share group type object"""
share_group_type = {
'name': 'fake_group_type',
'is_public': True,
}
return _create_db_row(db.share_group_type_create, share_group_type,
kwargs)
def create_share_network(**kwargs):
"""Create a share network object."""
net = {

4
manila/tests/test_quota.py
View File

@ -440,14 +440,14 @@ class DbQuotaDriverTestCase(test.TestCase):
def test_destroy_all_by_project_and_share_type(self):
mock_destroy_all = self.mock_object(
quota.db, 'quota_destroy_all_by_project_and_share_type')
quota.db, 'quota_destroy_all_by_share_type')
result = self.driver.destroy_all_by_project_and_share_type(
self.ctxt, self.project_id, self.share_type_id)
self.assertIsNone(result)
mock_destroy_all.assert_called_once_with(
self.ctxt, self.project_id, self.share_type_id)
self.ctxt, self.share_type_id, project_id=self.project_id)
def test_expire(self):
self.mock_object(quota.db, 'reservation_expire')

7
releasenotes/notes/bug-1811680-destroy-quotas-usages-reservations-when-deleting-share-type-a18f2e00a65fe922.yaml Normal file
View File

@ -0,0 +1,7 @@
---
fixes:
- |
Share type quotas, usages and reservations will now be correctly cleaned
up if a share type has been deleted. See `launchpad bug #1811680
<https://bugs.launchpad.net/manila/+bug/1811680>`_ for details regarding
the bug that prevented this cleanup prior.