diff --git a/changelogs/fragments/skipping.yml b/changelogs/fragments/skipping.yml new file mode 100644 index 00000000000..6b88b3e0bc4 --- /dev/null +++ b/changelogs/fragments/skipping.yml @@ -0,0 +1,6 @@ +bugfixes: + - For a skipped task, the results will now contain both ``skip_reason`` and ``skipped_reason`` for all cases of skipping a task. + Until now it used one or the other depending on the situation, this will ensure consistent returns from now on. + +deprecated_features: + - The ``skipped_reason`` present in the results from a skipped task will be removed, use ``skip_reason`` instead. diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index a9fa2c22110..fb541745c39 100644 --- a/lib/ansible/executor/task_executor.py +++ b/lib/ansible/executor/task_executor.py @@ -147,7 +147,7 @@ class TaskExecutor: if res['skipped']: res['msg'] = 'All items skipped' else: - res = dict(changed=False, skipped=True, skipped_reason='No items in the list', results=[]) + res = dict(changed=False, skipped=True, skip_reason='No items in the list', results=[]) else: display.debug("calling self._execute()") res = self._execute(self._task_templar, self._job_vars) diff --git a/lib/ansible/executor/task_result.py b/lib/ansible/executor/task_result.py index b973a6a2796..53eeb712dbb 100644 --- a/lib/ansible/executor/task_result.py +++ b/lib/ansible/executor/task_result.py @@ -10,10 +10,11 @@ import functools import typing as t from ansible import constants -from ansible.utils import vars as _vars -from ansible.vars.clean import module_response_deepcopy, strip_internal_keys -from ansible.module_utils._internal import _messages from ansible._internal import _collection_proxy +from ansible.module_utils._internal import _messages, _deprecator +from ansible.module_utils._internal._datatag import _tags +from ansible.vars.clean import module_response_deepcopy, strip_internal_keys +from ansible.utils import vars as _vars if t.TYPE_CHECKING: from ansible.inventory.host import Host @@ -31,6 +32,13 @@ CLEAN_EXCEPTIONS = ( '_ansible_verbose_override', # controls display of ansible_facts, gathering would be very noise with -v otherwise ) +_DEPRECATE_SKIPPED_REASON = _tags.Deprecated( + msg='skipped_reason` is deprecated due to duplication.', + version='2.25', + deprecator=_deprecator.ANSIBLE_CORE_DEPRECATOR, + help_text='Use `skip_reason` instead.', +) + @t.final @dataclasses.dataclass(frozen=True, kw_only=True, slots=True) @@ -56,6 +64,12 @@ class _BaseTaskResult: self._return_data = return_data # FIXME: this should be immutable, but strategy result processing mutates it in some corner cases self.__task_fields = task_fields + # core now sends both for now, deprecate access to skipped_reason + skipped = return_data.get('skipped_reason', return_data.get('skip_reason', None)) + if skipped is not None: + return_data['skip_reason'] = skipped + return_data['skipped_reason'] = _DEPRECATE_SKIPPED_REASON.tag(skipped) + @property def host(self) -> Host: """The host associated with this result.""" diff --git a/lib/ansible/plugins/action/debug.py b/lib/ansible/plugins/action/debug.py index 55016e5b0b5..f23965b016e 100644 --- a/lib/ansible/plugins/action/debug.py +++ b/lib/ansible/plugins/action/debug.py @@ -83,7 +83,7 @@ class ActionModule(ActionBase): replacing_behavior.emit_warnings() else: - result['skipped_reason'] = "Verbosity threshold not met." + result['skip_reason'] = "Verbosity threshold not met." result['skipped'] = True result['failed'] = False diff --git a/lib/ansible/plugins/callback/__init__.py b/lib/ansible/plugins/callback/__init__.py index 944606f26ff..171eb942117 100644 --- a/lib/ansible/plugins/callback/__init__.py +++ b/lib/ansible/plugins/callback/__init__.py @@ -172,7 +172,7 @@ class CallbackBase(AnsiblePlugin): self.set_options(options) self._hide_in_debug = ( - 'changed', 'failed', 'skipped', 'invocation', 'skip_reason', + 'changed', 'failed', 'skipped', 'invocation', 'skip_reason', 'skipped_reason', 'ansible_loop_var', 'ansible_index_var', 'ansible_loop', ) 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 ce3d86dedad..d5e8afdce31 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 @@ -53,6 +53,7 @@ skipping: [testhost] => changed: false false_condition: false skip_reason: Conditional result was False + skipped_reason: Conditional result was False TASK [Task with var in name (foo bar)] ***************************************** changed: [testhost] => @@ -215,11 +216,11 @@ ok: [testhost] => TASK [debug] ******************************************************************* skipping: [testhost] => - skipped_reason: No items in the list + {} TASK [debug] ******************************************************************* skipping: [testhost] => - skipped_reason: No items in the list + {} TASK [debug] ******************************************************************* skipping: [testhost] => (item=1) => 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 9ec927558d7..c46df4d7d95 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 @@ -55,6 +55,7 @@ skipping: [testhost] => changed: false false_condition: false skip_reason: Conditional result was False + skipped_reason: Conditional result was False TASK [Task with var in name (foo bar)] ***************************************** changed: [testhost] => @@ -222,11 +223,11 @@ ok: [testhost] => TASK [debug] ******************************************************************* skipping: [testhost] => - skipped_reason: No items in the list + {} TASK [debug] ******************************************************************* skipping: [testhost] => - skipped_reason: No items in the list + {} TASK [debug] ******************************************************************* skipping: [testhost] => (item=1) => diff --git a/test/integration/targets/results/aliases b/test/integration/targets/results/aliases new file mode 100644 index 00000000000..498fedd558e --- /dev/null +++ b/test/integration/targets/results/aliases @@ -0,0 +1,2 @@ +shippable/posix/group4 +context/controller diff --git a/test/integration/targets/results/library/return_skipkey.sh b/test/integration/targets/results/library/return_skipkey.sh new file mode 100755 index 00000000000..028e6f16a11 --- /dev/null +++ b/test/integration/targets/results/library/return_skipkey.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +echo '{"skipped": true, "failed": false, "skip_reason": "skipped for test"}' diff --git a/test/integration/targets/results/library/return_skippedkey.sh b/test/integration/targets/results/library/return_skippedkey.sh new file mode 100755 index 00000000000..179722a2855 --- /dev/null +++ b/test/integration/targets/results/library/return_skippedkey.sh @@ -0,0 +1,3 @@ +#!/bin/sh + +echo '{"skipped": true, "failed": false, "skipped_reason": "skipped for test"}' diff --git a/test/integration/targets/results/tasks/main.yml b/test/integration/targets/results/tasks/main.yml new file mode 100644 index 00000000000..6b74e25f37f --- /dev/null +++ b/test/integration/targets/results/tasks/main.yml @@ -0,0 +1,34 @@ +- name: check for skipped keys + block: + - name: skip me + debug: + when: false + register: skipped + + - name: check for keys + assert: + that: + - ((skipped.keys() | intersect(['skip_reason', 'skipped_reason'])) | length) == 2 + + - name: check for keys + debug: + msg: '{{ skipped[item] }}' + loop: ['skip_reason', 'skipped_reason'] + + - name: skip me2 + return_skipkey: + register: skip_key + + - name: check for keys + assert: + that: + - ((skipped.keys() | intersect(['skip_reason', 'skipped_reason'])) | length) == 2 + + - name: skip me3 + return_skipkey: + register: skipped_key + + - name: check for keys + assert: + that: + - ((skipped.keys() | intersect(['skip_reason', 'skipped_reason'])) | length) == 2 diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt index bb296a812d9..8db52b9b18d 100644 --- a/test/sanity/ignore.txt +++ b/test/sanity/ignore.txt @@ -94,6 +94,8 @@ test/integration/targets/module_precedence/roles_with_extension/foo/library/a.in test/integration/targets/module_precedence/roles_with_extension/foo/library/ping.ini shebang test/integration/targets/old_style_modules_posix/library/helloworld.sh shebang test/integration/targets/template/files/encoding_1252_utf-8.expected no-smart-quotes +test/integration/targets/results/library/return_skipkey.sh shebang +test/integration/targets/results/library/return_skippedkey.sh shebang test/integration/targets/template/files/encoding_1252_utf-8.expected no-unwanted-characters test/integration/targets/template/files/encoding_1252_windows-1252.expected no-smart-quotes test/integration/targets/template/files/encoding_1252_windows-1252.expected no-unwanted-characters