Destroy type quotas when a share type is deleted

Upon share type deletion, we were leaving behind
share type quotas, usages and reservations. These
couldn't be cleaned up without resorting to manual
intervention.

Cleanup quotas when a share type is being destroyed,
since the resources that they pertained to are gone
too.

Closes-Bug: #1811680
Change-Id: I04b1fe88808596aa8c05429b964190d654587f9a
This commit is contained in:
Goutham Pacha Ravi 2019-03-15 00:07:32 -07:00
parent 45aa22934d
commit 4b6cfcf690
7 changed files with 214 additions and 29 deletions

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

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,

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.

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

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 = {

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')

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.