NetApp: Cleanup if E-Series volume create fails

The creation of a volume may fail in such a manner
that a volume in a partial state has been created.
When Openstack comes back re-attempting the creation
operation it fails because the partial volume with
the name already exists.

Solution: When there is a failure creating a volume,
check if a partial volume was created and delete it
so any retry will be successful and the storage device
does not contain any dead volumes.

Closes-Bug: 1506940
Change-Id: Id93527a4cab314f27a13d8c08b772fed538d1092

diff --git a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py
index de69391b6d1a1c5e1a18ef6c0422497c8a75ccb9..90e5c27d485e3ae562499c665c72afc1984d61fd 100644 (file)
--- a/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py
+++ b/cinder/tests/unit/volume/drivers/netapp/eseries/test_library.py
@@ -878,6 +878,78 @@ class NetAppEseriesLibraryTestCase(test.TestCase):
 
         mock_invoke.assert_not_called()
 
+    @mock.patch.object(library, 'LOG', mock.Mock())
+    def test_create_volume_fail_clean(self):
+        """Test volume creation fail w/o a partial volume being created.
+
+        Test the failed creation of a volume where a partial volume with
+        the name has not been created, thus no cleanup is required.
+        """
+        self.library._get_volume = mock.Mock(
+            side_effect = exception.VolumeNotFound(message=''))
+        self.library._client.create_volume = mock.Mock(
+            side_effect = exception.NetAppDriverException)
+        self.library._client.delete_volume = mock.Mock()
+        fake_volume = copy.deepcopy(get_fake_volume())
+
+        self.assertRaises(exception.NetAppDriverException,
+                          self.library.create_volume, fake_volume)
+
+        self.assertTrue(self.library._get_volume.called)
+        self.assertFalse(self.library._client.delete_volume.called)
+        self.assertEqual(1, library.LOG.error.call_count)
+
+    @mock.patch.object(library, 'LOG', mock.Mock())
+    def test_create_volume_fail_dirty(self):
+        """Test volume creation fail where a partial volume has been created.
+
+        Test scenario where the creation of a volume fails and a partial
+        volume is created with the name/id that was supplied by to the
+        original creation call.  In this situation the partial volume should
+        be detected and removed.
+        """
+        fake_volume = copy.deepcopy(get_fake_volume())
+        self.library._get_volume = mock.Mock(return_value=fake_volume)
+        self.library._client.list_volume = mock.Mock(return_value=fake_volume)
+        self.library._client.create_volume = mock.Mock(
+            side_effect = exception.NetAppDriverException)
+        self.library._client.delete_volume = mock.Mock()
+
+        self.assertRaises(exception.NetAppDriverException,
+                          self.library.create_volume, fake_volume)
+
+        self.assertTrue(self.library._get_volume.called)
+        self.assertTrue(self.library._client.delete_volume.called)
+        self.library._client.delete_volume.assert_called_once_with(
+            fake_volume["id"])
+        self.assertEqual(1, library.LOG.error.call_count)
+
+    @mock.patch.object(library, 'LOG', mock.Mock())
+    def test_create_volume_fail_dirty_fail_delete(self):
+        """Volume creation fail with partial volume deletion fails
+
+        Test scenario where the creation of a volume fails and a partial
+        volume is created with the name/id that was supplied by to the
+        original creation call. The partial volume is detected but when
+        the cleanup deletetion of that fragment volume is attempted it fails.
+        """
+        fake_volume = copy.deepcopy(get_fake_volume())
+        self.library._get_volume = mock.Mock(return_value=fake_volume)
+        self.library._client.list_volume = mock.Mock(return_value=fake_volume)
+        self.library._client.create_volume = mock.Mock(
+            side_effect = exception.NetAppDriverException)
+        self.library._client.delete_volume = mock.Mock(
+            side_effect = exception.NetAppDriverException)
+
+        self.assertRaises(exception.NetAppDriverException,
+                          self.library.create_volume, fake_volume)
+
+        self.assertTrue(self.library._get_volume.called)
+        self.assertTrue(self.library._client.delete_volume.called)
+        self.library._client.delete_volume.assert_called_once_with(
+            fake_volume["id"])
+        self.assertEqual(2, library.LOG.error.call_count)
+
 
 @ddt.ddt
 class NetAppEseriesLibraryMultiAttachTestCase(test.TestCase):

diff --git a/cinder/volume/drivers/netapp/eseries/library.py b/cinder/volume/drivers/netapp/eseries/library.py
index 323af2d6c810f18d006384acac9ae613fa61ca1d..a7ef5c119608214aabf87754ef649b5013befdeb 100644 (file)
--- a/cinder/volume/drivers/netapp/eseries/library.py
+++ b/cinder/volume/drivers/netapp/eseries/library.py
@@ -428,6 +428,26 @@ class NetAppESeriesLibrary(object):
         except exception.NetAppDriverException as e:
             with excutils.save_and_reraise_exception():
                 LOG.error(_LE("Error creating volume. Msg - %s."), e)
+                # There was some kind failure creating the volume, make sure no
+                # partial flawed work exists
+                try:
+                    bad_vol = self._get_volume(eseries_volume_label)
+                except Exception:
+                    # Swallowing the exception intentionally because this is
+                    # emergency cleanup to make sure no intermediate volumes
+                    # were left. In this whole error situation, the more
+                    # common route would be for no volume to have been created.
+                    pass
+                else:
+                    # Some sort of partial volume was created despite the
+                    # error.  Lets clean it out so no partial state volumes or
+                    # orphans are left.
+                    try:
+                        self._client.delete_volume(bad_vol["id"])
+                    except exception.NetAppDriverException as e2:
+                        LOG.error(_LE(
+                            "Error cleaning up failed volume creation.  "
+                            "Msg - %s."), e2)
 
         return vol