From 1e6b8249e7755ef76baf27c2ebd5f0d05ef322cc Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Wed, 15 Feb 2023 08:49:30 +1000 Subject: [PATCH] Add condition that causes a when to skip a task to output msg (#78918) * Add condition that causes a when to skip a task * Fix up tests * Use false_condition instead of failed_condition * Remove formatting accidentially added * Fix sanity --- changelogs/fragments/skip-conditional.yml | 2 ++ lib/ansible/executor/task_executor.py | 6 ++++-- lib/ansible/playbook/conditional.py | 16 ++++++++++++++-- ...t.out.result_format_yaml_lossy_verbose.stdout | 4 ++++ ...default.out.result_format_yaml_verbose.stdout | 4 ++++ test/integration/targets/no_log/runme.sh | 2 +- .../integration/targets/test_core/tasks/main.yml | 13 +++++++++++++ test/units/executor/test_task_executor.py | 1 + 8 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 changelogs/fragments/skip-conditional.yml diff --git a/changelogs/fragments/skip-conditional.yml b/changelogs/fragments/skip-conditional.yml new file mode 100644 index 00000000000..b53ab3827ee --- /dev/null +++ b/changelogs/fragments/skip-conditional.yml @@ -0,0 +1,2 @@ +minor_changes: +- Added the conditional that was False if ``when`` caused a task to skip under ``false_condition``. diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 5dc45d9d6e3..c956a31b6c1 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -450,9 +450,11 @@ class TaskExecutor: # the fact that the conditional may specify that the task be skipped due to a # variable not being present which would otherwise cause validation to fail try: - if not self._task.evaluate_conditional(templar, tempvars): + conditional_result, false_condition = self._task.evaluate_conditional_with_result(templar, tempvars) + if not conditional_result: display.debug("when evaluation is False, skipping this task") - return dict(changed=False, skipped=True, skip_reason='Conditional result was False', _ansible_no_log=no_log) + return dict(changed=False, skipped=True, skip_reason='Conditional result was False', + false_condition=false_condition, _ansible_no_log=no_log) except AnsibleError as e: # loop error takes precedence if self._loop_eval_error is not None: diff --git a/lib/ansible/playbook/conditional.py b/lib/ansible/playbook/conditional.py index ca1be9500ca..834725bf83c 100644 --- a/lib/ansible/playbook/conditional.py +++ b/lib/ansible/playbook/conditional.py @@ -20,6 +20,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import ast +import typing as t from jinja2.compiler import generate from jinja2.exceptions import UndefinedError @@ -28,6 +29,7 @@ from ansible.errors import AnsibleError, AnsibleUndefinedVariable from ansible.module_utils.six import text_type from ansible.module_utils._text import to_native from ansible.playbook.attribute import FieldAttribute +from ansible.template import Templar from ansible.utils.display import Display display = Display() @@ -57,11 +59,19 @@ class Conditional: if not isinstance(value, list): setattr(self, name, [value]) - def evaluate_conditional(self, templar, all_vars): + def evaluate_conditional(self, templar: Templar, all_vars: dict[str, t.Any]) -> bool: ''' Loops through the conditionals set on this object, returning False if any of them evaluate as such. ''' + return self.evaluate_conditional_with_result(templar, all_vars)[0] + + def evaluate_conditional_with_result(self, templar: Templar, all_vars: dict[str, t.Any]) -> tuple[bool, t.Optional[str]]: + """ + Loops through the conditionals set on this object, returning + False if any of them evaluate as such as well as the condition + that was false. + """ # since this is a mix-in, it may not have an underlying datastructure # associated with it, so we pull it out now in case we need it for @@ -71,6 +81,7 @@ class Conditional: ds = getattr(self, '_ds') result = True + false_condition: t.Optional[str] = None try: for conditional in self.when: @@ -88,12 +99,13 @@ class Conditional: display.debug("Evaluated conditional (%s): %s" % (conditional, res)) if not result: + false_condition = conditional break except Exception as e: raise AnsibleError("The conditional check '%s' failed. The error was: %s" % (to_native(conditional), to_native(e)), obj=ds) - return result + return result, false_condition def _check_conditional(self, conditional, templar, all_vars): ''' diff --git a/test/integration/targets/callback_default/callback_default.out.result_format_yaml_lossy_verbose.stdout b/test/integration/targets/callback_default/callback_default.out.result_format_yaml_lossy_verbose.stdout index 71a4ef9ebf6..ed455756ba8 100644 --- a/test/integration/targets/callback_default/callback_default.out.result_format_yaml_lossy_verbose.stdout +++ b/test/integration/targets/callback_default/callback_default.out.result_format_yaml_lossy_verbose.stdout @@ -43,6 +43,7 @@ fatal: [testhost]: FAILED! => TASK [Skipped task] ************************************************************ skipping: [testhost] => changed: false + false_condition: false skip_reason: Conditional result was False TASK [Task with var in name (foo bar)] ***************************************** @@ -120,6 +121,7 @@ ok: [testhost] => (item=debug-3) => msg: debug-3 skipping: [testhost] => (item=debug-4) => ansible_loop_var: item + false_condition: item != 4 item: 4 fatal: [testhost]: FAILED! => msg: One or more items failed @@ -199,9 +201,11 @@ skipping: [testhost] => TASK [debug] ******************************************************************* skipping: [testhost] => (item=1) => ansible_loop_var: item + false_condition: false item: 1 skipping: [testhost] => (item=2) => ansible_loop_var: item + false_condition: false item: 2 skipping: [testhost] => msg: All items skipped diff --git a/test/integration/targets/callback_default/callback_default.out.result_format_yaml_verbose.stdout b/test/integration/targets/callback_default/callback_default.out.result_format_yaml_verbose.stdout index 7a99cc74f70..3a121a5f9f1 100644 --- a/test/integration/targets/callback_default/callback_default.out.result_format_yaml_verbose.stdout +++ b/test/integration/targets/callback_default/callback_default.out.result_format_yaml_verbose.stdout @@ -45,6 +45,7 @@ fatal: [testhost]: FAILED! => TASK [Skipped task] ************************************************************ skipping: [testhost] => changed: false + false_condition: false skip_reason: Conditional result was False TASK [Task with var in name (foo bar)] ***************************************** @@ -126,6 +127,7 @@ ok: [testhost] => (item=debug-3) => msg: debug-3 skipping: [testhost] => (item=debug-4) => ansible_loop_var: item + false_condition: item != 4 item: 4 fatal: [testhost]: FAILED! => msg: One or more items failed @@ -206,9 +208,11 @@ skipping: [testhost] => TASK [debug] ******************************************************************* skipping: [testhost] => (item=1) => ansible_loop_var: item + false_condition: false item: 1 skipping: [testhost] => (item=2) => ansible_loop_var: item + false_condition: false item: 2 skipping: [testhost] => msg: All items skipped diff --git a/test/integration/targets/no_log/runme.sh b/test/integration/targets/no_log/runme.sh index bb5c048fc9a..795730bddd7 100755 --- a/test/integration/targets/no_log/runme.sh +++ b/test/integration/targets/no_log/runme.sh @@ -5,7 +5,7 @@ set -eux # This test expects 7 loggable vars and 0 non-loggable ones. # If either mismatches it fails, run the ansible-playbook command to debug. [ "$(ansible-playbook no_log_local.yml -i ../../inventory -vvvvv "$@" | awk \ -'BEGIN { logme = 0; nolog = 0; } /LOG_ME/ { logme += 1;} /DO_NOT_LOG/ { nolog += 1;} END { printf "%d/%d", logme, nolog; }')" = "26/0" ] +'BEGIN { logme = 0; nolog = 0; } /LOG_ME/ { logme += 1;} /DO_NOT_LOG/ { nolog += 1;} END { printf "%d/%d", logme, nolog; }')" = "27/0" ] # deal with corner cases with no log and loops # no log enabled, should produce 6 censored messages diff --git a/test/integration/targets/test_core/tasks/main.yml b/test/integration/targets/test_core/tasks/main.yml index 8c2decbdee9..ac06d67eb00 100644 --- a/test/integration/targets/test_core/tasks/main.yml +++ b/test/integration/targets/test_core/tasks/main.yml @@ -126,6 +126,16 @@ hello: world register: executed_task +- name: Skip me with multiple conditions + set_fact: + hello: world + when: + - True == True + - foo == 'bar' + vars: + foo: foo + register: skipped_task_multi_condition + - name: Try skipped test on non-dictionary set_fact: hello: "{{ 'nope' is skipped }}" @@ -136,8 +146,11 @@ assert: that: - skipped_task is skipped + - skipped_task.false_condition == False - executed_task is not skipped - misuse_of_skipped is failure + - skipped_task_multi_condition is skipped + - skipped_task_multi_condition.false_condition == "foo == 'bar'" - name: Not an async task set_fact: diff --git a/test/units/executor/test_task_executor.py b/test/units/executor/test_task_executor.py index 8576d21b344..47e339939db 100644 --- a/test/units/executor/test_task_executor.py +++ b/test/units/executor/test_task_executor.py @@ -329,6 +329,7 @@ class TestTaskExecutor(unittest.TestCase): # other reason is that if I specify 0 here, the test fails. ;) mock_task.async_val = 1 mock_task.poll = 0 + mock_task.evaluate_conditional_with_result.return_value = (True, None) mock_play_context = MagicMock() mock_play_context.post_validate.return_value = None