openshift_checks: improve comments/names

9 files changed, 32 insertions, 30 deletions

diff --git a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
index bde81ad2c..0aa11eba1 100644
--- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
@@ -22,14 +22,16 @@ DEPLOYMENT_IMAGE_INFO = {
 class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
     """Check that required Docker images are available.
 
-    This check attempts to ensure that required docker images are
-    either present locally, or able to be pulled down from available
-    registries defined in a host machine.
+    Determine docker images that an install would require and check that they
+    are either present in the host's docker index, or available for the host to pull
+    with known registries as defined in our inventory file (or defaults).
     """
 
     name = "docker_image_availability"
     tags = ["preflight"]
-    dependencies = ["skopeo", "python-docker-py"]
+    # we use python-docker-py to check local docker for images, and skopeo
+    # to look for images available remotely without waiting to pull them.
+    dependencies = ["python-docker-py", "skopeo"]
 
     @classmethod
     def is_active(cls, task_vars):
@@ -154,17 +156,18 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck):
 
         return list(regs)
 
-    def available_images(self, images, registries, task_vars):
-        """Inspect existing images using Skopeo and return all images successfully inspected."""
+    def available_images(self, images, default_registries, task_vars):
+        """Search remotely for images. Returns: list of images found."""
         return [
             image for image in images
-            if self.is_available_skopeo_image(image, registries, task_vars)
+            if self.is_available_skopeo_image(image, default_registries, task_vars)
         ]
 
-    def is_available_skopeo_image(self, image, registries, task_vars):
+    def is_available_skopeo_image(self, image, default_registries, task_vars):
         """Use Skopeo to determine if required image exists in known registry(s)."""
+        registries = default_registries
 
-        # if image does already includes a registry, just use that
+        # if image already includes a registry, only use that
         if image.count("/") > 1:
             registry, image = image.split("/", 1)
             registries = [registry]
diff --git a/roles/openshift_health_checker/openshift_checks/logging/curator.py b/roles/openshift_health_checker/openshift_checks/logging/curator.py
index c9fc59896..d8e64074f 100644
--- a/roles/openshift_health_checker/openshift_checks/logging/curator.py
+++ b/roles/openshift_health_checker/openshift_checks/logging/curator.py
@@ -1,13 +1,11 @@
-"""
-Module for performing checks on an Curator logging deployment
-"""
+"""Check for an aggregated logging Curator deployment"""
 
 from openshift_checks import get_var
 from openshift_checks.logging.logging import LoggingCheck
 
 
 class Curator(LoggingCheck):
-    """Module that checks an integrated logging Curator deployment"""
+    """Check for an aggregated logging Curator deployment"""
 
     name = "curator"
     tags = ["health", "logging"]
@@ -15,8 +13,6 @@ class Curator(LoggingCheck):
     logging_namespace = None
 
     def run(self, tmp, task_vars):
