move ensure_dependencies to OpenShiftCheck class

roles/openshift_health_checker/openshift_checks/__init__.py

@@ -53,6 +53,8 @@ def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None,
     which provides the check's stored task_vars and tmp.
     """
 
+    dependencies = []
+
     def __init__(self, execute_module=None, task_vars=None, tmp=None):
         self._execute_module = execute_module
         self.task_vars = task_vars or {}
@@ -174,6 +176,33 @@ def get_var(self, *keys, **kwargs):
                 '{error}'.format(var=".".join(keys), value=value, error=error)
             )
 
+    def ensure_dependencies(self):
+        """
+        Ensure that docker-related packages exist, but not on atomic hosts
+        (which would not be able to install but should already have them).
+        Returns: msg, failed
+        """
+        if self.get_var("openshift", "common", "is_atomic"):
+            return "", False
+
+        # NOTE: we would use the "package" module but it's actually an action plugin
+        # and it's not clear how to invoke one of those. This is about the same anyway:
+        result = self.execute_module(
+            self.get_var("ansible_pkg_mgr", default="yum"),
+            {"name": self.dependencies, "state": "present"},
+        )
+        msg = result.get("msg", "")
+        if result.get("failed"):
+            if "No package matching" in msg:
+                msg = "Ensure that all required dependencies can be installed via `yum`.\n"
+            msg = (
+                "Unable to install required packages on this host:\n"
+                "    {deps}\n{msg}"
+            ).format(deps=',\n    '.join(self.dependencies), msg=msg)
+        failed = result.get("failed", False) or result.get("rc", 0) != 0
+        self.changed = result.get("changed", False)
+        return msg, failed
+
     @staticmethod
     def get_major_minor_version(openshift_image_tag):
         """Parse and return the deployed version of OpenShift as a tuple."""

roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py

@@ -3,10 +3,9 @@
 """
 
 from openshift_checks import OpenShiftCheck
-from openshift_checks.mixins import DockerHostMixin
 
 
-class EtcdImageDataSize(DockerHostMixin, OpenShiftCheck):
+class EtcdImageDataSize(OpenShiftCheck):
     """Check that total size of OpenShift image data does not exceed the recommended limit in an etcd cluster"""
 
     name = "etcd_imagedata_size"

roles/openshift_health_checker/openshift_checks/mixins.py

@@ -16,38 +16,11 @@ def is_active(self):
 
 class DockerHostMixin(object):
     """Mixin for checks that are only active on hosts that require Docker."""
-
-    dependencies = []
+    # permanent # pylint: disable=too-few-public-methods
+    # Reason: The mixin is not intended to stand on its own as a class.
 
     def is_active(self):
         """Only run on hosts that depend on Docker."""
         is_containerized = self.get_var("openshift", "common", "is_containerized")
         is_node = "nodes" in self.get_var("group_names", default=[])
         return super(DockerHostMixin, self).is_active() and (is_containerized or is_node)
-
-    def ensure_dependencies(self):
-        """
-        Ensure that docker-related packages exist, but not on atomic hosts
-        (which would not be able to install but should already have them).
-        Returns: msg, failed
-        """
-        if self.get_var("openshift", "common", "is_atomic"):
-            return "", False
-
-        # NOTE: we would use the "package" module but it's actually an action plugin
-        # and it's not clear how to invoke one of those. This is about the same anyway:
-        result = self.execute_module(
-            self.get_var("ansible_pkg_mgr", default="yum"),
-            {"name": self.dependencies, "state": "present"},
-        )
-        msg = result.get("msg", "")
-        if result.get("failed"):
-            if "No package matching" in msg:
-                msg = "Ensure that all required dependencies can be installed via `yum`.\n"
-            msg = (
-                "Unable to install required packages on this host:\n"
-                "    {deps}\n{msg}"
-            ).format(deps=',\n    '.join(self.dependencies), msg=msg)
-        failed = result.get("failed", False) or result.get("rc", 0) != 0
-        self.changed = result.get("changed", False)
-        return msg, failed

roles/openshift_health_checker/test/etcd_imagedata_size_test.py

@@ -49,10 +49,19 @@ def fake_etcd_node(node, visited):
     ([{'mount': '/mnt'}], ['/mnt']),  # missing relevant mount paths
 ])
 def test_cannot_determine_available_mountpath(ansible_mounts, extra_words):
+    def execute_module(module_name, module_args, *_):
+        if module_name == 'yum':
+            return {'changed': True}
+
     task_vars = dict(
         ansible_mounts=ansible_mounts,
+        openshift=dict(
+            common=dict(
+                is_atomic=False,
+            ),
+        )
     )
-    check = EtcdImageDataSize(fake_execute_module, task_vars)
+    check = EtcdImageDataSize(execute_module, task_vars)
 
     with pytest.raises(OpenShiftCheckException) as excinfo:
         check.run()
@@ -113,6 +122,9 @@ def test_cannot_determine_available_mountpath(ansible_mounts, extra_words):
 ])
 def test_check_etcd_key_size_calculates_correct_limit(ansible_mounts, tree, size_limit, should_fail, extra_words):
     def execute_module(module_name, module_args, *_):
+        if module_name == 'yum':
+            return {'changed': True}
+
         if module_name != "etcdkeysize":
             return {
                 "changed": False,
@@ -128,7 +140,10 @@ def execute_module(module_name, module_args, *_):
         ansible_mounts=ansible_mounts,
         openshift=dict(
             master=dict(etcd_hosts=["localhost"]),
-            common=dict(config_base="/var/lib/origin")
+            common=dict(
+                config_base="/var/lib/origin",
+                is_atomic=False,
+            )
         )
     )
     if size_limit is None:
@@ -269,6 +284,9 @@ def execute_module(module_name, module_args, *_):
 ])
 def test_etcd_key_size_check_calculates_correct_size(ansible_mounts, tree, root_path, expected_size, extra_words):
     def execute_module(module_name, module_args, *_):
+        if module_name == 'yum':
+            return {'changed': True}
+
         if module_name != "etcdkeysize":
             return {
                 "changed": False,
@@ -286,7 +304,10 @@ def execute_module(module_name, module_args, *_):
         ansible_mounts=ansible_mounts,
         openshift=dict(
             master=dict(etcd_hosts=["localhost"]),
-            common=dict(config_base="/var/lib/origin")
+            common=dict(
+                config_base="/var/lib/origin",
+                is_atomic=False,
+            )
         )
     )
 
@@ -296,6 +317,9 @@ def execute_module(module_name, module_args, *_):
 
 def test_etcdkeysize_module_failure():
     def execute_module(module_name, *_):
+        if module_name == 'yum':
+            return {'changed': True}
+
         if module_name != "etcdkeysize":
             return {
                 "changed": False,
@@ -314,7 +338,10 @@ def execute_module(module_name, *_):
         }],
         openshift=dict(
             master=dict(etcd_hosts=["localhost"]),
-            common=dict(config_base="/var/lib/origin")
+            common=dict(
+                config_base="/var/lib/origin",
+                is_atomic=False,
+            )
         )
     )
 
@@ -323,7 +350,3 @@ def execute_module(module_name, *_):
     assert check["failed"]
     for word in "Failed to retrieve stats":
         assert word in check["msg"]
-
-
-def fake_execute_module(*args):
-    raise AssertionError('this function should not be called')