27 files changed, 65 insertions, 75 deletions
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index dafa73bad..502ef6aa5 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -152,3 +152,17 @@ are run on a different Jenkins host that is not publicly accessible, however the
 test results are posted to S3 buckets when complete.
 
 The test output of each job is also posted to the Pull Request as comments.
+
+---
+
+## Appendix
+
+### Finding unused Python code
+
+If you are contributing with Python code, you can use the tool
+[`vulture`](https://pypi.python.org/pypi/vulture) to verify that you are not
+introducing unused code by accident.
+
+This tool is not used in an automated form in CI nor otherwise because it may
+produce both false positives and false negatives. Still, it can be helpful to
+detect dead code that escapes our eyes.
diff --git a/openshift-ansible.spec b/openshift-ansible.spec
index e49c54347..c03d4eb8f 100644
--- a/openshift-ansible.spec
+++ b/openshift-ansible.spec
@@ -73,6 +73,10 @@ find -L %{buildroot}%{_datadir}/ansible/%{name}/playbooks -name filter_plugins -
 
 # openshift-ansible-roles install
 cp -rp roles %{buildroot}%{_datadir}/ansible/%{name}/
+# openshift_master_facts symlinks filter_plugins/oo_filters.py from ansible_plugins/filter_plugins
+pushd %{buildroot}%{_datadir}/ansible/%{name}/roles/openshift_master_facts/filter_plugins
+ln -sf ../../../../../ansible_plugins/filter_plugins/oo_filters.py oo_filters.py
+popd
 
 # openshift-ansible-filter-plugins install
 cp -rp filter_plugins %{buildroot}%{_datadir}/ansible_plugins/
diff --git a/requirements.txt b/requirements.txt
index 8f47033f8..5a6a161cb 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -2,3 +2,4 @@ ansible>=2.2
 six
 pyOpenSSL
 PyYAML
+ruamel.yaml
diff --git a/roles/lib_openshift/library/oc_route.py b/roles/lib_openshift/library/oc_route.py
index 73600cb13..4b5c4460c 100644
--- a/roles/lib_openshift/library/oc_route.py
+++ b/roles/lib_openshift/library/oc_route.py
@@ -1548,7 +1548,7 @@ class OCRoute(OpenShiftCLI):
     def needs_update(self):
         ''' verify an update is needed '''
         skip = []
-        return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=True)
+        return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=self.verbose)
 
     @staticmethod
     def get_cert_data(path, content):
diff --git a/roles/lib_openshift/src/class/oc_route.py b/roles/lib_openshift/src/class/oc_route.py
index cb743e19d..448457292 100644
--- a/roles/lib_openshift/src/class/oc_route.py
+++ b/roles/lib_openshift/src/class/oc_route.py
@@ -61,7 +61,7 @@ class OCRoute(OpenShiftCLI):
     def needs_update(self):
         ''' verify an update is needed '''
         skip = []
-        return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=True)
+        return not Utils.check_def_equal(self.config.data, self.route.yaml_dict, skip_keys=skip, debug=self.verbose)
 
     @staticmethod
     def get_cert_data(path, content):
