From 5e71e43a2a2e9089185d34e5406ee212cc478a75 Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Wed, 22 Mar 2017 16:29:37 +0100
Subject: Rename module_executor -> execute_module
It is a function/callable, the name should imply action, should be a
verb and not a noun.
Keep supporting the old name while we have PRs in-flight that use the
old name.
---
.../action_plugins/openshift_health_check.py | 2 +-
.../openshift_checks/__init__.py | 9 ++++--
.../openshift_checks/package_availability.py | 2 +-
.../openshift_checks/package_update.py | 2 +-
.../openshift_checks/package_version.py | 2 +-
.../test/openshift_check_test.py | 37 +++++++++++++++++++++-
6 files changed, 47 insertions(+), 7 deletions(-)
(limited to 'roles/openshift_health_checker')
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 8b23533c8..027abf398 100644
--- a/roles/openshift_health_checker/action_plugins/openshift_health_check.py
+++ b/roles/openshift_health_checker/action_plugins/openshift_health_check.py
@@ -91,7 +91,7 @@ class ActionModule(ActionBase):
check_name,
cls.__module__, cls.__name__,
other_cls.__module__, other_cls.__name__))
- known_checks[check_name] = cls(module_executor=self._execute_module)
+ known_checks[check_name] = cls(execute_module=self._execute_module)
return known_checks
diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py
index 93547a2e0..72d0b26df 100644
--- a/roles/openshift_health_checker/openshift_checks/__init__.py
+++ b/roles/openshift_health_checker/openshift_checks/__init__.py
@@ -21,8 +21,13 @@ class OpenShiftCheckException(Exception):
class OpenShiftCheck(object):
"""A base class for defining checks for an OpenShift cluster environment."""
- def __init__(self, module_executor):
- self.module_executor = module_executor
+ def __init__(self, execute_module=None, module_executor=None):
+ if execute_module is module_executor is None:
+ raise TypeError(
+ "__init__() takes either execute_module (recommended) "
+ "or module_executor (deprecated), none given")
+ self.execute_module = execute_module or module_executor
+ self.module_executor = self.execute_module
@abstractproperty
def name(self):
diff --git a/roles/openshift_health_checker/openshift_checks/package_availability.py b/roles/openshift_health_checker/openshift_checks/package_availability.py
index 771123d61..9891972a6 100644
--- a/roles/openshift_health_checker/openshift_checks/package_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/package_availability.py
@@ -21,7 +21,7 @@ class PackageAvailability(NotContainerizedMixin, OpenShiftCheck):
packages.update(self.node_packages(rpm_prefix))
args = {"packages": sorted(set(packages))}
- return self.module_executor("check_yum_update", args, tmp, task_vars)
+ return self.execute_module("check_yum_update", args, tmp, task_vars)
@staticmethod
def master_packages(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 c5a226954..fd0c0a755 100644
--- a/roles/openshift_health_checker/openshift_checks/package_update.py
+++ b/roles/openshift_health_checker/openshift_checks/package_update.py
@@ -11,4 +11,4 @@ class PackageUpdate(NotContainerizedMixin, OpenShiftCheck):
def run(self, tmp, task_vars):
args = {"packages": []}
- return self.module_executor("check_yum_update", args, tmp, task_vars)
+ return self.execute_module("check_yum_update", args, tmp, task_vars)
diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py
index 2e9d07deb..42193a1c6 100644
--- a/roles/openshift_health_checker/openshift_checks/package_version.py
+++ b/roles/openshift_health_checker/openshift_checks/package_version.py
@@ -17,4 +17,4 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck):
"prefix": rpm_prefix,
"version": openshift_release,
}
- return self.module_executor("aos_version", args, tmp, task_vars)
+ return self.execute_module("aos_version", args, tmp, task_vars)
diff --git a/roles/openshift_health_checker/test/openshift_check_test.py b/roles/openshift_health_checker/test/openshift_check_test.py
index c4c8cd1c2..9cbd5b11e 100644
--- a/roles/openshift_health_checker/test/openshift_check_test.py
+++ b/roles/openshift_health_checker/test/openshift_check_test.py
@@ -1,6 +1,6 @@
import pytest
-from openshift_checks import get_var, OpenShiftCheckException
+from openshift_checks import OpenShiftCheck, get_var, OpenShiftCheckException
# Fixtures
@@ -22,6 +22,41 @@ def missing_keys(request):
# Tests
+def test_OpenShiftCheck_init():
+ class TestCheck(OpenShiftCheck):
+ name = "test_check"
+ run = NotImplemented
+
+ # initialization requires at least one argument (apart from self)
+ with pytest.raises(TypeError) as excinfo:
+ TestCheck()
+ assert 'execute_module' in str(excinfo.value)
+ assert 'module_executor' in str(excinfo.value)
+
+ execute_module = object()
+
+ # initialize with positional argument
+ check = TestCheck(execute_module)
+ # new recommended name
+ assert check.execute_module == execute_module
+ # deprecated attribute name
+ assert check.module_executor == execute_module
+
+ # initialize with keyword argument, recommended name
+ check = TestCheck(execute_module=execute_module)
+ # new recommended name
+ assert check.execute_module == execute_module
+ # deprecated attribute name
+ assert check.module_executor == execute_module
+
+ # initialize with keyword argument, deprecated name
+ check = TestCheck(module_executor=execute_module)
+ # new recommended name
+ assert check.execute_module == execute_module
+ # deprecated attribute name
+ assert check.module_executor == execute_module
+
+
@pytest.mark.parametrize("keys,expected", [
(("foo",), 42),
(("bar", "baz"), "openshift"),
--
cgit v1.2.3
From 4b657031ff309cb8b004cd7fbd44ae479ce09432 Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Wed, 22 Mar 2017 16:51:25 +0100
Subject: Test OpenShift health check loader
---
.../action_plugins/openshift_health_check.py | 4 +++-
.../openshift_checks/__init__.py | 27 ++++++++++++----------
.../test/openshift_check_test.py | 9 +++++++-
3 files changed, 26 insertions(+), 14 deletions(-)
(limited to 'roles/openshift_health_checker')
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 027abf398..cf0fe19f1 100644
--- a/roles/openshift_health_checker/action_plugins/openshift_health_check.py
+++ b/roles/openshift_health_checker/action_plugins/openshift_health_check.py
@@ -17,7 +17,7 @@ from ansible.plugins.action import ActionBase
# this callback plugin.
sys.path.insert(1, os.path.dirname(os.path.dirname(__file__)))
-from openshift_checks import OpenShiftCheck, OpenShiftCheckException # noqa: E402
+from openshift_checks import OpenShiftCheck, OpenShiftCheckException, load_checks # noqa: E402
class ActionModule(ActionBase):
@@ -78,6 +78,8 @@ class ActionModule(ActionBase):
return result
def load_known_checks(self):
+ load_checks()
+
known_checks = {}
known_check_classes = set(cls for cls in OpenShiftCheck.subclasses())
diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py
index 72d0b26df..be63d864a 100644
--- a/roles/openshift_health_checker/openshift_checks/__init__.py
+++ b/roles/openshift_health_checker/openshift_checks/__init__.py
@@ -63,6 +63,21 @@ class OpenShiftCheck(object):
yield subclass
+LOADER_EXCLUDES = (
+ "__init__.py",
+ "mixins.py",
+)
+
+
+def load_checks():
+ """Dynamically import all check modules for the side effect of registering checks."""
+ return [
+ import_module(__package__ + "." + name[:-3])
+ for name in os.listdir(os.path.dirname(__file__))
+ if name.endswith(".py") and name not in LOADER_EXCLUDES
+ ]
+
+
def get_var(task_vars, *keys, **kwargs):
"""Helper function to get deeply nested values from task_vars.
@@ -78,15 +93,3 @@ def get_var(task_vars, *keys, **kwargs):
return kwargs["default"]
raise OpenShiftCheckException("'{}' is undefined".format(".".join(map(str, keys))))
return value
-
-
-# Dynamically import all submodules for the side effect of loading checks.
-
-EXCLUDES = (
- "__init__.py",
- "mixins.py",
-)
-
-for name in os.listdir(os.path.dirname(__file__)):
- if name.endswith(".py") and name not in EXCLUDES:
- import_module(__package__ + "." + name[:-3])
diff --git a/roles/openshift_health_checker/test/openshift_check_test.py b/roles/openshift_health_checker/test/openshift_check_test.py
index 9cbd5b11e..03465a7c3 100644
--- a/roles/openshift_health_checker/test/openshift_check_test.py
+++ b/roles/openshift_health_checker/test/openshift_check_test.py
@@ -1,6 +1,7 @@
import pytest
-from openshift_checks import OpenShiftCheck, get_var, OpenShiftCheckException
+from openshift_checks import OpenShiftCheck, OpenShiftCheckException
+from openshift_checks import load_checks, get_var
# Fixtures
@@ -57,6 +58,12 @@ def test_OpenShiftCheck_init():
assert check.module_executor == execute_module
+def test_load_checks():
+ """Loading checks should load and return Python modules."""
+ modules = load_checks()
+ assert modules
+
+
@pytest.mark.parametrize("keys,expected", [
(("foo",), 42),
(("bar", "baz"), "openshift"),
--
cgit v1.2.3
From 0af4d72753fa411ddbd17e180aca2c3f4a4df9a6 Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Wed, 22 Mar 2017 16:52:37 +0100
Subject: Test recursively finding subclasses
---
.../test/openshift_check_test.py | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
(limited to 'roles/openshift_health_checker')
diff --git a/roles/openshift_health_checker/test/openshift_check_test.py b/roles/openshift_health_checker/test/openshift_check_test.py
index 03465a7c3..e3153979c 100644
--- a/roles/openshift_health_checker/test/openshift_check_test.py
+++ b/roles/openshift_health_checker/test/openshift_check_test.py
@@ -58,6 +58,23 @@ def test_OpenShiftCheck_init():
assert check.module_executor == execute_module
+def test_subclasses():
+ """OpenShiftCheck.subclasses should find all subclasses recursively."""
+ class TestCheck1(OpenShiftCheck):
+ pass
+
+ class TestCheck2(OpenShiftCheck):
+ pass
+
+ class TestCheck1A(TestCheck1):
+ pass
+
+ local_subclasses = set([TestCheck1, TestCheck1A, TestCheck2])
+ known_subclasses = set(OpenShiftCheck.subclasses())
+
+ assert local_subclasses - known_subclasses == set(), "local_subclasses should be a subset of known_subclasses"
+
+
def test_load_checks():
"""Loading checks should load and return Python modules."""
modules = load_checks()
--
cgit v1.2.3
From 209810c9905c0ebf89b1e03ccb8593f78a6e5396 Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Wed, 22 Mar 2017 21:05:18 +0100
Subject: Add unit tests for mixins.py
---
roles/openshift_health_checker/test/mixins_test.py | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 roles/openshift_health_checker/test/mixins_test.py
(limited to 'roles/openshift_health_checker')
diff --git a/roles/openshift_health_checker/test/mixins_test.py b/roles/openshift_health_checker/test/mixins_test.py
new file mode 100644
index 000000000..2d83e207d
--- /dev/null
+++ b/roles/openshift_health_checker/test/mixins_test.py
@@ -0,0 +1,23 @@
+import pytest
+
+from openshift_checks import OpenShiftCheck, OpenShiftCheckException
+from openshift_checks.mixins import NotContainerizedMixin
+
+
+class NotContainerizedCheck(NotContainerizedMixin, OpenShiftCheck):
+ name = "not_containerized"
+ run = NotImplemented
+
+
+@pytest.mark.parametrize('task_vars,expected', [
+ (dict(openshift=dict(common=dict(is_containerized=False))), True),
+ (dict(openshift=dict(common=dict(is_containerized=True))), False),
+])
+def test_is_active(task_vars, expected):
+ assert NotContainerizedCheck.is_active(task_vars) == expected
+
+
+def test_is_active_missing_task_vars():
+ with pytest.raises(OpenShiftCheckException) as excinfo:
+ NotContainerizedCheck.is_active(task_vars={})
+ assert 'is_containerized' in str(excinfo.value)
--
cgit v1.2.3
From a39620a07602db95d0d59702a9a5466b270a2be8 Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Fri, 24 Mar 2017 22:09:00 +0100
Subject: Add unit tests for package_availability.py
---
.../test/package_availability_test.py | 49 ++++++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 roles/openshift_health_checker/test/package_availability_test.py
(limited to 'roles/openshift_health_checker')
diff --git a/roles/openshift_health_checker/test/package_availability_test.py b/roles/openshift_health_checker/test/package_availability_test.py
new file mode 100644
index 000000000..25385339a
--- /dev/null
+++ b/roles/openshift_health_checker/test/package_availability_test.py
@@ -0,0 +1,49 @@
+import pytest
+
+from openshift_checks.package_availability import PackageAvailability
+
+
+@pytest.mark.parametrize('task_vars,must_have_packages,must_not_have_packages', [
+ (
+ dict(openshift=dict(common=dict(service_type='openshift'))),
+ set(),
+ set(['openshift-master', 'openshift-node']),
+ ),
+ (
+ dict(
+ openshift=dict(common=dict(service_type='origin')),
+ group_names=['masters'],
+ ),
+ set(['origin-master']),
+ set(['origin-node']),
+ ),
+ (
+ dict(
+ openshift=dict(common=dict(service_type='atomic-openshift')),
+ group_names=['nodes'],
+ ),
+ set(['atomic-openshift-node']),
+ set(['atomic-openshift-master']),
+ ),
+ (
+ dict(
+ openshift=dict(common=dict(service_type='atomic-openshift')),
+ group_names=['masters', 'nodes'],
+ ),
+ set(['atomic-openshift-master', 'atomic-openshift-node']),
+ set(),
+ ),
+])
+def test_package_availability(task_vars, must_have_packages, must_not_have_packages):
+ return_value = object()
+
+ def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
+ assert module_name == 'check_yum_update'
+ assert 'packages' in module_args
+ assert set(module_args['packages']).issuperset(must_have_packages)
+ assert not set(module_args['packages']).intersection(must_not_have_packages)
+ return return_value
+
+ check = PackageAvailability(execute_module=execute_module)
+ result = check.run(tmp=None, task_vars=task_vars)
+ assert result is return_value
--
cgit v1.2.3
From 5acb711c3ebbb273351f1a1f3d418c85a6abf2c3 Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Mon, 27 Mar 2017 14:58:13 +0200
Subject: Add unit tests for package_update.py
---
.../openshift_health_checker/test/package_update_test.py | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
create mode 100644 roles/openshift_health_checker/test/package_update_test.py
(limited to 'roles/openshift_health_checker')
diff --git a/roles/openshift_health_checker/test/package_update_test.py b/roles/openshift_health_checker/test/package_update_test.py
new file mode 100644
index 000000000..5e000cff5
--- /dev/null
+++ b/roles/openshift_health_checker/test/package_update_test.py
@@ -0,0 +1,16 @@
+from openshift_checks.package_update import PackageUpdate
+
+
+def test_package_update():
+ return_value = object()
+
+ def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
+ assert module_name == 'check_yum_update'
+ assert 'packages' in module_args
+ # empty list of packages means "generic check if 'yum update' will work"
+ assert module_args['packages'] == []
+ return return_value
+
+ check = PackageUpdate(execute_module=execute_module)
+ result = check.run(tmp=None, task_vars=None)
+ assert result is return_value
--
cgit v1.2.3
From ca2eaab70716a23d1d8064d16d183cb7f2ffc898 Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Mon, 27 Mar 2017 15:08:10 +0200
Subject: Add unit tests for package_version.py
---
.../test/package_version_test.py | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
create mode 100644 roles/openshift_health_checker/test/package_version_test.py
(limited to 'roles/openshift_health_checker')
diff --git a/roles/openshift_health_checker/test/package_version_test.py b/roles/openshift_health_checker/test/package_version_test.py
new file mode 100644
index 000000000..cc1d263bc
--- /dev/null
+++ b/roles/openshift_health_checker/test/package_version_test.py
@@ -0,0 +1,21 @@
+from openshift_checks.package_version import PackageVersion
+
+
+def test_package_version():
+ task_vars = dict(
+ openshift=dict(common=dict(service_type='origin')),
+ openshift_release='v3.5',
+ )
+ return_value = object()
+
+ def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
+ assert module_name == 'aos_version'
+ assert 'prefix' in module_args
+ assert 'version' in module_args
+ assert module_args['prefix'] == task_vars['openshift']['common']['service_type']
+ assert module_args['version'] == task_vars['openshift_release']
+ return return_value
+
+ check = PackageVersion(execute_module=execute_module)
+ result = check.run(tmp=None, task_vars=task_vars)
+ assert result is return_value
--
cgit v1.2.3
From d6cf72a4f488d8c275bfbaf27620ff716da1ae5d Mon Sep 17 00:00:00 2001
From: Rodolfo Carvalho <rhcarvalho@gmail.com>
Date: Mon, 27 Mar 2017 16:08:22 +0200
Subject: Add test scaffold for docker_image_availability.py
The intention is to set a starting point and let another team member
work on the code to gain experience with tests.
---
.../openshift_checks/docker_image_availability.py | 11 +++++++++
.../test/docker_image_availability_test.py | 28 ++++++++++++++++++++++
2 files changed, 39 insertions(+)
create mode 100644 roles/openshift_health_checker/test/docker_image_availability_test.py
(limited to 'roles/openshift_health_checker')
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 7a7498cb7..cce289b95 100644
--- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
+++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py
@@ -15,6 +15,9 @@ class DockerImageAvailability(OpenShiftCheck):
skopeo_image = "openshift/openshift-ansible"
+ # FIXME(juanvallejo): we should consider other possible values of
+ # `deployment_type` (the key here). See
+ # https://github.com/openshift/openshift-ansible/blob/8e26f8c/roles/openshift_repos/vars/main.yml#L7
docker_image_base = {
"origin": {
"repo": "openshift",
@@ -62,9 +65,15 @@ class DockerImageAvailability(OpenShiftCheck):
def required_images(self, task_vars):
deployment_type = get_var(task_vars, "deployment_type")
+ # FIXME(juanvallejo): we should handle gracefully with a proper error
+ # message when given an unexpected value for `deployment_type`.
image_base_name = self.docker_image_base[deployment_type]
openshift_release = get_var(task_vars, "openshift_release")
+ # FIXME(juanvallejo): this variable is not required when the
+ # installation is non-containerized. The example inventories have it
+ # commented out. We should handle gracefully and with a proper error
+ # message when this variable is required and not set.
openshift_image_tag = get_var(task_vars, "openshift_image_tag")
is_containerized = get_var(task_vars, "openshift", "common", "is_containerized")
@@ -104,6 +113,8 @@ class DockerImageAvailability(OpenShiftCheck):
if result.get("failed", False):
return []
+ # FIXME(juanvallejo): wrong default type, result["info"] is expected to
+ # contain a dictionary (see how we call `docker_info.get` below).
docker_info = result.get("info", "")
return [registry.get("Name", "") for registry in docker_info.get("Registries", {})]
diff --git a/roles/openshift_health_checker/test/docker_image_availability_test.py b/roles/openshift_health_checker/test/docker_image_availability_test.py
new file mode 100644
index 000000000..2a9c32f77
--- /dev/null
+++ b/roles/openshift_health_checker/test/docker_image_availability_test.py
@@ -0,0 +1,28 @@
+import pytest
+
+from openshift_checks.docker_image_availability import DockerImageAvailability
+
+
+@pytest.mark.xfail(strict=True) # TODO: remove this once this test is fully implemented.
+@pytest.mark.parametrize('task_vars,expected_result', [
+ (
+ dict(
+ openshift=dict(common=dict(
+ service_type='origin',
+ is_containerized=False,
+ )),
+ openshift_release='v3.5',
+ deployment_type='origin',
+ openshift_image_tag='', # FIXME: should not be required
+ ),
+ {'changed': False},
+ ),
+ # TODO: add more parameters here to test the multiple possible inputs that affect behavior.
+])
+def test_docker_image_availability(task_vars, expected_result):
+ def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None):
+ return {'info': {}} # TODO: this will vary depending on input parameters.
+
+ check = DockerImageAvailability(execute_module=execute_module)
+ result = check.run(tmp=None, task_vars=task_vars)
+ assert result == expected_result
--
cgit v1.2.3