async handling changed to use 'true' booleans(#85074)

async_status and async_wrrapper now use booleans instead of 0/1 for 'thruthyness'
gather_facts also updated

Co-authored-by: 🇺🇦 Sviatoslav Sydorenko (Святослав Сидоренко) <wk.cvs.github@sydorenko.org.ua>
Co-authored-by: Matt Clay <matt@mystile.com>
(cherry picked from commit d41a3430b7)

changelogs/fragments/async_really_true.yml

@@ -0,0 +1,2 @@
+bugfixes:
+- async_status module - The ``started`` and ``finished`` return values are now ``True`` or ``False`` instead of ``1`` or ``0``.

lib/ansible/executor/powershell/async_watchdog.ps1

@@ -64,7 +64,7 @@ $jobError = $null
 try {
     $jobAsyncResult = $ps.BeginInvoke($pipelineInput, $invocationSettings, $null, $null)
     $jobAsyncResult.AsyncWaitHandle.WaitOne($Timeout * 1000) > $null
-    $result.finished = 1
+    $result.finished = $true
 
     if ($jobAsyncResult.IsCompleted) {
         $jobOutput = $ps.EndInvoke($jobAsyncResult)

lib/ansible/executor/powershell/async_wrapper.ps1

@@ -135,8 +135,8 @@ try {
     # We need to write the result file before the process is started to ensure
     # it can read the file.
     $result = @{
-        started = 1
-        finished = 0
+        started = $true
+        finished = $false
         results_file = $resultsPath
         ansible_job_id = $localJid
         _ansible_suppress_tmpdir_delete = $true

lib/ansible/executor/task_executor.py

@@ -910,9 +910,9 @@ class TaskExecutor:
                 # have issues which result in a half-written/unparseable result
                 # file on disk, which manifests to the user as a timeout happening
                 # before it's time to timeout.
-                if (int(async_result.get('finished', 0)) == 1 or
-                        ('failed' in async_result and async_result.get('_ansible_parsed', False)) or
-                        'skipped' in async_result):
+                if (async_result.get('finished', False) or
+                   (async_result.get('failed', False) and async_result.get('_ansible_parsed', False)) or
+                   async_result.get('skipped', False)):
                     break
             except Exception as e:
                 # Connections can raise exceptions during polling (eg, network bounce, reboot); these should be non-fatal.
@@ -941,7 +941,7 @@ class TaskExecutor:
                     ),
                 )
 
-        if int(async_result.get('finished', 0)) != 1:
+        if not async_result.get('finished', False):
             if async_result.get('_ansible_parsed'):
                 return dict(failed=True, msg="async task did not complete within the requested time - %ss" % self._task.async_val, async_result=async_result)
             else:

lib/ansible/modules/async_status.py

@@ -28,6 +28,8 @@ options:
     type: str
     choices: [ cleanup, status ]
     default: status
+notes:
+  - The RV(started) and RV(finished) return values were updated to return V(True) or V(False) instead of V(1) or V(0) in ansible-core 2.19.
 extends_documentation_fragment:
 - action_common_attributes
 - action_common_attributes.flow
@@ -85,15 +87,15 @@ ansible_job_id:
   type: str
   sample: '360874038559.4169'
 finished:
-  description: Whether the asynchronous job has finished (V(1)) or not (V(0))
+  description: Whether the asynchronous job has finished or not
   returned: always
-  type: int
-  sample: 1
+  type: bool
+  sample: true
 started:
-  description: Whether the asynchronous job has started (V(1)) or not (V(0))
+  description: Whether the asynchronous job has started or not
   returned: always
-  type: int
-  sample: 1
+  type: bool
+  sample: true
 stdout:
   description: Any output returned by async_wrapper
   returned: always
@@ -134,7 +136,7 @@ def main():
     log_path = os.path.join(async_dir, jid)
 
     if not os.path.exists(log_path):
-        module.fail_json(msg="could not find job", ansible_job_id=jid, started=1, finished=1)
+        module.fail_json(msg="could not find job", ansible_job_id=jid, started=True, finished=True)
 
     if mode == 'cleanup':
         os.unlink(log_path)
@@ -151,16 +153,16 @@ def main():
     except Exception:
         if not data:
             # file not written yet?  That means it is running
-            module.exit_json(results_file=log_path, ansible_job_id=jid, started=1, finished=0)
+            module.exit_json(results_file=log_path, ansible_job_id=jid, started=True, finished=False)
         else:
             module.fail_json(ansible_job_id=jid, results_file=log_path,
-                             msg="Could not parse job output: %s" % data, started=1, finished=1)
+                             msg="Could not parse job output: %s" % data, started=True, finished=True)
 
     if 'started' not in data:
-        data['finished'] = 1
+        data['finished'] = True
         data['ansible_job_id'] = jid
     elif 'finished' not in data:
-        data['finished'] = 0
+        data['finished'] = False
 
     # just write the module output directly to stdout and exit; bypass other processing done by exit_json since it's already been done
     print(f"\n{json.dumps(data)}")  # pylint: disable=ansible-bad-function

lib/ansible/modules/async_wrapper.py

@@ -149,7 +149,7 @@ def _run_module(wrapped_cmd, jid):
 
     # DTFIX-FUTURE: needs rework for serialization profiles
 
-    jwrite({"started": 1, "finished": 0, "ansible_job_id": jid})
+    jwrite({"started": True, "finished": False, "ansible_job_id": jid})
 
     result = {}
 
@@ -203,7 +203,7 @@ def _run_module(wrapped_cmd, jid):
     except (OSError, IOError):
         e = sys.exc_info()[1]
         result = {
-            "failed": 1,
+            "failed": True,
             "cmd": wrapped_cmd,
             "msg": to_text(e),
             "outdata": outdata,  # temporary notice only
@@ -214,7 +214,7 @@ def _run_module(wrapped_cmd, jid):
 
     except (ValueError, Exception):
         result = {
-            "failed": 1,
+            "failed": True,
             "cmd": wrapped_cmd,
             "data": outdata,  # temporary notice only
             "stderr": stderr,
@@ -260,7 +260,7 @@ def main():
         _make_temp_dir(jobdir)
     except Exception as e:
         end({
-            "failed": 1,
+            "failed": True,
             "msg": "could not create directory: %s - %s" % (jobdir, to_text(e)),
             "exception": to_text(traceback.format_exc()),  # NB: task executor compat will coerce to the correct dataclass type
         }, 1)
@@ -293,7 +293,7 @@ def main():
                     continue
 
             notice("Return async_wrapper task started.")
-            end({"failed": 0, "started": 1, "finished": 0, "ansible_job_id": jid, "results_file": job_path,
+            end({"failed": False, "started": True, "finished": False, "ansible_job_id": jid, "results_file": job_path,
                  "_ansible_suppress_tmpdir_delete": (not preserve_tmp)}, 0)
         else:
             # The actual wrapper process

lib/ansible/plugins/action/__init__.py

@@ -119,8 +119,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin):
 
         * Module parameters.  These are stored in self._task.args
         """
-
-        # does not default to {'changed': False, 'failed': False}, as it breaks async
+        # does not default to {'changed': False, 'failed': False}, as it used to break async
         result = {}
 
         if tmp is not None:
@@ -1113,7 +1112,7 @@ class ActionBase(ABC, _AnsiblePluginInfoMixin):
         if wrap_async and not self._connection.always_pipeline_modules:
             # configure, upload, and chmod the async_wrapper module
             (async_module_bits, async_module_path) = self._configure_module(module_name='ansible.legacy.async_wrapper', module_args=dict(), task_vars=task_vars)
-            (async_module_style, shebang, async_module_data) = (async_module_bits.module_style, async_module_bits.shebang, async_module_bits.b_module_data)
+            (shebang, async_module_data) = (async_module_bits.shebang, async_module_bits.b_module_data)
             async_module_remote_filename = self._connection._shell.get_remote_filename(async_module_path)
             remote_async_module_path = self._connection._shell.join_path(tmpdir, async_module_remote_filename)
             self._transfer_data(remote_async_module_path, async_module_data)

lib/ansible/plugins/action/async_status.py

@@ -28,7 +28,7 @@ class ActionModule(ActionBase):
         )
 
         # initialize response
-        results['started'] = results['finished'] = 0
+        results['started'] = results['finished'] = False
         results['stdout'] = results['stderr'] = ''
         results['stdout_lines'] = results['stderr_lines'] = []
 
@@ -43,9 +43,14 @@ class ActionModule(ActionBase):
             results['erased'] = log_path
         else:
             results['results_file'] = log_path
-            results['started'] = 1
+            results['started'] = True
 
         new_module_args['_async_dir'] = async_dir
         results = merge_hash(results, self._execute_module(module_name='ansible.legacy.async_status', task_vars=task_vars, module_args=new_module_args))
 
+        # Backwards compat shim for when started/finished were ints,
+        # mostly to work with ansible.windows.async_status
+        for convert in ('started', 'finished'):
+            results[convert] = bool(results[convert])
+
         return results

lib/ansible/plugins/action/gather_facts.py

@@ -157,7 +157,7 @@ class ActionModule(ActionBase):
                 for module in jobs:
                     poll_args = {'jid': jobs[module]['ansible_job_id'], '_async_dir': os.path.dirname(jobs[module]['results_file'])}
                     res = self._execute_module(module_name='ansible.legacy.async_status', module_args=poll_args, task_vars=task_vars, wrap_async=False)
-                    if res.get('finished', 0) == 1:
+                    if res.get('finished', False):
                         if res.get('failed', False):
                             failed[module] = res
                         elif res.get('skipped', False):

lib/ansible/plugins/test/core.py

@@ -116,7 +116,6 @@ def started(result):
 
     if 'started' in result:
         # For async tasks, return status
-        # NOTE: The value of started is 0 or 1, not False or True :-/
         return bool(result.get('started'))
     else:
         # For non-async tasks, warn user, but return as if started
@@ -131,7 +130,6 @@ def finished(result):
 
     if 'finished' in result:
         # For async tasks, return status
-        # NOTE: The value of finished is 0 or 1, not False or True :-/
         return bool(result.get('finished'))
     else:
         # For non-async tasks, warn user, but return as if finished

lib/ansible/plugins/test/finished.yml

@@ -5,7 +5,7 @@ DOCUMENTATION:
   short_description: Did async task finish
   description:
     - Used to test if an async task has finished, it will also work with normal tasks but will issue a warning.
-    - This test checks for the existence of a C(finished) key in the input dictionary and that it is V(1) if present
+    - This test checks for the existence of a C(finished) key in the input dictionary and that it is V(True) if present
   options:
     _input:
       description: registered result from an Ansible task

test/integration/targets/async/tasks/main.yml

@@ -94,8 +94,7 @@
 - name: assert task was successfully started
   assert:
     that:
-    - fnf_task.started == 1
-    - fnf_task is started
+    - fnf_task.started
     - "'ansible_job_id' in fnf_task"
 
 - name: 'check on task started as a "fire-and-forget"'
@@ -123,10 +122,9 @@
   assert:
     that:
     - async_result.ansible_job_id is match('j\d+\.\d+')
-    - async_result.finished == 1
-    - async_result is finished
-    - async_result is not changed
-    - async_result is failed
+    - async_result.finished
+    - async_result.changed is false
+    - async_result.failed
     - async_result.msg == 'failed gracefully'
 
 - name: test exception module failure
@@ -143,9 +141,10 @@
     - async_result.ansible_job_id is match('j\d+\.\d+')
     - async_result.finished == 1
     - async_result is finished
-    - async_result.changed == false
     - async_result is not changed
-    - async_result.failed == true
+    - async_result.finished
+    - async_result.changed is false
+    - async_result.failed
     - async_result is failed
     - async_result.msg is contains 'failing via exception'
     # DTFIX5: enable tracebacks, ensure exception is populated
@@ -162,11 +161,9 @@
   assert:
     that:
     - async_result.ansible_job_id is match('j\d+\.\d+')
-    - async_result.finished == 1
-    - async_result is finished
-    - async_result.changed == true
-    - async_result is changed
-    - async_result is successful
+    - async_result.finished
+    - async_result.changed
+    - async_result.failed is false
 
 - name: test trailing junk after JSON
   async_test:
@@ -179,11 +176,9 @@
   assert:
     that:
     - async_result.ansible_job_id is match('j\d+\.\d+')
-    - async_result.finished == 1
-    - async_result is finished
-    - async_result.changed == true
-    - async_result is changed
-    - async_result is successful
+    - async_result.finished
+    - async_result.changed
+    - async_result.failed is false
     - async_result.warnings[0] is search('trailing junk after module output')
 
 - name: test stderr handling

test/support/windows-integration/plugins/modules/async_status.ps1

@@ -17,7 +17,7 @@ $log_path = [System.IO.Path]::Combine($async_dir, $jid)
 
 If(-not $(Test-Path $log_path))
 {
-    Fail-Json @{ansible_job_id=$jid; started=1; finished=1} "could not find job at '$async_dir'"
+    Fail-Json @{ansible_job_id=$jid; started=$true; finished=$true} "could not find job at '$async_dir'"
 }
 
 If($mode -eq "cleanup") {
@@ -48,11 +48,11 @@ Catch {
 }
 
 If (-not $data.ContainsKey("started")) {
-    $data['finished'] = 1
+    $data['finished'] = $true
     $data['ansible_job_id'] = $jid
 }
 ElseIf (-not $data.ContainsKey("finished")) {
-    $data['finished'] = 0
+    $data['finished'] = $false
 }
 
 Exit-Json $data

test/units/plugins/action/test_action.py

@@ -107,7 +107,7 @@ class TestActionBase(unittest.TestCase):
         mock_task.async_val = None
         action_base = DerivedActionBase(mock_task, mock_connection, play_context, None, None, None)
         results = action_base.run()
-        self.assertEqual(results, dict())
+        self.assertEqual(results, {})
 
         mock_task.async_val = 0
         action_base = DerivedActionBase(mock_task, mock_connection, play_context, None, None, None)