From: John Griffith <john.griffith@solidfire.com>
Date: Mon, 9 Feb 2015 21:45:40 +0000 (-0700)
Subject: Don't fail target_delete if target doesn't exist
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=19a31d33a4fa732eedc3a6c6c9a5956c70498779;p=openstack-build%2Fcinder-build.git

Don't fail target_delete if target doesn't exist

There are cases seen in the Gate where a target delete is
called and an exception is raised because the target does
not exist. In the cinder target driver code we raise this
as an ISCSITargetRemoveFailed exception, but if we're asking
to delete the target and the target doesn't exist we can
probably safely move along.

This patch adds a check for this specific case and logs a warning
and continues rather than failing.  We also add a unit test to
check this case.

Change-Id: I7021cafc9ee48bb8ad54433e4482ff1d61e865ae
Closes-Bug: #1420010
---

diff --git a/cinder/tests/targets/test_lio_driver.py b/cinder/tests/targets/test_lio_driver.py
index 06547a9ed..f2cb81596 100644
--- a/cinder/tests/targets/test_lio_driver.py
+++ b/cinder/tests/targets/test_lio_driver.py
@@ -80,6 +80,14 @@ class TestLioAdmDriver(test_tgt.TestTgtAdmDriver):
                           self.fake_volumes_dir,
                           chap_auth)
 
+    def test_delete_target_not_found(self):
+        # NOTE(jdg): This test inherits from the
+        # tgt driver tests, this particular test
+        # is tgt driver specific and does not apply here.
+        # We implement it and pass because if we don't it
+        # calls the parent and fails due to missing mocks.
+        pass
+
     @mock.patch.object(lio.LioAdm, 'create_iscsi_target')
     def test_ensure_export(self, _mock_create):
 
diff --git a/cinder/tests/targets/test_tgt_driver.py b/cinder/tests/targets/test_tgt_driver.py
index efbf599d1..306c8b165 100644
--- a/cinder/tests/targets/test_tgt_driver.py
+++ b/cinder/tests/targets/test_tgt_driver.py
@@ -18,6 +18,7 @@ from oslo_concurrency import processutils as putils
 from oslo_utils import timeutils
 
 from cinder import context
+from cinder import exception
 from cinder import test
 from cinder import utils
 from cinder.volume import configuration as conf
@@ -237,6 +238,51 @@ class TestTgtAdmDriver(test.TestCase):
                 0,
                 self.fake_volumes_dir))
 
+    @mock.patch('os.path.isfile', return_value=True)
+    @mock.patch('os.path.exists', return_value=True)
+    @mock.patch.object(utils, 'execute')
+    @mock.patch('os.unlink', return_value=None)
+    def test_delete_target_not_found(self,
+                                     mock_unlink,
+                                     mock_exec,
+                                     mock_pathexists,
+                                     mock_isfile):
+        def _fake_execute(*args, **kwargs):
+            raise putils.ProcessExecutionError(
+                exit_code=1,
+                stdout='',
+                stderr='can\'t find the target',
+                cmd='tgt-admin --force --delete')
+
+        def _fake_execute_wrong_message(*args, **kwargs):
+            raise putils.ProcessExecutionError(
+                exit_code=1,
+                stdout='',
+                stderr='this isnt the error your looking for',
+                cmd='tgt-admin --force --delete')
+
+        mock_exec.side_effect = _fake_execute
+
+        test_vol_id = '83c2e877-feed-46be-8435-77884fe55b45'
+        test_vol_name = 'volume-83c2e877-feed-46be-8435-77884fe55b45'
+
+        with mock.patch.object(self.target, '_get_target', return_value=False):
+            self.assertEqual(
+                None,
+                self.target.remove_iscsi_target(
+                    1,
+                    0,
+                    test_vol_id,
+                    test_vol_name))
+
+            mock_exec.side_effect = _fake_execute_wrong_message
+            self.assertRaises(exception.ISCSITargetRemoveFailed,
+                              self.target.remove_iscsi_target,
+                              1,
+                              0,
+                              test_vol_id,
+                              test_vol_name)
+
     def test_create_export(self):
 
         def _fake_execute(*args, **kwargs):
diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py
index 9f8878a03..3b0c00903 100644
--- a/cinder/volume/targets/tgt.py
+++ b/cinder/volume/targets/tgt.py
@@ -389,10 +389,14 @@ class TgtAdm(iscsi.ISCSITarget):
                           iqn,
                           run_as_root=True)
         except putils.ProcessExecutionError as e:
-            LOG.error(_LE("Failed to remove iscsi target for volume "
-                          "id:%(vol_id)s: %(e)s")
-                      % {'vol_id': vol_id, 'e': e})
-            raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
+            if "can't find the target" in e.stderr:
+                LOG.warning(_LW("Failed target removal because target "
+                                "couldn't be found for iqn: %s."), iqn)
+            else:
+                LOG.error(_LE("Failed to remove iscsi target for volume "
+                              "id:%(vol_id)s: %(e)s"),
+                          {'vol_id': vol_id, 'e': e})
+                raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
         # NOTE(jdg): There's a bug in some versions of tgt that
         # will sometimes fail silently when using the force flag
         #    https://bugs.launchpad.net/ubuntu/+source/tgt/+bug/1305343