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 e62af497c34251468487ac0a1cd643268c0bc9f7..f1cda9ec857bbc2753bff2d4df0b3aba3ac42ec7 100644 (file)
--- 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 a59aae4cbadc644c38c38ad10797becd9bf4c1fc..049638ad96a4ef12e386a342b88faf167d4a6c7f 100644 (file)
--- 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 4e5f9134b5a682fd8a63332626841a0c18849ee7..14bbf744604dce9db7fb17bfaf415cccd1101b04 100644 (file)
--- 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 664c6ce25af27076a59702c2a48be86bc3b58c0b..876b86ce33f5c86b1b9e9cab4d713a71a6849a61 100644 (file)
--- 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