Merge "Prevent share type deletion if linked to group types"

manila/db/sqlalchemy/api.py

@@ -4270,18 +4270,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())
@@ -5170,6 +5177,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(

manila/exception.py

@@ -652,8 +652,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):

manila/tests/db/sqlalchemy/test_api.py

@@ -1250,6 +1250,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):
 
@@ -3389,29 +3440,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,
@@ -3423,13 +3471,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']))

releasenotes/notes/bug-1699836-disallow-share-type-deletion-with-active-share-group-types-83809532d06ef0dd.yaml

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