Fix format errors in brick/iscsi LOG messages

Currently in cinder.brick.iscsi.iscs:LioADM.create_iscsi_target
The Log message for the exception is:
  LOG.error("%s" % e),

This rightfully results in:
  *** UnicodeError: UnicodeError(u'Message objects
      do not support str() because they may contain
      non-ascii characters. Please use unicode() or
      translate() instead.',)

In some cases this causes the Volume service to stop and
doesn't help a ton with debug. While looking at this also
noticed a number of other similar cases where invalid LOG
messages were set up.  Following the i8n guidelines here:
http://docs.openstack.org/developer/oslo.i18n/guidelines.html

Went ahead and cleaned the bulk of the LOG messages in this
file up to adhere to the guidelines as well as fixing the
UnicodeError described in the bug.

Change-Id: I33adf99ca25a2d8f869de5bfa85b4ca8429be05e

diff --git a/cinder/brick/iscsi/iscsi.py b/cinder/brick/iscsi/iscsi.py
index 4f6822661b04b24e96637532047288d5c052a71d..934cdc29022b920336bd55115da9016ddf7a6164 100644 (file)
--- a/cinder/brick/iscsi/iscsi.py
+++ b/cinder/brick/iscsi/iscsi.py
@@ -165,13 +165,11 @@ class TgtAdm(TargetAdmin):
                                    'logicalunit', '--tid',
                                    tid, '--lun', '1', '-b',
                                    path, run_as_root=True)
-            LOG.debug('StdOut from recreate backing lun: %s' % out)
-            LOG.debug('StdErr from recreate backing lun: %s' % err)
         except putils.ProcessExecutionError as e:
             LOG.error(_LE("Failed to recover attempt to create "
-                          "iscsi backing lun for volume "
-                          "id:%(vol_id)s: %(e)s")
-                      % {'vol_id': name, 'e': e})
+                          "iscsi backing lun for Volume "
+                          "ID: %(vol_id)s: %(e)s"),
+                      {'vol_id': name, 'e': e})
 
     def _get_target_chap_auth(self, name):
         volumes_dir = self.volumes_dir
@@ -185,14 +183,14 @@ class TgtAdm(TargetAdmin):
             # NOTE(jdg): Debug is ok here because the caller
             # will just generate the CHAP creds and create the
             # file based on the None return
-            LOG.debug('Failed to open config for %(vol_id)s: %(e)s'
-                      % {'vol_id': vol_id, 'e': six.text_type(e)})
+            LOG.debug('Failed to open config for %(vol_id)s: %(e)s',
+                      {'vol_id': vol_id, 'e': six.text_type(e)})
             return None
 
         m = re.search('incominguser (\w+) (\w+)', volume_conf)
         if m:
             return (m.group(1), m.group(2))
-        LOG.debug('Failed to find CHAP auth from config for %s' % vol_id)
+        LOG.debug('Failed to find CHAP auth from config for %s', vol_id)
         return None
 
     def create_iscsi_target(self, name, tid, lun, path,
@@ -212,7 +210,7 @@ class TgtAdm(TargetAdmin):
                                                              path, chap_str,
                                                              write_cache)
 
-        LOG.info(_LI('Creating iscsi_target for: %s') % vol_id)
+        LOG.info(_LI('Creating iscsi_target for: %s'), vol_id)
         volumes_dir = self.volumes_dir
         volume_path = os.path.join(volumes_dir, vol_id)
 
@@ -220,8 +218,8 @@ class TgtAdm(TargetAdmin):
         f.write(volume_conf)
         f.close()
         LOG.debug('Created volume path %(vp)s,\n'
-                  'content: %(vc)s'
-                  % {'vp': volume_path, 'vc': volume_conf})
+                  'content: %(vc)s',
+                  {'vp': volume_path, 'vc': volume_conf})
 
         old_persist_file = None
         old_name = kwargs.get('old_name', None)
@@ -235,8 +233,6 @@ class TgtAdm(TargetAdmin):
             # created.
             (out, err) = self._run('tgt-admin', '--update', name,
                                    run_as_root=True)
-            LOG.debug("StdOut from tgt-admin --update: %s", out)
-            LOG.debug("StdErr from tgt-admin --update: %s", err)
 
             # Grab targets list for debug
             # Consider adding a check for lun 0 and 1 for tgtadm
@@ -249,12 +245,13 @@ class TgtAdm(TargetAdmin):
                                    '--mode',
                                    'target',
                                    run_as_root=True)
-            LOG.debug("Targets after update: %s" % out)
+            LOG.debug("Targets after update: %s", out)
         except putils.ProcessExecutionError as e:
             LOG.warning(_LW("Failed to create iscsi target for volume "
-                            "id:%(vol_id)s: %(e)s")
-                        % {'vol_id': vol_id, 'e': e})
+                            "id:%(vol_id)s: %(e)s"),
+                        {'vol_id': vol_id, 'e': e.stderr})
             if "target already exists" in e.stderr:
+
                 LOG.warning(_LW('Create iscsi target failed for '
                                 'target already exists'))
                 # NOTE(jdg):  We've run into some cases where the cmd being
@@ -273,9 +270,9 @@ class TgtAdm(TargetAdmin):
         iqn = '%s%s' % (self.iscsi_target_prefix, vol_id)
         tid = self._get_target(iqn)
         if tid is None:
-            LOG.error(_LE("Failed to create iscsi target for volume "
-                          "id:%(vol_id)s. Please ensure your tgtd config file "
-                          "contains 'include %(volumes_dir)s/*'") % {
+            LOG.error(_LE("Failed to create iscsi target for Volume "
+                          "ID: %(vol_id)s. Ensure the tgtd config file "
+                          "contains 'include %(volumes_dir)s/*'"), {
                       'vol_id': vol_id, 'volumes_dir': volumes_dir, })
             raise exception.NotFound()
 
@@ -302,12 +299,12 @@ class TgtAdm(TargetAdmin):
         return tid
 
     def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs):
-        LOG.info(_LI('Removing iscsi_target for: %s') % vol_id)
+        LOG.info(_LI('Removing iscsi_target for: %s'), vol_id)
         vol_uuid_file = vol_name
         volume_path = os.path.join(self.volumes_dir, vol_uuid_file)
         if not os.path.exists(volume_path):
             LOG.warning(_LW('Volume path %s does not exist, '
-                            'nothing to remove.') % volume_path)
+                            'nothing to remove.'), volume_path)
             return
 
         if os.path.isfile(volume_path):
@@ -325,9 +322,9 @@ class TgtAdm(TargetAdmin):
                       run_as_root=True,
                       attempts=CONF.num_shell_tries)
         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': e})
+            LOG.error(_LE("Failed to remove iscsi target for Volume "
+                          "ID: %(vol_id)s: %(e)s"),
+                      {'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
@@ -349,9 +346,9 @@ class TgtAdm(TargetAdmin):
                           iqn,
                           run_as_root=True)
             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': e})
+                LOG.error(_LE("Failed to remove iscsi target for Volume "
+                              "ID: %(vol_id)s: %(e)s"),
+                          {'vol_id': vol_id, 'e': e})
                 raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
 
         # NOTE(jdg): This *should* be there still but incase
@@ -362,7 +359,7 @@ class TgtAdm(TargetAdmin):
             os.unlink(volume_path)
         else:
             LOG.debug('Volume path %s not found at end, '
-                      'of remove_iscsi_target.' % volume_path)
+                      'of remove_iscsi_target.', volume_path)
 
     def show_target(self, tid, iqn=None, **kwargs):
         if iqn is None:
@@ -420,14 +417,14 @@ class IetAdm(TargetAdmin):
                     f.close()
             except putils.ProcessExecutionError as e:
                 vol_id = name.split(':')[1]
-                LOG.error(_LE("Failed to create iscsi target for volume "
-                              "id:%(vol_id)s: %(e)s")
-                          % {'vol_id': vol_id, 'e': e})
+                LOG.error(_LE("Failed to create iscsi target for Volume "
+                              "ID: %(vol_id)s: %(e)s"),
+                          {'vol_id': vol_id, 'e': e})
                 raise exception.ISCSITargetCreateFailed(volume_id=vol_id)
         return tid
 
     def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs):
-        LOG.info(_LI('Removing iscsi_target for volume: %s') % vol_id)
+        LOG.info(_LI('Removing iscsi_target for volume: %s'), vol_id)
         self._delete_logicalunit(tid, lun, **kwargs)
         self._delete_target(tid, **kwargs)
         vol_uuid_file = vol_name
@@ -540,7 +537,7 @@ class LioAdm(TargetAdmin):
 
         vol_id = name.split(':')[1]
 
-        LOG.info(_LI('Creating iscsi_target for volume: %s') % vol_id)
+        LOG.info(_LI('Creating iscsi_target for volume: %s'), vol_id)
 
         if chap_auth is not None:
             (chap_auth_userid, chap_auth_password) = chap_auth.split(' ')[1:]
@@ -553,23 +550,23 @@ class LioAdm(TargetAdmin):
                             chap_auth_password]
             self._run('cinder-rtstool', *command_args, run_as_root=True)
         except putils.ProcessExecutionError as e:
-            LOG.error(_LE("Failed to create iscsi target for volume "
-                          "id:%s.") % vol_id)
-            LOG.error("%s" % e)
+            LOG.error(_LE("Failed to create iscsi target for Volume "
+                          "ID: %(vol_id)s, Error: %(err)s."),
+                      {'vol_id': vol_id, 'err': e.stderr})
 
             raise exception.ISCSITargetCreateFailed(volume_id=vol_id)
 
         iqn = '%s%s' % (self.iscsi_target_prefix, vol_id)
         tid = self._get_target(iqn)
         if tid is None:
-            LOG.error(_LE("Failed to create iscsi target for volume "
-                          "id:%s.") % vol_id)
+            LOG.error(_LE("Failed to create iscsi target for Volume "
+                          "ID: %s."), vol_id)
             raise exception.NotFound()
 
         return tid
 
     def remove_iscsi_target(self, tid, lun, vol_id, vol_name, **kwargs):
-        LOG.info(_LI('Removing iscsi_target: %s') % vol_id)
+        LOG.info(_LI('Removing iscsi_target: %s'), vol_id)
         vol_uuid_name = vol_name
         iqn = '%s%s' % (self.iscsi_target_prefix, vol_uuid_name)
 
@@ -579,9 +576,9 @@ class LioAdm(TargetAdmin):
                       iqn,
                       run_as_root=True)
         except putils.ProcessExecutionError as e:
-            LOG.error(_LE("Failed to remove iscsi target for volume "
-                          "id:%s.") % vol_id)
-            LOG.error("%s" % e)
+            LOG.error(_LE("Failed to remove iscsi target for Volume "
+                          "ID: %(vol_id)s, Error: %(err)s."),
+                      {'vol_id': vol_id, 'err': e.stderr})
             raise exception.ISCSITargetRemoveFailed(volume_id=vol_id)
 
     def show_target(self, tid, iqn=None, **kwargs):
@@ -608,7 +605,7 @@ class LioAdm(TargetAdmin):
                       connector['initiator'],
                       run_as_root=True)
         except putils.ProcessExecutionError:
-            LOG.error(_LE("Failed to add initiator iqn %s to target.") %
+            LOG.error(_LE("Failed to add initiator iqn %s to target."),
                       connector['initiator'])
             raise exception.ISCSITargetAttachFailed(volume_id=volume['id'])
 
@@ -622,7 +619,7 @@ class LioAdm(TargetAdmin):
                       connector['initiator'],
                       run_as_root=True)
         except putils.ProcessExecutionError:
-            LOG.error(_LE("Failed to delete initiator iqn %s to target.") %
+            LOG.error(_LE("Failed to delete initiator iqn %s to target."),
                       connector['initiator'])
             raise exception.ISCSITargetAttachFailed(volume_id=volume['id'])