create_volume: cleanup consistencygroup when driver exception

Creating a volume with group_id, if driver raises an exception,
the volume status will always be “creating”.The status doesn’t
update to “error” because “ObjectActionError: consistency group
changed”
This patch fixes the issue by invoking _cleanup_cg_in_volume
when driver raises an exception.

Change-Id: Ibbb077f66b41c359b50fff053aa183382c2cde14
Closes-Bug: #1729215

cinder/tests/unit/group/test_groups_manager.py

@@ -65,6 +65,72 @@ class GroupManagerTestCase(test.TestCase):
         self.assertRaises(exception.InvalidVolume,
                           self.volume_api.delete, self.context, volume)
 
+    @mock.patch(
+        'cinder.tests.fake_driver.FakeLoggingVolumeDriver.'
+        'create_cloned_volume')
+    @mock.patch(
+        'cinder.tests.fake_driver.FakeLoggingVolumeDriver.'
+        'create_volume_from_snapshot')
+    @mock.patch('cinder.tests.fake_driver.FakeLoggingVolumeDriver.'
+                'create_volume')
+    def test_create_vol_with_group_id_driver_exception(self,
+                                                       mock_create_volume,
+                                                       mock_create_from_snap,
+                                                       mock_create_cloned_vol):
+        """Test create a volume with group_id but driver exception."""
+        # create_raw_volume with group id, but driver exception
+        mock_create_volume.side_effect = exception.CinderException
+        group = tests_utils.create_group(
+            self.context,
+            availability_zone=CONF.storage_availability_zone,
+            volume_type_ids=[fake.VOLUME_TYPE_ID],
+            group_type_id=fake.GROUP_TYPE_ID,
+            host=CONF.host)
+        self.volume.create_group(self.context, group)
+
+        volume = tests_utils.create_volume(
+            self.context,
+            group_id=group.id,
+            volume_type_id=fake.VOLUME_TYPE_ID,
+            status='available',
+            host=group.host)
+        self.assertRaises(exception.CinderException,
+                          self.volume.create_volume,
+                          self.context,
+                          volume)
+        self.assertIsNone(volume.consistencygroup_id)
+
+        # create volume from_snapshot with group id but driver exception
+        mock_create_from_snap.side_effect = exception.CinderException
+        snapshot = tests_utils.create_snapshot(self.context, volume.id)
+        volume2 = tests_utils.create_volume(
+            self.context,
+            group_id=group.id,
+            snapshot_id=snapshot.id,
+            status='available',
+            host=group.host,
+            volume_type_id=fake.VOLUME_TYPE_ID)
+        self.assertRaises(exception.CinderException,
+                          self.volume.create_volume,
+                          self.context,
+                          volume2)
+        self.assertIsNone(volume2.consistencygroup_id)
+
+        # create cloned volume with group_id but driver exception
+        mock_create_cloned_vol.side_effect = exception.CinderException
+        volume3 = tests_utils.create_volume(
+            self.context,
+            group_id=group.id,
+            source_volid=volume.id,
+            status='available',
+            host=group.host,
+            volume_type_id=fake.VOLUME_TYPE_ID)
+        self.assertRaises(exception.CinderException,
+                          self.volume.create_volume,
+                          self.context,
+                          volume3)
+        self.assertIsNone(volume3.consistencygroup_id)
+
     @mock.patch.object(GROUP_QUOTAS, "reserve",
                        return_value=["RESERVATION"])
     @mock.patch.object(GROUP_QUOTAS, "commit")

cinder/volume/flows/manager/create_volume.py

@@ -427,9 +427,11 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
     def _create_from_snapshot(self, context, volume, snapshot_id,
                               **kwargs):
         snapshot = objects.Snapshot.get_by_id(context, snapshot_id)
-        model_update = self.driver.create_volume_from_snapshot(volume,
-                                                               snapshot)
-        self._cleanup_cg_in_volume(volume)
+        try:
+            model_update = self.driver.create_volume_from_snapshot(volume,
+                                                                   snapshot)
+        finally:
+            self._cleanup_cg_in_volume(volume)
         # NOTE(harlowja): Subtasks would be useful here since after this
         # point the volume has already been created and further failures
         # will not destroy the volume (although they could in the future).
@@ -469,8 +471,10 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
         # and we should have proper locks on the source volume while actions
         # that use the source volume are underway.
         srcvol_ref = objects.Volume.get_by_id(context, source_volid)
-        model_update = self.driver.create_cloned_volume(volume, srcvol_ref)
-        self._cleanup_cg_in_volume(volume)
+        try:
+            model_update = self.driver.create_cloned_volume(volume, srcvol_ref)
+        finally:
+            self._cleanup_cg_in_volume(volume)
         # NOTE(harlowja): Subtasks would be useful here since after this
         # point the volume has already been created and further failures
         # will not destroy the volume (although they could in the future).
@@ -861,8 +865,10 @@ class CreateVolumeFromSpecTask(flow_utils.CinderTask):
         return model_update
 
     def _create_raw_volume(self, volume, **kwargs):
-        ret = self.driver.create_volume(volume)
-        self._cleanup_cg_in_volume(volume)
+        try:
+            ret = self.driver.create_volume(volume)
+        finally:
+            self._cleanup_cg_in_volume(volume)
         return ret
 
     def execute(self, context, volume, volume_spec):