pylint fixes

2 files changed, 35 insertions, 37 deletions

diff --git a/roles/openshift_facts/library/openshift_facts.py b/roles/openshift_facts/library/openshift_facts.py
index fdd3e8708..3de8faace 100755
--- a/roles/openshift_facts/library/openshift_facts.py
+++ b/roles/openshift_facts/library/openshift_facts.py
@@ -7,16 +7,6 @@
 
 """Ansible module for retrieving and setting openshift related facts"""
 
-DOCUMENTATION = '''
----
-module: openshift_facts
-short_description: Cluster Facts
-author: Jason DeTiberus
-requirements: [ ]
-'''
-EXAMPLES = '''
-'''
-
 import ConfigParser
 import copy
 import io
@@ -30,6 +20,17 @@ from dbus import SystemBus, Interface
 from dbus.exceptions import DBusException
 
 
+DOCUMENTATION = '''
+---
+module: openshift_facts
+short_description: Cluster Facts
+author: Jason DeTiberus
+requirements: [ ]
+'''
+EXAMPLES = '''
+'''
+
+
 def migrate_docker_facts(facts):
     """ Apply migrations for docker facts """
     params = {
@@ -499,10 +500,8 @@ def set_dnsmasq_facts_if_unset(facts):
     """
 
     if 'common' in facts:
-        if 'use_dnsmasq' not in facts['common'] and safe_get_bool(facts['common']['version_gte_3_2_or_1_2']):
-            facts['common']['use_dnsmasq'] = True
-        else:
-            facts['common']['use_dnsmasq'] = False
+        facts['common']['use_dnsmasq'] = bool('use_dnsmasq' not in facts['common'] and
+                                              safe_get_bool(facts['common']['version_gte_3_2_or_1_2']))
         if 'master' in facts and 'dns_port' not in facts['master']:
             if safe_get_bool(facts['common']['use_dnsmasq']):
                 facts['master']['dns_port'] = 8053
@@ -1143,7 +1142,7 @@ def get_openshift_version(facts):
         _, output, _ = module.run_command(['/usr/bin/openshift', 'version'])
         version = parse_openshift_version(output)
     elif 'common' in facts and 'is_containerized' in facts['common']:
-        version = get_containerized_openshift_version(facts)
+        version = get_container_openshift_version(facts)
 
     # Handle containerized masters that have not yet been configured as a node.
     # This can be very slow and may get re-run multiple times, so we only use this
@@ -1155,15 +1154,18 @@ def get_openshift_version(facts):
     return version
 
 
-def get_containerized_openshift_version(facts):
-    # If containerized, see if we can determine the installed version via the systemd environment files:
+def get_container_openshift_version(facts):
+    """
+    If containerized, see if we can determine the installed version via the
+    systemd environment files.
+    """
     for filename in ['/etc/sysconfig/%s-master', '/etc/sysconfig/%s-node']:
-        env_file = filename % facts['common']['service_type']
-        if not os.path.exists(env_file):
+        env_path = filename % facts['common']['service_type']
+        if not os.path.exists(env_path):
             continue
 
-        with open(env_file) as f:
-            for line in f:
+        with open(env_path) as env_file:
+            for line in env_file:
                 if line.startswith("IMAGE_VERSION="):
                     tag = line[len("IMAGE_VERSION="):].strip()
                     # Remove leading "v" and any trailing release info, we just want
@@ -1218,7 +1220,7 @@ def apply_provider_facts(facts, provider_facts):
 
 # Disabling pylint too many branches. This function needs refactored
 # but is a very core part of openshift_facts.
-# pylint: disable=too-many-branches
+# pylint: disable=too-many-branches,too-many-nested-blocks
 def merge_facts(orig, new, additive_facts_to_overwrite, protected_facts_to_overwrite):
     """ Recursively merge facts dicts
 
@@ -1766,10 +1768,7 @@ class OpenShiftFacts(object):
 
         if 'clock' in roles:
             exit_code, _, _ = module.run_command(['rpm', '-q', 'chrony'])
-            if exit_code == 0:
-                chrony_installed = True
-            else:
-                chrony_installed = False
+            chrony_installed = bool(exit_code == 0)
             defaults['clock'] = dict(
                 enabled=True,
                 chrony_installed=chrony_installed)
@@ -2153,7 +2152,7 @@ def main():
                             ansible_facts=openshift_facts.facts)
 
 # ignore pylint errors related to the module_utils import