diff --git a/roles/lib_openshift/src/test/unit/oadm_manage_node.py b/roles/lib_openshift/src/test/unit/test_oadm_manage_node.py
index b0786dfac..b0786dfac 100755..100644
--- a/roles/lib_openshift/src/test/unit/oadm_manage_node.py
+++ b/roles/lib_openshift/src/test/unit/test_oadm_manage_node.py
diff --git a/roles/lib_openshift/src/test/unit/oc_env.py b/roles/lib_openshift/src/test/unit/test_oc_env.py
index 15bd7e464..15bd7e464 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_env.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_env.py
diff --git a/roles/lib_openshift/src/test/unit/oc_label.py b/roles/lib_openshift/src/test/unit/test_oc_label.py
index 3176987b0..3176987b0 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_label.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_label.py
diff --git a/roles/lib_openshift/src/test/unit/oc_process.py b/roles/lib_openshift/src/test/unit/test_oc_process.py
index 450ff7071..450ff7071 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_process.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_process.py
diff --git a/roles/lib_openshift/src/test/unit/oc_route.py b/roles/lib_openshift/src/test/unit/test_oc_route.py
index 361b61f4b..361b61f4b 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_route.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_route.py
diff --git a/roles/lib_openshift/src/test/unit/oc_scale.py b/roles/lib_openshift/src/test/unit/test_oc_scale.py
index f15eb164d..f15eb164d 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_scale.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_scale.py
diff --git a/roles/lib_openshift/src/test/unit/oc_secret.py b/roles/lib_openshift/src/test/unit/test_oc_secret.py
index 645aac82b..645aac82b 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_secret.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_secret.py
diff --git a/roles/lib_openshift/src/test/unit/oc_service.py b/roles/lib_openshift/src/test/unit/test_oc_service.py
index 4a845e9f3..4a845e9f3 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_service.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_service.py
diff --git a/roles/lib_openshift/src/test/unit/oc_serviceaccount.py b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount.py
index 256b569eb..256b569eb 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_serviceaccount.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount.py
diff --git a/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount_secret.py
index 213c581aa..213c581aa 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_serviceaccount_secret.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_serviceaccount_secret.py
diff --git a/roles/lib_openshift/src/test/unit/oc_version.py b/roles/lib_openshift/src/test/unit/test_oc_version.py
index 67dea415b..67dea415b 100755..100644
--- a/roles/lib_openshift/src/test/unit/oc_version.py
+++ b/roles/lib_openshift/src/test/unit/test_oc_version.py
diff --git a/roles/lib_utils/library/repoquery.py b/roles/lib_utils/library/repoquery.py
index cc7aa04fd..f31c8911b 100644
--- a/roles/lib_utils/library/repoquery.py
+++ b/roles/lib_utils/library/repoquery.py
@@ -477,7 +477,7 @@ class Repoquery(RepoqueryCLI):
         ''' Gather and present the versions of each package '''
 
         versions_dict = {}
-        versions_dict['available_versions_full'] = formatted_versions.keys()
+        versions_dict['available_versions_full'] = list(formatted_versions.keys())
 
         # set the match version, if called
         if self.match_version:
diff --git a/roles/lib_utils/src/test/unit/repoquery.py b/roles/lib_utils/src/test/unit/test_repoquery.py
index c487ab254..c487ab254 100755..100644
--- a/roles/lib_utils/src/test/unit/repoquery.py
+++ b/roles/lib_utils/src/test/unit/test_repoquery.py
diff --git a/roles/lib_utils/src/test/unit/yedit_test.py b/roles/lib_utils/src/test/unit/test_yedit.py
index 2793c5c1a..ed07ac96e 100755..100644
--- a/roles/lib_utils/src/test/unit/yedit_test.py
+++ b/roles/lib_utils/src/test/unit/test_yedit.py
@@ -256,7 +256,7 @@ class YeditTest(unittest.TestCase):
 
     def test_pop_list_item_2(self):
         '''test dict value with none value'''
-        z = range(10)
+        z = list(range(10))
         yed = Yedit(content=z, separator=':')
         yed.pop('', 5)
         z.pop(5)
diff --git a/roles/openshift_facts/tasks/main.yml b/roles/openshift_facts/tasks/main.yml
index 9a1982076..0ec294bbc 100644
--- a/roles/openshift_facts/tasks/main.yml
+++ b/roles/openshift_facts/tasks/main.yml
@@ -14,6 +14,18 @@
     l_is_master_system_container: "{{ (use_master_system_container | default(use_system_containers) | bool) }}"
     l_is_etcd_system_container: "{{ (use_etcd_system_container | default(use_system_containers) | bool) }}"
 
