From 5f51278dee925c47055aacbb8ffac5e351ea7ee2 Mon Sep 17 00:00:00 2001 From: Luke Meyer Date: Mon, 10 Jul 2017 16:58:31 -0400 Subject: openshift_checks: fix execute_module params Fix where execute_module was being passed task_vars in place of tmp param. Most modules don't seem to use either and so this doesn't fail; but under some conditions (perhaps different per version of ansible?) it tried to treat the dict as a string and came back with a python stack trace. --- roles/openshift_health_checker/openshift_checks/logging/kibana.py | 2 +- roles/openshift_health_checker/openshift_checks/logging/logging.py | 2 +- roles/openshift_health_checker/test/kibana_test.py | 2 +- roles/openshift_health_checker/test/logging_check_test.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/logging/kibana.py b/roles/openshift_health_checker/openshift_checks/logging/kibana.py index 442f407b1..551e8dfa0 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/kibana.py +++ b/roles/openshift_health_checker/openshift_checks/logging/kibana.py @@ -62,7 +62,7 @@ class Kibana(LoggingCheck): # TODO(lmeyer): give users option to validate certs status_code=302, ) - result = self.execute_module('uri', args, task_vars) + result = self.execute_module('uri', args, None, task_vars) if result.get('failed'): return result['msg'] return None diff --git a/roles/openshift_health_checker/openshift_checks/logging/logging.py b/roles/openshift_health_checker/openshift_checks/logging/logging.py index 05b4d300c..e8821f0ba 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/logging.py +++ b/roles/openshift_health_checker/openshift_checks/logging/logging.py @@ -78,7 +78,7 @@ class LoggingCheck(OpenShiftCheck): "extra_args": list(extra_args) if extra_args else [], } - result = execute_module("ocutil", args, task_vars) + result = execute_module("ocutil", args, None, task_vars) if result.get("failed"): msg = ( 'Unexpected error using `oc` to validate the logging stack components.\n' diff --git a/roles/openshift_health_checker/test/kibana_test.py b/roles/openshift_health_checker/test/kibana_test.py index 19140a1b6..40a5d19d8 100644 --- a/roles/openshift_health_checker/test/kibana_test.py +++ b/roles/openshift_health_checker/test/kibana_test.py @@ -169,7 +169,7 @@ def test_get_kibana_url(route, expect_url, expect_error): ), ]) def test_verify_url_internal_failure(exec_result, expect): - check = Kibana(execute_module=lambda module_name, args, task_vars: dict(failed=True, msg=exec_result)) + check = Kibana(execute_module=lambda module_name, args, tmp, task_vars: dict(failed=True, msg=exec_result)) check._get_kibana_url = lambda task_vars: ('url', None) error = check._check_kibana_route({}) diff --git a/roles/openshift_health_checker/test/logging_check_test.py b/roles/openshift_health_checker/test/logging_check_test.py index b6db34fe3..3b9e3fa8d 100644 --- a/roles/openshift_health_checker/test/logging_check_test.py +++ b/roles/openshift_health_checker/test/logging_check_test.py @@ -80,7 +80,7 @@ plain_curator_pod = { ("Permission denied", "Unexpected error using `oc`"), ]) def test_oc_failure(problem, expect): - def execute_module(module_name, args, task_vars): + def execute_module(module_name, args, tmp, task_vars): if module_name == "ocutil": return dict(failed=True, result=problem) return dict(changed=False) -- cgit v1.2.3 From e1c3499ad16f332d7684fb2eb896f980740c95b7 Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Mon, 10 Jul 2017 17:45:17 +0200 Subject: Add overlay to supported Docker storage drivers Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1467809 As a next step, we can refine under which conditions the overlay driver is supported. --- roles/openshift_health_checker/openshift_checks/docker_storage.py | 2 +- roles/openshift_health_checker/test/docker_storage_test.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/docker_storage.py b/roles/openshift_health_checker/openshift_checks/docker_storage.py index 8d0fbcc9c..e80691ef3 100644 --- a/roles/openshift_health_checker/openshift_checks/docker_storage.py +++ b/roles/openshift_health_checker/openshift_checks/docker_storage.py @@ -17,7 +17,7 @@ class DockerStorage(DockerHostMixin, OpenShiftCheck): tags = ["pre-install", "health", "preflight"] dependencies = ["python-docker-py"] - storage_drivers = ["devicemapper", "overlay2"] + storage_drivers = ["devicemapper", "overlay", "overlay2"] max_thinpool_data_usage_percent = 90.0 max_thinpool_meta_usage_percent = 90.0 diff --git a/roles/openshift_health_checker/test/docker_storage_test.py b/roles/openshift_health_checker/test/docker_storage_test.py index 876614b1d..bb25e3f66 100644 --- a/roles/openshift_health_checker/test/docker_storage_test.py +++ b/roles/openshift_health_checker/test/docker_storage_test.py @@ -65,8 +65,8 @@ non_atomic_task_vars = {"openshift": {"common": {"is_atomic": False}}} dict(info={ "Driver": "overlay", }), - True, - ["unsupported Docker storage driver"], + False, + [], ), ( dict(info={ -- cgit v1.2.3 From c630e6dbd29e80ad57cb230244fe1cb830a891aa Mon Sep 17 00:00:00 2001 From: Rodolfo Carvalho Date: Fri, 7 Jul 2017 16:39:07 +0200 Subject: Only store failures that were not ignored. In the past, health checks were implemented with ignore_errors: True in the playbook level, requiring us to store all failures, ignored or not, so that we could report on all failed checks. Now checks are run from a single action plugin entry point, without ignoring errors (all errors are aggregated via the action plugin). Since the integration of the openshift_health_checker role with the install playbook, failure summaries are part of the output of a lot more calls to ansible-playbook. We shall report only failures that caused the execution to stop, as ignored failures in the summary only serve to confuse users. --- roles/openshift_health_checker/callback_plugins/zz_failure_summary.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/callback_plugins/zz_failure_summary.py b/roles/openshift_health_checker/callback_plugins/zz_failure_summary.py index 64c29a8d9..443b76ea1 100644 --- a/roles/openshift_health_checker/callback_plugins/zz_failure_summary.py +++ b/roles/openshift_health_checker/callback_plugins/zz_failure_summary.py @@ -39,7 +39,8 @@ class CallbackModule(CallbackBase): def v2_runner_on_failed(self, result, ignore_errors=False): super(CallbackModule, self).v2_runner_on_failed(result, ignore_errors) - self.__failures.append(dict(result=result, ignore_errors=ignore_errors)) + if not ignore_errors: + self.__failures.append(dict(result=result, ignore_errors=ignore_errors)) def v2_playbook_on_stats(self, stats): super(CallbackModule, self).v2_playbook_on_stats(stats) -- cgit v1.2.3 From 1c213b51468fa698acb08d18ce5d6b2a5796d93d Mon Sep 17 00:00:00 2001 From: juanvallejo Date: Mon, 10 Jul 2017 15:38:22 -0400 Subject: add scheduled pods check --- .../openshift_checks/logging/logging.py | 4 +-- .../test/logging_check_test.py | 30 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) (limited to 'roles') diff --git a/roles/openshift_health_checker/openshift_checks/logging/logging.py b/roles/openshift_health_checker/openshift_checks/logging/logging.py index e8821f0ba..6e951e82c 100644 --- a/roles/openshift_health_checker/openshift_checks/logging/logging.py +++ b/roles/openshift_health_checker/openshift_checks/logging/logging.py @@ -54,12 +54,12 @@ class LoggingCheck(OpenShiftCheck): """Returns: list of pods not in a ready and running state""" return [ pod for pod in pods - if any( + if not pod.get("status", {}).get("containerStatuses") or any( container['ready'] is False for container in pod['status']['containerStatuses'] ) or not any( condition['type'] == 'Ready' and condition['status'] == 'True' - for condition in pod['status']['conditions'] + for condition in pod['status'].get('conditions', []) ) ] diff --git a/roles/openshift_health_checker/test/logging_check_test.py b/roles/openshift_health_checker/test/logging_check_test.py index 3b9e3fa8d..128b76b12 100644 --- a/roles/openshift_health_checker/test/logging_check_test.py +++ b/roles/openshift_health_checker/test/logging_check_test.py @@ -50,6 +50,16 @@ plain_kibana_pod = { } } +plain_kibana_pod_no_containerstatus = { + "metadata": { + "labels": {"component": "kibana", "deploymentconfig": "logging-kibana"}, + "name": "logging-kibana-1", + }, + "status": { + "conditions": [{"status": "True", "type": "Ready"}], + } +} + fluentd_pod_node1 = { "metadata": { "labels": {"component": "fluentd", "deploymentconfig": "logging-fluentd"}, @@ -135,3 +145,23 @@ def test_get_pods_for_component(pod_output, expect_pods, expect_error): {} ) assert_error(error, expect_error) + + +@pytest.mark.parametrize('name, pods, expected_pods', [ + ( + 'test single pod found, scheduled, but no containerStatuses field', + [plain_kibana_pod_no_containerstatus], + [plain_kibana_pod_no_containerstatus], + ), + ( + 'set of pods has at least one pod with containerStatuses (scheduled); should still fail', + [plain_kibana_pod_no_containerstatus, plain_kibana_pod], + [plain_kibana_pod_no_containerstatus], + ), + +], 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: '') + result = check.not_running_pods(pods) + + assert result == expected_pods -- cgit v1.2.3