-# pylint: disable=redefined-builtin, unused-wildcard-import, wildcard-import
+# pylint: disable=redefined-builtin, unused-wildcard-import, wildcard-import, wrong-import-position
 # import module snippets
 from ansible.module_utils.basic import *
 from ansible.module_utils.facts import *
diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py
index cd74ed233..71734e792 100644
--- a/utils/src/ooinstall/cli_installer.py
+++ b/utils/src/ooinstall/cli_installer.py
@@ -1,17 +1,17 @@
 # TODO: Temporarily disabled due to importing old code into openshift-ansible
 # repo. We will work on these over time.
-# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,no-value-for-parameter
+# pylint: disable=bad-continuation,missing-docstring,no-self-use,invalid-name,no-value-for-parameter,too-many-lines
 
-import click
 import os
 import re
 import sys
+from distutils.version import LooseVersion
+import click
 from ooinstall import openshift_ansible
 from ooinstall import OOConfig
 from ooinstall.oo_config import OOConfigInvalidHostError
 from ooinstall.oo_config import Host
 from ooinstall.variants import find_variant, get_variant_version_combos
-from distutils.version import LooseVersion
 
 DEFAULT_ANSIBLE_CONFIG = '/usr/share/atomic-openshift-utils/ansible.cfg'
 DEFAULT_PLAYBOOK_DIR = '/usr/share/ansible/openshift-ansible/'
@@ -32,7 +32,7 @@ def is_valid_hostname(hostname):
     return all(allowed.match(x) for x in hostname.split("."))
 
 def validate_prompt_hostname(hostname):
-    if '' == hostname or is_valid_hostname(hostname):
+    if hostname == '' or is_valid_hostname(hostname):
         return hostname
     raise click.BadParameter('Invalid hostname. Please double-check this value and re-enter it.')
 
@@ -60,6 +60,7 @@ def list_hosts(hosts):
     for idx in hosts_idx:
         click.echo('   {}: {}'.format(idx, hosts[idx]))
 
+# pylint: disable=redefined-variable-type
 def delete_hosts(hosts):
     while True:
         list_hosts(hosts)
@@ -146,10 +147,7 @@ http://docs.openshift.com/enterprise/latest/architecture/infrastructure_componen
             if rpm_or_container == 'container':
                 host_props['containerized'] = True
 
-        if existing_env:
-            host_props['new_host'] = True
-        else:
-            host_props['new_host'] = False
+        host_props['new_host'] = existing_env
 
         host = Host(**host_props)
 
@@ -377,7 +375,7 @@ Notes:
     default_facts_lines = []
     default_facts = {}
     for h in hosts:
-        if h.preconfigured == True:
+        if h.preconfigured:
             continue
         try:
             default_facts[h.connect_to] = {}
@@ -824,6 +822,7 @@ def uninstall(ctx):
 @click.option('--latest-minor', '-l', is_flag=True, default=False)
 @click.option('--next-major', '-n', is_flag=True, default=False)
 @click.pass_context
+#pylint: disable=bad-builtin,too-many-statements
 def upgrade(ctx, latest_minor, next_major):
     oo_cfg = ctx.obj['oo_cfg']
     verbose = ctx.obj['verbose']
@@ -883,7 +882,7 @@ def upgrade(ctx, latest_minor, next_major):
     if next_major:
         if 'major_playbook' not in mapping:
             click.echo("No major upgrade supported for %s %s with this version "\
-                       "of atomic-openshift-utils." % (variant, version))
+                       "of atomic-openshift-utils." % (variant, old_version))
             sys.exit(0)
         playbook = mapping['major_playbook']
         new_version = mapping['major_version']