Merge "Check groups before deleting group type"

cinder/db/sqlalchemy/api.py

@@ -3962,13 +3962,12 @@ def group_type_destroy(context, id):
     session = get_session()
     with session.begin():
         _group_type_get(context, id, session)
-        # TODO(xyang): Uncomment the following after groups table is added.
-        # results = model_query(context, models.Group, session=session). \
-        #     filter_by(group_type_id=id).all()
-        # if results:
-        #     LOG.error('GroupType %s deletion failed, '
-        #               'GroupType in use.', id)
-        #     raise exception.GroupTypeInUse(group_type_id=id)
+        results = model_query(context, models.Group, session=session). \
+            filter_by(group_type_id=id).all()
+        if results:
+            LOG.error('GroupType %s deletion failed, '
+                      'GroupType in use.', id)
+            raise exception.GroupTypeInUse(group_type_id=id)
         model_query(context, models.GroupTypes, session=session).\
             filter_by(id=id).\
             update({'deleted': True,

cinder/tests/unit/api/v3/test_group_types.py

@@ -34,6 +34,7 @@ from cinder.tests.unit import fake_constants as fake
 from cinder.volume import group_types
 
 GROUP_TYPE_MICRO_VERSION = '3.11'
+IN_USE_GROUP_TYPE = fake.GROUP_TYPE3_ID
 
 
 def stub_group_type(id):
@@ -88,6 +89,11 @@ def return_group_types_get_default_not_found():
     return {}
 
 
+def return_group_types_with_groups_destroy(context, id):
+    if id == IN_USE_GROUP_TYPE:
+        raise exception.GroupTypeInUse(group_type_id=id)
+
+
 @ddt.ddt
 class GroupTypesApiTest(test.TestCase):
 
@@ -136,6 +142,35 @@ class GroupTypesApiTest(test.TestCase):
             self.ctxt, 'group_type1', {},
             boolean_is_public, description=None)
 
+    @ddt.data(fake.GROUP_TYPE_ID, IN_USE_GROUP_TYPE)
+    def test_group_type_destroy(self, grp_type_id):
+        grp_type = {'id': grp_type_id, 'name': 'grp' + grp_type_id}
+        self.mock_object(group_types, 'get_group_type',
+                         return_value=grp_type)
+        self.mock_object(group_types, 'destroy',
+                         return_group_types_with_groups_destroy)
+        mock_notify_info = self.mock_object(
+            v3_group_types.GroupTypesController,
+            '_notify_group_type_info')
+        mock_notify_error = self.mock_object(
+            v3_group_types.GroupTypesController,
+            '_notify_group_type_error')
+        req = fakes.HTTPRequest.blank('/v3/%s/group_types/%s' % (
+            fake.PROJECT_ID, grp_type_id),
+            version=GROUP_TYPE_MICRO_VERSION)
+        req.environ['cinder.context'] = self.ctxt
+        if grp_type_id == IN_USE_GROUP_TYPE:
+            self.assertRaises(webob.exc.HTTPBadRequest,
+                              self.controller.delete,
+                              req, grp_type_id)
+            mock_notify_error.assert_called_once_with(
+                self.ctxt, 'group_type.delete', mock.ANY,
+                group_type=grp_type)
+        else:
+            self.controller.delete(req, grp_type_id)
+            mock_notify_info.assert_called_once_with(
+                self.ctxt, 'group_type.delete', grp_type)
+
     def test_group_types_index(self):
         self.mock_object(group_types, 'get_all_group_types',
                          return_group_types_get_all_types)

cinder/volume/group_types.py

@@ -18,6 +18,8 @@
 from oslo_config import cfg
 from oslo_db import exception as db_exc
 from oslo_log import log as logging
+import six
+import webob
 
 from cinder import context
 from cinder import db
@@ -75,7 +77,11 @@ def destroy(context, id):
         raise exception.InvalidGroupType(reason=msg)
     else:
         elevated = context if context.is_admin else context.elevated()
-        db.group_type_destroy(elevated, id)
+        try:
+            db.group_type_destroy(elevated, id)
+        except exception.GroupTypeInUse as e:
+            msg = _('Target group type is still in use. %s') % six.text_type(e)
+            raise webob.exc.HTTPBadRequest(explanation=msg)
 
 
 def get_all_group_types(context, inactive=0, filters=None, marker=None,