From d2955189066364461a846e0e34d344c94b26a16b Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Mon, 27 Feb 2017 17:52:02 -0500 Subject: add ram and storage preflight check --- .../openshift_checks/disk_availability.py | 73 ++++++++++++++++++++++ .../openshift_checks/memory_availability.py | 35 +++++++++++ 2 files changed, 108 insertions(+) create mode 100644 roles/openshift_health_checker/openshift_checks/disk_availability.py create mode 100644 roles/openshift_health_checker/openshift_checks/memory_availability.py (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/disk_availability.py b/roles/openshift_health_checker/openshift_checks/disk_availability.py new file mode 100644 index 000000000..ecf5147fd --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/disk_availability.py @@ -0,0 +1,73 @@ +# pylint: disable=missing-docstring +from openshift_checks import OpenShiftCheck, OpenShiftCheckException, get_var +from openshift_checks.mixins import NotContainerizedMixin + + +class DiskAvailability(NotContainerizedMixin, OpenShiftCheck): + """Check that recommended disk space is available before a first-time install.""" + + name = "disk_availability" + tags = ["preflight"] + + # all values are base-10 as they are taken, as is, from + # the latest requirements for an OpenShift installation + # https://docs.openshift.org/latest/install_config/install/prerequisites.html#system-requirements + recommended_diskspace = { + "nodes": 15 * 10 ** 9, + "masters": 40 * 10 ** 9, + "etcd": 20 * 10 ** 9, + } + + def run(self, tmp, task_vars): + ansible_mounts = get_var(task_vars, "ansible_mounts") + self.recommended_diskspace["nodes"] = get_var(task_vars, + "min_recommended_diskspace_node", + default=self.recommended_diskspace["nodes"]) + self.recommended_diskspace["masters"] = get_var(task_vars, + "min_recommended_diskspace_master", + default=self.recommended_diskspace["masters"]) + self.recommended_diskspace["etcd"] = get_var(task_vars, + "min_recommended_diskspace_etcd", + default=self.recommended_diskspace["etcd"]) + + failed, msg = self.volume_check(ansible_mounts, task_vars) + return {"failed": failed, "msg": msg} + + def volume_check(self, ansible_mounts, task_vars): + group_names = get_var(task_vars, "group_names", default=[]) + + if not set(self.recommended_diskspace).intersection(group_names): + msg = "Unable to determine recommended volume size for group_name {group_name}" + raise OpenShiftCheckException(msg.format(group_name=group_names)) + + recommended_diskspace_bytes = max(self.recommended_diskspace.get(group, 0) for group in group_names) + openshift_diskfree_bytes = self.get_openshift_disk_availability(ansible_mounts) + + if openshift_diskfree_bytes < recommended_diskspace_bytes: + msg = ("Available disk space ({diskfree} GB) for the volume containing \"/var\" is " + "below recommended storage. Minimum required disk space: {recommended} GB") + return True, msg.format(diskfree=self.to_gigabytes(openshift_diskfree_bytes), + recommended=self.to_gigabytes(recommended_diskspace_bytes)) + + return False, "" + + @staticmethod + def get_openshift_disk_availability(ansible_mounts): + """Iterates through a map of mounted volumes to determine space remaining on the OpenShift volume""" + if not ansible_mounts: + msg = "Unable to determine existing volume mounts from ansible_mounts" + raise OpenShiftCheckException(msg) + + # priority list in descending order + supported_mnt_paths = ["/var", "/"] + available_mnts = {mnt.get("mount"): mnt for mnt in ansible_mounts} + + for path in supported_mnt_paths: + if path in available_mnts: + return available_mnts[path].get("size_available") + + return 0 + + @staticmethod + def to_gigabytes(total_bytes): + return total_bytes / 10**9 diff --git a/roles/openshift_health_checker/openshift_checks/memory_availability.py b/roles/openshift_health_checker/openshift_checks/memory_availability.py new file mode 100644 index 000000000..aa54b9535 --- /dev/null +++ b/roles/openshift_health_checker/openshift_checks/memory_availability.py @@ -0,0 +1,35 @@ +# pylint: disable=missing-docstring +from openshift_checks import OpenShiftCheck, get_var + + +class MemoryAvailability(OpenShiftCheck): + """Check that recommended memory is available.""" + + name = "memory_availability" + tags = ["preflight"] + + recommended_memory_mb = { + "nodes": 8 * 1000, + "masters": 16 * 1000, + "etcd": 10 * 1000, + } + + @classmethod + def is_active(cls, task_vars): + """Skip hosts that do not have recommended memory space requirements.""" + group_names = get_var(task_vars, "group_names", default=[]) + has_mem_space_recommendation = bool(set(group_names).intersection(cls.recommended_memory_mb)) + return super(MemoryAvailability, cls).is_active(task_vars) and has_mem_space_recommendation + + def run(self, tmp, task_vars): + group_names = get_var(task_vars, "group_names", default=[]) + total_memory = get_var(task_vars, "ansible_memtotal_mb") + + recommended_memory_mb = max(self.recommended_memory_mb.get(group, 0) for group in group_names) + + if total_memory < recommended_memory_mb: + msg = ("Available memory ({available} MB) below recommended storage. " + "Minimum required memory: {recommended} MB") + return {"failed": True, "msg": msg.format(available=total_memory, recommended=recommended_memory_mb)} + + return {"changed": False} -- cgit v1.2.3 From 58fdef2696f7f1f0c6f3b5aa404427a2fc5a00f2 Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Fri, 7 Apr 2017 18:37:14 -0400 Subject: add disk and memory availability check tests --- .../openshift_checks/memory_availability.py | 5 +- .../test/disk_availability_test.py | 106 +++++++++++++++++++++ .../test/memory_availability_test.py | 84 ++++++++++++++++ 3 files changed, 194 insertions(+), 1 deletion(-) create mode 100644 roles/openshift_health_checker/test/disk_availability_test.py create mode 100644 roles/openshift_health_checker/test/memory_availability_test.py (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/memory_availability.py b/roles/openshift_health_checker/openshift_checks/memory_availability.py index aa54b9535..b1629aa89 100644 --- a/roles/openshift_health_checker/openshift_checks/memory_availability.py +++ b/roles/openshift_health_checker/openshift_checks/memory_availability.py @@ -1,5 +1,5 @@ # pylint: disable=missing-docstring -from openshift_checks import OpenShiftCheck, get_var +from openshift_checks import OpenShiftCheck, OpenShiftCheckException, get_var class MemoryAvailability(OpenShiftCheck): @@ -26,6 +26,9 @@ class MemoryAvailability(OpenShiftCheck): total_memory = get_var(task_vars, "ansible_memtotal_mb") recommended_memory_mb = max(self.recommended_memory_mb.get(group, 0) for group in group_names) + if not recommended_memory_mb: + msg = "Unable to determine recommended memory size for group_name {group_name}" + raise OpenShiftCheckException(msg.format(group_name=group_names)) if total_memory < recommended_memory_mb: msg = ("Available memory ({available} MB) below recommended storage. " diff --git a/roles/openshift_health_checker/test/disk_availability_test.py b/roles/openshift_health_checker/test/disk_availability_test.py new file mode 100644 index 000000000..e9d1097a1 --- /dev/null +++ b/roles/openshift_health_checker/test/disk_availability_test.py @@ -0,0 +1,106 @@ +import pytest + +from openshift_checks.disk_availability import DiskAvailability, OpenShiftCheckException + + +def test_exception_raised_on_empty_ansible_mounts(): + with pytest.raises(OpenShiftCheckException) as excinfo: + DiskAvailability(execute_module=NotImplementedError).get_openshift_disk_availability([]) + + assert "existing volume mounts from ansible_mounts" in str(excinfo.value) + + +@pytest.mark.parametrize("group_name,size_available", [ + ( + "masters", + 41110980608, + ), + ( + "nodes", + 21110980608, + ), + ( + "etcd", + 21110980608, + ), +]) +def test_volume_check_with_recommended_diskspace(group_name, size_available): + result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( + openshift=dict(common=dict( + service_type='origin', + is_containerized=False, + )), + group_names=[group_name], + ansible_mounts=[{ + "mount": "/", + "size_available": size_available, + }] + )) + + assert not result['failed'] + assert not result['msg'] + + +@pytest.mark.parametrize("group_name,size_available", [ + ( + "masters", + 21110980608, + ), + ( + "nodes", + 1110980608, + ), + ( + "etcd", + 1110980608, + ), +]) +def test_volume_check_with_insufficient_diskspace(group_name, size_available): + result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( + openshift=dict(common=dict( + service_type='origin', + is_containerized=False, + )), + group_names=[group_name], + ansible_mounts=[{ + "mount": "/", + "size_available": size_available, + }] + )) + + assert result['failed'] + assert "is below recommended storage" in result['msg'] + + +def test_volume_check_with_unsupported_mountpaths(): + result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( + openshift=dict(common=dict( + service_type='origin', + is_containerized=False, + )), + group_names=["masters", "nodes"], + ansible_mounts=[{ + "mount": "/unsupported", + "size_available": 12345, + }] + )) + + assert result['failed'] + assert "0 GB" in result['msg'] + + +def test_volume_check_with_invalid_groupname(): + with pytest.raises(OpenShiftCheckException) as excinfo: + result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( + openshift=dict(common=dict( + service_type='origin', + is_containerized=False, + )), + group_names=["invalid"], + ansible_mounts=[{ + "mount": "/unsupported", + "size_available": 12345, + }] + )) + + assert "'invalid'" in str(excinfo.value) diff --git a/roles/openshift_health_checker/test/memory_availability_test.py b/roles/openshift_health_checker/test/memory_availability_test.py new file mode 100644 index 000000000..a8ba032cc --- /dev/null +++ b/roles/openshift_health_checker/test/memory_availability_test.py @@ -0,0 +1,84 @@ +import pytest + +from openshift_checks.memory_availability import MemoryAvailability, OpenShiftCheckException + + +@pytest.mark.parametrize('group_names,is_containerized,is_active', [ + (['masters'], False, True), + # ensure check is skipped on containerized installs + (['masters'], True, True), + (['nodes'], True, True), + (['etcd'], False, True), + (['masters', 'nodes'], False, True), + (['masters', 'etcd'], False, True), + ([], False, False), + (['lb'], False, False), + (['nfs'], False, False), +]) +def test_is_active(group_names, is_containerized, is_active): + task_vars = dict( + group_names=group_names, + openshift=dict(common=dict(is_containerized=is_containerized)), + ) + assert MemoryAvailability.is_active(task_vars=task_vars) == is_active + + +@pytest.mark.parametrize("group_name,size_available", [ + ( + "masters", + 17200, + ), + ( + "nodes", + 8200, + ), + ( + "etcd", + 12200, + ), +]) +def test_mem_check_with_recommended_memtotal(group_name, size_available): + result = MemoryAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( + group_names=[group_name], + ansible_memtotal_mb=size_available, + )) + + assert not result.get('failed', False) + + +@pytest.mark.parametrize("group_name,size_available", [ + ( + "masters", + 1, + ), + ( + "nodes", + 2, + ), + ( + "etcd", + 3, + ), +]) +def test_mem_check_with_insufficient_memtotal(group_name, size_available): + result = MemoryAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( + group_names=[group_name], + ansible_memtotal_mb=size_available, + )) + + assert result['failed'] + assert "below recommended storage" in result['msg'] + + +def test_mem_check_with_invalid_groupname(): + with pytest.raises(OpenShiftCheckException) as excinfo: + result = MemoryAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( + openshift=dict(common=dict( + service_type='origin', + is_containerized=False, + )), + group_names=["invalid"], + ansible_memtotal_mb=1234567, + )) + + assert "'invalid'" in str(excinfo.value) -- cgit v1.2.3 From c74543f55b14b8153298baa0115f8ab05d2c6961 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 10 Apr 2017 19:25:55 +0200 Subject: Simplify disk availability check, review tests - only support a fixed list of recommended values for now, no overwriting via Ansible variables (keep it simple, add features as needed). - implement is_active: run this check only for hosts that have a recommended disk space. - test priority of mount paths / and /var. --- .../openshift_checks/disk_availability.py | 84 ++++----- .../test/disk_availability_test.py | 205 +++++++++++++-------- 2 files changed, 165 insertions(+), 124 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/disk_availability.py b/roles/openshift_health_checker/openshift_checks/disk_availability.py index ecf5147fd..c2792a0fe 100644 --- a/roles/openshift_health_checker/openshift_checks/disk_availability.py +++ b/roles/openshift_health_checker/openshift_checks/disk_availability.py @@ -9,65 +9,57 @@ class DiskAvailability(NotContainerizedMixin, OpenShiftCheck): name = "disk_availability" tags = ["preflight"] - # all values are base-10 as they are taken, as is, from - # the latest requirements for an OpenShift installation + # Values taken from the official installation documentation: # https://docs.openshift.org/latest/install_config/install/prerequisites.html#system-requirements - recommended_diskspace = { - "nodes": 15 * 10 ** 9, - "masters": 40 * 10 ** 9, - "etcd": 20 * 10 ** 9, + recommended_disk_space_bytes = { + "masters": 40 * 10**9, + "nodes": 15 * 10**9, + "etcd": 20 * 10**9, } - def run(self, tmp, task_vars): - ansible_mounts = get_var(task_vars, "ansible_mounts") - self.recommended_diskspace["nodes"] = get_var(task_vars, - "min_recommended_diskspace_node", - default=self.recommended_diskspace["nodes"]) - self.recommended_diskspace["masters"] = get_var(task_vars, - "min_recommended_diskspace_master", - default=self.recommended_diskspace["masters"]) - self.recommended_diskspace["etcd"] = get_var(task_vars, - "min_recommended_diskspace_etcd", - default=self.recommended_diskspace["etcd"]) - - failed, msg = self.volume_check(ansible_mounts, task_vars) - return {"failed": failed, "msg": msg} - - def volume_check(self, ansible_mounts, task_vars): + @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)) + return super(DiskAvailability, cls).is_active(task_vars) and has_disk_space_recommendation - if not set(self.recommended_diskspace).intersection(group_names): - msg = "Unable to determine recommended volume size for group_name {group_name}" - raise OpenShiftCheckException(msg.format(group_name=group_names)) + def run(self, tmp, task_vars): + group_names = get_var(task_vars, "group_names") + ansible_mounts = get_var(task_vars, "ansible_mounts") - recommended_diskspace_bytes = max(self.recommended_diskspace.get(group, 0) for group in group_names) - openshift_diskfree_bytes = self.get_openshift_disk_availability(ansible_mounts) + min_free_bytes = max(self.recommended_disk_space_bytes.get(name, 0) for name in group_names) + free_bytes = self.openshift_available_disk(ansible_mounts) - if openshift_diskfree_bytes < recommended_diskspace_bytes: - msg = ("Available disk space ({diskfree} GB) for the volume containing \"/var\" is " - "below recommended storage. Minimum required disk space: {recommended} GB") - return True, msg.format(diskfree=self.to_gigabytes(openshift_diskfree_bytes), - recommended=self.to_gigabytes(recommended_diskspace_bytes)) + 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) + } - return False, "" + return {} @staticmethod - def get_openshift_disk_availability(ansible_mounts): - """Iterates through a map of mounted volumes to determine space remaining on the OpenShift volume""" - if not ansible_mounts: - msg = "Unable to determine existing volume mounts from ansible_mounts" - raise OpenShiftCheckException(msg) + 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} - for path in supported_mnt_paths: - if path in available_mnts: - return available_mnts[path].get("size_available") + try: + for path in supported_mnt_paths: + if path in available_mnts: + return available_mnts[path]["size_available"] + except KeyError: + pass - return 0 - - @staticmethod - def to_gigabytes(total_bytes): - return total_bytes / 10**9 + paths = ''.join(sorted(available_mnts)) or 'none' + msg = "Unable to determine available disk space. Paths mounted: {}.".format(paths) + raise OpenShiftCheckException(msg) diff --git a/roles/openshift_health_checker/test/disk_availability_test.py b/roles/openshift_health_checker/test/disk_availability_test.py index e9d1097a1..8054ad25c 100644 --- a/roles/openshift_health_checker/test/disk_availability_test.py +++ b/roles/openshift_health_checker/test/disk_availability_test.py @@ -3,104 +3,153 @@ import pytest from openshift_checks.disk_availability import DiskAvailability, OpenShiftCheckException -def test_exception_raised_on_empty_ansible_mounts(): +@pytest.mark.parametrize('group_names,is_containerized,is_active', [ + (['masters'], False, True), + # ensure check is skipped on containerized installs + (['masters'], True, False), + (['nodes'], False, True), + (['etcd'], False, True), + (['masters', 'nodes'], False, True), + (['masters', 'etcd'], False, True), + ([], False, False), + (['lb'], False, False), + (['nfs'], False, False), +]) +def test_is_active(group_names, is_containerized, is_active): + task_vars = dict( + group_names=group_names, + openshift=dict(common=dict(is_containerized=is_containerized)), + ) + assert DiskAvailability.is_active(task_vars=task_vars) == 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 +]) +def test_cannot_determine_available_disk(ansible_mounts, extra_words): + task_vars = dict( + group_names=['masters'], + ansible_mounts=ansible_mounts, + ) + check = DiskAvailability(execute_module=fake_execute_module) + with pytest.raises(OpenShiftCheckException) as excinfo: - DiskAvailability(execute_module=NotImplementedError).get_openshift_disk_availability([]) + check.run(tmp=None, task_vars=task_vars) - assert "existing volume mounts from ansible_mounts" in str(excinfo.value) + for word in 'determine available disk'.split() + extra_words: + assert word in str(excinfo.value) -@pytest.mark.parametrize("group_name,size_available", [ +@pytest.mark.parametrize('group_names,ansible_mounts', [ + ( + ['masters'], + [{ + 'mount': '/', + 'size_available': 40 * 10**9 + 1, + }], + ), ( - "masters", - 41110980608, + ['nodes'], + [{ + 'mount': '/', + 'size_available': 15 * 10**9 + 1, + }], ), ( - "nodes", - 21110980608, + ['etcd'], + [{ + 'mount': '/', + 'size_available': 20 * 10**9 + 1, + }], ), ( - "etcd", - 21110980608, + ['etcd'], + [{ + # not enough space on / ... + 'mount': '/', + 'size_available': 0, + }, { + # ... but enough on /var + 'mount': '/var', + 'size_available': 20 * 10**9 + 1, + }], ), ]) -def test_volume_check_with_recommended_diskspace(group_name, size_available): - result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( - openshift=dict(common=dict( - service_type='origin', - is_containerized=False, - )), - group_names=[group_name], - ansible_mounts=[{ - "mount": "/", - "size_available": size_available, - }] - )) - - assert not result['failed'] - assert not result['msg'] - - -@pytest.mark.parametrize("group_name,size_available", [ +def test_succeeds_with_recommended_disk_space(group_names, ansible_mounts): + task_vars = dict( + group_names=group_names, + ansible_mounts=ansible_mounts, + ) + + check = DiskAvailability(execute_module=fake_execute_module) + result = check.run(tmp=None, task_vars=task_vars) + + assert not result.get('failed', False) + + +@pytest.mark.parametrize('group_names,ansible_mounts,extra_words', [ + ( + ['masters'], + [{ + 'mount': '/', + 'size_available': 1, + }], + ['0.0 GB'], + ), ( - "masters", - 21110980608, + ['nodes'], + [{ + 'mount': '/', + 'size_available': 1 * 10**9, + }], + ['1.0 GB'], ), ( - "nodes", - 1110980608, + ['etcd'], + [{ + 'mount': '/', + 'size_available': 1, + }], + ['0.0 GB'], ), ( - "etcd", - 1110980608, + ['nodes', 'masters'], + [{ + 'mount': '/', + # enough space for a node, not enough for a master + 'size_available': 15 * 10**9 + 1, + }], + ['15.0 GB'], + ), + ( + ['etcd'], + [{ + # enough space on / ... + 'mount': '/', + 'size_available': 20 * 10**9 + 1, + }, { + # .. but not enough on /var + 'mount': '/var', + 'size_available': 0, + }], + ['0.0 GB'], ), ]) -def test_volume_check_with_insufficient_diskspace(group_name, size_available): - result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( - openshift=dict(common=dict( - service_type='origin', - is_containerized=False, - )), - group_names=[group_name], - ansible_mounts=[{ - "mount": "/", - "size_available": size_available, - }] - )) +def test_fails_with_insufficient_disk_space(group_names, ansible_mounts, extra_words): + task_vars = dict( + group_names=group_names, + ansible_mounts=ansible_mounts, + ) - assert result['failed'] - assert "is below recommended storage" in result['msg'] - - -def test_volume_check_with_unsupported_mountpaths(): - result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( - openshift=dict(common=dict( - service_type='origin', - is_containerized=False, - )), - group_names=["masters", "nodes"], - ansible_mounts=[{ - "mount": "/unsupported", - "size_available": 12345, - }] - )) + check = DiskAvailability(execute_module=fake_execute_module) + result = check.run(tmp=None, task_vars=task_vars) assert result['failed'] - assert "0 GB" in result['msg'] + for word in 'below recommended'.split() + extra_words: + assert word in result['msg'] -def test_volume_check_with_invalid_groupname(): - with pytest.raises(OpenShiftCheckException) as excinfo: - result = DiskAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( - openshift=dict(common=dict( - service_type='origin', - is_containerized=False, - )), - group_names=["invalid"], - ansible_mounts=[{ - "mount": "/unsupported", - "size_available": 12345, - }] - )) - - assert "'invalid'" in str(excinfo.value) +def fake_execute_module(*args): + raise AssertionError('this function should not be called') -- cgit v1.2.3 From 59e781baaa33d1ffb0d57529c23fd9a1c01a377a Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Tue, 11 Apr 2017 14:31:09 +0200 Subject: Simplify mixin class - Expose only is_active and no other method. - Move general comment to module docstring. --- .../openshift_health_checker/openshift_checks/mixins.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/mixins.py b/roles/openshift_health_checker/openshift_checks/mixins.py index 657e15160..20d160eaf 100644 --- a/roles/openshift_health_checker/openshift_checks/mixins.py +++ b/roles/openshift_health_checker/openshift_checks/mixins.py @@ -1,4 +1,8 @@ -# pylint: disable=missing-docstring +# pylint: disable=missing-docstring,too-few-public-methods +""" +Mixin classes meant to be used with subclasses of OpenShiftCheck. +""" + from openshift_checks import get_var @@ -7,12 +11,5 @@ class NotContainerizedMixin(object): @classmethod def is_active(cls, task_vars): - return ( - # This mixin is meant to be used with subclasses of OpenShiftCheck. - super(NotContainerizedMixin, cls).is_active(task_vars) and - not cls.is_containerized(task_vars) - ) - - @staticmethod - def is_containerized(task_vars): - return get_var(task_vars, "openshift", "common", "is_containerized") + is_containerized = get_var(task_vars, "openshift", "common", "is_containerized") + return super(NotContainerizedMixin, cls).is_active(task_vars) and not is_containerized -- cgit v1.2.3 From 2aca8a4b833897ace556df407040e9eaeff1ebc4 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Tue, 11 Apr 2017 17:09:33 +0200 Subject: Simplify memory availability check, review tests - Fix required memory for etcd hosts (10 -> 20 GB), as per documentation. - Some changes to make the code more similar to the similar DiskAvailability check. - Do not raise exception for hosts that do not have a recommended memory value (those are ignored anyway through `is_active`, so that was essentially dead code). - Test that the required memory is the max of the recommended memories for all groups assigned to a host. E.g. if a host is master and node, we should check that it has enough memory to be a master, because the memory requirement for a master is higher than for a node. --- .../openshift_checks/memory_availability.py | 50 +++++----- .../test/disk_availability_test.py | 6 +- .../test/memory_availability_test.py | 105 +++++++++++---------- 3 files changed, 87 insertions(+), 74 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/memory_availability.py b/roles/openshift_health_checker/openshift_checks/memory_availability.py index b1629aa89..28805dc37 100644 --- a/roles/openshift_health_checker/openshift_checks/memory_availability.py +++ b/roles/openshift_health_checker/openshift_checks/memory_availability.py @@ -1,5 +1,5 @@ # pylint: disable=missing-docstring -from openshift_checks import OpenShiftCheck, OpenShiftCheckException, get_var +from openshift_checks import OpenShiftCheck, get_var class MemoryAvailability(OpenShiftCheck): @@ -8,31 +8,37 @@ class MemoryAvailability(OpenShiftCheck): name = "memory_availability" tags = ["preflight"] - recommended_memory_mb = { - "nodes": 8 * 1000, - "masters": 16 * 1000, - "etcd": 10 * 1000, + # Values taken from the official installation documentation: + # https://docs.openshift.org/latest/install_config/install/prerequisites.html#system-requirements + recommended_memory_bytes = { + "masters": 16 * 10**9, + "nodes": 8 * 10**9, + "etcd": 20 * 10**9, } @classmethod def is_active(cls, task_vars): - """Skip hosts that do not have recommended memory space requirements.""" + """Skip hosts that do not have recommended memory requirements.""" group_names = get_var(task_vars, "group_names", default=[]) - has_mem_space_recommendation = bool(set(group_names).intersection(cls.recommended_memory_mb)) - return super(MemoryAvailability, cls).is_active(task_vars) and has_mem_space_recommendation + has_memory_recommendation = bool(set(group_names).intersection(cls.recommended_memory_bytes)) + return super(MemoryAvailability, cls).is_active(task_vars) and has_memory_recommendation def run(self, tmp, task_vars): - group_names = get_var(task_vars, "group_names", default=[]) - total_memory = get_var(task_vars, "ansible_memtotal_mb") - - recommended_memory_mb = max(self.recommended_memory_mb.get(group, 0) for group in group_names) - if not recommended_memory_mb: - msg = "Unable to determine recommended memory size for group_name {group_name}" - raise OpenShiftCheckException(msg.format(group_name=group_names)) - - if total_memory < recommended_memory_mb: - msg = ("Available memory ({available} MB) below recommended storage. " - "Minimum required memory: {recommended} MB") - return {"failed": True, "msg": msg.format(available=total_memory, recommended=recommended_memory_mb)} - - return {"changed": False} + group_names = get_var(task_vars, "group_names") + total_memory_bytes = get_var(task_vars, "ansible_memtotal_mb") * 10**6 + + min_memory_bytes = max(self.recommended_memory_bytes.get(name, 0) for name in group_names) + + if total_memory_bytes < min_memory_bytes: + return { + 'failed': True, + 'msg': ( + 'Available memory ({available:.1f} GB) ' + 'below recommended value ({recommended:.1f} GB)' + ).format( + available=float(total_memory_bytes) / 10**9, + recommended=float(min_memory_bytes) / 10**9, + ), + } + + return {} diff --git a/roles/openshift_health_checker/test/disk_availability_test.py b/roles/openshift_health_checker/test/disk_availability_test.py index 8054ad25c..970b474d7 100644 --- a/roles/openshift_health_checker/test/disk_availability_test.py +++ b/roles/openshift_health_checker/test/disk_availability_test.py @@ -24,9 +24,9 @@ def test_is_active(group_names, is_containerized, 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 + ([], ['none']), # empty ansible_mounts + ([{'mount': '/mnt'}], ['/mnt']), # missing relevant mount paths + ([{'mount': '/var'}], ['/var']), # missing size_available ]) def test_cannot_determine_available_disk(ansible_mounts, extra_words): task_vars = dict( diff --git a/roles/openshift_health_checker/test/memory_availability_test.py b/roles/openshift_health_checker/test/memory_availability_test.py index a8ba032cc..e161a5b9e 100644 --- a/roles/openshift_health_checker/test/memory_availability_test.py +++ b/roles/openshift_health_checker/test/memory_availability_test.py @@ -1,84 +1,91 @@ import pytest -from openshift_checks.memory_availability import MemoryAvailability, OpenShiftCheckException +from openshift_checks.memory_availability import MemoryAvailability -@pytest.mark.parametrize('group_names,is_containerized,is_active', [ - (['masters'], False, True), - # ensure check is skipped on containerized installs - (['masters'], True, True), - (['nodes'], True, True), - (['etcd'], False, True), - (['masters', 'nodes'], False, True), - (['masters', 'etcd'], False, True), - ([], False, False), - (['lb'], False, False), - (['nfs'], False, False), +@pytest.mark.parametrize('group_names,is_active', [ + (['masters'], True), + (['nodes'], True), + (['etcd'], True), + (['masters', 'nodes'], True), + (['masters', 'etcd'], True), + ([], False), + (['lb'], False), + (['nfs'], False), ]) -def test_is_active(group_names, is_containerized, is_active): +def test_is_active(group_names, is_active): task_vars = dict( group_names=group_names, - openshift=dict(common=dict(is_containerized=is_containerized)), ) assert MemoryAvailability.is_active(task_vars=task_vars) == is_active -@pytest.mark.parametrize("group_name,size_available", [ +@pytest.mark.parametrize('group_names,ansible_memtotal_mb', [ ( - "masters", + ['masters'], 17200, ), ( - "nodes", + ['nodes'], 8200, ), ( - "etcd", - 12200, + ['etcd'], + 22200, + ), + ( + ['masters', 'nodes'], + 17000, ), ]) -def test_mem_check_with_recommended_memtotal(group_name, size_available): - result = MemoryAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( - group_names=[group_name], - ansible_memtotal_mb=size_available, - )) +def test_succeeds_with_recommended_memory(group_names, ansible_memtotal_mb): + task_vars = dict( + group_names=group_names, + ansible_memtotal_mb=ansible_memtotal_mb, + ) + + check = MemoryAvailability(execute_module=fake_execute_module) + result = check.run(tmp=None, task_vars=task_vars) assert not result.get('failed', False) -@pytest.mark.parametrize("group_name,size_available", [ +@pytest.mark.parametrize('group_names,ansible_memtotal_mb,extra_words', [ ( - "masters", - 1, + ['masters'], + 0, + ['0.0 GB'], ), ( - "nodes", - 2, + ['nodes'], + 100, + ['0.1 GB'], ), ( - "etcd", - 3, + ['etcd'], + -1, + ['0.0 GB'], + ), + ( + ['nodes', 'masters'], + # enough memory for a node, not enough for a master + 11000, + ['11.0 GB'], ), ]) -def test_mem_check_with_insufficient_memtotal(group_name, size_available): - result = MemoryAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( - group_names=[group_name], - ansible_memtotal_mb=size_available, - )) +def test_fails_with_insufficient_memory(group_names, ansible_memtotal_mb, extra_words): + task_vars = dict( + group_names=group_names, + ansible_memtotal_mb=ansible_memtotal_mb, + ) - assert result['failed'] - assert "below recommended storage" in result['msg'] + check = MemoryAvailability(execute_module=fake_execute_module) + result = check.run(tmp=None, task_vars=task_vars) + assert result['failed'] + for word in 'below recommended'.split() + extra_words: + assert word in result['msg'] -def test_mem_check_with_invalid_groupname(): - with pytest.raises(OpenShiftCheckException) as excinfo: - result = MemoryAvailability(execute_module=NotImplementedError).run(tmp=None, task_vars=dict( - openshift=dict(common=dict( - service_type='origin', - is_containerized=False, - )), - group_names=["invalid"], - ansible_memtotal_mb=1234567, - )) - assert "'invalid'" in str(excinfo.value) +def fake_execute_module(*args): + raise AssertionError('this function should not be called') -- cgit v1.2.3