From: Anish Bhatt Date: Tue, 10 Feb 2015 23:59:04 +0000 (-0800) Subject: Improve error handling in refactored Tgt driver X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=ad08c7a1f0c4e6a1e44b5ad02f9efbf86a2a878e;p=openstack-build%2Fcinder-build.git Improve error handling in refactored Tgt driver Only allow IOError to fall through when file not found is acceptable. Cleanup a few error messages, remove uses of six as this is not required Closes-Bug: 1422095 Change-Id: I8a8f1ee561b15b38860b31cae1b444527e998869 --- diff --git a/cinder/tests/targets/test_cxt_driver.py b/cinder/tests/targets/test_cxt_driver.py index e62af497c..f1cda9ec8 100644 --- a/cinder/tests/targets/test_cxt_driver.py +++ b/cinder/tests/targets/test_cxt_driver.py @@ -128,6 +128,23 @@ class TestCxtAdmDriver(test.TestCase): self.target._get_target_chap_auth(ctx, test_vol)) self.assertTrue(mock_open.called) + def test_get_target_chap_auth_negative(self): + test_vol =\ + 'iqn.2010-10.org.openstack:'\ + 'volume-83c2e877-feed-46be-8435-77884fe55b45' + with mock.patch('__builtin__.open') as mock_open: + e = IOError() + e.errno = 123 + mock_open.side_effect = e + ctxt = context.get_admin_context() + self.assertRaises(IOError, + self.target._get_target_chap_auth, + ctxt, test_vol) + mock_open.side_effect = StandardError() + self.assertRaises(StandardError, + self.target._get_target_chap_auth, + ctxt, test_vol) + @mock.patch('cinder.volume.targets.cxt.CxtAdm._get_target', return_value=1) @mock.patch('cinder.utils.execute') diff --git a/cinder/tests/targets/test_tgt_driver.py b/cinder/tests/targets/test_tgt_driver.py index a59aae4cb..049638ad9 100644 --- a/cinder/tests/targets/test_tgt_driver.py +++ b/cinder/tests/targets/test_tgt_driver.py @@ -231,6 +231,23 @@ class TestTgtAdmDriver(test.TestCase): self.assertEqual(expected, self.target._get_target_chap_auth(ctxt, test_vol)) + def test_get_target_chap_auth_negative(self): + test_vol =\ + 'iqn.2010-10.org.openstack:'\ + 'volume-83c2e877-feed-46be-8435-77884fe55b45' + with mock.patch('__builtin__.open') as mock_open: + e = IOError() + e.errno = 123 + mock_open.side_effect = e + ctxt = context.get_admin_context() + self.assertRaises(IOError, + self.target._get_target_chap_auth, + ctxt, test_vol) + mock_open.side_effect = StandardError() + self.assertRaises(StandardError, + self.target._get_target_chap_auth, + ctxt, test_vol) + def test_create_iscsi_target(self): def _fake_execute(*args, **kwargs): diff --git a/cinder/volume/targets/cxt.py b/cinder/volume/targets/cxt.py index 4e5f9134b..14bbf7446 100644 --- a/cinder/volume/targets/cxt.py +++ b/cinder/volume/targets/cxt.py @@ -19,7 +19,6 @@ import re from oslo_concurrency import processutils as putils from oslo_utils import netutils -import six from cinder import exception from cinder.openstack.common import fileutils @@ -114,17 +113,15 @@ class CxtAdm(iscsi.ISCSITarget): volume_conf = f.read() except IOError as e_fnf: LOG.debug('Failed to open config for %(vol_id)s: %(e)s', - {'vol_id': vol_id, 'e': - six.text_type(e_fnf)}) + {'vol_id': vol_id, 'e': e_fnf}) # We don't run on anything non-linux if e_fnf.errno == 2: return None else: raise except Exception as e_vol: - LOG.debug('Failed to open config for %(vol_id)s: %(e)s', - {'vol_id': vol_id, 'e': - six.text_type(e_vol)}) + LOG.error(_LE('Failed to open config for %(vol_id)s: %(e)s'), + {'vol_id': vol_id, 'e': e_vol}) raise m = re.search('Auth_CHAP_Initiator="(\w+)":"(\w+)"', volume_conf) diff --git a/cinder/volume/targets/tgt.py b/cinder/volume/targets/tgt.py index 664c6ce25..876b86ce3 100644 --- a/cinder/volume/targets/tgt.py +++ b/cinder/volume/targets/tgt.py @@ -15,7 +15,6 @@ import re import time from oslo_concurrency import processutils as putils -import six from cinder import exception from cinder.openstack.common import fileutils @@ -107,8 +106,8 @@ class TgtAdm(iscsi.ISCSITarget): except putils.ProcessExecutionError as e: LOG.error(_LE("Failed recovery attempt to create " "iscsi backing lun for Volume " - "id:%(vol_id)s: %(e)s"), - {'vol_id': name, 'e': six.text_type(e)}) + "ID:%(vol_id)s: %(e)s"), + {'vol_id': name, 'e': e}) finally: LOG.debug('StdOut from recreate backing lun: %s', out) LOG.debug('StdErr from recreate backing lun: %s', err) @@ -130,10 +129,18 @@ class TgtAdm(iscsi.ISCSITarget): try: with open(volume_path, 'r') as f: volume_conf = f.read() - except Exception as e: + except IOError as e_fnf: LOG.debug('Failed to open config for Volume %(vol_id)s: %(e)s', - {'vol_id': vol_id, 'e': six.text_type(e)}) - return None + {'vol_id': vol_id, 'e': e_fnf}) + # tgt is linux specific + if e_fnf.errno == 2: + return None + else: + raise + except Exception as e_vol: + LOG.error(_LE('Failed to open config for %(vol_id)s: %(e)s'), + {'vol_id': vol_id, 'e': e_vol}) + raise m = re.search('incominguser (\w+) (\w+)', volume_conf) if m: @@ -214,11 +221,12 @@ class TgtAdm(iscsi.ISCSITarget): # ER marker (Ref bug: #1398078). LOG.warning(_LW('Could not create target because ' 'it already exists for volume: %s'), vol_id) - LOG.debug('Exception was: %s', six.text_type(e)) + LOG.debug('Exception was: %s', e) - LOG.error(_LE("Failed to create iscsi target for Volume " - "ID: %(vol_id)s: %(e)s"), - {'vol_id': vol_id, 'e': six.text_type(e)}) + else: + LOG.error(_LE("Failed to create iscsi target for Volume " + "ID: %(vol_id)s: %(e)s"), + {'vol_id': vol_id, 'e': e}) # Don't forget to remove the persistent file we created os.unlink(volume_path) @@ -305,9 +313,9 @@ class TgtAdm(iscsi.ISCSITarget): 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 " + LOG.error(_LE("Failed to remove iscsi target for Volume " "ID: %(vol_id)s: %(e)s"), - {'vol_id': vol_id, 'e': six.text_type(e)}) + {'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 @@ -330,7 +338,7 @@ class TgtAdm(iscsi.ISCSITarget): 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': six.text_type(e)}) + {'vol_id': vol_id, 'e': e}) raise exception.ISCSITargetRemoveFailed(volume_id=vol_id) # NOTE(jdg): This *should* be there still but incase