From: Mike Perez <thingee@gmail.com>
Date: Sat, 7 Mar 2015 01:24:00 +0000 (-0800)
Subject: Don't fail target_delete if ACL's don't exist
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=9f49be03b2e53b9e5eae36b58ba03ee3ff3e4ff4;p=openstack-build%2Fcinder-build.git

Don't fail target_delete if ACL's don't exist

Seen in the gate a bit where a target delete is called and fails because
the ACL's don't exist for that target. If we're deleting a target and
the ACL's don't exist, it's probably safe to move on.

This patch adds a check for this specific case, logs a warning, and
continues rather than raising an exception.

Closes-Bug: #1425310
Change-Id: Ideada8ddea8624aff7add95e9caad2aba4e31ae3
---

diff --git a/cinder/tests/targets/test_tgt_driver.py b/cinder/tests/targets/test_tgt_driver.py
index a59aae4cb..73b3f625c 100644
--- a/cinder/tests/targets/test_tgt_driver.py
+++ b/cinder/tests/targets/test_tgt_driver.py
@@ -336,6 +336,51 @@ class TestTgtAdmDriver(test.TestCase):
                               test_vol_id,
                               test_vol_name)
 
+    @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_acl_not_found(self,
+                                         mock_unlink,
+                                         mock_exec,
+                                         mock_pathexists,
+                                         mock_isfile):
+        def _fake_execute(*args, **kwargs):
+            raise putils.ProcessExecutionError(
+                exit_code=1,
+                stdout='',
+                stderr='this access control rule does not exist',
+                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)
+
     @mock.patch.object(tgt.TgtAdm, '_get_iscsi_properties')
     def test_initialize_connection(self, mock_get_iscsi):
 
diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py
index 664c6ce25..4aca33f3c 100644
--- a/cinder/volume/targets/tgt.py
+++ b/cinder/volume/targets/tgt.py
@@ -301,9 +301,12 @@ class TgtAdm(iscsi.ISCSITarget):
                           iqn,
                           run_as_root=True)
         except putils.ProcessExecutionError as e:
-            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)
+            non_fatal_errors = ("can't find the target",
+                                "access control rule does not exist")
+
+            if any(error in e.stderr for error in non_fatal_errors):
+                LOG.warning(_LW("Failed target removal because target or "
+                                "ACL's 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"),