openshift_checks: refactor logging checks

Turn failure messages into exceptions that tests can look for without
depending on text meant for humans.
Turn logging_namespace property into a method.
Get rid of _exec_oc and just use logging.exec_oc.

roles/openshift_health_checker/openshift_checks/__init__.py

@@ -13,8 +13,30 @@
 
 
 class OpenShiftCheckException(Exception):
-    """Raised when a check cannot proceed."""
-    pass
+    """Raised when a check encounters a failure condition."""
+
+    def __init__(self, name, msg=None):
+        # msg is for the message the user will see when this is raised.
+        # name is for test code to identify the error without looking at msg text.
+        if msg is None:  # for parameter backward compatibility
+            msg = name
+            name = self.__class__.__name__
+        self.name = name
+        super(OpenShiftCheckException, self).__init__(msg)
+
+
+class OpenShiftCheckExceptionList(OpenShiftCheckException):
+    """A container for multiple logging errors that may be detected in one check."""
+    def __init__(self, errors):
+        self.errors = errors
+        super(OpenShiftCheckExceptionList, self).__init__(
+            'OpenShiftCheckExceptionList',
+            '\n'.join(str(msg) for msg in errors)
+        )
+
+    # make iterable
+    def __getitem__(self, index):
+        return self.errors[index]
 
 
 @six.add_metaclass(ABCMeta)
@@ -34,7 +56,8 @@ def __init__(self, execute_module=None, task_vars=None, tmp=None):
         self._execute_module = execute_module
         self.task_vars = task_vars or {}
         self.tmp = tmp
-        # set True when the check makes a change to the host so it can be reported to the user:
+
+        # set to True when the check changes the host, for accurate total "changed" count
         self.changed = False
 
     @abstractproperty

roles/openshift_health_checker/openshift_checks/logging/curator.py

@@ -1,6 +1,6 @@
 """Check for an aggregated logging Curator deployment"""
 
-from openshift_checks.logging.logging import LoggingCheck
+from openshift_checks.logging.logging import OpenShiftCheckException, LoggingCheck
 
 
 class Curator(LoggingCheck):
@@ -9,46 +9,33 @@ class Curator(LoggingCheck):
     name = "curator"
     tags = ["health", "logging"]
 
-    logging_namespace = None
-
     def run(self):
-        self.logging_namespace = self.get_var("openshift_logging_namespace", default="logging")
-        curator_pods, error = super(Curator, self).get_pods_for_component(
-            self.logging_namespace,
-            "curator",
-        )
-        if error:
-            return {"failed": True, "msg": error}
-        check_error = self.check_curator(curator_pods)
-
-        if check_error:
-            msg = ("The following Curator deployment issue was found:"
-                   "\n-------\n"
-                   "{}".format(check_error))
-            return {"failed": True, "msg": msg}
-
+        curator_pods = self.get_pods_for_component("curator")
+        self.check_curator(curator_pods)
         # TODO(lmeyer): run it all again for the ops cluster
-        return {"failed": False, "msg": 'No problems found with Curator deployment.'}
+
+        return {}
 
     def check_curator(self, pods):
         """Check to see if curator is up and working. Returns: error string"""
         if not pods:
-            return (
+            raise OpenShiftCheckException(
+                "MissingComponentPods",
                 "There are no Curator pods for the logging stack,\n"
                 "so nothing will prune Elasticsearch indexes.\n"
                 "Is Curator correctly deployed?"
             )
 
-        not_running = super(Curator, self).not_running_pods(pods)
+        not_running = self.not_running_pods(pods)
         if len(not_running) == len(pods):
-            return (
+            raise OpenShiftCheckException(
+                "CuratorNotRunning",
                 "The Curator pod is not currently in a running state,\n"
                 "so Elasticsearch indexes may increase without bound."
             )
         if len(pods) - len(not_running) > 1:
-            return (
+            raise OpenShiftCheckException(
+                "TooManyCurators",
                 "There is more than one Curator pod running. This should not normally happen.\n"
                 "Although this doesn't cause any problems, you may want to investigate."
             )
-
-        return None