Prevent share type deletion if linked to group types
Share types can be associated with share group types. This relationship needs to be dropped in case share types are to be deleted, or we'll be leaving behind dead references in the database. Share groups cannot exist without share group types, and hence we don't need to check on existing groups when deleting share types; checking for existing group types will suffice. Change-Id: Id86a93f1923a493cefe73e2d90d5094005b42ae4 Closes-Bug: #1699836 Signed-off-by: Goutham Pacha Ravi <gouthampravi@gmail.com>
This commit is contained in:
Goutham Pacha Ravi
parent
1edd0c39a6
commit
470bab7c71
@ -4219,18 +4219,25 @@ def share_type_destroy(context, id):
|
||||
session = get_session()
|
||||
with session.begin():
|
||||
_share_type_get(context, id, session)
|
||||
results = (model_query(context, models.ShareInstance, session=session,
|
||||
read_deleted="no").
|
||||
filter_by(share_type_id=id).count())
|
||||
share_group_count = model_query(
|
||||
shares_count = model_query(
|
||||
context,
|
||||
models.ShareGroupShareTypeMapping,
|
||||
models.ShareInstance,
|
||||
read_deleted="no",
|
||||
session=session,
|
||||
).filter_by(share_type_id=id).count()
|
||||
if results or share_group_count:
|
||||
LOG.error('ShareType %s deletion failed, ShareType in use.',
|
||||
id)
|
||||
share_group_types_count = model_query(
|
||||
context,
|
||||
models.ShareGroupTypeShareTypeMapping,
|
||||
read_deleted="no",
|
||||
session=session,
|
||||
).filter_by(share_type_id=id).count()
|
||||
if shares_count or share_group_types_count:
|
||||
msg = ("Deletion of share type %(stype)s failed; it in use by "
|
||||
"%(shares)d shares and %(gtypes)d share group types")
|
||||
msg_args = {'stype': id,
|
||||
'shares': shares_count,
|
||||
'gtypes': share_group_types_count}
|
||||
LOG.error(msg, msg_args)
|
||||
raise exception.ShareTypeInUse(share_type_id=id)
|
||||
(model_query(context, models.ShareTypeExtraSpecs, session=session).
|
||||
filter_by(share_type_id=id).soft_delete())
|
||||
@ -5094,6 +5101,11 @@ def share_group_type_destroy(context, type_id):
|
||||
).filter_by(
|
||||
share_group_type_id=type_id,
|
||||
).soft_delete()
|
||||
model_query(
|
||||
context, models.ShareGroupTypeShareTypeMapping, session=session
|
||||
).filter_by(
|
||||
share_group_type_id=type_id,
|
||||
).soft_delete()
|
||||
model_query(
|
||||
context, models.ShareGroupTypes, session=session
|
||||
).filter_by(
|
||||
|
||||
@ -641,8 +641,8 @@ class ShareGroupTypeSpecsNotFound(NotFound):
|
||||
|
||||
|
||||
class ShareTypeInUse(ManilaException):
|
||||
message = _("Share Type %(share_type_id)s deletion is not allowed with "
|
||||
"shares present with the type.")
|
||||
message = _("Share Type %(share_type_id)s deletion is not allowed while "
|
||||
"shares or share group types are associated with the type.")
|
||||
|
||||
|
||||
class IPAddressInUse(InUse):
|
||||
|
||||
@ -1194,6 +1194,57 @@ class ShareGroupDatabaseAPITestCase(test.TestCase):
|
||||
self.assertEqual(constants.STATUS_AVAILABLE, member['status'])
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class ShareGroupTypeAPITestCase(test.TestCase):
|
||||
|
||||
def setUp(self):
|
||||
super(ShareGroupTypeAPITestCase, self).setUp()
|
||||
self.ctxt = context.RequestContext(
|
||||
user_id='user_id', project_id='project_id', is_admin=True)
|
||||
|
||||
@ddt.data(True, False)
|
||||
def test_share_type_destroy_in_use(self, used_by_groups):
|
||||
share_type_1 = db_utils.create_share_type(name='fike')
|
||||
share_type_2 = db_utils.create_share_type(name='bowman')
|
||||
share_group_type_1 = db_utils.create_share_group_type(
|
||||
name='orange', is_public=False, share_types=[share_type_1['id']],
|
||||
group_specs={'dabo': 'allin', 'cadence': 'count'},
|
||||
override_defaults=True)
|
||||
share_group_type_2 = db_utils.create_share_group_type(
|
||||
name='regalia', share_types=[share_type_2['id']])
|
||||
if used_by_groups:
|
||||
share_group_1 = db_utils.create_share_group(
|
||||
share_group_type_id=share_group_type_1['id'],
|
||||
share_types=[share_type_1['id']])
|
||||
share_group_2 = db_utils.create_share_group(
|
||||
share_group_type_id=share_group_type_2['id'],
|
||||
share_types=[share_type_2['id']])
|
||||
self.assertRaises(exception.ShareGroupTypeInUse,
|
||||
db_api.share_group_type_destroy,
|
||||
self.ctxt, share_group_type_1['id'])
|
||||
self.assertRaises(exception.ShareGroupTypeInUse,
|
||||
db_api.share_group_type_destroy,
|
||||
self.ctxt, share_group_type_2['id'])
|
||||
# Cleanup share groups
|
||||
db_api.share_group_destroy(self.ctxt, share_group_1['id'])
|
||||
db_api.share_group_destroy(self.ctxt, share_group_2['id'])
|
||||
|
||||
# Let's cleanup share_group_type_1 and verify it is gone
|
||||
self.assertIsNone(db_api.share_group_type_destroy(
|
||||
self.ctxt, share_group_type_1['id']))
|
||||
self.assertDictMatch(
|
||||
{}, db_api.share_group_type_specs_get(
|
||||
self.ctxt, share_group_type_1['id']))
|
||||
self.assertRaises(exception.ShareGroupTypeNotFound,
|
||||
db_api.share_group_type_get,
|
||||
self.ctxt, share_group_type_1['id'])
|
||||
|
||||
# share_group_type_2 must still be around
|
||||
self.assertEqual(share_group_type_2['id'],
|
||||
db_api.share_group_type_get(
|
||||
self.ctxt, share_group_type_2['id'])['id'])
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class ShareSnapshotDatabaseAPITestCase(test.TestCase):
|
||||
|
||||
@ -3333,29 +3384,26 @@ 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.data({'used_by_shares': True, 'used_by_group_types': False},
|
||||
{'used_by_shares': False, 'used_by_group_types': True},
|
||||
{'used_by_shares': True, 'used_by_group_types': True})
|
||||
@ddt.unpack
|
||||
def test_share_type_destroy_in_use(self, used_by_shares,
|
||||
used_by_groups):
|
||||
used_by_group_types):
|
||||
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:
|
||||
if used_by_group_types:
|
||||
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(
|
||||
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,
|
||||
@ -3367,13 +3415,13 @@ class ShareTypeAPITestCase(test.TestCase):
|
||||
# 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:
|
||||
if used_by_group_types:
|
||||
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.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']))
|
||||
|
||||
@ -0,0 +1,6 @@
|
||||
---
|
||||
fixes:
|
||||
- |
|
||||
Fixed `Launchpad bug 1699836 <https://launchpad.net/bugs/1699836>`_ by
|
||||
preventing share type deletion when there are share group types
|
||||
associated with them.
|
||||
Loading…
Reference in New Issue
Block a user