+- name: Validate python version
+  fail:
+    msg: |
+      openshift-ansible requires Python 3 for {{ ansible_distribution }};
+      For information on enabling Python 3 with Ansible, see https://docs.ansible.com/ansible/python_3_support.html
+  when: ansible_distribution == 'Fedora' and ansible_python['version']['major'] != 3
+
+- name: Validate python version
+  fail:
+    msg: "openshift-ansible requires Python 2 for {{ ansible_distribution }}"
+  when: ansible_distribution != 'Fedora' and ansible_python['version']['major'] != 2
+
 - name: Ensure various deps are installed
   package: name={{ item }} state=present
   with_items: "{{ required_packages }}"
diff --git a/roles/openshift_hosted/tasks/router/router.yml b/roles/openshift_hosted/tasks/router/router.yml
index b944fa522..3d5713d6b 100644
--- a/roles/openshift_hosted/tasks/router/router.yml
+++ b/roles/openshift_hosted/tasks/router/router.yml
@@ -78,3 +78,27 @@
   changed_when: "'service exists' not in openshift_hosted_router_results.stdout"
   failed_when: "openshift_hosted_router_results.rc != 0 and 'service exists' not in openshift_hosted_router_results.stdout and 'deployment_config' not in openshift_hosted_router_results.stderr and 'service' not in openshift_hosted_router_results.stderr"
   when: replicas | int > 0
+
+- command: >
+    {{ openshift.common.client_binary }}
+    {% if openshift.hosted.router.name | default(none) is not none -%}
+    get dc/{{ openshift.hosted.router.name }}
+    {% else %}
+    get dc/router
+    {% endif%}
+    --template=\\{\\{.spec.replicas\\}\\}
+    --namespace={{ openshift.hosted.router.namespace | default('default') }}
+  register: current_replicas
+  when: replicas | int > 0
+
+- name: Ensure router replica count matches desired
+  command: >
+    {{ openshift.common.client_binary }}
+    scale --replicas={{ replicas }}
+    {% if openshift.hosted.router.name | default(none) is not none -%}
+    dc/{{ openshift.hosted.router.name }}
+    {% else %}
+    dc/router
+    {% endif%}
+    --namespace={{ openshift.hosted.router.namespace | default('default') }}
+  when: replicas | int > 0 and replicas | int != current_replicas.stdout | int
diff --git a/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py b/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py
index 68b6deb88..bcfaa55c8 100644
--- a/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py
+++ b/roles/openshift_master_facts/test/openshift_master_facts_default_predicates_tests.py
@@ -175,23 +175,6 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
         else:
             assert_equal(results, default_predicates)
 
-    def check_defaults_override_vars(self, release, deployment_type,
-                                     default_predicates, regions_enabled,
-                                     extra_facts=None):
-        facts = copy.deepcopy(self.default_facts)
-        facts['openshift']['common']['short_version'] = release
-        facts['openshift']['common']['deployment_type'] = deployment_type
-        if extra_facts is not None:
-            for fact in extra_facts:
-                facts[fact] = extra_facts[fact]
-        results = self.lookup.run(None, variables=facts,
-                                  regions_enabled=regions_enabled,
-                                  return_set_vars=False)
-        if regions_enabled:
-            assert_equal(results, default_predicates + [REGION_PREDICATE])
-        else:
-            assert_equal(results, default_predicates)
-
     def test_openshift_version(self):
         for regions_enabled in (True, False):
             for release, deployment_type, default_predicates in TEST_VARS:
diff --git a/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py b/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py
index 4e44a2b3d..7ddc5dcb2 100644
--- a/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py
+++ b/roles/openshift_master_facts/test/openshift_master_facts_default_priorities_tests.py
@@ -160,23 +160,6 @@ class TestOpenShiftMasterFactsDefaultPredicates(object):
         else:
             assert_equal(results, default_priorities)
 
