Trivial code cleanups

Cleaning up comments and logging to make sure they properly adhere
to Openstack standards.

* Consistently use """ instead of ''' for comments.
* Always lazy-load logging parameters.
* Fixed bad log line in cert_manager.

Change-Id: I547f5dfa61609a899aef9b1470be8d8a6d8e4b81

magnum/api/controllers/v1/bay.py

@@ -290,7 +290,7 @@ class BaysController(base.Controller):
     }
 
     def _generate_name_for_bay(self, context):
-        '''Generate a random name like: zeta-22-bay.'''
+        """Generate a random name like: zeta-22-bay."""
         name_gen = name_generator.NameGenerator()
         name = name_gen.generate()
         return name + '-bay'

magnum/api/controllers/v1/baymodel.py

@@ -236,7 +236,7 @@ class BayModelsController(base.Controller):
     }
 
     def _generate_name_for_baymodel(self, context):
-        '''Generate a random name like: zeta-22-model.'''
+        """Generate a random name like: zeta-22-model."""
 
         name_gen = name_generator.NameGenerator()
         name = name_gen.generate()

magnum/common/cert_manager/barbican_cert_manager.py

@@ -87,8 +87,7 @@ class CertManager(cert_manager.CertManager):
         """
         connection = get_admin_clients().barbican()
 
-        LOG.info("Storing certificate container '{0}' in Barbican."
+        LOG.info("Storing certificate container '%s' in Barbican.", name)
-                 .format(name))
 
         certificate_secret = None
         private_key_secret = None
@@ -139,13 +138,13 @@ class CertManager(cert_manager.CertManager):
                     old_ref = secret.secret_ref
                     try:
                         secret.delete()
-                        LOG.info("Deleted secret {0} ({1}) during rollback."
+                        LOG.info("Deleted secret %s (%s) during rollback.",
-                                 .format(secret.name, old_ref))
+                                 secret.name, old_ref)
                     except Exception:
                         LOG.warning(
-                            "Failed to delete {0} ({1}) during rollback. "
+                            "Failed to delete %s (%s) during rollback. "
-                            "This is probably not a problem."
+                            "This is probably not a problem.",
-                            .format(secret.name, old_ref))
+                            secret.name, old_ref)
             with excutils.save_and_reraise_exception():
                 LOG.exception("Error storing certificate data")
 
@@ -165,9 +164,7 @@ class CertManager(cert_manager.CertManager):
         """
         connection = get_admin_clients().barbican()
 
-        LOG.info(
+        LOG.info("Loading certificate container %s from Barbican.", cert_ref)
-            "Loading certificate container {0} from Barbican."
-            .format(cert_ref))
         try:
             if check_only:
                 cert_container = connection.containers.get(
@@ -182,7 +179,7 @@ class CertManager(cert_manager.CertManager):
             return Cert(cert_container)
         except barbican_exc.HTTPClientError:
             with excutils.save_and_reraise_exception():
-                LOG.exception("Error getting {0}".format(cert_ref))
+                LOG.exception("Error getting %s", cert_ref)
 
     @staticmethod
     def delete_cert(cert_ref, service_name='Magnum', resource_ref=None,
@@ -195,8 +192,8 @@ class CertManager(cert_manager.CertManager):
         connection = get_admin_clients().barbican()
 
         LOG.info(
-            "Recursively deleting certificate container {0} from Barbican."
+            "Recursively deleting certificate container %s from Barbican.",
-            .format(cert_ref))
+            cert_ref)
         try:
             certificate_container = connection.containers.get(cert_ref)
             certificate_container.certificate.delete()
@@ -209,5 +206,5 @@ class CertManager(cert_manager.CertManager):
         except barbican_exc.HTTPClientError:
             with excutils.save_and_reraise_exception():
                 LOG.exception(
-                    "Error recursively deleting certificate container {0}"
+                    "Error recursively deleting certificate container %s",
-                    .format(cert_ref))
+                    cert_ref)

magnum/common/cert_manager/local_cert_manager.py

@@ -114,9 +114,9 @@ class CertManager(cert_manager.CertManager):
         :raises CertificateStorageException: if certificate retrieval fails
         """
         LOG.warning(
-            "Loading certificate {0} from the local filesystem. "
+            "Loading certificate %s from the local filesystem. "
-            "CertManager type 'local' should be used for testing purpose."
+            "CertManager type 'local' should be used for testing purpose.",
-            .format(cert_ref))
+            cert_ref)
 
         filename_base = os.path.join(CONF.certificates.storage_path, cert_ref)
 