-        """Check various things and gather errors. Returns: result as hash"""
-
         self.logging_namespace = get_var(task_vars, "openshift_logging_namespace", default="logging")
         curator_pods, error = super(Curator, self).get_pods_for_component(
             self.module_executor,
diff --git a/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py b/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py
index 01cb35b81..d57cb9376 100644
--- a/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py
+++ b/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py
@@ -1,6 +1,4 @@
-"""
-Module for performing checks on an Elasticsearch logging deployment
-"""
+"""Check for an aggregated logging Elasticsearch deployment"""
 
 import json
 import re
@@ -10,7 +8,7 @@ from openshift_checks.logging.logging import LoggingCheck
 
 
 class Elasticsearch(LoggingCheck):
-    """Module that checks an integrated logging Elasticsearch deployment"""
+    """Check for an aggregated logging Elasticsearch deployment"""
 
     name = "elasticsearch"
     tags = ["health", "logging"]
@@ -41,7 +39,7 @@ class Elasticsearch(LoggingCheck):
         return {"failed": False, "changed": False, "msg": 'No problems found with Elasticsearch deployment.'}
 
     def _not_running_elasticsearch_pods(self, es_pods):
-        """Returns: list of running pods, list of errors about non-running pods"""
+        """Returns: list of pods that are not running, list of errors about non-running pods"""
         not_running = super(Elasticsearch, self).not_running_pods(es_pods)
         if not_running:
             return not_running, [(
diff --git a/roles/openshift_health_checker/openshift_checks/logging/fluentd.py b/roles/openshift_health_checker/openshift_checks/logging/fluentd.py
index 627567293..1e1e7f2bd 100644
--- a/roles/openshift_health_checker/openshift_checks/logging/fluentd.py
+++ b/roles/openshift_health_checker/openshift_checks/logging/fluentd.py
@@ -1,6 +1,4 @@
-"""
-Module for performing checks on an Fluentd logging deployment
-"""
+"""Check for an aggregated logging Fluentd deployment"""
 
 import json
 
@@ -9,7 +7,8 @@ from openshift_checks.logging.logging import LoggingCheck
 
 
 class Fluentd(LoggingCheck):
-    """Module that checks an integrated logging Fluentd deployment"""
+    """Check for an aggregated logging Fluentd deployment"""
+
     name = "fluentd"
     tags = ["health", "logging"]
 
diff --git a/roles/openshift_health_checker/openshift_checks/logging/logging.py b/roles/openshift_health_checker/openshift_checks/logging/logging.py
index 02a094007..46fd4f5c7 100644
--- a/roles/openshift_health_checker/openshift_checks/logging/logging.py
+++ b/roles/openshift_health_checker/openshift_checks/logging/logging.py
@@ -9,7 +9,7 @@ from openshift_checks import OpenShiftCheck, OpenShiftCheckException, get_var
 
 
 class LoggingCheck(OpenShiftCheck):
-    """Base class for logging component checks"""
+    """Base class for OpenShift aggregated logging component checks"""
 
     name = "logging"
     logging_namespace = "logging"
diff --git a/roles/openshift_health_checker/openshift_checks/memory_availability.py b/roles/openshift_health_checker/openshift_checks/memory_availability.py
index f4e31065f..2b42c72a9 100644
--- a/roles/openshift_health_checker/openshift_checks/memory_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/memory_availability.py
@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring
+"""Check that recommended memory is available."""
 from openshift_checks import OpenShiftCheck, get_var
 
 MIB = 2**20
diff --git a/roles/openshift_health_checker/openshift_checks/package_availability.py b/roles/openshift_health_checker/openshift_checks/package_availability.py
index 0dd2b1286..a67b68d1b 100644
--- a/roles/openshift_health_checker/openshift_checks/package_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/package_availability.py
@@ -1,4 +1,5 @@
-# pylint: disable=missing-docstring
+"""Check that required RPM packages are available."""
+
 from openshift_checks import OpenShiftCheck, get_var
 from openshift_checks.mixins import NotContainerizedMixin
 
@@ -11,6 +12,7 @@ class PackageAvailability(NotContainerizedMixin, OpenShiftCheck):
 
     @classmethod
     def is_active(cls, task_vars):
+        """Run only when yum is the package manager as the code is specific to it."""
         return super(PackageAvailability, cls).is_active(task_vars) and task_vars["ansible_pkg_mgr"] == "yum"
 
     def run(self, tmp, task_vars):
@@ -29,6 +31,7 @@ class PackageAvailability(NotContainerizedMixin, OpenShiftCheck):
 
     @staticmethod
     def master_packages(rpm_prefix):
+        """Return a list of RPMs that we expect a master install to have available."""
         return [
             "{rpm_prefix}".format(rpm_prefix=rpm_prefix),
             "{rpm_prefix}-clients".format(rpm_prefix=rpm_prefix),
@@ -44,6 +47,7 @@ class PackageAvailability(NotContainerizedMixin, OpenShiftCheck):
 
     @staticmethod
     def node_packages(rpm_prefix):
+        """Return a list of RPMs that we expect a node install to have available."""
         return [
             "{rpm_prefix}".format(rpm_prefix=rpm_prefix),
             "{rpm_prefix}-node".format(rpm_prefix=rpm_prefix),
diff --git a/roles/openshift_health_checker/openshift_checks/package_update.py b/roles/openshift_health_checker/openshift_checks/package_update.py
index f432380c6..db063158c 100644
--- a/roles/openshift_health_checker/openshift_checks/package_update.py
+++ b/roles/openshift_health_checker/openshift_checks/package_update.py
@@ -1,10 +1,10 @@
-# pylint: disable=missing-docstring
+"""Check that a yum update would not run into conflicts with available packages."""
 from openshift_checks import OpenShiftCheck
 from openshift_checks.mixins import NotContainerizedMixin
 
 
 class PackageUpdate(NotContainerizedMixin, OpenShiftCheck):
-    """Check that there are no conflicts in RPM packages."""
+    """Check that a yum update would not run into conflicts with available packages."""
 
     name = "package_update"
     tags = ["preflight"]
diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py
index 204752bd0..ab4295770 100644
--- a/roles/openshift_health_checker/openshift_checks/package_version.py
+++ b/roles/openshift_health_checker/openshift_checks/package_version.py
@@ -1,4 +1,4 @@
-# pylint: disable=missing-docstring
+"""Check that available RPM packages match the required versions."""
 from openshift_checks import OpenShiftCheck, OpenShiftCheckException, get_var
 from openshift_checks.mixins import NotContainerizedMixin
 
@@ -107,6 +107,7 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck):
         raise OpenShiftCheckException(msg.format(openshift_version))
 
     def get_openshift_version(self, task_vars):
+        """Return received image tag as a normalized X.Y minor version string."""
         openshift_version = get_var(task_vars, "openshift_image_tag")
         if openshift_version and openshift_version[0] == 'v':
             openshift_version = openshift_version[1:]
@@ -114,6 +115,7 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck):
         return self.parse_version(openshift_version)
 
     def parse_version(self, version):
+        """Return a normalized X.Y minor version string."""
         components = version.split(".")
         if not components or len(components) < 2:
             msg = "An invalid version of OpenShift was found for this host: {}"