-    def check_defaults_override_vars(self, release, deployment_type,
-                                     default_priorities, zones_enabled,
-                                     extra_facts=None):
-        facts = copy.deepcopy(self.default_facts)
-        facts['openshift']['common']['short_version'] = release
-        facts['openshift']['common']['deployment_type'] = deployment_type
-        if extra_facts is not None:
-            for fact in extra_facts:
-                facts[fact] = extra_facts[fact]
-        results = self.lookup.run(None, variables=facts,
-                                  zones_enabled=zones_enabled,
-                                  return_set_vars=False)
-        if zones_enabled:
-            assert_equal(results, default_priorities + [ZONE_PRIORITY])
-        else:
-            assert_equal(results, default_priorities)
-
     def test_openshift_version(self):
         for zones_enabled in (True, False):
             for release, deployment_type, default_priorities in TEST_VARS:
diff --git a/setup.cfg b/setup.cfg
index 06346852c..757d3ddea 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -5,12 +5,15 @@
 universal=1
 
 [nosetests]
-tests=roles/openshift_master_facts/test/, test/
+tests=roles/lib_openshift/src/test/unit,
+      roles/lib_utils/src/test/unit,
+      roles/openshift_master_facts/test,
+      test
 verbosity=2
 with-coverage=1
 cover-html=1
 cover-inclusive=1
-cover-min-percentage=70
+cover-min-percentage=25
 cover-erase=1
 detailed-errors=1
 cover-branches=1
diff --git a/test-requirements.txt b/test-requirements.txt
index 2ee1e657d..8bfc1b594 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -9,3 +9,4 @@ PyYAML
 yamllint
 nose
 coverage
+mock
diff --git a/utils/src/ooinstall/cli_installer.py b/utils/src/ooinstall/cli_installer.py
index b787741d7..a6d784dea 100644
--- a/utils/src/ooinstall/cli_installer.py
+++ b/utils/src/ooinstall/cli_installer.py
@@ -72,12 +72,6 @@ You might want to override the default subdomain used for exposed routes. If you
     return click.prompt('New default subdomain (ENTER for none)', default='')
 
 
-def list_hosts(hosts):
-    hosts_idx = range(len(hosts))
-    for idx in hosts_idx:
-        click.echo('   {}: {}'.format(idx, hosts[idx]))
-
-
 def collect_hosts(oo_cfg, existing_env=False, masters_set=False, print_summary=True):
     """
         Collect host information from user. This will later be filled in using
@@ -656,20 +650,6 @@ https://docs.openshift.com/enterprise/latest/admin_guide/install/prerequisites.h
     return oo_cfg
 
 
-def get_role_variable(oo_cfg, role_name, variable_name):
-    try:
-        target_role = next(role for role in oo_cfg.deployment.roles if role.name is role_name)
-        target_variable = target_role.variables[variable_name]
-        return target_variable
-    except (StopIteration, KeyError):
-        return None
-
-
-def set_role_variable(oo_cfg, role_name, variable_name, variable_value):
-    target_role = next(role for role in oo_cfg.deployment.roles if role.name is role_name)
-    target_role[variable_name] = variable_value
-
-
 def collect_new_nodes(oo_cfg):
     click.clear()
     click.echo('*** New Node Configuration ***')
diff --git a/utils/src/ooinstall/oo_config.py b/utils/src/ooinstall/oo_config.py
index cf14105af..9670c03bb 100644
--- a/utils/src/ooinstall/oo_config.py
+++ b/utils/src/ooinstall/oo_config.py
@@ -126,15 +126,6 @@ class Host(object):
         """ Does this host have the etcd role """
         return 'etcd' in self.roles
 
-    def is_etcd_member(self, all_hosts):
-        """ Will this host be a member of a standalone etcd cluster. """
-        if not self.is_master():
-            return False
-        masters = [host for host in all_hosts if host.is_master()]
-        if len(masters) > 1:
-            return True
-        return False
-
     def is_dedicated_node(self):
         """ Will this host be a dedicated node. (not a master) """
         return self.is_node() and not self.is_master()
@@ -436,12 +427,6 @@ class OOConfig(object):
     def __str__(self):
         return self.yaml()
 
-    def get_host(self, name):
-        for host in self.deployment.hosts:
-            if host.connect_to == name:
-                return host
-        return None
-
     def get_host_roles_set(self):
         roles_set = set()
         for host in self.deployment.hosts: