From 9f49be03b2e53b9e5eae36b58ba03ee3ff3e4ff4 Mon Sep 17 00:00:00 2001 From: Mike Perez Date: Fri, 6 Mar 2015 17:24:00 -0800 Subject: [PATCH] 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 --- cinder/tests/targets/test_tgt_driver.py | 45 +++++++++++++++++++++++++ cinder/volume/targets/tgt.py | 9 +++-- 2 files changed, 51 insertions(+), 3 deletions(-) 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"), -- 2.45.2