@@ -131,9 +131,7 @@ class CertManager(cert_manager.CertManager):
             with open(filename_certificate, 'r') as cert_file:
                 cert_data['certificate'] = cert_file.read()
         except IOError:
-            LOG.error(
+            LOG.error("Failed to read certificate for %s.", cert_ref)
-                "Failed to read certificate for {0}."
-                .format(cert_ref))
             raise exception.CertificateStorageException(
                 msg=_("Certificate could not be read.")
             )
@@ -141,9 +139,7 @@ class CertManager(cert_manager.CertManager):
             with open(filename_private_key, 'r') as key_file:
                 cert_data['private_key'] = key_file.read()
         except IOError:
-            LOG.error(
+            LOG.error("Failed to read private key for %s.", cert_ref)
-                "Failed to read private key for {0}."
-                .format(cert_ref))
             raise exception.CertificateStorageException(
                 msg=_("Private Key could not be read.")
             )
@@ -175,9 +171,9 @@ class CertManager(cert_manager.CertManager):
         :raises CertificateStorageException: if certificate deletion fails
         """
         LOG.warning(
-            "Deleting certificate {0} from the local filesystem. "
+            "Deleting certificate %s from the local filesystem. "
-            "CertManager type 'local' should be used for testing purpose."
+            "CertManager type 'local' should be used for testing purpose.",
-            .format(cert_ref))
+            cert_ref)
 
         filename_base = os.path.join(CONF.certificates.storage_path, cert_ref)
 
@@ -194,7 +190,5 @@ class CertManager(cert_manager.CertManager):
             if path.isfile(filename_pkp):
                 os.remove(filename_pkp)
         except IOError as ioe:
-            LOG.error(
+            LOG.error("Failed to delete certificate %s.", cert_ref)
-                "Failed to delete certificate {0}."
-                .format(cert_ref))
             raise exception.CertificateStorageException(msg=str(ioe))

magnum/common/name_generator.py

@@ -26,10 +26,10 @@ class NameGenerator(object):
         self.random = random.Random()
 
     def generate(self):
-        '''Generate a random name compose of a Greek leter and
+        """Generate a random name compose of a Greek leter and
 
         a number, like: beta_2.
-        '''
+        """
 
         letter = self.random.choice(self.letters)
         number = self.random.randint(1, 24)

magnum/common/x509/operations.py

@@ -197,7 +197,7 @@ def sign(csr, issuer_name, ca_key, ca_key_password=None,
         try:
             csr = x509.load_pem_x509_csr(csr, backend=default_backend())
         except ValueError:
-            LOG.exception("Received invalid csr {0}.".format(csr))
+            LOG.exception("Received invalid csr %s.", csr)
             raise exception.InvalidCsr(csr=csr)
 
     term_of_validity = CONF.x509.term_of_validity

magnum/conductor/handlers/common/cert_manager.py

@@ -144,8 +144,8 @@ def get_cluster_magnum_cert(cluster, context=None):
 
 def create_client_files(cluster, context=None):
     if not os.path.isdir(CONF.cluster.temp_cache_dir):
-        LOG.debug("Certificates will not be cached in the filesystem: they \
+        LOG.debug("Certificates will not be cached in the filesystem: they "
-            will be created as tempfiles.")
+                  "will be created as tempfiles.")
         ca_cert = get_cluster_ca_certificate(cluster, context)
         magnum_cert = get_cluster_magnum_cert(cluster, context)
 

magnum/conductor/handlers/conductor_listener.py

@@ -15,15 +15,15 @@ from magnum.common import profiler
 
 @profiler.trace_cls("rpc")
 class Handler(object):
-    '''Listen on an AMQP queue named for the conductor.
+    """Listen on an AMQP queue named for the conductor.
 
     Allows individual conductors to communicate with each other for
     multi-conductor support.
-    '''
+    """
     def ping_conductor(self, context):
