]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Dell: Failed vol create could leave dead volumes
authorTom Swanson <tom_swanson@dell.com>
Mon, 15 Feb 2016 17:42:34 +0000 (11:42 -0600)
committerTom Swanson <tom_swanson@dell.com>
Tue, 16 Feb 2016 15:47:27 +0000 (09:47 -0600)
Added _cleanup_failed_create_volume which tries to delete any
partially created volumes.

Change-Id: I1f2f549b364a013b713e193591d08b9e9376569e
Closes-Bug: 1546161

cinder/tests/unit/test_dellsc.py
cinder/volume/drivers/dell/dell_storagecenter_common.py

index 41da543030cf00cfef086b0825bc3399080a9c0b..d636bdad0f3c846b8faa80a0e61c2a23ed48e410 100644 (file)
@@ -537,7 +537,10 @@ class DellSCSanISCSIDriverTestCase(test.TestCase):
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
                        'find_sc',
                        return_value=12345)
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       'delete_volume')
     def test_create_volume_failure(self,
+                                   mock_delete_volume,
                                    mock_find_sc,
                                    mock_create_volume,
                                    mock_close_connection,
@@ -546,6 +549,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase):
         volume = {'id': self.volume_name, 'size': 1}
         self.assertRaises(exception.VolumeBackendAPIException,
                           self.driver.create_volume, volume)
+        self.assertTrue(mock_delete_volume.called)
 
     @mock.patch.object(dell_storagecenter_iscsi.DellStorageCenterISCSIDriver,
                        '_delete_replications')
@@ -1076,7 +1080,10 @@ class DellSCSanISCSIDriverTestCase(test.TestCase):
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
                        'create_view_volume',
                        return_value=None)
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       'delete_volume')
     def test_create_volume_from_snapshot_failed(self,
+                                                mock_delete_volume,
                                                 mock_create_view_volume,
                                                 mock_find_replay_profile,
                                                 mock_find_replay,
@@ -1093,6 +1100,7 @@ class DellSCSanISCSIDriverTestCase(test.TestCase):
         self.assertTrue(mock_find_replay.called)
         self.assertTrue(mock_find_volume.called)
         self.assertFalse(mock_find_replay_profile.called)
+        self.assertTrue(mock_delete_volume.called)
 
     @mock.patch.object(dell_storagecenter_iscsi.DellStorageCenterISCSIDriver,
                        '_create_replications')
@@ -1189,6 +1197,36 @@ class DellSCSanISCSIDriverTestCase(test.TestCase):
         self.assertTrue(mock_find_volume.called)
         self.assertEqual({}, ret)
 
+    @mock.patch.object(dell_storagecenter_iscsi.DellStorageCenterISCSIDriver,
+                       '_create_replications',
+                       return_value={})
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       'find_sc',
+                       return_value=12345)
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       'find_volume',
+                       return_value=VOLUME)
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       'create_cloned_volume',
+                       return_value=None)
+    @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
+                       'delete_volume')
+    def test_create_cloned_volume_failed(self,
+                                         mock_delete_volume,
+                                         mock_create_cloned_volume,
+                                         mock_find_volume,
+                                         mock_find_sc,
+                                         mock_create_replications,
+                                         mock_close_connection,
+                                         mock_open_connection,
+                                         mock_init):
+        volume = {'id': self.volume_name + '_clone'}
+        src_vref = {'id': self.volume_name}
+        self.assertRaises(exception.VolumeBackendAPIException,
+                          self.driver.create_cloned_volume,
+                          volume, src_vref)
+        self.assertTrue(mock_delete_volume.called)
+
     @mock.patch.object(dell_storagecenter_api.StorageCenterApi,
                        'delete_volume')
     @mock.patch.object(dell_storagecenter_iscsi.DellStorageCenterISCSIDriver,
index fad3edd53e3797d5bd74cb3a7f4f1282ce65f1c1..8888119302d45baddcbc6e5bdf446abd7c26e1a4 100644 (file)
@@ -199,6 +199,13 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD,
                             'replication_driver_data': replication_driver_data}
         return model_update
 
+    @staticmethod
+    def _cleanup_failed_create_volume(api, volumename):
+        try:
+            api.delete_volume(volumename)
+        except exception.VolumeBackendAPIException as ex:
+            LOG.info(_LI('Non fatal cleanup error: %s.'), ex.msg)
+
     def create_volume(self, volume):
         """Create a volume."""
         model_update = {}
@@ -224,6 +231,10 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD,
                                                  volume_size,
                                                  storage_profile,
                                                  replay_profile_string)
+                    if scvolume is None:
+                        raise exception.VolumeBackendAPIException(
+                            message=_('Unable to create volume %s') %
+                            volume_name)
 
                 # Update Consistency Group
                 self._add_volume_to_consistency_group(api, scvolume, volume)
@@ -234,14 +245,13 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD,
             except Exception:
                 # if we actually created a volume but failed elsewhere
                 # clean up the volume now.
-                if scvolume:
-                    api.delete_volume(volume_name)
+                self._cleanup_failed_create_volume(api, volume_name)
                 with excutils.save_and_reraise_exception():
                     LOG.error(_LE('Failed to create volume %s'),
                               volume_name)
         if scvolume is None:
             raise exception.VolumeBackendAPIException(
-                data=_('Unable to create volume'))
+                data=_('Unable to create volume. Backend down.'))
 
         return model_update
 
@@ -366,6 +376,12 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD,
                                 'storagetype:replayprofiles')
                             scvolume = api.create_view_volume(
                                 volume_name, replay, replay_profile_string)
+                            if scvolume is None:
+                                raise exception.VolumeBackendAPIException(
+                                    message=_('Unable to create volume '
+                                              '%(name)s from %(snap)s.') %
+                                    {'name': volume_name,
+                                     'snap': snapshot_id})
 
                             # Update Consistency Group
                             self._add_volume_to_consistency_group(api,
@@ -378,8 +394,7 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD,
 
             except Exception:
                 # Clean up after ourselves.
-                if scvolume:
-                    api.delete_volume(volume_name)
+                self._cleanup_failed_create_volume(api, volume_name)
                 with excutils.save_and_reraise_exception():
                     LOG.error(_LE('Failed to create volume %s'),
                               volume_name)
@@ -414,6 +429,12 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD,
                         # Create our volume
                         scvolume = api.create_cloned_volume(
                             volume_name, srcvol, replay_profile_string)
+                        if scvolume is None:
+                            raise exception.VolumeBackendAPIException(
+                                message=_('Unable to create volume '
+                                          '%(name)s from %(vol)s.') %
+                                {'name': volume_name,
+                                 'vol': src_volume_name})
 
                         # Update Consistency Group
                         self._add_volume_to_consistency_group(api,
@@ -425,8 +446,7 @@ class DellCommonDriver(driver.ConsistencyGroupVD, driver.ManageableVD,
                                                                  scvolume)
             except Exception:
                 # Clean up after ourselves.
-                if scvolume:
-                    api.delete_volume(volume_name)
+                self._cleanup_failed_create_volume(api, volume_name)
                 with excutils.save_and_reraise_exception():
                     LOG.error(_LE('Failed to create volume %s'),
                               volume_name)