From 7a643daa7ed629cd904cfb5fd5eec4260f0f1582 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Tue, 13 Jun 2017 19:09:19 +0200 Subject: Refactor DiskAvailability for arbitrary paths Prepare the check to support verifying multiple paths, not only /var. --- .../openshift_checks/disk_availability.py | 96 ++++++++++++++-------- .../test/disk_availability_test.py | 2 +- 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/roles/openshift_health_checker/openshift_checks/disk_availability.py b/roles/openshift_health_checker/openshift_checks/disk_availability.py index 962148cb8..dbcbfbc8e 100644 --- a/roles/openshift_health_checker/openshift_checks/disk_availability.py +++ b/roles/openshift_health_checker/openshift_checks/disk_availability.py @@ -1,4 +1,6 @@ # pylint: disable=missing-docstring +import os.path + from openshift_checks import OpenShiftCheck, OpenShiftCheckException, get_var from openshift_checks.mixins import NotContainerizedMixin @@ -12,56 +14,84 @@ class DiskAvailability(NotContainerizedMixin, OpenShiftCheck): # Values taken from the official installation documentation: # https://docs.openshift.org/latest/install_config/install/prerequisites.html#system-requirements recommended_disk_space_bytes = { - "masters": 40 * 10**9, - "nodes": 15 * 10**9, - "etcd": 20 * 10**9, + '/var': { + 'masters': 40 * 10**9, + 'nodes': 15 * 10**9, + 'etcd': 20 * 10**9, + }, } @classmethod def is_active(cls, task_vars): """Skip hosts that do not have recommended disk space requirements.""" group_names = get_var(task_vars, "group_names", default=[]) - has_disk_space_recommendation = bool(set(group_names).intersection(cls.recommended_disk_space_bytes)) + active_groups = set() + for recommendation in cls.recommended_disk_space_bytes.values(): + active_groups.update(recommendation.keys()) + has_disk_space_recommendation = bool(active_groups.intersection(group_names)) return super(DiskAvailability, cls).is_active(task_vars) and has_disk_space_recommendation def run(self, tmp, task_vars): group_names = get_var(task_vars, "group_names") ansible_mounts = get_var(task_vars, "ansible_mounts") - free_bytes = self.openshift_available_disk(ansible_mounts) - - recommended_min = max(self.recommended_disk_space_bytes.get(name, 0) for name in group_names) - configured_min = int(get_var(task_vars, "openshift_check_min_host_disk_gb", default=0)) * 10**9 - min_free_bytes = configured_min or recommended_min - - if free_bytes < min_free_bytes: - return { - 'failed': True, - 'msg': ( - 'Available disk space ({:.1f} GB) for the volume containing ' - '"/var" is below minimum recommended space ({:.1f} GB)' - ).format(float(free_bytes) / 10**9, float(min_free_bytes) / 10**9) + ansible_mounts = {mount['mount']: mount for mount in ansible_mounts} + + user_config = get_var(task_vars, "openshift_check_min_host_disk_gb", default={}) + try: + # For backwards-compatibility, if openshift_check_min_host_disk_gb + # is a number, then it overrides the required config for '/var'. + number = float(user_config) + user_config = { + '/var': { + 'masters': number, + 'nodes': number, + 'etcd': number, + }, } + except TypeError: + # If it is not a number, then it should be a nested dict. + pass + + + for path, recommendation in self.recommended_disk_space_bytes.items(): + free_bytes = self.free_bytes(path, ansible_mounts) + recommended_bytes = max(recommendation.get(name, 0) for name in group_names) + + config = user_config.get(path, {}) + # NOTE: the user config is in GB, but we compare bytes, thus the + # conversion. + config_bytes = max(config.get(name, 0) for name in group_names) * 10**9 + recommended_bytes = config_bytes or recommended_bytes + + if free_bytes < recommended_bytes: + free_gb = float(free_bytes) / 10**9 + recommended_gb = float(recommended_bytes) / 10**9 + return { + 'failed': True, + 'msg': ( + 'Available disk space in "{}" ({:.1f} GB) ' + 'is below minimum recommended ({:.1f} GB)' + ).format(path, free_gb, recommended_gb) + } return {} @staticmethod - def openshift_available_disk(ansible_mounts): - """Determine the available disk space for an OpenShift installation. - - ansible_mounts should be a list of dicts like the 'setup' Ansible module - returns. - """ - # priority list in descending order - supported_mnt_paths = ["/var", "/"] - available_mnts = {mnt.get("mount"): mnt for mnt in ansible_mounts} + def free_bytes(path, ansible_mounts): + """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 try: - for path in supported_mnt_paths: - if path in available_mnts: - return available_mnts[path]["size_available"] + free_bytes = ansible_mounts[mount_point]['size_available'] except KeyError: - pass + 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)) - paths = ''.join(sorted(available_mnts)) or 'none' - msg = "Unable to determine available disk space. Paths mounted: {}.".format(paths) - raise OpenShiftCheckException(msg) + return free_bytes diff --git a/roles/openshift_health_checker/test/disk_availability_test.py b/roles/openshift_health_checker/test/disk_availability_test.py index b353fa610..de09c51da 100644 --- a/roles/openshift_health_checker/test/disk_availability_test.py +++ b/roles/openshift_health_checker/test/disk_availability_test.py @@ -38,7 +38,7 @@ def test_cannot_determine_available_disk(ansible_mounts, extra_words): with pytest.raises(OpenShiftCheckException) as excinfo: check.run(tmp=None, task_vars=task_vars) - for word in 'determine available disk'.split() + extra_words: + for word in 'determine disk availability'.split() + extra_words: assert word in str(excinfo.value) -- cgit v1.2.3