-        '''Respond to conductor.
+        """Respond to conductor.
 
         Respond affirmatively to confirm that the conductor performing the
         action is still alive.
-        '''
+        """
         return True

magnum/drivers/common/driver.py

@@ -39,7 +39,7 @@ class Driver(object):
 
     @classmethod
     def get_drivers(cls):
-        '''Retrieves cluster drivers from python entry_points.
+        """Retrieves cluster drivers from python entry_points.
 
         Example:
 
@@ -69,7 +69,7 @@ class Driver(object):
             }
 
         :return: dict
-        '''
+        """
 
         if not cls.definitions:
             cls.definitions = dict()
@@ -87,7 +87,7 @@ class Driver(object):
 
     @classmethod
     def get_driver(cls, server_type, os, coe):
-        '''Get Driver.
+        """Get Driver.
 
         Returns the Driver class for the provided cluster_type.
 
@@ -118,7 +118,7 @@ class Driver(object):
                     produce
 
         :return: class
-        '''
+        """
 
         definition_map = cls.get_drivers()
         cluster_type = (server_type, os, coe)
@@ -142,20 +142,20 @@ class Driver(object):
         return cls.get_driver(ct.server_type, ct.cluster_distro, ct.coe)
 
     def update_cluster_status(self, context, cluster):
-        '''Update the cluster status based on underlying orchestration
+        """Update the cluster status based on underlying orchestration
 
            This is an optional method if your implementation does not need
            to poll the orchestration for status updates (for example, your
            driver uses some notification-based mechanism instead).
-        '''
+        """
         return
 
     @abc.abstractproperty
     def provides(self):
-        '''return a list of (server_type, os, coe) tuples
+        """return a list of (server_type, os, coe) tuples
 
            Returns a list of cluster configurations supported by this driver
-        '''
+        """
         raise NotImplementedError("Subclasses must implement 'provides'.")
 
     @abc.abstractmethod

magnum/drivers/heat/driver.py

@@ -41,11 +41,11 @@ LOG = logging.getLogger(__name__)
 
 @six.add_metaclass(abc.ABCMeta)
 class HeatDriver(driver.Driver):
-    '''Base Driver class for using Heat
+    """Base Driver class for using Heat
 
        Abstract class for implementing Drivers that leverage OpenStack Heat for
        orchestrating cluster lifecycle operations
-    '''
+    """
 
     def _extract_template_definition(self, context, cluster,
                                      scale_manager=None):
@@ -67,10 +67,10 @@ class HeatDriver(driver.Driver):
 
     @abc.abstractmethod
     def get_template_definition(self):
-        '''return an implementation of
+        """return an implementation of
 
            magnum.drivers.common.drivers.heat.TemplateDefinition
-        '''
+        """
 
         raise NotImplementedError("Must implement 'get_template_definition'")
 

magnum/drivers/heat/template_def.py

@@ -108,12 +108,12 @@ class OutputMapping(object):
 
 @six.add_metaclass(abc.ABCMeta)
 class TemplateDefinition(object):
-    '''A mapping between Magnum objects and Heat templates.
+    """A mapping between Magnum objects and Heat templates.
 
     A TemplateDefinition is essentially a mapping between Magnum objects
     and Heat templates. Each TemplateDefinition has a mapping of Heat
     parameters.
