diff options
Diffstat (limited to 'roles/openshift_health_checker')
37 files changed, 1617 insertions, 908 deletions
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 23da53940..05e53333d 100644 --- a/roles/openshift_health_checker/action_plugins/openshift_health_check.py +++ b/roles/openshift_health_checker/action_plugins/openshift_health_check.py @@ -68,13 +68,15 @@ class ActionModule(ActionBase): msg=str(e), ) + if check.changed: + r["changed"] = True check_results[check_name] = r - if r.get("failed", False): - result["failed"] = True - result["msg"] = "One or more checks failed" + result["changed"] = any(r.get("changed") for r in check_results.values()) + if any(r.get("failed") for r in check_results.values()): + result["failed"] = True + result["msg"] = "One or more checks failed" - result["changed"] = any(r.get("changed", False) for r in check_results.values()) return result def load_known_checks(self, tmp, task_vars): diff --git a/roles/openshift_health_checker/library/aos_version.py b/roles/openshift_health_checker/library/aos_version.py index f9babebb9..c8769b511 100644 --- a/roles/openshift_health_checker/library/aos_version.py +++ b/roles/openshift_health_checker/library/aos_version.py @@ -24,11 +24,19 @@ from ansible.module_utils.basic import AnsibleModule # Python 3, we use six for cross compatibility in this module alone: from ansible.module_utils.six import string_types -IMPORT_EXCEPTION = None +YUM_IMPORT_EXCEPTION = None +DNF_IMPORT_EXCEPTION = None +PKG_MGR = None try: import yum # pylint: disable=import-error + PKG_MGR = "yum" except ImportError as err: - IMPORT_EXCEPTION = err + YUM_IMPORT_EXCEPTION = err +try: + import dnf # pylint: disable=import-error + PKG_MGR = "dnf" +except ImportError as err: + DNF_IMPORT_EXCEPTION = err class AosVersionException(Exception): @@ -47,8 +55,11 @@ def main(): supports_check_mode=True ) - if IMPORT_EXCEPTION: - module.fail_json(msg="aos_version module could not import yum: %s" % IMPORT_EXCEPTION) + if YUM_IMPORT_EXCEPTION and DNF_IMPORT_EXCEPTION: + module.fail_json( + msg="aos_version module could not import yum or dnf: %s %s" % + (YUM_IMPORT_EXCEPTION, DNF_IMPORT_EXCEPTION) + ) # determine the packages we will look for package_list = module.params['package_list'] @@ -83,9 +94,6 @@ def _to_dict(pkg_list): def _retrieve_available_packages(expected_pkgs): - # search for package versions available for openshift pkgs - yb = yum.YumBase() # pylint: disable=invalid-name - # The openshift excluder prevents unintended updates to openshift # packages by setting yum excludes on those packages. See: # https://wiki.centos.org/SpecialInterestGroup/PaaS/OpenShift-Origin-Control-Updates @@ -94,17 +102,41 @@ def _retrieve_available_packages(expected_pkgs): # attempt to determine what packages are available via yum they may # be excluded. So, for our purposes here, disable excludes to see # what will really be available during an install or upgrade. - yb.conf.disable_excludes = ['all'] - try: - pkgs = yb.pkgSack.returnPackages(patterns=expected_pkgs) - except yum.Errors.PackageSackError as excinfo: - # you only hit this if *none* of the packages are available - raise AosVersionException('\n'.join([ - 'Unable to find any OpenShift packages.', - 'Check your subscription and repo settings.', - str(excinfo), - ])) + if PKG_MGR == "yum": + # search for package versions available for openshift pkgs + yb = yum.YumBase() # pylint: disable=invalid-name + + yb.conf.disable_excludes = ['all'] + + try: + pkgs = yb.pkgSack.returnPackages(patterns=expected_pkgs) + except yum.Errors.PackageSackError as excinfo: + # you only hit this if *none* of the packages are available + raise AosVersionException('\n'.join([ + 'Unable to find any OpenShift packages.', + 'Check your subscription and repo settings.', + str(excinfo), + ])) + elif PKG_MGR == "dnf": + dbase = dnf.Base() # pyling: disable=invalid-name + + dbase.conf.disable_excludes = ['all'] + dbase.read_all_repos() + dbase.fill_sack(load_system_repo=False, load_available_repos=True) + + dquery = dbase.sack.query() + aquery = dquery.available() + + pkgs = list(aquery.filter(name=expected_pkgs)) + + if not pkgs: + # pkgs list is empty, raise because no expected packages found + raise AosVersionException('\n'.join([ + 'Unable to find any OpenShift packages.', + 'Check your subscription and repo settings.', + ])) + return pkgs diff --git a/roles/openshift_health_checker/library/rpm_version.py b/roles/openshift_health_checker/library/rpm_version.py index 8ea223055..c24fbba3b 100644 --- a/roles/openshift_health_checker/library/rpm_version.py +++ b/roles/openshift_health_checker/library/rpm_version.py @@ -4,6 +4,7 @@ Ansible module for rpm-based systems determining existing package version inform """ from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.six import string_types IMPORT_EXCEPTION = None try: @@ -82,11 +83,16 @@ def _check_pkg_versions(found_pkgs_dict, expected_pkgs_dict): continue found_versions = [_parse_version(version) for version in found_pkgs_dict[pkg_name]] - expected_version = _parse_version(pkg["version"]) - if expected_version not in found_versions: + + if isinstance(pkg["version"], string_types): + expected_versions = [_parse_version(pkg["version"])] + else: + expected_versions = [_parse_version(version) for version in pkg["version"]] + + if not set(expected_versions) & set(found_versions): invalid_pkg_versions[pkg_name] = { "found_versions": found_versions, - "required_version": expected_version, + "required_versions": expected_versions, } if not_found_pkgs: @@ -106,7 +112,7 @@ def _check_pkg_versions(found_pkgs_dict, expected_pkgs_dict): "The following packages were found to be installed with an incorrect version: {}".format('\n'.join([ " \n{}\n Required version: {}\n Found versions: {}".format( pkg_name, - pkg["required_version"], + ', '.join(pkg["required_versions"]), ', '.join([version for version in pkg["found_versions"]])) for pkg_name, pkg in invalid_pkg_versions.items() ])) diff --git a/roles/openshift_health_checker/meta/main.yml b/roles/openshift_health_checker/meta/main.yml index 4d141974c..bc8e7bdcf 100644 --- a/roles/openshift_health_checker/meta/main.yml +++ b/roles/openshift_health_checker/meta/main.yml @@ -1,5 +1,3 @@ --- dependencies: - - role: openshift_facts - - role: openshift_repos - - role: openshift_version +- role: openshift_facts diff --git a/roles/openshift_health_checker/openshift_checks/__init__.py b/roles/openshift_health_checker/openshift_checks/__init__.py index 40a28cde5..07ec6f7ef 100644 --- a/roles/openshift_health_checker/openshift_checks/__init__.py +++ b/roles/openshift_health_checker/openshift_checks/__init__.py @@ -10,11 +10,34 @@ from importlib import import_module from ansible.module_utils import six from ansible.module_utils.six.moves import reduce # pylint: disable=import-error,redefined-builtin +from ansible.plugins.filter.core import to_bool as ansible_to_bool 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) @@ -35,6 +58,9 @@ class OpenShiftCheck(object): self.task_vars = task_vars or {} self.tmp = tmp + # set to True when the check changes the host, for accurate total "changed" count + self.changed = False + @abstractproperty def name(self): """The name of this check, usually derived from the class name.""" @@ -94,16 +120,107 @@ class OpenShiftCheck(object): 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 or returning a default value - provided as a keyword argument. + OpenShiftCheckException when a key is not found. + + Keyword args: + default: + On missing key, return this as default value instead of raising exception. + convert: + Supply a function to apply to normalize the value before returning it. + None is the default (return as-is). + This function should raise ValueError if the user has provided a value + that cannot be converted, or OpenShiftCheckException if some other + problem needs to be described to the user. """ + if len(keys) == 1: + keys = keys[0].split(".") + try: value = reduce(operator.getitem, keys, self.task_vars) except (KeyError, TypeError): - if "default" in kwargs: - return kwargs["default"] - raise OpenShiftCheckException("'{}' is undefined".format(".".join(map(str, keys)))) - return value + if "default" not in kwargs: + raise OpenShiftCheckException( + "This check expects the '{}' inventory variable to be defined\n" + "in order to proceed, but it is undefined. There may be a bug\n" + "in Ansible, the checks, or their dependencies." + "".format(".".join(map(str, keys))) + ) + value = kwargs["default"] + + convert = kwargs.get("convert", None) + try: + if convert is None: + return value + elif convert is bool: # interpret bool as Ansible does, instead of python truthiness + return ansible_to_bool(value) + else: + return convert(value) + + except ValueError as error: # user error in specifying value + raise OpenShiftCheckException( + 'Cannot convert inventory variable to expected type:\n' + ' "{var}={value}"\n' + '{error}'.format(var=".".join(keys), value=value, error=error) + ) + + except OpenShiftCheckException: # some other check-specific problem + raise + + except Exception as error: # probably a bug in the function + raise OpenShiftCheckException( + 'There is a bug in this check. While trying to convert variable \n' + ' "{var}={value}"\n' + 'the given converter cannot be used or failed unexpectedly:\n' + '{error}'.format(var=".".join(keys), value=value, error=error) + ) + + @staticmethod + def get_major_minor_version(openshift_image_tag): + """Parse and return the deployed version of OpenShift as a tuple.""" + if openshift_image_tag and openshift_image_tag[0] == 'v': + openshift_image_tag = openshift_image_tag[1:] + + # map major release versions across releases + # to a common major version + openshift_major_release_version = { + "1": "3", + } + + components = openshift_image_tag.split(".") + if not components or len(components) < 2: + msg = "An invalid version of OpenShift was found for this host: {}" + raise OpenShiftCheckException(msg.format(openshift_image_tag)) + + if components[0] in openshift_major_release_version: + components[0] = openshift_major_release_version[components[0]] + + components = tuple(int(x) for x in components[:2]) + return components + + def find_ansible_mount(self, path): + """Return the mount point for path from ansible_mounts.""" + + # reorganize list of mounts into dict by path + mount_for_path = { + mount['mount']: mount + for mount + in self.get_var('ansible_mounts') + } + + # NOTE: including base cases '/' and '' to ensure the loop ends + mount_targets = set(mount_for_path.keys()) | {'/', ''} + mount_point = path + while mount_point not in mount_targets: + mount_point = os.path.dirname(mount_point) + + try: + return mount_for_path[mount_point] + except KeyError: + known_mounts = ', '.join('"{}"'.format(mount) for mount in sorted(mount_for_path)) + raise OpenShiftCheckException( + 'Unable to determine mount point for path "{}".\n' + 'Known mount points: {}.'.format(path, known_mounts or 'none') + ) LOADER_EXCLUDES = ( diff --git a/roles/openshift_health_checker/openshift_checks/disk_availability.py b/roles/openshift_health_checker/openshift_checks/disk_availability.py index 283461294..6d1dea9ce 100644 --- a/roles/openshift_health_checker/openshift_checks/disk_availability.py +++ b/roles/openshift_health_checker/openshift_checks/disk_availability.py @@ -1,6 +1,5 @@ """Check that there is enough disk space in predefined paths.""" -import os.path import tempfile from openshift_checks import OpenShiftCheck, OpenShiftCheckException @@ -35,6 +34,15 @@ class DiskAvailability(OpenShiftCheck): }, } + # recommended disk space for each location under an upgrade context + recommended_disk_upgrade_bytes = { + '/var': { + 'masters': 10 * 10**9, + 'nodes': 5 * 10 ** 9, + 'etcd': 5 * 10 ** 9, + }, + } + def is_active(self): """Skip hosts that do not have recommended disk space requirements.""" group_names = self.get_var("group_names", default=[]) @@ -46,9 +54,6 @@ class DiskAvailability(OpenShiftCheck): def run(self): group_names = self.get_var("group_names") - ansible_mounts = self.get_var("ansible_mounts") - ansible_mounts = {mount['mount']: mount for mount in ansible_mounts} - user_config = self.get_var("openshift_check_min_host_disk_gb", default={}) try: # For backwards-compatibility, if openshift_check_min_host_disk_gb @@ -71,7 +76,7 @@ class DiskAvailability(OpenShiftCheck): # not part of the official recommendation but present in the user # configuration. for path, recommendation in self.recommended_disk_space_bytes.items(): - free_bytes = self.free_bytes(path, ansible_mounts) + free_bytes = self.free_bytes(path) recommended_bytes = max(recommendation.get(name, 0) for name in group_names) config = user_config.get(path, {}) @@ -80,9 +85,34 @@ class DiskAvailability(OpenShiftCheck): config_bytes = max(config.get(name, 0) for name in group_names) * 10**9 recommended_bytes = config_bytes or recommended_bytes + # if an "upgrade" context is set, update the minimum disk requirement + # as this signifies an in-place upgrade - the node might have the + # required total disk space, but some of that space may already be + # in use by the existing OpenShift deployment. + context = self.get_var("r_openshift_health_checker_playbook_context", default="") + if context == "upgrade": + recommended_upgrade_paths = self.recommended_disk_upgrade_bytes.get(path, {}) + if recommended_upgrade_paths: + recommended_bytes = config_bytes or max(recommended_upgrade_paths.get(name, 0) + for name in group_names) + if free_bytes < recommended_bytes: free_gb = float(free_bytes) / 10**9 recommended_gb = float(recommended_bytes) / 10**9 + msg = ( + 'Available disk space in "{}" ({:.1f} GB) ' + 'is below minimum recommended ({:.1f} GB)' + ).format(path, free_gb, recommended_gb) + + # warn if check failed under an "upgrade" context + # due to limits imposed by the user config + if config_bytes and context == "upgrade": + msg += ('\n\nMake sure to account for decreased disk space during an upgrade\n' + 'due to an existing OpenShift deployment. Please check the value of\n' + ' openshift_check_min_host_disk_gb={}\n' + 'in your Ansible inventory, and lower the recommended disk space availability\n' + 'if necessary for this upgrade.').format(config_bytes) + return { 'failed': True, 'msg': ( @@ -93,22 +123,17 @@ class DiskAvailability(OpenShiftCheck): return {} - @staticmethod - def free_bytes(path, ansible_mounts): + def free_bytes(self, path): """Return the size available in path based on ansible_mounts.""" - mount_point = path - # arbitry value to prevent an infinite loop, in the unlike case that '/' - # is not in ansible_mounts. - max_depth = 32 - while mount_point not in ansible_mounts and max_depth > 0: - mount_point = os.path.dirname(mount_point) - max_depth -= 1 - + mount = self.find_ansible_mount(path) try: - free_bytes = ansible_mounts[mount_point]['size_available'] + return mount['size_available'] except KeyError: - known_mounts = ', '.join('"{}"'.format(mount) for mount in sorted(ansible_mounts)) or 'none' - msg = 'Unable to determine disk availability for "{}". Known mount points: {}.' - raise OpenShiftCheckException(msg.format(path, known_mounts)) - - return free_bytes + raise OpenShiftCheckException( + 'Unable to retrieve disk availability for "{path}".\n' + 'Ansible facts included a matching mount point for this path:\n' + ' {mount}\n' + 'however it is missing the size_available field.\n' + 'To investigate, you can inspect the output of `ansible -m setup <host>`' + ''.format(path=path, mount=mount) + ) 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 77180223e..85a922f86 100644 --- a/roles/openshift_health_checker/openshift_checks/docker_image_availability.py +++ b/roles/openshift_health_checker/openshift_checks/docker_image_availability.py @@ -41,11 +41,10 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): return super(DockerImageAvailability, self).is_active() and has_valid_deployment_type def run(self): - msg, failed, changed = self.ensure_dependencies() + msg, failed = self.ensure_dependencies() if failed: return { "failed": True, - "changed": changed, "msg": "Some dependencies are required in order to check Docker image availability.\n" + msg } @@ -54,11 +53,11 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): # exit early if all images were found locally if not missing_images: - return {"changed": changed} + return {} registries = self.known_docker_registries() if not registries: - return {"failed": True, "msg": "Unable to retrieve any docker registries.", "changed": changed} + return {"failed": True, "msg": "Unable to retrieve any docker registries."} available_images = self.available_images(missing_images, registries) unavailable_images = set(missing_images) - set(available_images) @@ -70,10 +69,9 @@ class DockerImageAvailability(DockerHostMixin, OpenShiftCheck): "One or more required Docker images are not available:\n {}\n" "Configured registries: {}" ).format(",\n ".join(sorted(unavailable_images)), ", ".join(registries)), - "changed": changed, } - return {"changed": changed} + return {} def required_images(self): """ diff --git a/roles/openshift_health_checker/openshift_checks/docker_storage.py b/roles/openshift_health_checker/openshift_checks/docker_storage.py index dea15a56e..0558ddf14 100644 --- a/roles/openshift_health_checker/openshift_checks/docker_storage.py +++ b/roles/openshift_health_checker/openshift_checks/docker_storage.py @@ -1,6 +1,5 @@ """Check Docker storage driver and usage.""" import json -import os.path import re from openshift_checks import OpenShiftCheck, OpenShiftCheckException from openshift_checks.mixins import DockerHostMixin @@ -43,21 +42,20 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck): ] def run(self): - msg, failed, changed = self.ensure_dependencies() + msg, failed = self.ensure_dependencies() if failed: return { "failed": True, - "changed": changed, "msg": "Some dependencies are required in order to query docker storage on host:\n" + msg } # attempt to get the docker info hash from the API docker_info = self.execute_module("docker_info", {}) if docker_info.get("failed"): - return {"failed": True, "changed": changed, + return {"failed": True, "msg": "Failed to query Docker API. Is docker running on this host?"} if not docker_info.get("info"): # this would be very strange - return {"failed": True, "changed": changed, + return {"failed": True, "msg": "Docker API query missing info:\n{}".format(json.dumps(docker_info))} docker_info = docker_info["info"] @@ -68,7 +66,7 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck): "Detected unsupported Docker storage driver '{driver}'.\n" "Supported storage drivers are: {drivers}" ).format(driver=driver, drivers=', '.join(self.storage_drivers)) - return {"failed": True, "changed": changed, "msg": msg} + return {"failed": True, "msg": msg} # driver status info is a list of tuples; convert to dict and validate based on driver driver_status = {item[0]: item[1] for item in docker_info.get("DriverStatus", [])} @@ -81,7 +79,6 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck): if driver in ['overlay', 'overlay2']: result = self.check_overlay_support(docker_info, driver_status) - result['changed'] = result.get('changed', False) or changed return result def check_devicemapper_support(self, driver_status): @@ -254,7 +251,7 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck): "msg": "Specified 'max_overlay_usage_percent' is not a percentage: {}".format(threshold), } - mount = self.find_ansible_mount(path, self.get_var("ansible_mounts")) + mount = self.find_ansible_mount(path) try: free_bytes = mount['size_available'] total_bytes = mount['size_total'] @@ -277,22 +274,3 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck): } return {} - - # TODO(lmeyer): migrate to base class - @staticmethod - def find_ansible_mount(path, ansible_mounts): - """Return the mount point for path from ansible_mounts.""" - - mount_for_path = {mount['mount']: mount for mount in ansible_mounts} - mount_point = path - while mount_point not in mount_for_path: - if mount_point in ["/", ""]: # "/" not in ansible_mounts??? - break - mount_point = os.path.dirname(mount_point) - - try: - return mount_for_path[mount_point] - except KeyError: - known_mounts = ', '.join('"{}"'.format(mount) for mount in sorted(mount_for_path)) or 'none' - msg = 'Unable to determine mount point for path "{}". Known mount points: {}.' - raise OpenShiftCheckException(msg.format(path, known_mounts)) diff --git a/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py b/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py index 28c38504d..f4296753a 100644 --- a/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py +++ b/roles/openshift_health_checker/openshift_checks/etcd_imagedata_size.py @@ -2,7 +2,7 @@ Ansible module for determining if the size of OpenShift image data exceeds a specified limit in an etcd cluster. """ -from openshift_checks import OpenShiftCheck, OpenShiftCheckException +from openshift_checks import OpenShiftCheck class EtcdImageDataSize(OpenShiftCheck): @@ -12,7 +12,7 @@ class EtcdImageDataSize(OpenShiftCheck): tags = ["etcd"] def run(self): - etcd_mountpath = self._get_etcd_mountpath(self.get_var("ansible_mounts")) + etcd_mountpath = self.find_ansible_mount("/var/lib/etcd") etcd_avail_diskspace = etcd_mountpath["size_available"] etcd_total_diskspace = etcd_mountpath["size_total"] @@ -56,7 +56,7 @@ class EtcdImageDataSize(OpenShiftCheck): reason = etcdkeysize["module_stderr"] msg = msg.format(host=etcd_host, reason=reason) - return {"failed": True, "changed": False, "msg": msg} + return {"failed": True, "msg": msg} if etcdkeysize["size_limit_exceeded"]: limit = self._to_gigabytes(etcd_imagedata_size_limit) @@ -65,20 +65,7 @@ class EtcdImageDataSize(OpenShiftCheck): "Use the `oadm prune images` command to cleanup unused Docker images.") return {"failed": True, "msg": msg.format(host=etcd_host, limit=limit)} - return {"changed": False} - - @staticmethod - def _get_etcd_mountpath(ansible_mounts): - valid_etcd_mount_paths = ["/var/lib/etcd", "/var/lib", "/var", "/"] - - mount_for_path = {mnt.get("mount"): mnt for mnt in ansible_mounts} - for path in valid_etcd_mount_paths: - if path in mount_for_path: - return mount_for_path[path] - - paths = ', '.join(sorted(mount_for_path)) or 'none' - msg = "Unable to determine a valid etcd mountpath. Paths mounted: {}.".format(paths) - raise OpenShiftCheckException(msg) + return {} @staticmethod def _to_gigabytes(byte_size): diff --git a/roles/openshift_health_checker/openshift_checks/etcd_traffic.py b/roles/openshift_health_checker/openshift_checks/etcd_traffic.py index cc1b14d8a..b4c8957e9 100644 --- a/roles/openshift_health_checker/openshift_checks/etcd_traffic.py +++ b/roles/openshift_health_checker/openshift_checks/etcd_traffic.py @@ -14,8 +14,8 @@ class EtcdTraffic(OpenShiftCheck): group_names = self.get_var("group_names", default=[]) valid_group_names = "etcd" in group_names - version = self.get_var("openshift", "common", "short_version") - valid_version = version in ("3.4", "3.5", "1.4", "1.5") + version = self.get_major_minor_version(self.get_var("openshift_image_tag")) + valid_version = version in ((3, 4), (3, 5)) return super(EtcdTraffic, self).is_active() and valid_group_names and valid_version diff --git a/roles/openshift_health_checker/openshift_checks/etcd_volume.py b/roles/openshift_health_checker/openshift_checks/etcd_volume.py index da7d0364a..e5d93ff3f 100644 --- a/roles/openshift_health_checker/openshift_checks/etcd_volume.py +++ b/roles/openshift_health_checker/openshift_checks/etcd_volume.py @@ -1,6 +1,6 @@ """A health check for OpenShift clusters.""" -from openshift_checks import OpenShiftCheck, OpenShiftCheckException +from openshift_checks import OpenShiftCheck class EtcdVolume(OpenShiftCheck): @@ -11,8 +11,8 @@ class EtcdVolume(OpenShiftCheck): # Default device usage threshold. Value should be in the range [0, 100]. default_threshold_percent = 90 - # Where to find ectd data, higher priority first. - supported_mount_paths = ["/var/lib/etcd", "/var/lib", "/var", "/"] + # Where to find etcd data + etcd_mount_path = "/var/lib/etcd" def is_active(self): etcd_hosts = self.get_var("groups", "etcd", default=[]) or self.get_var("groups", "masters", default=[]) or [] @@ -20,7 +20,7 @@ class EtcdVolume(OpenShiftCheck): return super(EtcdVolume, self).is_active() and is_etcd_host def run(self): - mount_info = self._etcd_mount_info() + mount_info = self.find_ansible_mount(self.etcd_mount_path) available = mount_info["size_available"] total = mount_info["size_total"] used = total - available @@ -40,16 +40,4 @@ class EtcdVolume(OpenShiftCheck): ) return {"failed": True, "msg": msg} - return {"changed": False} - - def _etcd_mount_info(self): - ansible_mounts = self.get_var("ansible_mounts") - mounts = {mnt.get("mount"): mnt for mnt in ansible_mounts} - - for path in self.supported_mount_paths: - if path in mounts: - return mounts[path] - - paths = ', '.join(sorted(mounts)) or 'none' - msg = "Unable to find etcd storage mount point. Paths mounted: {}.".format(paths) - raise OpenShiftCheckException(msg) + return {} diff --git a/roles/openshift_health_checker/openshift_checks/logging/curator.py b/roles/openshift_health_checker/openshift_checks/logging/curator.py index f82ae64d7..b27f97172 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/curator.py +++ b/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,35 @@ 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, "changed": False, "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, "changed": False, "msg": msg} + """Check various things and gather errors. Returns: result as hash""" + 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, "changed": 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 diff --git a/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py b/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py index 1e478c04d..7fc843fd7 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py +++ b/roles/openshift_health_checker/openshift_checks/logging/elasticsearch.py @@ -3,6 +3,7 @@ import json import re +from openshift_checks import OpenShiftCheckException, OpenShiftCheckExceptionList from openshift_checks.logging.logging import LoggingCheck @@ -12,174 +13,181 @@ class Elasticsearch(LoggingCheck): name = "elasticsearch" tags = ["health", "logging"] - logging_namespace = None - def run(self): """Check various things and gather errors. Returns: result as hash""" - self.logging_namespace = self.get_var("openshift_logging_namespace", default="logging") - es_pods, error = super(Elasticsearch, self).get_pods_for_component( - self.logging_namespace, - "es", - ) - if error: - return {"failed": True, "changed": False, "msg": error} - check_error = self.check_elasticsearch(es_pods) - - if check_error: - msg = ("The following Elasticsearch deployment issue was found:" - "\n-------\n" - "{}".format(check_error)) - return {"failed": True, "changed": False, "msg": msg} - + es_pods = self.get_pods_for_component("es") + self.check_elasticsearch(es_pods) # TODO(lmeyer): run it all again for the ops cluster - return {"failed": False, "changed": False, "msg": 'No problems found with Elasticsearch deployment.'} - def _not_running_elasticsearch_pods(self, es_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, [( - 'The following Elasticsearch pods are not running:\n' - '{pods}' - 'These pods will not aggregate logs from their nodes.' - ).format(pods=''.join( - " {} ({})\n".format(pod['metadata']['name'], pod['spec'].get('host', 'None')) - for pod in not_running - ))] - return not_running, [] + return {} def check_elasticsearch(self, es_pods): - """Various checks for elasticsearch. Returns: error string""" - not_running_pods, error_msgs = self._not_running_elasticsearch_pods(es_pods) - running_pods = [pod for pod in es_pods if pod not in not_running_pods] + """Perform checks for Elasticsearch. Raises OpenShiftCheckExceptionList on any errors.""" + running_pods, errors = self.running_elasticsearch_pods(es_pods) pods_by_name = { pod['metadata']['name']: pod for pod in running_pods # Filter out pods that are not members of a DC if pod['metadata'].get('labels', {}).get('deploymentconfig') } if not pods_by_name: - return 'No logging Elasticsearch pods were found. Is logging deployed?' - error_msgs += self._check_elasticsearch_masters(pods_by_name) - error_msgs += self._check_elasticsearch_node_list(pods_by_name) - error_msgs += self._check_es_cluster_health(pods_by_name) - error_msgs += self._check_elasticsearch_diskspace(pods_by_name) - return '\n'.join(error_msgs) + # nothing running, cannot run the rest of the check + errors.append(OpenShiftCheckException( + 'NoRunningPods', + 'No logging Elasticsearch pods were found running, so no logs are being aggregated.' + )) + raise OpenShiftCheckExceptionList(errors) + + errors += self.check_elasticsearch_masters(pods_by_name) + errors += self.check_elasticsearch_node_list(pods_by_name) + errors += self.check_es_cluster_health(pods_by_name) + errors += self.check_elasticsearch_diskspace(pods_by_name) + if errors: + raise OpenShiftCheckExceptionList(errors) + + def running_elasticsearch_pods(self, es_pods): + """Returns: list of running pods, list of errors about non-running pods""" + not_running = self.not_running_pods(es_pods) + running_pods = [pod for pod in es_pods if pod not in not_running] + if not_running: + return running_pods, [OpenShiftCheckException( + 'PodNotRunning', + 'The following Elasticsearch pods are defined but not running:\n' + '{pods}'.format(pods=''.join( + " {} ({})\n".format(pod['metadata']['name'], pod['spec'].get('host', 'None')) + for pod in not_running + )) + )] + return running_pods, [] @staticmethod def _build_es_curl_cmd(pod_name, url): base = "exec {name} -- curl -s --cert {base}cert --key {base}key --cacert {base}ca -XGET '{url}'" return base.format(base="/etc/elasticsearch/secret/admin-", name=pod_name, url=url) - def _check_elasticsearch_masters(self, pods_by_name): - """Check that Elasticsearch masters are sane. Returns: list of error strings""" + def check_elasticsearch_masters(self, pods_by_name): + """Check that Elasticsearch masters are sane. Returns: list of errors""" es_master_names = set() - error_msgs = [] + errors = [] for pod_name in pods_by_name.keys(): # Compare what each ES node reports as master and compare for split brain get_master_cmd = self._build_es_curl_cmd(pod_name, "https://localhost:9200/_cat/master") - master_name_str = self._exec_oc(get_master_cmd, []) + master_name_str = self.exec_oc(get_master_cmd, []) master_names = (master_name_str or '').split(' ') if len(master_names) > 1: es_master_names.add(master_names[1]) else: - error_msgs.append( - 'No master? Elasticsearch {pod} returned bad string when asked master name:\n' + errors.append(OpenShiftCheckException( + 'NoMasterName', + 'Elasticsearch {pod} gave unexpected response when asked master name:\n' ' {response}'.format(pod=pod_name, response=master_name_str) - ) + )) if not es_master_names: - error_msgs.append('No logging Elasticsearch masters were found. Is logging deployed?') - return '\n'.join(error_msgs) + errors.append(OpenShiftCheckException( + 'NoMasterFound', + 'No logging Elasticsearch masters were found.' + )) + return errors if len(es_master_names) > 1: - error_msgs.append( + errors.append(OpenShiftCheckException( + 'SplitBrainMasters', 'Found multiple Elasticsearch masters according to the pods:\n' '{master_list}\n' 'This implies that the masters have "split brain" and are not correctly\n' 'replicating data for the logging cluster. Log loss is likely to occur.' .format(master_list='\n'.join(' ' + master for master in es_master_names)) - ) + )) - return error_msgs + return errors - def _check_elasticsearch_node_list(self, pods_by_name): - """Check that reported ES masters are accounted for by pods. Returns: list of error strings""" + def check_elasticsearch_node_list(self, pods_by_name): + """Check that reported ES masters are accounted for by pods. Returns: list of errors""" if not pods_by_name: - return ['No logging Elasticsearch masters were found. Is logging deployed?'] + return [OpenShiftCheckException( + 'MissingComponentPods', + 'No logging Elasticsearch pods were found.' + )] # get ES cluster nodes node_cmd = self._build_es_curl_cmd(list(pods_by_name.keys())[0], 'https://localhost:9200/_nodes') - cluster_node_data = self._exec_oc(node_cmd, []) + cluster_node_data = self.exec_oc(node_cmd, []) try: cluster_nodes = json.loads(cluster_node_data)['nodes'] except (ValueError, KeyError): - return [ + return [OpenShiftCheckException( + 'MissingNodeList', 'Failed to query Elasticsearch for the list of ES nodes. The output was:\n' + cluster_node_data - ] + )] # Try to match all ES-reported node hosts to known pods. - error_msgs = [] + errors = [] for node in cluster_nodes.values(): # Note that with 1.4/3.4 the pod IP may be used as the master name if not any(node['host'] in (pod_name, pod['status'].get('podIP')) for pod_name, pod in pods_by_name.items()): - error_msgs.append( + errors.append(OpenShiftCheckException( + 'EsPodNodeMismatch', 'The Elasticsearch cluster reports a member node "{node}"\n' 'that does not correspond to any known ES pod.'.format(node=node['host']) - ) + )) - return error_msgs + return errors - def _check_es_cluster_health(self, pods_by_name): + def check_es_cluster_health(self, pods_by_name): """Exec into the elasticsearch pods and check the cluster health. Returns: list of errors""" - error_msgs = [] + errors = [] for pod_name in pods_by_name.keys(): cluster_health_cmd = self._build_es_curl_cmd(pod_name, 'https://localhost:9200/_cluster/health?pretty=true') - cluster_health_data = self._exec_oc(cluster_health_cmd, []) + cluster_health_data = self.exec_oc(cluster_health_cmd, []) try: health_res = json.loads(cluster_health_data) if not health_res or not health_res.get('status'): raise ValueError() except ValueError: - error_msgs.append( + errors.append(OpenShiftCheckException( + 'BadEsResponse', 'Could not retrieve cluster health status from logging ES pod "{pod}".\n' 'Response was:\n{output}'.format(pod=pod_name, output=cluster_health_data) - ) + )) continue if health_res['status'] not in ['green', 'yellow']: - error_msgs.append( + errors.append(OpenShiftCheckException( + 'EsClusterHealthRed', 'Elasticsearch cluster health status is RED according to pod "{}"'.format(pod_name) - ) + )) - return error_msgs + return errors - def _check_elasticsearch_diskspace(self, pods_by_name): + def check_elasticsearch_diskspace(self, pods_by_name): """ Exec into an ES pod and query the diskspace on the persistent volume. Returns: list of errors """ - error_msgs = [] + errors = [] for pod_name in pods_by_name.keys(): df_cmd = 'exec {} -- df --output=ipcent,pcent /elasticsearch/persistent'.format(pod_name) - disk_output = self._exec_oc(df_cmd, []) + disk_output = self.exec_oc(df_cmd, []) lines = disk_output.splitlines() # expecting one header looking like 'IUse% Use%' and one body line body_re = r'\s*(\d+)%?\s+(\d+)%?\s*$' if len(lines) != 2 or len(lines[0].split()) != 2 or not re.match(body_re, lines[1]): - error_msgs.append( + errors.append(OpenShiftCheckException( + 'BadDfResponse', 'Could not retrieve storage usage from logging ES pod "{pod}".\n' 'Response to `df` command was:\n{output}'.format(pod=pod_name, output=disk_output) - ) + )) continue inode_pct, disk_pct = re.match(body_re, lines[1]).groups() inode_pct_thresh = self.get_var('openshift_check_efk_es_inode_pct', default='90') if int(inode_pct) >= int(inode_pct_thresh): - error_msgs.append( + errors.append(OpenShiftCheckException( + 'InodeUsageTooHigh', 'Inode percent usage on the storage volume for logging ES pod "{pod}"\n' ' is {pct}, greater than threshold {limit}.\n' ' Note: threshold can be specified in inventory with {param}'.format( @@ -187,10 +195,11 @@ class Elasticsearch(LoggingCheck): pct=str(inode_pct), limit=str(inode_pct_thresh), param='openshift_check_efk_es_inode_pct', - )) + ))) disk_pct_thresh = self.get_var('openshift_check_efk_es_storage_pct', default='80') if int(disk_pct) >= int(disk_pct_thresh): - error_msgs.append( + errors.append(OpenShiftCheckException( + 'DiskUsageTooHigh', 'Disk percent usage on the storage volume for logging ES pod "{pod}"\n' ' is {pct}, greater than threshold {limit}.\n' ' Note: threshold can be specified in inventory with {param}'.format( @@ -198,13 +207,6 @@ class Elasticsearch(LoggingCheck): pct=str(disk_pct), limit=str(disk_pct_thresh), param='openshift_check_efk_es_storage_pct', - )) - - return error_msgs + ))) - def _exec_oc(self, cmd_str, extra_args): - return super(Elasticsearch, self).exec_oc( - self.logging_namespace, - cmd_str, - extra_args, - ) + return errors diff --git a/roles/openshift_health_checker/openshift_checks/logging/fluentd.py b/roles/openshift_health_checker/openshift_checks/logging/fluentd.py index 063e707a9..3b192a281 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/fluentd.py +++ b/roles/openshift_health_checker/openshift_checks/logging/fluentd.py @@ -2,6 +2,8 @@ import json + +from openshift_checks import OpenShiftCheckException, OpenShiftCheckExceptionList from openshift_checks.logging.logging import LoggingCheck @@ -11,61 +13,97 @@ class Fluentd(LoggingCheck): name = "fluentd" tags = ["health", "logging"] - logging_namespace = None - def run(self): - """Check various things and gather errors. Returns: result as hash""" + """Check the Fluentd deployment and raise an error if any problems are found.""" + + fluentd_pods = self.get_pods_for_component("fluentd") + self.check_fluentd(fluentd_pods) + return {} + + def check_fluentd(self, pods): + """Verify fluentd is running everywhere. Raises OpenShiftCheckExceptionList if error(s) found.""" - self.logging_namespace = self.get_var("openshift_logging_namespace", default="logging") - fluentd_pods, error = super(Fluentd, self).get_pods_for_component( - self.logging_namespace, - "fluentd", + node_selector = self.get_var( + 'openshift_logging_fluentd_nodeselector', + default='logging-infra-fluentd=true' ) - if error: - return {"failed": True, "changed": False, "msg": error} - check_error = self.check_fluentd(fluentd_pods) - if check_error: - msg = ("The following Fluentd deployment issue was found:" - "\n-------\n" - "{}".format(check_error)) - return {"failed": True, "changed": False, "msg": msg} + nodes_by_name = self.get_nodes_by_name() + fluentd_nodes = self.filter_fluentd_labeled_nodes(nodes_by_name, node_selector) + + errors = [] + errors += self.check_node_labeling(nodes_by_name, fluentd_nodes, node_selector) + errors += self.check_nodes_have_fluentd(pods, fluentd_nodes) + errors += self.check_fluentd_pods_running(pods) + + # Make sure there are no extra fluentd pods + if len(pods) > len(fluentd_nodes): + errors.append(OpenShiftCheckException( + 'TooManyFluentdPods', + 'There are more Fluentd pods running than nodes labeled.\n' + 'This may not cause problems with logging but it likely indicates something wrong.' + )) + + if errors: + raise OpenShiftCheckExceptionList(errors) - # TODO(lmeyer): run it all again for the ops cluster - return {"failed": False, "changed": False, "msg": 'No problems found with Fluentd deployment.'} + def get_nodes_by_name(self): + """Retrieve all the node definitions. Returns: dict(name: node)""" + nodes_json = self.exec_oc("get nodes -o json", []) + try: + nodes = json.loads(nodes_json) + except ValueError: # no valid json - should not happen + raise OpenShiftCheckException( + "BadOcNodeList", + "Could not obtain a list of nodes to validate fluentd.\n" + "Output from oc get:\n" + nodes_json + ) + if not nodes or not nodes.get('items'): # also should not happen + raise OpenShiftCheckException( + "NoNodesDefined", + "No nodes appear to be defined according to the API." + ) + return { + node['metadata']['name']: node + for node in nodes['items'] + } @staticmethod - def _filter_fluentd_labeled_nodes(nodes_by_name, node_selector): - """Filter to all nodes with fluentd label. Returns dict(name: node), error string""" + def filter_fluentd_labeled_nodes(nodes_by_name, node_selector): + """Filter to all nodes with fluentd label. Returns dict(name: node)""" label, value = node_selector.split('=', 1) fluentd_nodes = { name: node for name, node in nodes_by_name.items() if node['metadata']['labels'].get(label) == value } if not fluentd_nodes: - return None, ( + raise OpenShiftCheckException( + 'NoNodesLabeled', 'There are no nodes with the fluentd label {label}.\n' - 'This means no logs will be aggregated from the nodes.' - ).format(label=node_selector) - return fluentd_nodes, None + 'This means no logs will be aggregated from the nodes.'.format(label=node_selector) + ) + return fluentd_nodes - def _check_node_labeling(self, nodes_by_name, fluentd_nodes, node_selector): - """Note if nodes are not labeled as expected. Returns: error string""" + def check_node_labeling(self, nodes_by_name, fluentd_nodes, node_selector): + """Note if nodes are not labeled as expected. Returns: error list""" intended_nodes = self.get_var('openshift_logging_fluentd_hosts', default=['--all']) if not intended_nodes or '--all' in intended_nodes: intended_nodes = nodes_by_name.keys() nodes_missing_labels = set(intended_nodes) - set(fluentd_nodes.keys()) if nodes_missing_labels: - return ( + return [OpenShiftCheckException( + 'NodesUnlabeled', 'The following nodes are supposed to be labeled with {label} but are not:\n' ' {nodes}\n' - 'Fluentd will not aggregate logs from these nodes.' - ).format(label=node_selector, nodes=', '.join(nodes_missing_labels)) - return None + 'Fluentd will not aggregate logs from these nodes.'.format( + label=node_selector, nodes=', '.join(nodes_missing_labels) + ))] + + return [] @staticmethod - def _check_nodes_have_fluentd(pods, fluentd_nodes): - """Make sure fluentd is on all the labeled nodes. Returns: error string""" + def check_nodes_have_fluentd(pods, fluentd_nodes): + """Make sure fluentd is on all the labeled nodes. Returns: error list""" unmatched_nodes = fluentd_nodes.copy() node_names_by_label = { node['metadata']['labels']['kubernetes.io/hostname']: name @@ -85,83 +123,32 @@ class Fluentd(LoggingCheck): ]: unmatched_nodes.pop(name, None) if unmatched_nodes: - return ( + return [OpenShiftCheckException( + 'MissingFluentdPod', 'The following nodes are supposed to have a Fluentd pod but do not:\n' - '{nodes}' - 'These nodes will not have their logs aggregated.' - ).format(nodes=''.join( - " {}\n".format(name) - for name in unmatched_nodes.keys() - )) - return None + ' {nodes}\n' + 'These nodes will not have their logs aggregated.'.format( + nodes='\n '.join(unmatched_nodes.keys()) + ))] + + return [] - def _check_fluentd_pods_running(self, pods): + def check_fluentd_pods_running(self, pods): """Make sure all fluentd pods are running. Returns: error string""" not_running = super(Fluentd, self).not_running_pods(pods) if not_running: - return ( + return [OpenShiftCheckException( + 'FluentdNotRunning', 'The following Fluentd pods are supposed to be running but are not:\n' - '{pods}' - 'These pods will not aggregate logs from their nodes.' - ).format(pods=''.join( - " {} ({})\n".format(pod['metadata']['name'], pod['spec'].get('host', 'None')) - for pod in not_running - )) - return None - - def check_fluentd(self, pods): - """Verify fluentd is running everywhere. Returns: error string""" - - node_selector = self.get_var( - 'openshift_logging_fluentd_nodeselector', - default='logging-infra-fluentd=true' - ) - - nodes_by_name, error = self.get_nodes_by_name() - - if error: - return error - fluentd_nodes, error = self._filter_fluentd_labeled_nodes(nodes_by_name, node_selector) - if error: - return error - - error_msgs = [] - error = self._check_node_labeling(nodes_by_name, fluentd_nodes, node_selector) - if error: - error_msgs.append(error) - error = self._check_nodes_have_fluentd(pods, fluentd_nodes) - if error: - error_msgs.append(error) - error = self._check_fluentd_pods_running(pods) - if error: - error_msgs.append(error) - - # Make sure there are no extra fluentd pods - if len(pods) > len(fluentd_nodes): - error_msgs.append( - 'There are more Fluentd pods running than nodes labeled.\n' - 'This may not cause problems with logging but it likely indicates something wrong.' - ) - - return '\n'.join(error_msgs) - - def get_nodes_by_name(self): - """Retrieve all the node definitions. Returns: dict(name: node), error string""" - nodes_json = self._exec_oc("get nodes -o json", []) - try: - nodes = json.loads(nodes_json) - except ValueError: # no valid json - should not happen - return None, "Could not obtain a list of nodes to validate fluentd. Output from oc get:\n" + nodes_json - if not nodes or not nodes.get('items'): # also should not happen - return None, "No nodes appear to be defined according to the API." - return { - node['metadata']['name']: node - for node in nodes['items'] - }, None - - def _exec_oc(self, cmd_str, extra_args): - return super(Fluentd, self).exec_oc( - self.logging_namespace, - cmd_str, - extra_args, - ) + ' {pods}\n' + 'These pods will not aggregate logs from their nodes.'.format( + pods='\n'.join( + " {name} ({host})".format( + name=pod['metadata']['name'], + host=pod['spec'].get('host', 'None') + ) + for pod in not_running + ) + ))] + + return [] diff --git a/roles/openshift_health_checker/openshift_checks/logging/fluentd_config.py b/roles/openshift_health_checker/openshift_checks/logging/fluentd_config.py new file mode 100644 index 000000000..d783e6760 --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/logging/fluentd_config.py @@ -0,0 +1,131 @@ +""" +Module for performing checks on a Fluentd logging deployment configuration +""" + +from openshift_checks import OpenShiftCheckException +from openshift_checks.logging.logging import LoggingCheck + + +class FluentdConfig(LoggingCheck): + """Module that checks logging configuration of an integrated logging Fluentd deployment""" + name = "fluentd_config" + tags = ["health"] + + def is_active(self): + logging_deployed = self.get_var("openshift_hosted_logging_deploy", default=False) + + try: + version = self.get_major_minor_version(self.get_var("openshift_image_tag")) + except ValueError: + # if failed to parse OpenShift version, perform check anyway (if logging enabled) + return logging_deployed + + return logging_deployed and version < (3, 6) + + def run(self): + """Check that Fluentd has running pods, and that its logging config matches Docker's logging config.""" + config_error = self.check_logging_config() + if config_error: + msg = ("The following Fluentd logging configuration problem was found:" + "\n{}".format(config_error)) + return {"failed": True, "msg": msg} + + return {} + + def check_logging_config(self): + """Ensure that the configured Docker logging driver matches fluentd settings. + This means that, at least for now, if the following condition is met: + + openshift_logging_fluentd_use_journal == True + + then the value of the configured Docker logging driver should be "journald". + Otherwise, the value of the Docker logging driver should be "json-file". + Returns an error string if the above condition is not met, or None otherwise.""" + use_journald = self.get_var("openshift_logging_fluentd_use_journal", default=True) + + # if check is running on a master, retrieve all running pods + # and check any pod's container for the env var "USE_JOURNAL" + group_names = self.get_var("group_names") + if "masters" in group_names: + use_journald = self.check_fluentd_env_var() + + docker_info = self.execute_module("docker_info", {}) + try: + logging_driver = docker_info["info"]["LoggingDriver"] + except KeyError: + return "Unable to determine Docker logging driver." + + logging_driver = docker_info["info"]["LoggingDriver"] + recommended_logging_driver = "journald" + error = None + + # If fluentd is set to use journald but Docker is not, recommend setting the `--log-driver` + # option as an inventory file variable, or adding the log driver value as part of the + # Docker configuration in /etc/docker/daemon.json. There is no global --log-driver flag that + # can be passed to the Docker binary; the only other recommendation that can be made, would be + # to pass the `--log-driver` flag to the "run" sub-command of the `docker` binary when running + # individual containers. + if use_journald and logging_driver != "journald": + error = ('Your Fluentd configuration is set to aggregate Docker container logs from "journald".\n' + 'This differs from your Docker configuration, which has been set to use "{driver}" ' + 'as the default method of storing logs.\n' + 'This discrepancy in configuration will prevent Fluentd from receiving any logs' + 'from your Docker containers.').format(driver=logging_driver) + elif not use_journald and logging_driver != "json-file": + recommended_logging_driver = "json-file" + error = ('Your Fluentd configuration is set to aggregate Docker container logs from ' + 'individual json log files per container.\n ' + 'This differs from your Docker configuration, which has been set to use ' + '"{driver}" as the default method of storing logs.\n' + 'This discrepancy in configuration will prevent Fluentd from receiving any logs' + 'from your Docker containers.').format(driver=logging_driver) + + if error: + error += ('\nTo resolve this issue, add the following variable to your Ansible inventory file:\n\n' + ' openshift_docker_options="--log-driver={driver}"\n\n' + 'Alternatively, you can add the following option to your Docker configuration, located in' + '"/etc/docker/daemon.json":\n\n' + '{{ "log-driver": "{driver}" }}\n\n' + 'See https://docs.docker.com/engine/admin/logging/json-file ' + 'for more information.').format(driver=recommended_logging_driver) + + return error + + def check_fluentd_env_var(self): + """Read and return the value of the 'USE_JOURNAL' environment variable on a fluentd pod.""" + running_pods = self.running_fluentd_pods() + + try: + pod_containers = running_pods[0]["spec"]["containers"] + except KeyError: + return "Unable to detect running containers on selected Fluentd pod." + + if not pod_containers: + msg = ('There are no running containers on selected Fluentd pod "{}".\n' + 'Unable to calculate expected logging driver.').format(running_pods[0]["metadata"].get("name", "")) + raise OpenShiftCheckException(msg) + + pod_env = pod_containers[0].get("env") + if not pod_env: + msg = ('There are no environment variables set on the Fluentd container "{}".\n' + 'Unable to calculate expected logging driver.').format(pod_containers[0].get("name")) + raise OpenShiftCheckException(msg) + + for env in pod_env: + if env["name"] == "USE_JOURNAL": + return env.get("value", "false") != "false" + + return False + + def running_fluentd_pods(self): + """Return a list of running fluentd pods.""" + fluentd_pods = self.get_pods_for_component("fluentd") + + running_fluentd_pods = [pod for pod in fluentd_pods if pod['status']['phase'] == 'Running'] + if not running_fluentd_pods: + raise OpenShiftCheckException( + 'No Fluentd pods were found to be in the "Running" state. ' + 'At least one Fluentd pod is required in order to perform this check.' + ) + + return running_fluentd_pods diff --git a/roles/openshift_health_checker/openshift_checks/logging/kibana.py b/roles/openshift_health_checker/openshift_checks/logging/kibana.py index 60f94e106..3b1cf8baa 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/kibana.py +++ b/roles/openshift_health_checker/openshift_checks/logging/kibana.py @@ -12,7 +12,7 @@ except ImportError: from urllib.error import HTTPError, URLError import urllib.request as urllib2 -from openshift_checks.logging.logging import LoggingCheck +from openshift_checks.logging.logging import LoggingCheck, OpenShiftCheckException class Kibana(LoggingCheck): @@ -21,31 +21,15 @@ class Kibana(LoggingCheck): name = "kibana" tags = ["health", "logging"] - logging_namespace = None - def run(self): """Check various things and gather errors. Returns: result as hash""" - self.logging_namespace = self.get_var("openshift_logging_namespace", default="logging") - kibana_pods, error = super(Kibana, self).get_pods_for_component( - self.logging_namespace, - "kibana", - ) - if error: - return {"failed": True, "changed": False, "msg": error} - check_error = self.check_kibana(kibana_pods) - - if not check_error: - check_error = self._check_kibana_route() - - if check_error: - msg = ("The following Kibana deployment issue was found:" - "\n-------\n" - "{}".format(check_error)) - return {"failed": True, "changed": False, "msg": msg} - + kibana_pods = self.get_pods_for_component("kibana") + self.check_kibana(kibana_pods) + self.check_kibana_route() # TODO(lmeyer): run it all again for the ops cluster - return {"failed": False, "changed": False, "msg": 'No problems found with Kibana deployment.'} + + return {} def _verify_url_internal(self, url): """ @@ -68,7 +52,7 @@ class Kibana(LoggingCheck): def _verify_url_external(url): """ Try to reach a URL from ansible control host. - Returns: success (bool), reason (for failure) + Raise an OpenShiftCheckException if anything goes wrong. """ # This actually checks from the ansible control host, which may or may not # really be "external" to the cluster. @@ -94,133 +78,149 @@ class Kibana(LoggingCheck): return None def check_kibana(self, pods): - """Check to see if Kibana is up and working. Returns: error string.""" + """Check to see if Kibana is up and working. Raises OpenShiftCheckException if not.""" if not pods: - return "There are no Kibana pods deployed, so no access to the logging UI." + raise OpenShiftCheckException( + "MissingComponentPods", + "There are no Kibana pods deployed, so no access to the logging UI." + ) not_running = self.not_running_pods(pods) if len(not_running) == len(pods): - return "No Kibana pod is in a running state, so there is no access to the logging UI." + raise OpenShiftCheckException( + "NoRunningPods", + "No Kibana pod is in a running state, so there is no access to the logging UI." + ) elif not_running: - return ( + raise OpenShiftCheckException( + "PodNotRunning", "The following Kibana pods are not currently in a running state:\n" - "{pods}" - "However at least one is, so service may not be impacted." - ).format(pods="".join(" " + pod['metadata']['name'] + "\n" for pod in not_running)) - - return None + " {pods}\n" + "However at least one is, so service may not be impacted.".format( + pods="\n ".join(pod['metadata']['name'] for pod in not_running) + ) + ) def _get_kibana_url(self): """ Get kibana route or report error. - Returns: url (or empty), reason for failure + Returns: url """ # Get logging url - get_route = self._exec_oc("get route logging-kibana -o json", []) + get_route = self.exec_oc("get route logging-kibana -o json", []) if not get_route: - return None, 'no_route_exists' + raise OpenShiftCheckException( + 'no_route_exists', + 'No route is defined for Kibana in the logging namespace,\n' + 'so the logging stack is not accessible. Is logging deployed?\n' + 'Did something remove the logging-kibana route?' + ) - route = json.loads(get_route) + try: + route = json.loads(get_route) + # check that the route has been accepted by a router + ingress = route["status"]["ingress"] + except (ValueError, KeyError): + raise OpenShiftCheckException( + 'get_route_failed', + '"oc get route" returned an unexpected response:\n' + get_route + ) - # check that the route has been accepted by a router - ingress = route["status"]["ingress"] # ingress can be null if there is no router, or empty if not routed if not ingress or not ingress[0]: - return None, 'route_not_accepted' + raise OpenShiftCheckException( + 'route_not_accepted', + 'The logging-kibana route is not being routed by any router.\n' + 'Is the router deployed and working?' + ) host = route.get("spec", {}).get("host") if not host: - return None, 'route_missing_host' + raise OpenShiftCheckException( + 'route_missing_host', + 'The logging-kibana route has no hostname defined,\n' + 'which should never happen. Did something alter its definition?' + ) - return 'https://{}/'.format(host), None + return 'https://{}/'.format(host) - def _check_kibana_route(self): + def check_kibana_route(self): """ Check to see if kibana route is up and working. - Returns: error string + Raises exception if not. """ - known_errors = dict( - no_route_exists=( - 'No route is defined for Kibana in the logging namespace,\n' - 'so the logging stack is not accessible. Is logging deployed?\n' - 'Did something remove the logging-kibana route?' - ), - route_not_accepted=( - 'The logging-kibana route is not being routed by any router.\n' - 'Is the router deployed and working?' - ), - route_missing_host=( - 'The logging-kibana route has no hostname defined,\n' - 'which should never happen. Did something alter its definition?' - ), - ) - kibana_url, error = self._get_kibana_url() - if not kibana_url: - return known_errors.get(error, error) + kibana_url = self._get_kibana_url() # first, check that kibana is reachable from the master. error = self._verify_url_internal(kibana_url) if error: if 'urlopen error [Errno 111] Connection refused' in error: - error = ( + raise OpenShiftCheckException( + 'FailedToConnectInternal', 'Failed to connect from this master to Kibana URL {url}\n' - 'Is kibana running, and is at least one router routing to it?' - ).format(url=kibana_url) + 'Is kibana running, and is at least one router routing to it?'.format(url=kibana_url) + ) elif 'urlopen error [Errno -2] Name or service not known' in error: - error = ( + raise OpenShiftCheckException( + 'FailedToResolveInternal', 'Failed to connect from this master to Kibana URL {url}\n' 'because the hostname does not resolve.\n' - 'Is DNS configured for the Kibana hostname?' - ).format(url=kibana_url) + 'Is DNS configured for the Kibana hostname?'.format(url=kibana_url) + ) elif 'Status code was not' in error: - error = ( + raise OpenShiftCheckException( + 'WrongReturnCodeInternal', 'A request from this master to the Kibana URL {url}\n' 'did not return the correct status code (302).\n' 'This could mean that Kibana is malfunctioning, the hostname is\n' 'resolving incorrectly, or other network issues. The output was:\n' - ' {error}' - ).format(url=kibana_url, error=error) - return 'Error validating the logging Kibana route:\n' + error + ' {error}'.format(url=kibana_url, error=error) + ) + raise OpenShiftCheckException( + 'MiscRouteErrorInternal', + 'Error validating the logging Kibana route internally:\n' + error + ) # in production we would like the kibana route to work from outside the # cluster too; but that may not be the case, so allow disabling just this part. - if not self.get_var("openshift_check_efk_kibana_external", default=True): - return None + if self.get_var("openshift_check_efk_kibana_external", default="True").lower() != "true": + return error = self._verify_url_external(kibana_url) - if error: - if 'urlopen error [Errno 111] Connection refused' in error: - error = ( - 'Failed to connect from the Ansible control host to Kibana URL {url}\n' - 'Is the router for the Kibana hostname exposed externally?' - ).format(url=kibana_url) - elif 'urlopen error [Errno -2] Name or service not known' in error: - error = ( - 'Failed to resolve the Kibana hostname in {url}\n' - 'from the Ansible control host.\n' - 'Is DNS configured to resolve this Kibana hostname externally?' - ).format(url=kibana_url) - elif 'Expected success (200)' in error: - error = ( - 'A request to Kibana at {url}\n' - 'returned the wrong error code:\n' - ' {error}\n' - 'This could mean that Kibana is malfunctioning, the hostname is\n' - 'resolving incorrectly, or other network issues.' - ).format(url=kibana_url, error=error) - error = ( - 'Error validating the logging Kibana route:\n{error}\n' - 'To disable external Kibana route validation, set in your inventory:\n' - ' openshift_check_efk_kibana_external=False' - ).format(error=error) - return error - return None - def _exec_oc(self, cmd_str, extra_args): - return super(Kibana, self).exec_oc( - self.logging_namespace, - cmd_str, - extra_args, + if not error: + return + + error_fmt = ( + 'Error validating the logging Kibana route:\n{error}\n' + 'To disable external Kibana route validation, set the variable:\n' + ' openshift_check_efk_kibana_external=False' + ) + if 'urlopen error [Errno 111] Connection refused' in error: + msg = ( + 'Failed to connect from the Ansible control host to Kibana URL {url}\n' + 'Is the router for the Kibana hostname exposed externally?' + ).format(url=kibana_url) + raise OpenShiftCheckException('FailedToConnect', error_fmt.format(error=msg)) + elif 'urlopen error [Errno -2] Name or service not known' in error: + msg = ( + 'Failed to resolve the Kibana hostname in {url}\n' + 'from the Ansible control host.\n' + 'Is DNS configured to resolve this Kibana hostname externally?' + ).format(url=kibana_url) + raise OpenShiftCheckException('FailedToResolve', error_fmt.format(error=msg)) + elif 'Expected success (200)' in error: + msg = ( + 'A request to Kibana at {url}\n' + 'returned the wrong error code:\n' + ' {error}\n' + 'This could mean that Kibana is malfunctioning, the hostname is\n' + 'resolving incorrectly, or other network issues.' + ).format(url=kibana_url, error=error) + raise OpenShiftCheckException('WrongReturnCode', error_fmt.format(error=msg)) + raise OpenShiftCheckException( + 'MiscRouteError', + 'Error validating the logging Kibana route externally:\n' + error ) diff --git a/roles/openshift_health_checker/openshift_checks/logging/logging.py b/roles/openshift_health_checker/openshift_checks/logging/logging.py index 43ba6c406..ecd8adb64 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/logging.py +++ b/roles/openshift_health_checker/openshift_checks/logging/logging.py @@ -8,6 +8,16 @@ import os from openshift_checks import OpenShiftCheck, OpenShiftCheckException +class MissingComponentPods(OpenShiftCheckException): + """Raised when a component has no pods in the namespace.""" + pass + + +class CouldNotUseOc(OpenShiftCheckException): + """Raised when ocutil has a failure running oc.""" + pass + + class LoggingCheck(OpenShiftCheck): """Base class for OpenShift aggregated logging component checks""" @@ -15,10 +25,9 @@ class LoggingCheck(OpenShiftCheck): # run by itself. name = "logging" - logging_namespace = "logging" def is_active(self): - logging_deployed = self.get_var("openshift_hosted_logging_deploy", default=False) + logging_deployed = self.get_var("openshift_hosted_logging_deploy", convert=bool, default=False) return logging_deployed and super(LoggingCheck, self).is_active() and self.is_first_master() def is_first_master(self): @@ -32,22 +41,24 @@ class LoggingCheck(OpenShiftCheck): def run(self): return {} - def get_pods_for_component(self, namespace, logging_component): - """Get all pods for a given component. Returns: list of pods for component, error string""" + def get_pods_for_component(self, logging_component): + """Get all pods for a given component. Returns: list of pods.""" pod_output = self.exec_oc( - namespace, "get pods -l component={} -o json".format(logging_component), [], ) try: - pods = json.loads(pod_output) - if not pods or not pods.get('items'): + pods = json.loads(pod_output) # raises ValueError if deserialize fails + if not pods or not pods.get('items'): # also a broken response, treat the same raise ValueError() except ValueError: - # successful run but non-parsing data generally means there were no pods in the namespace - return None, 'No pods were found for the "{}" logging component.'.format(logging_component) + # successful run but non-parsing data generally means there were no pods to be found + raise MissingComponentPods( + 'There are no "{}" component pods in the "{}" namespace.\n' + 'Is logging deployed?'.format(logging_component, self.logging_namespace()) + ) - return pods['items'], None + return pods['items'] @staticmethod def not_running_pods(pods): @@ -63,15 +74,19 @@ class LoggingCheck(OpenShiftCheck): ) ] - def exec_oc(self, namespace="logging", cmd_str="", extra_args=None): + def logging_namespace(self): + """Returns the namespace in which logging is configured to deploy.""" + return self.get_var("openshift_logging_namespace", default="logging") + + def exec_oc(self, cmd_str="", extra_args=None): """ Execute an 'oc' command in the remote host. Returns: output of command and namespace, - or raises OpenShiftCheckException on error + or raises CouldNotUseOc on error """ config_base = self.get_var("openshift", "common", "config_base") args = { - "namespace": namespace, + "namespace": self.logging_namespace(), "config_file": os.path.join(config_base, "master", "admin.kubeconfig"), "cmd": cmd_str, "extra_args": list(extra_args) if extra_args else [], @@ -79,17 +94,16 @@ class LoggingCheck(OpenShiftCheck): result = self.execute_module("ocutil", args) if result.get("failed"): - msg = ( - 'Unexpected error using `oc` to validate the logging stack components.\n' - 'Error executing `oc {cmd}`:\n' - '{error}' - ).format(cmd=args['cmd'], error=result['result']) - if result['result'] == '[Errno 2] No such file or directory': - msg = ( + raise CouldNotUseOc( "This host is supposed to be a master but does not have the `oc` command where expected.\n" "Has an installation been run on this host yet?" ) - raise OpenShiftCheckException(msg) + + raise CouldNotUseOc( + 'Unexpected error using `oc` to validate the logging stack components.\n' + 'Error executing `oc {cmd}`:\n' + '{error}'.format(cmd=args['cmd'], error=result['result']) + ) return result.get("result", "") diff --git a/roles/openshift_health_checker/openshift_checks/logging/logging_index_time.py b/roles/openshift_health_checker/openshift_checks/logging/logging_index_time.py index b24e88e05..d781db649 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/logging_index_time.py +++ b/roles/openshift_health_checker/openshift_checks/logging/logging_index_time.py @@ -19,8 +19,6 @@ class LoggingIndexTime(LoggingCheck): name = "logging_index_time" tags = ["health", "logging"] - logging_namespace = "logging" - def run(self): """Add log entry by making unique request to Kibana. Check for unique entry in the ElasticSearch pod logs.""" try: @@ -28,29 +26,25 @@ class LoggingIndexTime(LoggingCheck): self.get_var("openshift_check_logging_index_timeout_seconds", default=ES_CMD_TIMEOUT_SECONDS) ) except ValueError: - return { - "failed": True, - "msg": ('Invalid value provided for "openshift_check_logging_index_timeout_seconds". ' - 'Value must be an integer representing an amount in seconds.'), - } + raise OpenShiftCheckException( + 'InvalidTimeout', + 'Invalid value provided for "openshift_check_logging_index_timeout_seconds". ' + 'Value must be an integer representing an amount in seconds.' + ) running_component_pods = dict() # get all component pods - self.logging_namespace = self.get_var("openshift_logging_namespace", default=self.logging_namespace) for component, name in (['kibana', 'Kibana'], ['es', 'Elasticsearch']): - pods, error = self.get_pods_for_component(self.logging_namespace, component) - - if error: - msg = 'Unable to retrieve pods for the {} logging component: {}' - return {"failed": True, "changed": False, "msg": msg.format(name, error)} - + pods = self.get_pods_for_component(component) running_pods = self.running_pods(pods) if not running_pods: - msg = ('No {} pods in the "Running" state were found.' - 'At least one pod is required in order to perform this check.') - return {"failed": True, "changed": False, "msg": msg.format(name)} + raise OpenShiftCheckException( + component + 'NoRunningPods', + 'No {} pods in the "Running" state were found.' + 'At least one pod is required in order to perform this check.'.format(name) + ) running_component_pods[component] = running_pods @@ -65,8 +59,11 @@ class LoggingIndexTime(LoggingCheck): interval = 1 while not self.query_es_from_es(es_pod, uuid): if time.time() + interval > deadline: - msg = "expecting match in Elasticsearch for message with uuid {}, but no matches were found after {}s." - raise OpenShiftCheckException(msg.format(uuid, timeout_secs)) + raise OpenShiftCheckException( + "NoMatchFound", + "expecting match in Elasticsearch for message with uuid {}, " + "but no matches were found after {}s.".format(uuid, timeout_secs) + ) time.sleep(interval) def curl_kibana_with_uuid(self, kibana_pod): @@ -76,22 +73,23 @@ class LoggingIndexTime(LoggingCheck): exec_cmd = "exec {pod_name} -c kibana -- curl --max-time 30 -s http://localhost:5601/{uuid}" exec_cmd = exec_cmd.format(pod_name=pod_name, uuid=uuid) - error_str = self.exec_oc(self.logging_namespace, exec_cmd, []) + error_str = self.exec_oc(exec_cmd, []) try: error_code = json.loads(error_str)["statusCode"] - except KeyError: - msg = ('invalid response returned from Kibana request (Missing "statusCode" key):\n' - 'Command: {}\nResponse: {}').format(exec_cmd, error_str) - raise OpenShiftCheckException(msg) - except ValueError: - msg = ('invalid response returned from Kibana request (Non-JSON output):\n' - 'Command: {}\nResponse: {}').format(exec_cmd, error_str) - raise OpenShiftCheckException(msg) + except (KeyError, ValueError): + raise OpenShiftCheckException( + 'kibanaInvalidResponse', + 'invalid response returned from Kibana request:\n' + 'Command: {}\nResponse: {}'.format(exec_cmd, error_str) + ) if error_code != 404: - msg = 'invalid error code returned from Kibana request. Expecting error code "404", but got "{}" instead.' - raise OpenShiftCheckException(msg.format(error_code)) + raise OpenShiftCheckException( + 'kibanaInvalidReturnCode', + 'invalid error code returned from Kibana request.\n' + 'Expecting error code "404", but got "{}" instead.'.format(error_code) + ) return uuid @@ -105,17 +103,18 @@ class LoggingIndexTime(LoggingCheck): "--key /etc/elasticsearch/secret/admin-key " "https://logging-es:9200/project.{namespace}*/_count?q=message:{uuid}" ) - exec_cmd = exec_cmd.format(pod_name=pod_name, namespace=self.logging_namespace, uuid=uuid) - result = self.exec_oc(self.logging_namespace, exec_cmd, []) + exec_cmd = exec_cmd.format(pod_name=pod_name, namespace=self.logging_namespace(), uuid=uuid) + result = self.exec_oc(exec_cmd, []) try: count = json.loads(result)["count"] - except KeyError: - msg = 'invalid response from Elasticsearch query:\n"{}"\nMissing "count" key:\n{}' - raise OpenShiftCheckException(msg.format(exec_cmd, result)) - except ValueError: - msg = 'invalid response from Elasticsearch query:\n"{}"\nNon-JSON output:\n{}' - raise OpenShiftCheckException(msg.format(exec_cmd, result)) + except (KeyError, ValueError): + raise OpenShiftCheckException( + 'esInvalidResponse', + 'Invalid response from Elasticsearch query:\n' + ' {}\n' + 'Response was:\n{}'.format(exec_cmd, result) + ) return count diff --git a/roles/openshift_health_checker/openshift_checks/mixins.py b/roles/openshift_health_checker/openshift_checks/mixins.py index 3b2c64e6a..e9bae60a3 100644 --- a/roles/openshift_health_checker/openshift_checks/mixins.py +++ b/roles/openshift_health_checker/openshift_checks/mixins.py @@ -29,10 +29,10 @@ class DockerHostMixin(object): """ 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, changed + Returns: msg, failed """ if self.get_var("openshift", "common", "is_atomic"): - return "", False, False + 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: @@ -49,5 +49,5 @@ class DockerHostMixin(object): " {deps}\n{msg}" ).format(deps=',\n '.join(self.dependencies), msg=msg) failed = result.get("failed", False) or result.get("rc", 0) != 0 - changed = result.get("changed", False) - return msg, failed, changed + self.changed = result.get("changed", False) + return msg, failed diff --git a/roles/openshift_health_checker/openshift_checks/ovs_version.py b/roles/openshift_health_checker/openshift_checks/ovs_version.py index cd6ebd493..363c12def 100644 --- a/roles/openshift_health_checker/openshift_checks/ovs_version.py +++ b/roles/openshift_health_checker/openshift_checks/ovs_version.py @@ -16,17 +16,11 @@ class OvsVersion(NotContainerizedMixin, OpenShiftCheck): tags = ["health"] openshift_to_ovs_version = { - "3.6": "2.6", - "3.5": "2.6", + "3.6": ["2.6", "2.7"], + "3.5": ["2.6", "2.7"], "3.4": "2.4", } - # map major release versions across releases - # to a common major version - openshift_major_release_version = { - "1": "3", - } - def is_active(self): """Skip hosts that do not have package requirements.""" group_names = self.get_var("group_names", default=[]) @@ -46,32 +40,15 @@ class OvsVersion(NotContainerizedMixin, OpenShiftCheck): def get_required_ovs_version(self): """Return the correct Open vSwitch version for the current OpenShift version""" - openshift_version = self._get_openshift_version() + openshift_version_tuple = self.get_major_minor_version(self.get_var("openshift_image_tag")) - if float(openshift_version) < 3.5: + if openshift_version_tuple < (3, 5): return self.openshift_to_ovs_version["3.4"] - ovs_version = self.openshift_to_ovs_version.get(str(openshift_version)) + openshift_version = ".".join(str(x) for x in openshift_version_tuple) + ovs_version = self.openshift_to_ovs_version.get(openshift_version) if ovs_version: - return self.openshift_to_ovs_version[str(openshift_version)] + return self.openshift_to_ovs_version[openshift_version] msg = "There is no recommended version of Open vSwitch for the current version of OpenShift: {}" raise OpenShiftCheckException(msg.format(openshift_version)) - - def _get_openshift_version(self): - openshift_version = self.get_var("openshift_image_tag") - if openshift_version and openshift_version[0] == 'v': - openshift_version = openshift_version[1:] - - return self._parse_version(openshift_version) - - def _parse_version(self, version): - components = version.split(".") - if not components or len(components) < 2: - msg = "An invalid version of OpenShift was found for this host: {}" - raise OpenShiftCheckException(msg.format(version)) - - if components[0] in self.openshift_major_release_version: - components[0] = self.openshift_major_release_version[components[0]] - - return '.'.join(components[:2]) diff --git a/roles/openshift_health_checker/openshift_checks/package_version.py b/roles/openshift_health_checker/openshift_checks/package_version.py index 020786804..8b780114f 100644 --- a/roles/openshift_health_checker/openshift_checks/package_version.py +++ b/roles/openshift_health_checker/openshift_checks/package_version.py @@ -1,4 +1,7 @@ """Check that available RPM packages match the required versions.""" + +import re + from openshift_checks import OpenShiftCheck, OpenShiftCheckException from openshift_checks.mixins import NotContainerizedMixin @@ -9,23 +12,25 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck): name = "package_version" tags = ["preflight"] + # NOTE: versions outside those specified are mapped to least/greatest openshift_to_ovs_version = { - "3.6": ["2.6", "2.7"], - "3.5": ["2.6", "2.7"], - "3.4": "2.4", + (3, 4): "2.4", + (3, 5): ["2.6", "2.7"], + (3, 6): ["2.6", "2.7"], } openshift_to_docker_version = { - "3.1": "1.8", - "3.2": "1.10", - "3.3": "1.10", - "3.4": "1.12", + (3, 1): "1.8", + (3, 2): "1.10", + (3, 3): "1.10", + (3, 4): "1.12", + (3, 5): "1.12", + (3, 6): "1.12", } - # map major release versions across releases - # to a common major version - openshift_major_release_version = { - "1": "3", + # map major OpenShift release versions across releases to a common major version + map_major_release_version = { + 1: 3, } def is_active(self): @@ -73,54 +78,49 @@ class PackageVersion(NotContainerizedMixin, OpenShiftCheck): return self.execute_module("aos_version", args) def get_required_ovs_version(self): - """Return the correct Open vSwitch version for the current OpenShift version. - If the current OpenShift version is >= 3.5, ensure Open vSwitch version 2.6, - Else ensure Open vSwitch version 2.4""" - openshift_version = self.get_openshift_version() + """Return the correct Open vSwitch version(s) for the current OpenShift version.""" + openshift_version = self.get_openshift_version_tuple() - if float(openshift_version) < 3.5: - return self.openshift_to_ovs_version["3.4"] + earliest = min(self.openshift_to_ovs_version) + latest = max(self.openshift_to_ovs_version) + if openshift_version < earliest: + return self.openshift_to_ovs_version[earliest] + if openshift_version > latest: + return self.openshift_to_ovs_version[latest] - ovs_version = self.openshift_to_ovs_version.get(str(openshift_version)) - if ovs_version: - return ovs_version + ovs_version = self.openshift_to_ovs_version.get(openshift_version) + if not ovs_version: + msg = "There is no recommended version of Open vSwitch for the current version of OpenShift: {}" + raise OpenShiftCheckException(msg.format(".".join(str(comp) for comp in openshift_version))) - msg = "There is no recommended version of Open vSwitch for the current version of OpenShift: {}" - raise OpenShiftCheckException(msg.format(openshift_version)) + return ovs_version def get_required_docker_version(self): - """Return the correct Docker version for the current OpenShift version. - If the OpenShift version is 3.1, ensure Docker version 1.8. - If the OpenShift version is 3.2 or 3.3, ensure Docker version 1.10. - If the current OpenShift version is >= 3.4, ensure Docker version 1.12.""" - openshift_version = self.get_openshift_version() - - if float(openshift_version) >= 3.4: - return self.openshift_to_docker_version["3.4"] - - docker_version = self.openshift_to_docker_version.get(str(openshift_version)) - if docker_version: - return docker_version - - msg = "There is no recommended version of Docker for the current version of OpenShift: {}" - raise OpenShiftCheckException(msg.format(openshift_version)) - - def get_openshift_version(self): - """Return received image tag as a normalized X.Y minor version string.""" - openshift_version = self.get_var("openshift_image_tag") - if openshift_version and openshift_version[0] == 'v': - openshift_version = openshift_version[1:] - - 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: + """Return the correct Docker version(s) for the current OpenShift version.""" + openshift_version = self.get_openshift_version_tuple() + + earliest = min(self.openshift_to_docker_version) + latest = max(self.openshift_to_docker_version) + if openshift_version < earliest: + return self.openshift_to_docker_version[earliest] + if openshift_version > latest: + return self.openshift_to_docker_version[latest] + + docker_version = self.openshift_to_docker_version.get(openshift_version) + if not docker_version: + msg = "There is no recommended version of Docker for the current version of OpenShift: {}" + raise OpenShiftCheckException(msg.format(".".join(str(comp) for comp in openshift_version))) + + return docker_version + + def get_openshift_version_tuple(self): + """Return received image tag as a normalized (X, Y) minor version tuple.""" + version = self.get_var("openshift_image_tag") + comps = [int(component) for component in re.findall(r'\d+', version)] + + if len(comps) < 2: msg = "An invalid version of OpenShift was found for this host: {}" raise OpenShiftCheckException(msg.format(version)) - if components[0] in self.openshift_major_release_version: - components[0] = self.openshift_major_release_version[components[0]] - - return '.'.join(components[:2]) + comps[0] = self.map_major_release_version.get(comps[0], comps[0]) + return tuple(comps[0:2]) diff --git a/roles/openshift_health_checker/test/action_plugin_test.py b/roles/openshift_health_checker/test/action_plugin_test.py index 2d068be3d..f5161d6f5 100644 --- a/roles/openshift_health_checker/test/action_plugin_test.py +++ b/roles/openshift_health_checker/test/action_plugin_test.py @@ -6,7 +6,7 @@ from openshift_health_check import ActionModule, resolve_checks from openshift_checks import OpenShiftCheckException -def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, run_exception=None): +def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, run_exception=None, changed=False): """Returns a new class that is compatible with OpenShiftCheck for testing.""" _name, _tags = name, tags @@ -14,6 +14,7 @@ def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, ru class FakeCheck(object): name = _name tags = _tags or [] + changed = False def __init__(self, execute_module=None, task_vars=None, tmp=None): pass @@ -22,6 +23,7 @@ def fake_check(name='fake_check', tags=None, is_active=True, run_return=None, ru return is_active def run(self): + self.changed = changed if run_exception is not None: raise run_exception return run_return @@ -135,14 +137,15 @@ def test_action_plugin_run_check_ok(plugin, task_vars, monkeypatch): def test_action_plugin_run_check_changed(plugin, task_vars, monkeypatch): - check_return_value = {'ok': 'test', 'changed': True} - check_class = fake_check(run_return=check_return_value) + check_return_value = {'ok': 'test'} + check_class = fake_check(run_return=check_return_value, changed=True) monkeypatch.setattr(plugin, 'load_known_checks', lambda tmp, task_vars: {'fake_check': check_class()}) monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check']) result = plugin.run(tmp=None, task_vars=task_vars) assert result['checks']['fake_check'] == check_return_value + assert changed(result['checks']['fake_check']) assert not failed(result) assert changed(result) assert not skipped(result) @@ -165,7 +168,7 @@ def test_action_plugin_run_check_fail(plugin, task_vars, monkeypatch): def test_action_plugin_run_check_exception(plugin, task_vars, monkeypatch): exception_msg = 'fake check has an exception' run_exception = OpenShiftCheckException(exception_msg) - check_class = fake_check(run_exception=run_exception) + check_class = fake_check(run_exception=run_exception, changed=True) monkeypatch.setattr(plugin, 'load_known_checks', lambda tmp, task_vars: {'fake_check': check_class()}) monkeypatch.setattr('openshift_health_check.resolve_checks', lambda *args: ['fake_check']) @@ -173,7 +176,8 @@ def test_action_plugin_run_check_exception(plugin, task_vars, monkeypatch): assert failed(result['checks']['fake_check'], msg_has=exception_msg) assert failed(result, msg_has=['failed']) - assert not changed(result) + assert changed(result['checks']['fake_check']) + assert changed(result) assert not skipped(result) diff --git a/roles/openshift_health_checker/test/curator_test.py b/roles/openshift_health_checker/test/curator_test.py index ae108c96e..62c680b74 100644 --- a/roles/openshift_health_checker/test/curator_test.py +++ b/roles/openshift_health_checker/test/curator_test.py @@ -1,22 +1,6 @@ import pytest -from openshift_checks.logging.curator import Curator - - -def canned_curator(exec_oc=None): - """Create a Curator check object with canned exec_oc method""" - check = Curator("dummy") # fails if a module is actually invoked - if exec_oc: - check._exec_oc = exec_oc - return check - - -def assert_error(error, expect_error): - if expect_error: - assert error - assert expect_error in error - else: - assert not error +from openshift_checks.logging.curator import Curator, OpenShiftCheckException plain_curator_pod = { @@ -44,25 +28,30 @@ not_running_curator_pod = { } +def test_get_curator_pods(): + check = Curator() + check.get_pods_for_component = lambda *_: [plain_curator_pod] + result = check.run() + assert "failed" not in result or not result["failed"] + + @pytest.mark.parametrize('pods, expect_error', [ ( [], - "no Curator pods", - ), - ( - [plain_curator_pod], - None, + 'MissingComponentPods', ), ( [not_running_curator_pod], - "not currently in a running state", + 'CuratorNotRunning', ), ( [plain_curator_pod, plain_curator_pod], - "more than one Curator pod", + 'TooManyCurators', ), ]) -def test_get_curator_pods(pods, expect_error): - check = canned_curator() - error = check.check_curator(pods) - assert_error(error, expect_error) +def test_get_curator_pods_fail(pods, expect_error): + check = Curator() + check.get_pods_for_component = lambda *_: pods + with pytest.raises(OpenShiftCheckException) as excinfo: + check.run() + assert excinfo.value.name == expect_error diff --git a/roles/openshift_health_checker/test/disk_availability_test.py b/roles/openshift_health_checker/test/disk_availability_test.py index e98d02c58..f4fd2dfed 100644 --- a/roles/openshift_health_checker/test/disk_availability_test.py +++ b/roles/openshift_health_checker/test/disk_availability_test.py @@ -20,12 +20,24 @@ def test_is_active(group_names, is_active): assert DiskAvailability(None, task_vars).is_active() == is_active -@pytest.mark.parametrize('ansible_mounts,extra_words', [ - ([], ['none']), # empty ansible_mounts - ([{'mount': '/mnt'}], ['/mnt']), # missing relevant mount paths - ([{'mount': '/var'}], ['/var']), # missing size_available +@pytest.mark.parametrize('desc, ansible_mounts, expect_chunks', [ + ( + 'empty ansible_mounts', + [], + ['determine mount point', 'none'], + ), + ( + 'missing relevant mount paths', + [{'mount': '/mnt'}], + ['determine mount point', '/mnt'], + ), + ( + 'missing size_available', + [{'mount': '/var'}, {'mount': '/usr'}, {'mount': '/tmp'}], + ['missing', 'size_available'], + ), ]) -def test_cannot_determine_available_disk(ansible_mounts, extra_words): +def test_cannot_determine_available_disk(desc, ansible_mounts, expect_chunks): task_vars = dict( group_names=['masters'], ansible_mounts=ansible_mounts, @@ -34,8 +46,8 @@ def test_cannot_determine_available_disk(ansible_mounts, extra_words): with pytest.raises(OpenShiftCheckException) as excinfo: DiskAvailability(fake_execute_module, task_vars).run() - for word in 'determine disk availability'.split() + extra_words: - assert word in str(excinfo.value) + for chunk in expect_chunks: + assert chunk in str(excinfo.value) @pytest.mark.parametrize('group_names,configured_min,ansible_mounts', [ @@ -97,8 +109,9 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib assert not result.get('failed', False) -@pytest.mark.parametrize('group_names,configured_min,ansible_mounts,extra_words', [ +@pytest.mark.parametrize('name,group_names,configured_min,ansible_mounts,expect_chunks', [ ( + 'test with no space available', ['masters'], 0, [{ @@ -108,6 +121,7 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib ['0.0 GB'], ), ( + 'test with a higher configured required value', ['masters'], 100, # set a higher threshold [{ @@ -117,6 +131,7 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib ['100.0 GB'], ), ( + 'test with 1GB available, but "0" GB space requirement', ['nodes'], 0, [{ @@ -126,6 +141,7 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib ['1.0 GB'], ), ( + 'test with no space available, but "0" GB space requirement', ['etcd'], 0, [{ @@ -135,16 +151,17 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib ['0.0 GB'], ), ( + 'test with enough space for a node, but not for a master', ['nodes', 'masters'], 0, [{ 'mount': '/', - # enough space for a node, not enough for a master 'size_available': 15 * 10**9 + 1, }], ['15.0 GB'], ), ( + 'test failure with enough space on "/", but not enough on "/var"', ['etcd'], 0, [{ @@ -158,8 +175,8 @@ def test_succeeds_with_recommended_disk_space(group_names, configured_min, ansib }], ['0.0 GB'], ), -]) -def test_fails_with_insufficient_disk_space(group_names, configured_min, ansible_mounts, extra_words): +], ids=lambda argval: argval[0]) +def test_fails_with_insufficient_disk_space(name, group_names, configured_min, ansible_mounts, expect_chunks): task_vars = dict( group_names=group_names, openshift_check_min_host_disk_gb=configured_min, @@ -169,8 +186,62 @@ def test_fails_with_insufficient_disk_space(group_names, configured_min, ansible result = DiskAvailability(fake_execute_module, task_vars).run() assert result['failed'] - for word in 'below recommended'.split() + extra_words: - assert word in result['msg'] + for chunk in 'below recommended'.split() + expect_chunks: + assert chunk in result.get('msg', '') + + +@pytest.mark.parametrize('name,group_names,context,ansible_mounts,failed,extra_words', [ + ( + 'test without enough space for master under "upgrade" context', + ['nodes', 'masters'], + "upgrade", + [{ + 'mount': '/', + 'size_available': 1 * 10**9 + 1, + 'size_total': 21 * 10**9 + 1, + }], + True, + ["1.0 GB"], + ), + ( + 'test with enough space for master under "upgrade" context', + ['nodes', 'masters'], + "upgrade", + [{ + 'mount': '/', + 'size_available': 10 * 10**9 + 1, + 'size_total': 21 * 10**9 + 1, + }], + False, + [], + ), + ( + 'test with not enough space for master, and non-upgrade context', + ['nodes', 'masters'], + "health", + [{ + 'mount': '/', + # not enough space for a master, + # "health" context should not lower requirement + 'size_available': 20 * 10**9 + 1, + }], + True, + ["20.0 GB", "below minimum"], + ), +], ids=lambda argval: argval[0]) +def test_min_required_space_changes_with_upgrade_context(name, group_names, context, ansible_mounts, failed, extra_words): + task_vars = dict( + r_openshift_health_checker_playbook_context=context, + group_names=group_names, + ansible_mounts=ansible_mounts, + ) + + check = DiskAvailability(fake_execute_module, task_vars) + result = check.run() + + assert result.get("failed", False) == failed + for word in extra_words: + assert word in result.get('msg', '') def fake_execute_module(*args): diff --git a/roles/openshift_health_checker/test/elasticsearch_test.py b/roles/openshift_health_checker/test/elasticsearch_test.py index 9edfc17c7..09bacd9ac 100644 --- a/roles/openshift_health_checker/test/elasticsearch_test.py +++ b/roles/openshift_health_checker/test/elasticsearch_test.py @@ -1,25 +1,26 @@ import pytest import json -from openshift_checks.logging.elasticsearch import Elasticsearch +from openshift_checks.logging.elasticsearch import Elasticsearch, OpenShiftCheckExceptionList + task_vars_config_base = dict(openshift=dict(common=dict(config_base='/etc/origin'))) def canned_elasticsearch(task_vars=None, exec_oc=None): - """Create an Elasticsearch check object with canned exec_oc method""" - check = Elasticsearch("dummy", task_vars or {}) # fails if a module is actually invoked + """Create an Elasticsearch check object with stubbed exec_oc method""" + check = Elasticsearch(None, task_vars or {}) if exec_oc: - check._exec_oc = exec_oc + check.exec_oc = exec_oc return check -def assert_error(error, expect_error): - if expect_error: - assert error - assert expect_error in error - else: - assert not error +def assert_error_in_list(expect_err, errorlist): + assert any(err.name == expect_err for err in errorlist), "{} in {}".format(str(expect_err), str(errorlist)) + + +def pods_by_name(pods): + return {pod['metadata']['name']: pod for pod in pods} plain_es_pod = { @@ -27,6 +28,7 @@ plain_es_pod = { "labels": {"component": "es", "deploymentconfig": "logging-es"}, "name": "logging-es", }, + "spec": {}, "status": { "conditions": [{"status": "True", "type": "Ready"}], "containerStatuses": [{"ready": True}], @@ -40,6 +42,7 @@ split_es_pod = { "labels": {"component": "es", "deploymentconfig": "logging-es-2"}, "name": "logging-es-2", }, + "spec": {}, "status": { "conditions": [{"status": "True", "type": "Ready"}], "containerStatuses": [{"ready": True}], @@ -48,12 +51,28 @@ split_es_pod = { "_test_master_name_str": "name logging-es-2", } +unready_es_pod = { + "metadata": { + "labels": {"component": "es", "deploymentconfig": "logging-es-3"}, + "name": "logging-es-3", + }, + "spec": {}, + "status": { + "conditions": [{"status": "False", "type": "Ready"}], + "containerStatuses": [{"ready": False}], + "podIP": "10.10.10.10", + }, + "_test_master_name_str": "BAD_NAME_RESPONSE", +} + def test_check_elasticsearch(): - assert 'No logging Elasticsearch pods' in canned_elasticsearch().check_elasticsearch([]) + with pytest.raises(OpenShiftCheckExceptionList) as excinfo: + canned_elasticsearch().check_elasticsearch([]) + assert_error_in_list('NoRunningPods', excinfo.value) # canned oc responses to match so all the checks pass - def _exec_oc(cmd, args): + def exec_oc(cmd, args): if '_cat/master' in cmd: return 'name logging-es' elif '/_nodes' in cmd: @@ -65,33 +84,41 @@ def test_check_elasticsearch(): else: raise Exception(cmd) - assert not canned_elasticsearch({}, _exec_oc).check_elasticsearch([plain_es_pod]) + check = canned_elasticsearch({}, exec_oc) + check.get_pods_for_component = lambda *_: [plain_es_pod] + assert {} == check.run() -def pods_by_name(pods): - return {pod['metadata']['name']: pod for pod in pods} +def test_check_running_es_pods(): + pods, errors = Elasticsearch().running_elasticsearch_pods([plain_es_pod, unready_es_pod]) + assert plain_es_pod in pods + assert_error_in_list('PodNotRunning', errors) + + +def test_check_elasticsearch_masters(): + pods = [plain_es_pod] + check = canned_elasticsearch(task_vars_config_base, lambda *_: plain_es_pod['_test_master_name_str']) + assert not check.check_elasticsearch_masters(pods_by_name(pods)) @pytest.mark.parametrize('pods, expect_error', [ ( [], - 'No logging Elasticsearch masters', + 'NoMasterFound', ), ( - [plain_es_pod], - None, + [unready_es_pod], + 'NoMasterName', ), ( [plain_es_pod, split_es_pod], - 'Found multiple Elasticsearch masters', + 'SplitBrainMasters', ), ]) -def test_check_elasticsearch_masters(pods, expect_error): +def test_check_elasticsearch_masters_error(pods, expect_error): test_pods = list(pods) - check = canned_elasticsearch(task_vars_config_base, lambda cmd, args: test_pods.pop(0)['_test_master_name_str']) - - errors = check._check_elasticsearch_masters(pods_by_name(pods)) - assert_error(''.join(errors), expect_error) + check = canned_elasticsearch(task_vars_config_base, lambda *_: test_pods.pop(0)['_test_master_name_str']) + assert_error_in_list(expect_error, check.check_elasticsearch_masters(pods_by_name(pods))) es_node_list = { @@ -101,80 +128,76 @@ es_node_list = { }}} +def test_check_elasticsearch_node_list(): + check = canned_elasticsearch(task_vars_config_base, lambda *_: json.dumps(es_node_list)) + assert not check.check_elasticsearch_node_list(pods_by_name([plain_es_pod])) + + @pytest.mark.parametrize('pods, node_list, expect_error', [ ( [], {}, - 'No logging Elasticsearch masters', - ), - ( - [plain_es_pod], - es_node_list, - None, + 'MissingComponentPods', ), ( [plain_es_pod], {}, # empty list of nodes triggers KeyError - "Failed to query", + 'MissingNodeList', ), ( [split_es_pod], es_node_list, - 'does not correspond to any known ES pod', + 'EsPodNodeMismatch', ), ]) -def test_check_elasticsearch_node_list(pods, node_list, expect_error): +def test_check_elasticsearch_node_list_errors(pods, node_list, expect_error): check = canned_elasticsearch(task_vars_config_base, lambda cmd, args: json.dumps(node_list)) + assert_error_in_list(expect_error, check.check_elasticsearch_node_list(pods_by_name(pods))) - errors = check._check_elasticsearch_node_list(pods_by_name(pods)) - assert_error(''.join(errors), expect_error) + +def test_check_elasticsearch_cluster_health(): + test_health_data = [{"status": "green"}] + check = canned_elasticsearch(exec_oc=lambda *_: json.dumps(test_health_data.pop(0))) + assert not check.check_es_cluster_health(pods_by_name([plain_es_pod])) @pytest.mark.parametrize('pods, health_data, expect_error', [ ( [plain_es_pod], - [{"status": "green"}], - None, - ), - ( - [plain_es_pod], [{"no-status": "should bomb"}], - 'Could not retrieve cluster health status', + 'BadEsResponse', ), ( [plain_es_pod, split_es_pod], [{"status": "green"}, {"status": "red"}], - 'Elasticsearch cluster health status is RED', + 'EsClusterHealthRed', ), ]) -def test_check_elasticsearch_cluster_health(pods, health_data, expect_error): +def test_check_elasticsearch_cluster_health_errors(pods, health_data, expect_error): test_health_data = list(health_data) - check = canned_elasticsearch(task_vars_config_base, lambda cmd, args: json.dumps(test_health_data.pop(0))) + check = canned_elasticsearch(exec_oc=lambda *_: json.dumps(test_health_data.pop(0))) + assert_error_in_list(expect_error, check.check_es_cluster_health(pods_by_name(pods))) - errors = check._check_es_cluster_health(pods_by_name(pods)) - assert_error(''.join(errors), expect_error) + +def test_check_elasticsearch_diskspace(): + check = canned_elasticsearch(exec_oc=lambda *_: 'IUse% Use%\n 3% 4%\n') + assert not check.check_elasticsearch_diskspace(pods_by_name([plain_es_pod])) @pytest.mark.parametrize('disk_data, expect_error', [ ( 'df: /elasticsearch/persistent: No such file or directory\n', - 'Could not retrieve storage usage', - ), - ( - 'IUse% Use%\n 3% 4%\n', - None, + 'BadDfResponse', ), ( 'IUse% Use%\n 95% 40%\n', - 'Inode percent usage on the storage volume', + 'InodeUsageTooHigh', ), ( 'IUse% Use%\n 3% 94%\n', - 'Disk percent usage on the storage volume', + 'DiskUsageTooHigh', ), ]) -def test_check_elasticsearch_diskspace(disk_data, expect_error): - check = canned_elasticsearch(task_vars_config_base, lambda cmd, args: disk_data) - - errors = check._check_elasticsearch_diskspace(pods_by_name([plain_es_pod])) - assert_error(''.join(errors), expect_error) +def test_check_elasticsearch_diskspace_errors(disk_data, expect_error): + check = canned_elasticsearch(exec_oc=lambda *_: disk_data) + assert_error_in_list(expect_error, check.check_elasticsearch_diskspace(pods_by_name([plain_es_pod]))) diff --git a/roles/openshift_health_checker/test/etcd_imagedata_size_test.py b/roles/openshift_health_checker/test/etcd_imagedata_size_test.py index e3d6706fa..d3aae98f2 100644 --- a/roles/openshift_health_checker/test/etcd_imagedata_size_test.py +++ b/roles/openshift_health_checker/test/etcd_imagedata_size_test.py @@ -1,7 +1,8 @@ import pytest from collections import namedtuple -from openshift_checks.etcd_imagedata_size import EtcdImageDataSize, OpenShiftCheckException +from openshift_checks.etcd_imagedata_size import EtcdImageDataSize +from openshift_checks import OpenShiftCheckException from etcdkeysize import check_etcd_key_size @@ -56,7 +57,7 @@ def test_cannot_determine_available_mountpath(ansible_mounts, extra_words): with pytest.raises(OpenShiftCheckException) as excinfo: check.run() - for word in 'determine valid etcd mountpath'.split() + extra_words: + for word in ['Unable to determine mount point'] + extra_words: assert word in str(excinfo.value) diff --git a/roles/openshift_health_checker/test/etcd_traffic_test.py b/roles/openshift_health_checker/test/etcd_traffic_test.py index f4316c423..fae3e578d 100644 --- a/roles/openshift_health_checker/test/etcd_traffic_test.py +++ b/roles/openshift_health_checker/test/etcd_traffic_test.py @@ -8,7 +8,7 @@ from openshift_checks.etcd_traffic import EtcdTraffic (['masters'], "3.6", False), (['nodes'], "3.4", False), (['etcd'], "3.4", True), - (['etcd'], "3.5", True), + (['etcd'], "1.5", True), (['etcd'], "3.1", False), (['masters', 'nodes'], "3.5", False), (['masters', 'etcd'], "3.5", True), @@ -17,9 +17,7 @@ from openshift_checks.etcd_traffic import EtcdTraffic def test_is_active(group_names, version, is_active): task_vars = dict( group_names=group_names, - openshift=dict( - common=dict(short_version=version), - ), + openshift_image_tag=version, ) assert EtcdTraffic(task_vars=task_vars).is_active() == is_active diff --git a/roles/openshift_health_checker/test/etcd_volume_test.py b/roles/openshift_health_checker/test/etcd_volume_test.py index 0b255136e..077cea3ea 100644 --- a/roles/openshift_health_checker/test/etcd_volume_test.py +++ b/roles/openshift_health_checker/test/etcd_volume_test.py @@ -1,6 +1,7 @@ import pytest -from openshift_checks.etcd_volume import EtcdVolume, OpenShiftCheckException +from openshift_checks.etcd_volume import EtcdVolume +from openshift_checks import OpenShiftCheckException @pytest.mark.parametrize('ansible_mounts,extra_words', [ @@ -15,7 +16,7 @@ def test_cannot_determine_available_disk(ansible_mounts, extra_words): with pytest.raises(OpenShiftCheckException) as excinfo: EtcdVolume(fake_execute_module, task_vars).run() - for word in 'Unable to find etcd storage mount point'.split() + extra_words: + for word in ['Unable to determine mount point'] + extra_words: assert word in str(excinfo.value) diff --git a/roles/openshift_health_checker/test/fluentd_config_test.py b/roles/openshift_health_checker/test/fluentd_config_test.py new file mode 100644 index 000000000..10db253bc --- /dev/null +++ b/roles/openshift_health_checker/test/fluentd_config_test.py @@ -0,0 +1,348 @@ +import pytest + +from openshift_checks.logging.fluentd_config import FluentdConfig, OpenShiftCheckException + + +def canned_fluentd_pod(containers): + return { + "metadata": { + "labels": {"component": "fluentd", "deploymentconfig": "logging-fluentd"}, + "name": "logging-fluentd-1", + }, + "spec": { + "host": "node1", + "nodeName": "node1", + "containers": containers, + }, + "status": { + "phase": "Running", + "containerStatuses": [{"ready": True}], + "conditions": [{"status": "True", "type": "Ready"}], + } + } + + +fluentd_pod = { + "metadata": { + "labels": {"component": "fluentd", "deploymentconfig": "logging-fluentd"}, + "name": "logging-fluentd-1", + }, + "spec": { + "host": "node1", + "nodeName": "node1", + "containers": [ + { + "name": "container1", + "env": [ + { + "name": "USE_JOURNAL", + "value": "true", + } + ], + } + ], + }, + "status": { + "phase": "Running", + "containerStatuses": [{"ready": True}], + "conditions": [{"status": "True", "type": "Ready"}], + } +} + +not_running_fluentd_pod = { + "metadata": { + "labels": {"component": "fluentd", "deploymentconfig": "logging-fluentd"}, + "name": "logging-fluentd-2", + }, + "status": { + "phase": "Unknown", + "containerStatuses": [{"ready": True}, {"ready": False}], + "conditions": [{"status": "True", "type": "Ready"}], + } +} + + +@pytest.mark.parametrize('name, use_journald, logging_driver, extra_words', [ + ( + 'test success with use_journald=false, and docker config set to use "json-file"', + False, + "json-file", + [], + ), +], ids=lambda argvals: argvals[0]) +def test_check_logging_config_non_master(name, use_journald, logging_driver, extra_words): + def execute_module(module_name, args): + if module_name == "docker_info": + return { + "info": { + "LoggingDriver": logging_driver, + } + } + + return {} + + task_vars = dict( + group_names=["nodes", "etcd"], + openshift_logging_fluentd_use_journal=use_journald, + openshift=dict( + common=dict(config_base=""), + ), + ) + + check = FluentdConfig(execute_module, task_vars) + check.execute_module = execute_module + error = check.check_logging_config() + + assert error is None + + +@pytest.mark.parametrize('name, use_journald, logging_driver, words', [ + ( + 'test failure with use_journald=false, but docker config set to use "journald"', + False, + "journald", + ['json log files', 'has been set to use "journald"'], + ), + ( + 'test failure with use_journald=false, but docker config set to use an "unsupported" driver', + False, + "unsupported", + ["json log files", 'has been set to use "unsupported"'], + ), + ( + 'test failure with use_journald=true, but docker config set to use "json-file"', + True, + "json-file", + ['logs from "journald"', 'has been set to use "json-file"'], + ), +], ids=lambda argvals: argvals[0]) +def test_check_logging_config_non_master_failed(name, use_journald, logging_driver, words): + def execute_module(module_name, args): + if module_name == "docker_info": + return { + "info": { + "LoggingDriver": logging_driver, + } + } + + return {} + + task_vars = dict( + group_names=["nodes", "etcd"], + openshift_logging_fluentd_use_journal=use_journald, + openshift=dict( + common=dict(config_base=""), + ), + ) + + check = FluentdConfig(execute_module, task_vars) + check.execute_module = execute_module + error = check.check_logging_config() + + assert error is not None + for word in words: + assert word in error + + +@pytest.mark.parametrize('name, pods, logging_driver, extra_words', [ + # use_journald returns false (not using journald), but check succeeds + # since docker is set to use json-file + ( + 'test success with use_journald=false, and docker config set to use default driver "json-file"', + [canned_fluentd_pod( + [ + { + "name": "container1", + "env": [{ + "name": "USE_JOURNAL", + "value": "false", + }], + }, + ] + )], + "json-file", + [], + ), + ( + 'test success with USE_JOURNAL env var missing and docker config set to use default driver "json-file"', + [canned_fluentd_pod( + [ + { + "name": "container1", + "env": [{ + "name": "RANDOM", + "value": "value", + }], + }, + ] + )], + "json-file", + [], + ), +], ids=lambda argvals: argvals[0]) +def test_check_logging_config_master(name, pods, logging_driver, extra_words): + def execute_module(module_name, args): + if module_name == "docker_info": + return { + "info": { + "LoggingDriver": logging_driver, + } + } + + return {} + + task_vars = dict( + group_names=["masters"], + openshift=dict( + common=dict(config_base=""), + ), + ) + + check = FluentdConfig(execute_module, task_vars) + check.execute_module = execute_module + check.get_pods_for_component = lambda _: pods + error = check.check_logging_config() + + assert error is None + + +@pytest.mark.parametrize('name, pods, logging_driver, words', [ + ( + 'test failure with use_journald=false, but docker config set to use "journald"', + [canned_fluentd_pod( + [ + { + "name": "container1", + "env": [{ + "name": "USE_JOURNAL", + "value": "false", + }], + }, + ] + )], + "journald", + ['json log files', 'has been set to use "journald"'], + ), + ( + 'test failure with use_journald=true, but docker config set to use "json-file"', + [fluentd_pod], + "json-file", + ['logs from "journald"', 'has been set to use "json-file"'], + ), + ( + 'test failure with use_journald=false, but docker set to use an "unsupported" driver', + [canned_fluentd_pod( + [ + { + "name": "container1", + "env": [{ + "name": "USE_JOURNAL", + "value": "false", + }], + }, + ] + )], + "unsupported", + ["json log files", 'has been set to use "unsupported"'], + ), + ( + 'test failure with USE_JOURNAL env var missing and docker config set to use "journald"', + [canned_fluentd_pod( + [ + { + "name": "container1", + "env": [{ + "name": "RANDOM", + "value": "value", + }], + }, + ] + )], + "journald", + ["configuration is set to", "json log files"], + ), +], ids=lambda argvals: argvals[0]) +def test_check_logging_config_master_failed(name, pods, logging_driver, words): + def execute_module(module_name, args): + if module_name == "docker_info": + return { + "info": { + "LoggingDriver": logging_driver, + } + } + + return {} + + task_vars = dict( + group_names=["masters"], + openshift=dict( + common=dict(config_base=""), + ), + ) + + check = FluentdConfig(execute_module, task_vars) + check.execute_module = execute_module + check.get_pods_for_component = lambda _: pods + error = check.check_logging_config() + + assert error is not None + for word in words: + assert word in error + + +@pytest.mark.parametrize('name, pods, response, logging_driver, extra_words', [ + ( + 'test OpenShiftCheckException with no running containers', + [canned_fluentd_pod([])], + { + "failed": True, + "result": "unexpected", + }, + "json-file", + ['no running containers'], + ), + ( + 'test OpenShiftCheckException one container and no env vars set', + [canned_fluentd_pod( + [ + { + "name": "container1", + "env": [], + }, + ] + )], + { + "failed": True, + "result": "unexpected", + }, + "json-file", + ['no environment variables'], + ), +], ids=lambda argvals: argvals[0]) +def test_check_logging_config_master_fails_on_unscheduled_deployment(name, pods, response, logging_driver, extra_words): + def execute_module(module_name, args): + if module_name == "docker_info": + return { + "info": { + "LoggingDriver": logging_driver, + } + } + + return {} + + task_vars = dict( + group_names=["masters"], + openshift=dict( + common=dict(config_base=""), + ), + ) + + check = FluentdConfig(execute_module, task_vars) + check.get_pods_for_component = lambda _: pods + + with pytest.raises(OpenShiftCheckException) as error: + check.check_logging_config() + + assert error is not None + for word in extra_words: + assert word in str(error) diff --git a/roles/openshift_health_checker/test/fluentd_test.py b/roles/openshift_health_checker/test/fluentd_test.py index 9cee57868..e7bf9818b 100644 --- a/roles/openshift_health_checker/test/fluentd_test.py +++ b/roles/openshift_health_checker/test/fluentd_test.py @@ -1,23 +1,11 @@ import pytest import json -from openshift_checks.logging.fluentd import Fluentd +from openshift_checks.logging.fluentd import Fluentd, OpenShiftCheckExceptionList, OpenShiftCheckException -def canned_fluentd(exec_oc=None): - """Create a Fluentd check object with canned exec_oc method""" - check = Fluentd("dummy") # fails if a module is actually invoked - if exec_oc: - check._exec_oc = exec_oc - return check - - -def assert_error(error, expect_error): - if expect_error: - assert error - assert expect_error in error - else: - assert not error +def assert_error_in_list(expect_err, errorlist): + assert any(err.name == expect_err for err in errorlist), "{} in {}".format(str(expect_err), str(errorlist)) fluentd_pod_node1 = { @@ -65,45 +53,60 @@ fluentd_node3_unlabeled = { } +def test_get_fluentd_pods(): + check = Fluentd() + check.exec_oc = lambda *_: json.dumps(dict(items=[fluentd_node1])) + check.get_pods_for_component = lambda *_: [fluentd_pod_node1] + assert not check.run() + + @pytest.mark.parametrize('pods, nodes, expect_error', [ ( [], [], - 'No nodes appear to be defined', + 'NoNodesDefined', ), ( [], [fluentd_node3_unlabeled], - 'There are no nodes with the fluentd label', + 'NoNodesLabeled', ), ( [], [fluentd_node1, fluentd_node3_unlabeled], - 'Fluentd will not aggregate logs from these nodes.', + 'NodesUnlabeled', ), ( [], [fluentd_node2], - "nodes are supposed to have a Fluentd pod but do not", + 'MissingFluentdPod', ), ( [fluentd_pod_node1, fluentd_pod_node1], [fluentd_node1], - 'more Fluentd pods running than nodes labeled', + 'TooManyFluentdPods', ), ( [fluentd_pod_node2_down], [fluentd_node2], - "Fluentd pods are supposed to be running", - ), - ( - [fluentd_pod_node1], - [fluentd_node1], - None, + 'FluentdNotRunning', ), ]) -def test_get_fluentd_pods(pods, nodes, expect_error): - check = canned_fluentd(exec_oc=lambda cmd, args: json.dumps(dict(items=nodes))) +def test_get_fluentd_pods_errors(pods, nodes, expect_error): + check = Fluentd() + check.exec_oc = lambda *_: json.dumps(dict(items=nodes)) + + with pytest.raises(OpenShiftCheckException) as excinfo: + check.check_fluentd(pods) + if isinstance(excinfo.value, OpenShiftCheckExceptionList): + assert_error_in_list(expect_error, excinfo.value) + else: + assert expect_error == excinfo.value.name + - error = check.check_fluentd(pods) - assert_error(error, expect_error) +def test_bad_oc_node_list(): + check = Fluentd() + check.exec_oc = lambda *_: "this isn't even json" + with pytest.raises(OpenShiftCheckException) as excinfo: + check.get_nodes_by_name() + assert 'BadOcNodeList' == excinfo.value.name diff --git a/roles/openshift_health_checker/test/kibana_test.py b/roles/openshift_health_checker/test/kibana_test.py index 3a880d300..04a5e89c4 100644 --- a/roles/openshift_health_checker/test/kibana_test.py +++ b/roles/openshift_health_checker/test/kibana_test.py @@ -8,23 +8,7 @@ except ImportError: from urllib.error import HTTPError, URLError import urllib.request as urllib2 -from openshift_checks.logging.kibana import Kibana - - -def canned_kibana(exec_oc=None): - """Create a Kibana check object with canned exec_oc method""" - check = Kibana() # fails if a module is actually invoked - if exec_oc: - check._exec_oc = exec_oc - return check - - -def assert_error(error, expect_error): - if expect_error: - assert error - assert expect_error in error - else: - assert not error +from openshift_checks.logging.kibana import Kibana, OpenShiftCheckException plain_kibana_pod = { @@ -49,39 +33,45 @@ not_running_kibana_pod = { } +def test_check_kibana(): + # should run without exception: + Kibana().check_kibana([plain_kibana_pod]) + + @pytest.mark.parametrize('pods, expect_error', [ ( [], - "There are no Kibana pods deployed", - ), - ( - [plain_kibana_pod], - None, + "MissingComponentPods", ), ( [not_running_kibana_pod], - "No Kibana pod is in a running state", + "NoRunningPods", ), ( [plain_kibana_pod, not_running_kibana_pod], - "The following Kibana pods are not currently in a running state", + "PodNotRunning", ), ]) -def test_check_kibana(pods, expect_error): - check = canned_kibana() - error = check.check_kibana(pods) - assert_error(error, expect_error) +def test_check_kibana_error(pods, expect_error): + with pytest.raises(OpenShiftCheckException) as excinfo: + Kibana().check_kibana(pods) + assert expect_error == excinfo.value.name -@pytest.mark.parametrize('route, expect_url, expect_error', [ +@pytest.mark.parametrize('comment, route, expect_error', [ ( + "No route returned", None, - None, - 'no_route_exists', + "no_route_exists", ), - # test route with no ingress ( + "broken route response", + {"status": {}}, + "get_route_failed", + ), + ( + "route with no ingress", { "metadata": { "labels": {"component": "kibana", "deploymentconfig": "logging-kibana"}, @@ -94,12 +84,11 @@ def test_check_kibana(pods, expect_error): "host": "hostname", } }, - None, - 'route_not_accepted', + "route_not_accepted", ), - # test route with no host ( + "route with no host", { "metadata": { "labels": {"component": "kibana", "deploymentconfig": "logging-kibana"}, @@ -112,12 +101,21 @@ def test_check_kibana(pods, expect_error): }, "spec": {}, }, - None, - 'route_missing_host', + "route_missing_host", ), +]) +def test_get_kibana_url_error(comment, route, expect_error): + check = Kibana() + check.exec_oc = lambda *_: json.dumps(route) if route else "" - # test route that looks fine + with pytest.raises(OpenShiftCheckException) as excinfo: + check._get_kibana_url() + assert excinfo.value.name == expect_error + + +@pytest.mark.parametrize('comment, route, expect_url', [ ( + "test route that looks fine", { "metadata": { "labels": {"component": "kibana", "deploymentconfig": "logging-kibana"}, @@ -133,61 +131,57 @@ def test_check_kibana(pods, expect_error): }, }, "https://hostname/", - None, ), ]) -def test_get_kibana_url(route, expect_url, expect_error): - check = canned_kibana(exec_oc=lambda cmd, args: json.dumps(route) if route else "") - - url, error = check._get_kibana_url() - if expect_url: - assert url == expect_url - else: - assert not url - if expect_error: - assert error == expect_error - else: - assert not error +def test_get_kibana_url(comment, route, expect_url): + check = Kibana() + check.exec_oc = lambda *_: json.dumps(route) + assert expect_url == check._get_kibana_url() @pytest.mark.parametrize('exec_result, expect', [ ( 'urlopen error [Errno 111] Connection refused', - 'at least one router routing to it?', + 'FailedToConnectInternal', ), ( 'urlopen error [Errno -2] Name or service not known', - 'DNS configured for the Kibana hostname?', + 'FailedToResolveInternal', ), ( 'Status code was not [302]: HTTP Error 500: Server error', - 'did not return the correct status code', + 'WrongReturnCodeInternal', ), ( 'bork bork bork', - 'bork bork bork', # should pass through + 'MiscRouteErrorInternal', ), ]) def test_verify_url_internal_failure(exec_result, expect): check = Kibana(execute_module=lambda *_: dict(failed=True, msg=exec_result)) - check._get_kibana_url = lambda: ('url', None) + check._get_kibana_url = lambda: 'url' - error = check._check_kibana_route() - assert_error(error, expect) + with pytest.raises(OpenShiftCheckException) as excinfo: + check.check_kibana_route() + assert expect == excinfo.value.name @pytest.mark.parametrize('lib_result, expect', [ ( - HTTPError('url', 500, "it broke", hdrs=None, fp=None), - 'it broke', + HTTPError('url', 500, 'it broke', hdrs=None, fp=None), + 'MiscRouteError', ), ( - URLError('it broke'), - 'it broke', + URLError('urlopen error [Errno 111] Connection refused'), + 'FailedToConnect', + ), + ( + URLError('urlopen error [Errno -2] Name or service not known'), + 'FailedToResolve', ), ( 302, - 'returned the wrong error code', + 'WrongReturnCode', ), ( 200, @@ -210,9 +204,41 @@ def test_verify_url_external_failure(lib_result, expect, monkeypatch): raise lib_result monkeypatch.setattr(urllib2, 'urlopen', urlopen) - check = canned_kibana() - check._get_kibana_url = lambda: ('url', None) + check = Kibana() + check._get_kibana_url = lambda: 'url' check._verify_url_internal = lambda url: None - error = check._check_kibana_route() - assert_error(error, expect) + if not expect: + check.check_kibana_route() + return + + with pytest.raises(OpenShiftCheckException) as excinfo: + check.check_kibana_route() + assert expect == excinfo.value.name + + +def test_verify_url_external_skip(): + check = Kibana(lambda *_: {}, dict(openshift_check_efk_kibana_external="false")) + check._get_kibana_url = lambda: 'url' + check.check_kibana_route() + + +# this is kind of silly but it adds coverage for the run() method... +def test_run(): + pods = ["foo"] + ran = dict(check_kibana=False, check_route=False) + + def check_kibana(pod_list): + ran["check_kibana"] = True + assert pod_list == pods + + def check_kibana_route(): + ran["check_route"] = True + + check = Kibana() + check.get_pods_for_component = lambda *_: pods + check.check_kibana = check_kibana + check.check_kibana_route = check_kibana_route + + check.run() + assert ran["check_kibana"] and ran["check_route"] diff --git a/roles/openshift_health_checker/test/logging_check_test.py b/roles/openshift_health_checker/test/logging_check_test.py index 6f1697ee6..1a1c190f6 100644 --- a/roles/openshift_health_checker/test/logging_check_test.py +++ b/roles/openshift_health_checker/test/logging_check_test.py @@ -1,18 +1,14 @@ import pytest import json -from openshift_checks.logging.logging import LoggingCheck, OpenShiftCheckException +from openshift_checks.logging.logging import LoggingCheck, MissingComponentPods, CouldNotUseOc task_vars_config_base = dict(openshift=dict(common=dict(config_base='/etc/origin'))) -logging_namespace = "logging" - - -def canned_loggingcheck(exec_oc=None): +def canned_loggingcheck(exec_oc=None, execute_module=None): """Create a LoggingCheck object with canned exec_oc method""" - check = LoggingCheck() # fails if a module is actually invoked - check.logging_namespace = 'logging' + check = LoggingCheck(execute_module) if exec_oc: check.exec_oc = exec_oc return check @@ -97,8 +93,8 @@ def test_oc_failure(problem, expect): check = LoggingCheck(execute_module, task_vars_config_base) - with pytest.raises(OpenShiftCheckException) as excinfo: - check.exec_oc(logging_namespace, 'get foo', []) + with pytest.raises(CouldNotUseOc) as excinfo: + check.exec_oc('get foo', []) assert expect in str(excinfo) @@ -124,25 +120,32 @@ def test_is_active(groups, logging_deployed, is_active): assert LoggingCheck(None, task_vars).is_active() == is_active -@pytest.mark.parametrize('pod_output, expect_pods, expect_error', [ +@pytest.mark.parametrize('pod_output, expect_pods', [ + ( + json.dumps({'items': [plain_es_pod]}), + [plain_es_pod], + ), +]) +def test_get_pods_for_component(pod_output, expect_pods): + check = canned_loggingcheck(lambda *_: pod_output) + pods = check.get_pods_for_component("es") + assert pods == expect_pods + + +@pytest.mark.parametrize('exec_oc_output, expect_error', [ ( 'No resources found.', - None, - 'No pods were found for the "es"', + MissingComponentPods, ), ( - json.dumps({'items': [plain_kibana_pod, plain_es_pod, plain_curator_pod, fluentd_pod_node1]}), - [plain_es_pod], - None, + '{"items": null}', + MissingComponentPods, ), ]) -def test_get_pods_for_component(pod_output, expect_pods, expect_error): - check = canned_loggingcheck(lambda namespace, cmd, args: pod_output) - pods, error = check.get_pods_for_component( - logging_namespace, - "es", - ) - assert_error(error, expect_error) +def test_get_pods_for_component_fail(exec_oc_output, expect_error): + check = canned_loggingcheck(lambda *_: exec_oc_output) + with pytest.raises(expect_error): + check.get_pods_for_component("es") @pytest.mark.parametrize('name, pods, expected_pods', [ @@ -159,7 +162,7 @@ def test_get_pods_for_component(pod_output, expect_pods, expect_error): ], ids=lambda argvals: argvals[0]) def test_get_not_running_pods_no_container_status(name, pods, expected_pods): - check = canned_loggingcheck(lambda exec_module, namespace, cmd, args, task_vars: '') + check = canned_loggingcheck(lambda *_: '') result = check.not_running_pods(pods) assert result == expected_pods diff --git a/roles/openshift_health_checker/test/logging_index_time_test.py b/roles/openshift_health_checker/test/logging_index_time_test.py index 178d7cd84..22566b295 100644 --- a/roles/openshift_health_checker/test/logging_index_time_test.py +++ b/roles/openshift_health_checker/test/logging_index_time_test.py @@ -69,7 +69,29 @@ def test_check_running_pods(pods, expect_pods): assert pods == expect_pods -@pytest.mark.parametrize('name, json_response, uuid, timeout, extra_words', [ +def test_bad_config_param(): + with pytest.raises(OpenShiftCheckException) as error: + LoggingIndexTime(task_vars=dict(openshift_check_logging_index_timeout_seconds="foo")).run() + assert 'InvalidTimeout' == error.value.name + + +def test_no_running_pods(): + check = LoggingIndexTime() + check.get_pods_for_component = lambda *_: [not_running_kibana_pod] + with pytest.raises(OpenShiftCheckException) as error: + check.run() + assert 'kibanaNoRunningPods' == error.value.name + + +def test_with_running_pods(): + check = LoggingIndexTime() + check.get_pods_for_component = lambda *_: [plain_running_kibana_pod, plain_running_elasticsearch_pod] + check.curl_kibana_with_uuid = lambda *_: SAMPLE_UUID + check.wait_until_cmd_or_err = lambda *_: None + assert not check.run().get("failed") + + +@pytest.mark.parametrize('name, json_response, uuid, timeout', [ ( 'valid count in response', { @@ -77,94 +99,72 @@ def test_check_running_pods(pods, expect_pods): }, SAMPLE_UUID, 0.001, - [], ), ], ids=lambda argval: argval[0]) -def test_wait_until_cmd_or_err_succeeds(name, json_response, uuid, timeout, extra_words): +def test_wait_until_cmd_or_err_succeeds(name, json_response, uuid, timeout): check = canned_loggingindextime(lambda *_: json.dumps(json_response)) check.wait_until_cmd_or_err(plain_running_elasticsearch_pod, uuid, timeout) -@pytest.mark.parametrize('name, json_response, uuid, timeout, extra_words', [ +@pytest.mark.parametrize('name, json_response, timeout, expect_error', [ ( 'invalid json response', { "invalid_field": 1, }, - SAMPLE_UUID, 0.001, - ["invalid response", "Elasticsearch"], + 'esInvalidResponse', ), ( 'empty response', {}, - SAMPLE_UUID, 0.001, - ["invalid response", "Elasticsearch"], + 'esInvalidResponse', ), ( 'valid response but invalid match count', { "count": 0, }, - SAMPLE_UUID, 0.005, - ["expecting match", SAMPLE_UUID, "0.005s"], + 'NoMatchFound', ) ], ids=lambda argval: argval[0]) -def test_wait_until_cmd_or_err(name, json_response, uuid, timeout, extra_words): +def test_wait_until_cmd_or_err(name, json_response, timeout, expect_error): check = canned_loggingindextime(lambda *_: json.dumps(json_response)) with pytest.raises(OpenShiftCheckException) as error: - check.wait_until_cmd_or_err(plain_running_elasticsearch_pod, uuid, timeout) + check.wait_until_cmd_or_err(plain_running_elasticsearch_pod, SAMPLE_UUID, timeout) - for word in extra_words: - assert word in str(error) + assert expect_error == error.value.name -@pytest.mark.parametrize('name, json_response, uuid, extra_words', [ - ( - 'correct response code, found unique id is returned', - { - "statusCode": 404, - }, - "sample unique id", - ["sample unique id"], - ), -], ids=lambda argval: argval[0]) -def test_curl_kibana_with_uuid(name, json_response, uuid, extra_words): - check = canned_loggingindextime(lambda *_: json.dumps(json_response)) - check.generate_uuid = lambda: uuid - - result = check.curl_kibana_with_uuid(plain_running_kibana_pod) - - for word in extra_words: - assert word in result +def test_curl_kibana_with_uuid(): + check = canned_loggingindextime(lambda *_: json.dumps({"statusCode": 404})) + check.generate_uuid = lambda: SAMPLE_UUID + assert SAMPLE_UUID == check.curl_kibana_with_uuid(plain_running_kibana_pod) -@pytest.mark.parametrize('name, json_response, uuid, extra_words', [ +@pytest.mark.parametrize('name, json_response, expect_error', [ ( 'invalid json response', { "invalid_field": "invalid", }, - SAMPLE_UUID, - ["invalid response returned", 'Missing "statusCode" key'], + 'kibanaInvalidResponse', ), ( 'wrong error code in response', { "statusCode": 500, }, - SAMPLE_UUID, - ["Expecting error code", "500"], + 'kibanaInvalidReturnCode', ), ], ids=lambda argval: argval[0]) -def test_failed_curl_kibana_with_uuid(name, json_response, uuid, extra_words): +def test_failed_curl_kibana_with_uuid(name, json_response, expect_error): check = canned_loggingindextime(lambda *_: json.dumps(json_response)) - check.generate_uuid = lambda: uuid + check.generate_uuid = lambda: SAMPLE_UUID with pytest.raises(OpenShiftCheckException) as error: check.curl_kibana_with_uuid(plain_running_kibana_pod) - for word in extra_words: - assert word in str(error) + assert expect_error == error.value.name diff --git a/roles/openshift_health_checker/test/openshift_check_test.py b/roles/openshift_health_checker/test/openshift_check_test.py index 43aa875f4..789784c77 100644 --- a/roles/openshift_health_checker/test/openshift_check_test.py +++ b/roles/openshift_health_checker/test/openshift_check_test.py @@ -81,6 +81,7 @@ def dummy_check(task_vars): @pytest.mark.parametrize("keys,expected", [ (("foo",), 42), (("bar", "baz"), "openshift"), + (("bar.baz",), "openshift"), ]) def test_get_var_ok(task_vars, keys, expected): assert dummy_check(task_vars).get_var(*keys) == expected @@ -94,3 +95,24 @@ def test_get_var_error(task_vars, missing_keys): def test_get_var_default(task_vars, missing_keys): default = object() assert dummy_check(task_vars).get_var(*missing_keys, default=default) == default + + +@pytest.mark.parametrize("keys, convert, expected", [ + (("foo",), str, "42"), + (("foo",), float, 42.0), + (("bar", "baz"), bool, False), +]) +def test_get_var_convert(task_vars, keys, convert, expected): + assert dummy_check(task_vars).get_var(*keys, convert=convert) == expected + + +@pytest.mark.parametrize("keys, convert", [ + (("bar", "baz"), int), + (("bar.baz"), float), + (("foo"), "bogus"), + (("foo"), lambda a, b: 1), + (("foo"), lambda a: 1 / 0), +]) +def test_get_var_convert_error(task_vars, keys, convert): + with pytest.raises(OpenShiftCheckException): + dummy_check(task_vars).get_var(*keys, convert=convert) diff --git a/roles/openshift_health_checker/test/ovs_version_test.py b/roles/openshift_health_checker/test/ovs_version_test.py index b6acef5a6..e1bf29d2a 100644 --- a/roles/openshift_health_checker/test/ovs_version_test.py +++ b/roles/openshift_health_checker/test/ovs_version_test.py @@ -38,8 +38,8 @@ def test_invalid_openshift_release_format(): @pytest.mark.parametrize('openshift_release,expected_ovs_version', [ - ("3.5", "2.6"), - ("3.6", "2.6"), + ("3.5", ["2.6", "2.7"]), + ("3.6", ["2.6", "2.7"]), ("3.4", "2.4"), ("3.3", "2.4"), ("1.0", "2.4"), diff --git a/roles/openshift_health_checker/test/package_version_test.py b/roles/openshift_health_checker/test/package_version_test.py index 1ddb9cecb..6054d3f3e 100644 --- a/roles/openshift_health_checker/test/package_version_test.py +++ b/roles/openshift_health_checker/test/package_version_test.py @@ -3,58 +3,53 @@ import pytest from openshift_checks.package_version import PackageVersion, OpenShiftCheckException -@pytest.mark.parametrize('openshift_release, extra_words', [ - ('111.7.0', ["no recommended version of Open vSwitch"]), - ('0.0.0', ["no recommended version of Docker"]), -]) -def test_openshift_version_not_supported(openshift_release, extra_words): - def execute_module(*_): - return {} - - task_vars = dict( - openshift=dict(common=dict(service_type='origin')), +def task_vars_for(openshift_release, deployment_type): + return dict( + openshift=dict(common=dict(service_type=deployment_type)), openshift_release=openshift_release, openshift_image_tag='v' + openshift_release, - openshift_deployment_type='origin', + openshift_deployment_type=deployment_type, ) - check = PackageVersion(execute_module, task_vars) + +def test_openshift_version_not_supported(): + check = PackageVersion(None, task_vars_for("1.2.3", 'origin')) + check.get_openshift_version_tuple = lambda: (3, 4, 1) # won't be in the dict + with pytest.raises(OpenShiftCheckException) as excinfo: - check.run() + check.get_required_ovs_version() + assert "no recommended version of Open vSwitch" in str(excinfo.value) - for word in extra_words: - assert word in str(excinfo.value) + with pytest.raises(OpenShiftCheckException) as excinfo: + check.get_required_docker_version() + assert "no recommended version of Docker" in str(excinfo.value) def test_invalid_openshift_release_format(): - def execute_module(*_): - return {} - task_vars = dict( openshift=dict(common=dict(service_type='origin')), openshift_image_tag='v0', openshift_deployment_type='origin', ) - check = PackageVersion(execute_module, task_vars) + check = PackageVersion(lambda *_: {}, task_vars) with pytest.raises(OpenShiftCheckException) as excinfo: check.run() assert "invalid version" in str(excinfo.value) @pytest.mark.parametrize('openshift_release', [ - "3.5", + "111.7.0", + "3.7", "3.6", + "3.5.1.2.3", + "3.5", "3.4", "3.3", + "2.1.0", ]) def test_package_version(openshift_release): - task_vars = dict( - openshift=dict(common=dict(service_type='origin')), - openshift_release=openshift_release, - openshift_image_tag='v' + openshift_release, - openshift_deployment_type='origin', - ) + return_value = object() def execute_module(module_name=None, module_args=None, tmp=None, task_vars=None, *_): @@ -67,26 +62,21 @@ def test_package_version(openshift_release): return return_value - check = PackageVersion(execute_module, task_vars) + check = PackageVersion(execute_module, task_vars_for(openshift_release, 'origin')) result = check.run() assert result is return_value @pytest.mark.parametrize('deployment_type,openshift_release,expected_docker_version', [ ("origin", "3.5", "1.12"), + ("origin", "1.3", "1.10"), + ("origin", "1.1", "1.8"), ("openshift-enterprise", "3.4", "1.12"), - ("origin", "3.3", "1.10"), ("openshift-enterprise", "3.2", "1.10"), - ("origin", "3.1", "1.8"), ("openshift-enterprise", "3.1", "1.8"), ]) def test_docker_package_version(deployment_type, openshift_release, expected_docker_version): - task_vars = dict( - openshift=dict(common=dict(service_type='origin')), - openshift_release=openshift_release, - openshift_image_tag='v' + openshift_release, - openshift_deployment_type=deployment_type, - ) + return_value = object() def execute_module(module_name=None, module_args=None, *_): @@ -99,7 +89,7 @@ def test_docker_package_version(deployment_type, openshift_release, expected_doc return return_value - check = PackageVersion(execute_module, task_vars) + check = PackageVersion(execute_module, task_vars_for(openshift_release, deployment_type)) result = check.run() assert result is return_value diff --git a/roles/openshift_health_checker/test/rpm_version_test.py b/roles/openshift_health_checker/test/rpm_version_test.py index 2f09ef965..2c1bcf876 100644 --- a/roles/openshift_health_checker/test/rpm_version_test.py +++ b/roles/openshift_health_checker/test/rpm_version_test.py @@ -49,7 +49,7 @@ def test_check_pkg_found(pkgs, expect_not_found): }, { "eggs": { - "required_version": "3.2", + "required_versions": ["3.2"], "found_versions": ["3.3"], } }, # not the right version @@ -61,11 +61,11 @@ def test_check_pkg_found(pkgs, expect_not_found): }, { "eggs": { - "required_version": "3.2", + "required_versions": ["3.2"], "found_versions": ["3.3", "1.2"], }, "spam": { - "required_version": "3.2", + "required_versions": ["3.2"], "found_versions": ["3.1", "3.3"], } }, # not the right version |