Merge 6a31cea55c into 7b4d4ed672

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.

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)

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."""

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

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',
         )
 

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)  => 

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)  => 

test/integration/targets/results/aliases

@@ -0,0 +1,2 @@
+shippable/posix/group4
+context/controller

test/integration/targets/results/library/return_skipkey.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+echo '{"skipped": true, "failed": false, "skip_reason": "skipped for test"}'

test/integration/targets/results/library/return_skippedkey.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+echo '{"skipped": true, "failed": false, "skipped_reason": "skipped for test"}'

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

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