-    '''
+    """
 
     def __init__(self):
         self.param_mappings = list()
@@ -279,7 +279,7 @@ class BaseTemplateDefinition(TemplateDefinition):
         try:
             result = requests.get(url).text
         except req_exceptions.RequestException as err:
-            LOG.error(six.text_type(err))
+            LOG.error(err)
             raise exception.GetClusterSizeFailed(
                 discovery_url=discovery_url)
 
@@ -323,7 +323,7 @@ class BaseTemplateDefinition(TemplateDefinition):
                         discovery_endpoint=discovery_endpoint)
                 discovery_url = discovery_request.text
             except req_exceptions.RequestException as err:
-                LOG.error(six.text_type(err))
+                LOG.error(err)
                 raise exception.GetDiscoveryUrlFailed(
                     discovery_endpoint=discovery_endpoint)
             if not discovery_url:

magnum/service/periodic.py

@@ -90,7 +90,7 @@ class ClusterUpdateJob(object):
 
 @profiler.trace_cls("rpc")
 class MagnumPeriodicTasks(periodic_task.PeriodicTasks):
-    '''Magnum periodic Task class
+    """Magnum periodic Task class
 
     Any periodic task job need to be added into this class
 
@@ -103,7 +103,7 @@ class MagnumPeriodicTasks(periodic_task.PeriodicTasks):
       try/catch block. The present try/catch block here helps putting
       magnum-periodic-task-specific log/error message.
 
-    '''
+    """
 
     def __init__(self, conf):
         super(MagnumPeriodicTasks, self).__init__(conf)

magnum/servicegroup/magnum_service_periodic.py

@@ -25,10 +25,10 @@ LOG = log.getLogger(__name__)
 
 
 class MagnumServicePeriodicTasks(periodic_task.PeriodicTasks):
-    '''Magnum periodic Task class
+    """Magnum periodic Task class
 
     Any periodic task job need to be added into this class
-    '''
+    """
 
     def __init__(self, conf, binary):
         self.magnum_service_ref = None

magnum/tests/fakes.py

@@ -97,11 +97,11 @@ class FakeAuthProtocol(mock.Mock):
 
 
 class FakeLoopingCall(object):
-    '''Fake a looping call without the eventlet stuff
+    """Fake a looping call without the eventlet stuff
 
        For tests, just do a simple implementation so that we can ensure the
        called logic works rather than testing LoopingCall
-    '''
+    """
 
     def __init__(self, **kwargs):
         func = kwargs.pop("f", None)

magnum/tests/functional/swarm/test_swarm_python_client.py

@@ -92,7 +92,7 @@ class TestSwarmAPIs(ClusterTest):
         # need to investigate the cause of this issue. See bug #1583337.
         for i in range(150):
             try:
-                self.LOG.info("Calling function " + func.__name__)
+                self.LOG.info("Calling function %s", func.__name__)
                 return func(*args, **kwargs)
             except req_exceptions.ConnectionError:
                 self.LOG.info("Connection aborted on calling Swarm API. "
@@ -100,7 +100,7 @@ class TestSwarmAPIs(ClusterTest):
             except errors.APIError as e:
                 if e.response.status_code != 500:
                     raise
-                self.LOG.info("Internal Server Error: " + str(e))
+                self.LOG.info("Internal Server Error: %s", e)
             time.sleep(2)
 
         raise Exception("Cannot connect to Swarm API.")

magnum/tests/unit/conductor/handlers/common/test_cert_manager.py

@@ -317,8 +317,9 @@ class CertManagerTestCase(base.BaseTestCase):
         (cluster_ca_cert, cluster_key, cluster_magnum_cert) = \
             cert_manager.create_client_files(mock_cluster)
 
-        mock_logging.debug.assert_called_once_with("Certificates will not be cached in the filesystem: they \
+        mock_logging.debug.assert_called_once_with(
-            will be created as tempfiles.")
+            "Certificates will not be cached in the filesystem: "
+            "they will be created as tempfiles.")
         self.assertEqual(self.CertManager.get_cert.call_count, 2)
         self.assertEqual(mock_cert.get_certificate.call_count, 2)
         self.assertEqual(mock_cert.get_decrypted_private_key.call_count, 1)