diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index 3a5e8d1653..c954080c4d 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -3908,18 +3908,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()) @@ -4780,6 +4787,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( diff --git a/manila/exception.py b/manila/exception.py index 024e57ea69..8448bfd477 100644 --- a/manila/exception.py +++ b/manila/exception.py @@ -628,8 +628,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): diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index e6e7a1b517..287dff5c69 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -1123,6 +1123,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): @@ -2921,29 +2972,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, @@ -2955,13 +3003,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'])) diff --git a/releasenotes/notes/bug-1699836-disallow-share-type-deletion-with-active-share-group-types-83809532d06ef0dd.yaml b/releasenotes/notes/bug-1699836-disallow-share-type-deletion-with-active-share-group-types-83809532d06ef0dd.yaml new file mode 100644 index 0000000000..5f8af0c50c --- /dev/null +++ b/releasenotes/notes/bug-1699836-disallow-share-type-deletion-with-active-share-group-types-83809532d06ef0dd.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed `Launchpad bug 1699836 `_ by + preventing share type deletion when there are share group types + associated with them.