Merge pull request #4899 from rhcarvalho/misc-cleanup-normalize-input

Merged by openshift-bot

5 files changed, 24 insertions, 17 deletions

diff --git a/playbooks/common/openshift-checks/health.yml b/playbooks/common/openshift-checks/health.yml
index c7766ff04..7e83b4aa6 100644
--- a/playbooks/common/openshift-checks/health.yml
+++ b/playbooks/common/openshift-checks/health.yml
@@ -1,16 +1,13 @@
 ---
-# openshift_health_checker depends on openshift_version which now requires group eval.
 - include: ../openshift-cluster/evaluate_groups.yml
-  tags:
-  - always
 
 - name: Run OpenShift health checks
   hosts: OSEv3
   roles:
   - openshift_health_checker
   vars:
-  - r_openshift_health_checker_playbook_context: "health"
+  - r_openshift_health_checker_playbook_context: health
   post_tasks:
-  - action: openshift_health_check  # https://github.com/ansible/ansible/issues/20513
+  - action: openshift_health_check
     args:
       checks: ['@health']
diff --git a/playbooks/common/openshift-checks/pre-install.yml b/playbooks/common/openshift-checks/pre-install.yml
index 7ca9f7e8b..afd4f95e0 100644
--- a/playbooks/common/openshift-checks/pre-install.yml
+++ b/playbooks/common/openshift-checks/pre-install.yml
@@ -1,16 +1,13 @@
 ---
-# openshift_health_checker depends on openshift_version which now requires group eval.
 - include: ../openshift-cluster/evaluate_groups.yml
-  tags:
-  - always
 
 - hosts: OSEv3
   name: run OpenShift pre-install checks
   roles:
   - openshift_health_checker
   vars:
-  - r_openshift_health_checker_playbook_context: "pre-install"
+  - r_openshift_health_checker_playbook_context: pre-install
   post_tasks:
-  - action: openshift_health_check  # https://github.com/ansible/ansible/issues/20513
+  - action: openshift_health_check
     args:
       checks: ['@preflight']
diff --git a/playbooks/common/openshift-cluster/config.yml b/playbooks/common/openshift-cluster/config.yml
index 7224ae712..31c4b04af 100644
--- a/playbooks/common/openshift-cluster/config.yml
+++ b/playbooks/common/openshift-cluster/config.yml
@@ -6,7 +6,7 @@
   roles:
   - openshift_health_checker
   vars:
-  - r_openshift_health_checker_playbook_context: "install"
+  - r_openshift_health_checker_playbook_context: install
   post_tasks:
   - action: openshift_health_check
     args:
diff --git a/roles/openshift_health_checker/action_plugins/openshift_health_check.py b/roles/openshift_health_checker/action_plugins/openshift_health_check.py
index 581dd7d15..23da53940 100644
--- a/roles/openshift_health_checker/action_plugins/openshift_health_check.py
+++ b/roles/openshift_health_checker/action_plugins/openshift_health_check.py
@@ -13,6 +13,7 @@ except ImportError:
     display = Display()
 
 from ansible.plugins.action import ActionBase
+from ansible.module_utils.six import string_types
 
 # Augment sys.path so that we can import checks from a directory relative to
 # this callback plugin.
@@ -39,7 +40,8 @@ class ActionModule(ActionBase):
         try:
             known_checks = self.load_known_checks(tmp, task_vars)
             args = self._task.args
-            resolved_checks = resolve_checks(args.get("checks", []), known_checks.values())
+            requested_checks = normalize(args.get('checks', []))
+            resolved_checks = resolve_checks(requested_checks, known_checks.values())
         except OpenShiftCheckException as e:
             result["failed"] = True
             result["msg"] = str(e)
@@ -47,10 +49,7 @@ class ActionModule(ActionBase):
 
         result["checks"] = check_results = {}
 
-        user_disabled_checks = [
-            check.strip()
-            for check in task_vars.get("openshift_disable_check", "").split(",")
-        ]
+        user_disabled_checks = normalize(task_vars.get('openshift_disable_check', []))
 
         for check_name in resolved_checks:
             display.banner("CHECK [{} : {}]".format(check_name, task_vars["ansible_host"]))
@@ -134,3 +133,14 @@ def resolve_checks(names, all_checks):
         resolved.update(tag_to_checks[tag])
 
     return resolved
+
+
+def normalize(checks):
+    """Return a clean list of check names.
+
+    The input may be a comma-separated string or a sequence. Leading and
+    trailing whitespace characters are removed. Empty items are discarded.
+    """
+    if isinstance(checks, string_types):
+        checks = checks.split(',')
+    return [name.strip() for name in checks if name.strip()]
diff --git a/roles/openshift_health_checker/openshift_checks/logging/logging.py b/roles/openshift_health_checker/openshift_checks/logging/logging.py
index a48e1c728..43ba6c406 100644
--- a/roles/openshift_health_checker/openshift_checks/logging/logging.py
+++ b/roles/openshift_health_checker/openshift_checks/logging/logging.py
@@ -11,6 +11,9 @@ from openshift_checks import OpenShiftCheck, OpenShiftCheckException
 class LoggingCheck(OpenShiftCheck):
     """Base class for OpenShift aggregated logging component checks"""
 
+    # FIXME: this should not be listed as a check, since it is not meant to be
+    # run by itself.
+
     name = "logging"
     logging_namespace = "logging"
 
@@ -27,7 +30,7 @@ class LoggingCheck(OpenShiftCheck):
         return masters[0] == hostname
 
     def run(self):
-        pass
+        return {}
 
     def get_pods_for_component(self, namespace, logging_component):
         """Get all pods for a given component. Returns: list of pods for component, error string"""