Add a hacking rule for string interpolation at logging

String interpolation should be delayed to be handled
by the logging code, rather than being done at the point
of the logging call.
See the oslo i18n guideline
* https://docs.openstack.org/oslo.i18n/latest/user/guidelines.html#adding-variables-to-log-messages
and
* https://github.com/openstack-dev/hacking/blob/master/hacking/checks/other.py#L39

Change-Id: I8a4f5f896865aebbff88ee894f0081e58cfce9ef

Author: coldmoment

HACKING.rst

@@ -21,3 +21,4 @@ Magnum Specific Commandments
 - [M339] Don't use xrange()
 - [M340] Check for explicit import of the _ function.
 - [M352] LOG.warn is deprecated. Enforce use of LOG.warning.
+- [M353] String interpolation should be delayed at logging calls.

magnum/api/app.py

@@ -59,7 +59,7 @@ def load_app():
 
     if not cfg_file:
         raise cfg.ConfigFilesNotFoundError([CONF.api.api_paste_config])
-    LOG.info("Full WSGI config used: %s" % cfg_file)
+    LOG.info("Full WSGI config used: %s", cfg_file)
     return deploy.loadapp("config:" + cfg_file)
 
 

magnum/api/controllers/v1/bay.py

@@ -361,7 +361,7 @@ def _collect_fault_info(self, context, bay):
             failed_resources = []
             LOG.warning("Failed to retrieve failed resources for "
                         "bay %(bay)s from Heat stack %(stack)s "
-                        "due to error: %(e)s" %
+                        "due to error: %(e)s",
                         {'bay': bay.uuid, 'stack': bay.stack_id, 'e': e},
                         exc_info=True)
 

magnum/api/controllers/v1/cluster.py

@@ -334,7 +334,7 @@ def _collect_fault_info(self, context, cluster):
             failed_resources = []
             LOG.warning("Failed to retrieve failed resources for "
                         "cluster %(cluster)s from Heat stack "
-                        "%(stack)s due to error: %(e)s" %
+                        "%(stack)s due to error: %(e)s",
                         {'cluster': cluster.uuid,
                          'stack': cluster.stack_id, 'e': e},
                         exc_info=True)

magnum/cmd/api.py

@@ -72,17 +72,17 @@ def main():
     # Create the WSGI server and start it
     host, port = CONF.api.host, CONF.api.port
 
-    LOG.info('Starting server in PID %s' % os.getpid())
+    LOG.info('Starting server in PID %s', os.getpid())
     LOG.debug("Configuration:")
     CONF.log_opt_values(LOG, logging.DEBUG)
 
-    LOG.info('Serving on %(proto)s://%(host)s:%(port)s' %
+    LOG.info('Serving on %(proto)s://%(host)s:%(port)s',
              dict(proto="https" if use_ssl else "http", host=host, port=port))
 
     workers = CONF.api.workers
     if not workers:
         workers = processutils.get_worker_count()
     LOG.info('Server will handle each request in a new process up to'
-             ' %s concurrent processes' % workers)
+             ' %s concurrent processes', workers)
     serving.run_simple(host, port, app, processes=workers,
                        ssl_context=_get_ssl_configs(use_ssl))

magnum/cmd/conductor.py

@@ -41,7 +41,7 @@ def main():
 
     gmr.TextGuruMeditation.setup_autorun(version)
 
-    LOG.info('Starting server in PID %s' % os.getpid())
+    LOG.info('Starting server in PID %s', os.getpid())
     LOG.debug("Configuration:")
     CONF.log_opt_values(LOG, logging.DEBUG)
 

magnum/common/exception.py

@@ -92,7 +92,7 @@ def __init__(self, message=None, **kwargs):
             # kwargs doesn't match a variable in the message
             # log the issue and the kwargs
             LOG.exception('Exception in string format operation, '
-                          'kwargs: %s' % kwargs)
+                          'kwargs: %s', kwargs)
             try:
                 if CONF.fatal_exception_format_errors:
                     raise

magnum/common/keystone.py

@@ -102,8 +102,8 @@ def _get_legacy_auth(self):
         LOG.warning('Auth plugin and its options for service user '
                     'must be provided in [%(new)s] section. '
                     'Using values from [%(old)s] section is '
-                    'deprecated.' % {'new': ksconf.CFG_GROUP,
-                                     'old': ksconf.CFG_LEGACY_GROUP})
+                    'deprecated.', {'new': ksconf.CFG_GROUP,
+                                    'old': ksconf.CFG_LEGACY_GROUP})
 
         conf = getattr(CONF, ksconf.CFG_LEGACY_GROUP)
 

magnum/common/urlfetch.py

@@ -38,7 +38,7 @@ def get(url, allowed_schemes=('http', 'https')):
     the allowed_schemes argument.
     Raise an IOError if getting the data fails.
     """
-    LOG.info('Fetching data from %s' % url)
+    LOG.info('Fetching data from %s', url)
 
     components = urllib.parse.urlparse(url)
 

magnum/common/utils.py

@@ -85,10 +85,10 @@ def execute(*cmd, **kwargs):
     if kwargs.get('run_as_root') and 'root_helper' not in kwargs:
         kwargs['root_helper'] = _get_root_helper()
     result = processutils.execute(*cmd, **kwargs)
-    LOG.debug('Execution completed, command line is "%s"' %
+    LOG.debug('Execution completed, command line is "%s"',
               ' '.join(map(str, cmd)))
-    LOG.debug('Command stdout is: "%s"' % result[0])
-    LOG.debug('Command stderr is: "%s"' % result[1])
+    LOG.debug('Command stdout is: "%s"', result[0])
+    LOG.debug('Command stderr is: "%s"', result[1])
     return result
 
 
@@ -125,15 +125,15 @@ def tempdir(**kwargs):
         try:
             shutil.rmtree(tmpdir)
         except OSError as e:
-            LOG.error('Could not remove tmpdir: %s' % e)
+            LOG.error('Could not remove tmpdir: %s', e)
 
 
 def rmtree_without_raise(path):
     try:
         if os.path.isdir(path):
             shutil.rmtree(path)
     except OSError as e:
-        LOG.warning("Failed to remove dir %(path)s, error: %(e)s" %
+        LOG.warning("Failed to remove dir %(path)s, error: %(e)s",
                     {'path': path, 'e': e})
 
 
@@ -148,7 +148,7 @@ def safe_rstrip(value, chars=None):
     if not isinstance(value, six.string_types):
         LOG.warning("Failed to remove trailing character. "
                     "Returning original object. "
-                    "Supplied object is not a string: %s," % value)
+                    "Supplied object is not a string: %s,", value)
         return value
 
     return value.rstrip(chars) or value

magnum/conductor/handlers/cluster_conductor.py

@@ -151,14 +151,14 @@ def cluster_delete(self, context, uuid):
             cluster.status_reason = None
         except exc.HTTPNotFound:
             LOG.info('The cluster %s was not found during cluster'
-                     ' deletion.' % cluster.id)
+                     ' deletion.', cluster.id)
             try:
                 trust_manager.delete_trustee_and_trust(osc, context, cluster)
                 cert_manager.delete_certificates_from_cluster(cluster,
                                                               context=context)
                 cluster.destroy()
             except exception.ClusterNotFound:
-                LOG.info('The cluster %s has been deleted by others.' %
+                LOG.info('The cluster %s has been deleted by others.',
                          uuid)
             conductor_utils.notify_about_cluster_operation(
                 context, taxonomy.ACTION_DELETE, taxonomy.OUTCOME_SUCCESS)

magnum/conductor/handlers/common/cert_manager.py

@@ -43,7 +43,7 @@ def _generate_ca_cert(issuer_name, context=None):
         name=issuer_name,
         context=context,
     )
-    LOG.debug('CA cert is created: %s' % ca_cert_ref)
+    LOG.debug('CA cert is created: %s', ca_cert_ref)
     return ca_cert_ref, ca_cert, ca_password
 
 
@@ -70,7 +70,7 @@ def _generate_client_cert(issuer_name, ca_cert, ca_password, context=None):
         name=CONDUCTOR_CLIENT_NAME,
         context=context
     )
-    LOG.debug('Magnum client cert is created: %s' % magnum_cert_ref)
+    LOG.debug('Magnum client cert is created: %s', magnum_cert_ref)
     return magnum_cert_ref
 
 
@@ -92,7 +92,7 @@ def generate_certificates_to_cluster(cluster, context=None):
     try:
         issuer_name = _get_issuer_name(cluster)
 
-        LOG.debug('Start to generate certificates: %s' % issuer_name)
+        LOG.debug('Start to generate certificates: %s', issuer_name)
 
         ca_cert_ref, ca_cert, ca_password = _generate_ca_cert(issuer_name,
                                                               context=context)
@@ -104,7 +104,7 @@ def generate_certificates_to_cluster(cluster, context=None):
         cluster.ca_cert_ref = ca_cert_ref
         cluster.magnum_cert_ref = magnum_cert_ref
     except Exception:
-        LOG.exception('Failed to generate certificates for Cluster: %s' %
+        LOG.exception('Failed to generate certificates for Cluster: %s',
                       cluster.uuid)
         raise exception.CertificatesToClusterFailed(cluster_uuid=cluster.uuid)
 
@@ -174,5 +174,5 @@ def delete_certificates_from_cluster(cluster, context=None):
                 cert_manager.get_backend().CertManager.delete_cert(
                     cert_ref, resource_ref=cluster.uuid, context=context)
         except Exception:
-            LOG.warning("Deleting certs is failed for Cluster %s" %
+            LOG.warning("Deleting certs is failed for Cluster %s",
                         cluster.uuid)

magnum/conductor/handlers/common/trust_manager.py

@@ -37,7 +37,7 @@ def create_trustee_and_trust(osc, cluster):
 
     except Exception:
         LOG.exception(
-            'Failed to create trustee and trust for Cluster: %s' %
+            'Failed to create trustee and trust for Cluster: %s',
             cluster.uuid)
         raise exception.TrusteeOrTrustToClusterFailed(
             cluster_uuid=cluster.uuid)

magnum/conductor/k8s_api.py

@@ -37,7 +37,7 @@ def _create_temp_file_with_content(self, content):
             tmp.write(content)
             tmp.flush()
         except Exception as err:
-            LOG.error("Error while creating temp file: %s" % err)
+            LOG.error("Error while creating temp file: %s", err)
             raise
         return tmp
 

magnum/conductor/scale_manager.py

@@ -67,13 +67,13 @@ def get_removal_nodes(self, hosts_output):
             LOG.warning(
                 "About to remove %(num_removal)d nodes, which is larger than "
                 "the number of empty nodes (%(num_empty)d). %(num_non_empty)d "
-                "non-empty nodes will be removed." % {
+                "non-empty nodes will be removed.", {
                     'num_removal': num_of_removal,
                     'num_empty': len(hosts_no_container),
                     'num_non_empty': num_of_removal - len(hosts_no_container)})
 
         hosts_to_remove = hosts_no_container[0:num_of_removal]
-        LOG.info('Require removal of hosts: %s' % hosts_to_remove)
+        LOG.info('Require removal of hosts: %s', hosts_to_remove)
 
         return hosts_to_remove
 

magnum/drivers/heat/driver.py

@@ -190,17 +190,17 @@ def poll_and_check(self):
             self._cluster_failed(stack)
 
     def _delete_complete(self):
-        LOG.info('Cluster has been deleted, stack_id: %s'
-                 % self.cluster.stack_id)
+        LOG.info('Cluster has been deleted, stack_id: %s',
+                 self.cluster.stack_id)
         try:
             trust_manager.delete_trustee_and_trust(self.openstack_client,
                                                    self.context,
                                                    self.cluster)
             cert_manager.delete_certificates_from_cluster(self.cluster,
                                                           context=self.context)
         except exception.ClusterNotFound:
-            LOG.info('The cluster %s has been deleted by others.'
-                     % self.cluster.uuid)
+            LOG.info('The cluster %s has been deleted by others.',
+                     self.cluster.uuid)
 
     def _sync_cluster_status(self, stack):
         self.cluster.status = stack.stack_status
@@ -233,7 +233,7 @@ def _sync_cluster_and_template_status(self, stack):
     def _cluster_failed(self, stack):
         LOG.error('Cluster error, stack status: %(cluster_status)s, '
                   'stack_id: %(stack_id)s, '
-                  'reason: %(reason)s' %
+                  'reason: %(reason)s',
                   {'cluster_status': stack.stack_status,
                    'stack_id': self.cluster.stack_id,
                    'reason': self.cluster.status_reason})
@@ -253,6 +253,6 @@ def _sync_missing_stack(self, new_status):
         self.cluster.save()
         LOG.info("Cluster with id %(id)s has been set to "
                  "%(status)s due to stack with id %(sid)s "
-                 "not found in Heat." %
+                 "not found in Heat.",
                  {'id': self.cluster.id, 'status': self.cluster.status,
                   'sid': self.cluster.stack_id})

magnum/drivers/heat/template_def.py

@@ -100,7 +100,7 @@ def get_output_value(self, stack):
             if output['output_key'] == self.heat_output:
                 return output['output_value']
 
-        LOG.warning('stack does not have output_key %s' % self.heat_output)
+        LOG.warning('stack does not have output_key %s', self.heat_output)
         return None
 
 

magnum/drivers/swarm_fedora_atomic_v1/monitor.py

@@ -51,7 +51,7 @@ def pull_data(self):
                     container = docker.inspect_container(container['Id'])
                 except Exception as e:
                     LOG.warning("Ignore error [%(e)s] when inspecting "
-                                "container %(container_id)s." %
+                                "container %(container_id)s.",
                                 {'e': e, 'container_id': container['Id']},
                                 exc_info=True)
                 containers.append(container)

magnum/service/periodic.py

@@ -135,7 +135,7 @@ def sync_cluster_status(self, ctx):
 
         except Exception as e:
             LOG.warning(
-                "Ignore error [%s] when syncing up cluster status." %
+                "Ignore error [%s] when syncing up cluster status.",
                 e, exc_info=True)
 
     @periodic_task.periodic_task(run_immediately=True)
@@ -157,7 +157,7 @@ def _send_cluster_metrics(self, ctx):
             except Exception as e:
                 LOG.warning(
                     "Skip pulling data from cluster %(cluster)s due to "
-                    "error: %(e)s" %
+                    "error: %(e)s",
                     {'e': e, 'cluster': cluster.uuid}, exc_info=True)
                 continue
 
@@ -172,7 +172,7 @@ def _send_cluster_metrics(self, ctx):
                     metrics.append(metric)
                 except Exception as e:
                     LOG.warning("Skip adding metric %(name)s due to "
-                                "error: %(e)s" %
+                                "error: %(e)s",
                                 {'e': e, 'name': name}, exc_info=True)
 
             message = dict(metrics=metrics,

magnum/tests/functional/api/base.py

@@ -106,7 +106,7 @@ def get_credentials_with_keypair(cls, name=None,
         except Exception:
             keypair_body = keypairs_client.create_keypair(
                 name=config.Config.keypair_id)
-            cls.LOG.debug("Keypair body: %s" % keypair_body)
+            cls.LOG.debug("Keypair body: %s", keypair_body)
             keypair = keypair_body['keypair']['private_key']
         return (creds, keypair)
 

magnum/tests/functional/api/v1/clients/bay_client.py

@@ -120,7 +120,7 @@ def wait_for_created_bay(self, bay_id, delete_on_error=True):
                 lambda: self.does_bay_exist(bay_id), 10, 1800)
         except Exception:
             # In error state.  Clean up the bay id if desired
-            self.LOG.error('Bay %s entered an exception state.' % bay_id)
+            self.LOG.error('Bay %s entered an exception state.', bay_id)
             if delete_on_error:
                 self.LOG.error('We will attempt to delete bays now.')
                 self.delete_bay(bay_id)
@@ -136,35 +136,35 @@ def is_bay_in_final_state(self, bay_id):
             resp, model = self.get_bay(bay_id)
             if model.status in ['CREATED', 'CREATE_COMPLETE',
                                 'ERROR', 'CREATE_FAILED']:
-                self.LOG.info('Bay %s succeeded.' % bay_id)
+                self.LOG.info('Bay %s succeeded.', bay_id)
                 return True
             else:
                 return False
         except exceptions.NotFound:
-            self.LOG.warning('Bay %s is not found.' % bay_id)
+            self.LOG.warning('Bay %s is not found.', bay_id)
             return False
 
     def does_bay_exist(self, bay_id):
         try:
             resp, model = self.get_bay(bay_id)
             if model.status in ['CREATED', 'CREATE_COMPLETE']:
-                self.LOG.info('Bay %s is created.' % bay_id)
+                self.LOG.info('Bay %s is created.', bay_id)
                 return True
             elif model.status in ['ERROR', 'CREATE_FAILED']:
-                self.LOG.error('Bay %s is in fail state.' % bay_id)
+                self.LOG.error('Bay %s is in fail state.', bay_id)
                 raise exceptions.ServerFault(
-                    "Got into an error condition: %s for %s" %
+                    "Got into an error condition: %s for %s",
                     (model.status, bay_id))
             else:
                 return False
         except exceptions.NotFound:
-            self.LOG.warning('Bay %s is not found.' % bay_id)
+            self.LOG.warning('Bay %s is not found.', bay_id)
             return False
 
     def does_bay_not_exist(self, bay_id):
         try:
             self.get_bay(bay_id)
         except exceptions.NotFound:
-            self.LOG.warning('Bay %s is not found.' % bay_id)
+            self.LOG.warning('Bay %s is not found.', bay_id)
             return True
         return False