From bb38413fcec7fb2640939782d57e494b40e3b41e Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 30 Jan 2017 18:29:06 +0100 Subject: Replace multi-role checks with action plugin This approach should make it easier to add new checks without having to write lots of YAML and doing things against Ansible (e.g. ignore_errors). A single action plugin determines what checks to run per each host, including arguments to the check. A check is implemented as a class with a run method, with the same signature as an action plugin and module, and is normally backed by a regular Ansible module. Each check is implemented as a separate Python file. This allows whoever adds a new check to focus solely in a single Python module, and potentially an Ansible module within library/ too. All checks are automatically loaded, and only active checks that are requested by the playbook get executed. --- .../openshift_checks/__init__.py | 57 ++++++++++++++++++ .../openshift_checks/mixins.py | 24 ++++++++ .../openshift_checks/package_availability.py | 69 ++++++++++++++++++++++ .../openshift_checks/package_update.py | 13 ++++ .../openshift_checks/package_version.py | 25 ++++++++ 5 files changed, 188 insertions(+) create mode 100644 roles/openshift_health_checker/openshift_checks/__init__.py create mode 100644 roles/openshift_health_checker/openshift_checks/mixins.py create mode 100644 roles/openshift_health_checker/openshift_checks/package_availability.py create mode 100644 roles/openshift_health_checker/openshift_checks/package_update.py create mode 100644 roles/openshift_health_checker/openshift_checks/package_version.py (limited to 'roles/openshift_health_checker/openshift_checks') diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py new file mode 100644 index 000000000..d893ba591 --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/__init__.py @@ -0,0 +1,57 @@ +""" +Health checks for OpenShift clusters. +""" + +import os +from abc import ABCMeta, abstractmethod, abstractproperty +from importlib import import_module + +import six + + +class OpenShiftCheckException(Exception): + """Raised when a check cannot proceed.""" + pass + + +@six.add_metaclass(ABCMeta) +class OpenShiftCheck(object): + """A base class for defining checks for an OpenShift cluster environment.""" + + def __init__(self, module_executor): + self.module_executor = module_executor + + @abstractproperty + def name(self): + """The name of this check, usually derived from the class name.""" + return "openshift_check" + + @classmethod + def is_active(cls, task_vars): # pylint: disable=unused-argument + """Returns true if this check applies to the ansible-playbook run.""" + return True + + @abstractmethod + def run(self, tmp, task_vars): + """Executes a check, normally implemented as a module.""" + return {} + + @classmethod + def subclasses(cls): + """Returns a generator of subclasses of this class and its subclasses.""" + for subclass in cls.__subclasses__(): # pylint: disable=no-member + yield subclass + for subclass in subclass.subclasses(): + yield subclass + + +# 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/openshift_checks/mixins.py b/roles/openshift_health_checker/openshift_checks/mixins.py new file mode 100644 index 000000000..4e0415944 --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/mixins.py @@ -0,0 +1,24 @@ +# pylint: disable=missing-docstring +from openshift_checks import OpenShiftCheckException + + +class NotContainerized(object): + """Mixin for checks that are only active when not in containerized mode.""" + + @classmethod + def is_active(cls, task_vars): + return ( + # This mixin is meant to be used with subclasses of + # OpenShiftCheck. Pylint disables this by default on mixins, + # though it relies on the class name ending in 'mixin'. + # pylint: disable=no-member + super(NotContainerized, cls).is_active(task_vars) and + not cls.is_containerized(task_vars) + ) + + @staticmethod + def is_containerized(task_vars): + try: + return task_vars["openshift"]["common"]["is_containerized"] + except (KeyError, TypeError): + raise OpenShiftCheckException("'openshift.common.is_containerized' is undefined") diff --git a/roles/openshift_health_checker/openshift_checks/package_availability.py b/roles/openshift_health_checker/openshift_checks/package_availability.py new file mode 100644 index 000000000..4260cbf7c --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/package_availability.py @@ -0,0 +1,69 @@ +# pylint: disable=missing-docstring +from openshift_checks import OpenShiftCheck, OpenShiftCheckException +from openshift_checks.mixins import NotContainerized + + +class PackageAvailability(NotContainerized, OpenShiftCheck): + """Check that required RPM packages are available.""" + + name = "package_availability" + + def run(self, tmp, task_vars): + try: + rpm_prefix = task_vars["openshift"]["common"]["service_type"] + except (KeyError, TypeError): + raise OpenShiftCheckException("'openshift.common.service_type' is undefined") + + group_names = task_vars.get("group_names", []) + + packages = set() + + if "masters" in group_names: + packages.update(self.master_packages(rpm_prefix)) + if "nodes" in group_names: + packages.update(self.node_packages(rpm_prefix)) + + args = {"packages": sorted(set(packages))} + return self.module_executor("check_yum_update", args, tmp, task_vars) + + @staticmethod + def master_packages(rpm_prefix): + return [ + "{rpm_prefix}".format(rpm_prefix=rpm_prefix), + "{rpm_prefix}-clients".format(rpm_prefix=rpm_prefix), + "{rpm_prefix}-master".format(rpm_prefix=rpm_prefix), + "bash-completion", + "cockpit-bridge", + "cockpit-docker", + "cockpit-kubernetes", + "cockpit-shell", + "cockpit-ws", + "etcd", + "httpd-tools", + ] + + @staticmethod + def node_packages(rpm_prefix): + return [ + "{rpm_prefix}".format(rpm_prefix=rpm_prefix), + "{rpm_prefix}-node".format(rpm_prefix=rpm_prefix), + "{rpm_prefix}-sdn-ovs".format(rpm_prefix=rpm_prefix), + "bind", + "ceph-common", + "dnsmasq", + "docker", + "firewalld", + "flannel", + "glusterfs-fuse", + "iptables-services", + "iptables", + "iscsi-initiator-utils", + "libselinux-python", + "nfs-utils", + "ntp", + "openssl", + "pyparted", + "python-httplib2", + "PyYAML", + "yum-utils", + ] diff --git a/roles/openshift_health_checker/openshift_checks/package_update.py b/roles/openshift_health_checker/openshift_checks/package_update.py new file mode 100644 index 000000000..316a776f5 --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/package_update.py @@ -0,0 +1,13 @@ +# pylint: disable=missing-docstring +from openshift_checks import OpenShiftCheck +from openshift_checks.mixins import NotContainerized + + +class PackageUpdate(NotContainerized, OpenShiftCheck): + """Check that there are no conflicts in RPM packages.""" + + name = "package_update" + + def run(self, tmp, task_vars): + args = {"packages": []} + return self.module_executor("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 new file mode 100644 index 000000000..a473119f3 --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/package_version.py @@ -0,0 +1,25 @@ +# pylint: disable=missing-docstring +from openshift_checks import OpenShiftCheck, OpenShiftCheckException +from openshift_checks.mixins import NotContainerized + + +class PackageVersion(NotContainerized, OpenShiftCheck): + """Check that available RPM packages match the required versions.""" + + name = "package_version" + + @classmethod + def is_active(cls, task_vars): + return ( + super(PackageVersion, cls).is_active(task_vars) + and task_vars.get("deployment_type") == "openshift-enterprise" + ) + + def run(self, tmp, task_vars): + try: + openshift_release = task_vars["openshift_release"] + except (KeyError, TypeError): + raise OpenShiftCheckException("'openshift_release' is undefined") + + args = {"version": openshift_release} + return self.module_executor("aos_version", args, tmp, task_vars) -- cgit v1.2.3 From c838e0f0b79b1471c47addf50c46fdb12281812c Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Tue, 31 Jan 2017 18:15:19 +0100 Subject: Introduce tag notation for checks This allows us to refer to a group of checks using a single handle. --- roles/openshift_health_checker/openshift_checks/__init__.py | 9 +++++++++ .../openshift_checks/package_availability.py | 1 + .../openshift_health_checker/openshift_checks/package_update.py | 1 + .../openshift_health_checker/openshift_checks/package_version.py | 1 + 4 files changed, 12 insertions(+) (limited to 'roles/openshift_health_checker/openshift_checks') diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py index d893ba591..ff99e7b4c 100644 --- a/roles/openshift_health_checker/openshift_checks/__init__.py +++ b/roles/openshift_health_checker/openshift_checks/__init__.py @@ -26,6 +26,15 @@ class OpenShiftCheck(object): """The name of this check, usually derived from the class name.""" return "openshift_check" + @property + def tags(self): + """A list of tags that this check satisfy. + + Tags are used to reference multiple checks with a single '@tagname' + special check name. + """ + return [] + @classmethod def is_active(cls, task_vars): # pylint: disable=unused-argument """Returns true if this check applies to the ansible-playbook run.""" diff --git a/roles/openshift_health_checker/openshift_checks/package_availability.py b/roles/openshift_health_checker/openshift_checks/package_availability.py index 4260cbf7c..31277a3b9 100644 --- a/roles/openshift_health_checker/openshift_checks/package_availability.py +++ b/roles/openshift_health_checker/openshift_checks/package_availability.py @@ -7,6 +7,7 @@ class PackageAvailability(NotContainerized, OpenShiftCheck): """Check that required RPM packages are available.""" name = "package_availability" + tags = ["preflight"] def run(self, tmp, task_vars): try: diff --git a/roles/openshift_health_checker/openshift_checks/package_update.py b/roles/openshift_health_checker/openshift_checks/package_update.py index 316a776f5..86b7b6245 100644 --- a/roles/openshift_health_checker/openshift_checks/package_update.py +++ b/roles/openshift_health_checker/openshift_checks/package_update.py @@ -7,6 +7,7 @@ class PackageUpdate(NotContainerized, OpenShiftCheck): """Check that there are no conflicts in RPM packages.""" name = "package_update" + tags = ["preflight"] def run(self, tmp, task_vars): args = {"packages": []} diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py index a473119f3..9394466f2 100644 --- a/roles/openshift_health_checker/openshift_checks/package_version.py +++ b/roles/openshift_health_checker/openshift_checks/package_version.py @@ -7,6 +7,7 @@ class PackageVersion(NotContainerized, OpenShiftCheck): """Check that available RPM packages match the required versions.""" name = "package_version" + tags = ["preflight"] @classmethod def is_active(cls, task_vars): -- cgit v1.2.3 From f502b09c103b5d8681854b7ab6a3c9655311f73b Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 6 Feb 2017 17:06:18 +0100 Subject: Refactor code to access values from task_vars --- .../openshift_checks/__init__.py | 18 ++++++++++++++++++ .../openshift_checks/mixins.py | 7 ++----- .../openshift_checks/package_availability.py | 10 +++------- .../openshift_checks/package_version.py | 7 ++----- 4 files changed, 25 insertions(+), 17 deletions(-) (limited to 'roles/openshift_health_checker/openshift_checks') diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py index ff99e7b4c..c31242624 100644 --- a/roles/openshift_health_checker/openshift_checks/__init__.py +++ b/roles/openshift_health_checker/openshift_checks/__init__.py @@ -5,8 +5,10 @@ Health checks for OpenShift clusters. import os from abc import ABCMeta, abstractmethod, abstractproperty from importlib import import_module +import operator import six +from six.moves import reduce class OpenShiftCheckException(Exception): @@ -54,6 +56,22 @@ class OpenShiftCheck(object): yield subclass +def get_var(task_vars, *keys, **kwargs): + """Helper function to get deeply nested values from task_vars. + + Ansible task_vars structures are Python dicts, often mapping strings to + other dicts. This helper makes it easier to get a nested value, raising + OpenShiftCheckException when a key is not found. + """ + try: + value = reduce(operator.getitem, keys, task_vars) + except (KeyError, TypeError): + if "default" in 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 = ( diff --git a/roles/openshift_health_checker/openshift_checks/mixins.py b/roles/openshift_health_checker/openshift_checks/mixins.py index 4e0415944..4029fba62 100644 --- a/roles/openshift_health_checker/openshift_checks/mixins.py +++ b/roles/openshift_health_checker/openshift_checks/mixins.py @@ -1,5 +1,5 @@ # pylint: disable=missing-docstring -from openshift_checks import OpenShiftCheckException +from openshift_checks import get_var class NotContainerized(object): @@ -18,7 +18,4 @@ class NotContainerized(object): @staticmethod def is_containerized(task_vars): - try: - return task_vars["openshift"]["common"]["is_containerized"] - except (KeyError, TypeError): - raise OpenShiftCheckException("'openshift.common.is_containerized' is undefined") + return get_var(task_vars, "openshift", "common", "is_containerized") diff --git a/roles/openshift_health_checker/openshift_checks/package_availability.py b/roles/openshift_health_checker/openshift_checks/package_availability.py index 31277a3b9..8faeef5ee 100644 --- a/roles/openshift_health_checker/openshift_checks/package_availability.py +++ b/roles/openshift_health_checker/openshift_checks/package_availability.py @@ -1,5 +1,5 @@ # pylint: disable=missing-docstring -from openshift_checks import OpenShiftCheck, OpenShiftCheckException +from openshift_checks import OpenShiftCheck, get_var from openshift_checks.mixins import NotContainerized @@ -10,12 +10,8 @@ class PackageAvailability(NotContainerized, OpenShiftCheck): tags = ["preflight"] def run(self, tmp, task_vars): - try: - rpm_prefix = task_vars["openshift"]["common"]["service_type"] - except (KeyError, TypeError): - raise OpenShiftCheckException("'openshift.common.service_type' is undefined") - - group_names = task_vars.get("group_names", []) + rpm_prefix = get_var(task_vars, "openshift", "common", "service_type") + group_names = get_var(task_vars, "group_names", default=[]) packages = set() diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py index 9394466f2..b31b4d401 100644 --- a/roles/openshift_health_checker/openshift_checks/package_version.py +++ b/roles/openshift_health_checker/openshift_checks/package_version.py @@ -1,5 +1,5 @@ # pylint: disable=missing-docstring -from openshift_checks import OpenShiftCheck, OpenShiftCheckException +from openshift_checks import OpenShiftCheck, get_var from openshift_checks.mixins import NotContainerized @@ -17,10 +17,7 @@ class PackageVersion(NotContainerized, OpenShiftCheck): ) def run(self, tmp, task_vars): - try: - openshift_release = task_vars["openshift_release"] - except (KeyError, TypeError): - raise OpenShiftCheckException("'openshift_release' is undefined") + openshift_release = get_var(task_vars, "openshift_release") args = {"version": openshift_release} return self.module_executor("aos_version", args, tmp, task_vars) -- cgit v1.2.3 From c24b9acb9d04f9613dcc7423791d09f83ef03670 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 6 Feb 2017 13:53:38 +0100 Subject: Do not hard code package names --- .../openshift_checks/package_version.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) (limited to 'roles/openshift_health_checker/openshift_checks') diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py index b31b4d401..7fa09cbfd 100644 --- a/roles/openshift_health_checker/openshift_checks/package_version.py +++ b/roles/openshift_health_checker/openshift_checks/package_version.py @@ -9,15 +9,12 @@ class PackageVersion(NotContainerized, OpenShiftCheck): name = "package_version" tags = ["preflight"] - @classmethod - def is_active(cls, task_vars): - return ( - super(PackageVersion, cls).is_active(task_vars) - and task_vars.get("deployment_type") == "openshift-enterprise" - ) - def run(self, tmp, task_vars): + rpm_prefix = get_var(task_vars, "openshift", "common", "service_type") openshift_release = get_var(task_vars, "openshift_release") - args = {"version": openshift_release} + args = { + "prefix": rpm_prefix, + "version": openshift_release, + } return self.module_executor("aos_version", args, tmp, task_vars) -- cgit v1.2.3