Permalink
Browse files

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
  • Loading branch information...
anish committed Feb 10, 2015
1 parent d911a31 commit ad08c7a1f0c4e6a1e44b5ad02f9efbf86a2a878e
@@ -128,6 +128,23 @@ def test_get_target_chap_auth(self):
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')
@@ -231,6 +231,23 @@ def test_get_target_chap_auth(self):
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):
@@ -19,7 +19,6 @@
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 @@ def _get_target_chap_auth(self, context, name):
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)
@@ -15,7 +15,6 @@
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 @@ def _recreate_backing_lun(self, iqn, tid, name, path):
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 @@ def _get_target_chap_auth(self, context, iscsi_name):
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 @@ def create_iscsi_target(self, name, tid, lun, path,
# 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 @@ def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs):
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 @@ def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs):
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

0 comments on commit ad08c7a

Please sign in to comment.