diff --git a/changelogs/fragments/async_really_true.yml b/changelogs/fragments/async_really_true.yml new file mode 100644 index 00000000000..5bb092ef94e --- /dev/null +++ b/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``. diff --git a/lib/ansible/executor/powershell/async_watchdog.ps1 b/lib/ansible/executor/powershell/async_watchdog.ps1 index 96439d1d347..391016de563 100644 --- a/lib/ansible/executor/powershell/async_watchdog.ps1 +++ b/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) diff --git a/lib/ansible/executor/powershell/async_wrapper.ps1 b/lib/ansible/executor/powershell/async_wrapper.ps1 index daaa2cd333b..dbd21e98f95 100644 --- a/lib/ansible/executor/powershell/async_wrapper.ps1 +++ b/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 diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py index 061d46e4c5e..872602ada35 100644 --- a/lib/ansible/executor/task_executor.py +++ b/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: diff --git a/lib/ansible/modules/async_status.py b/lib/ansible/modules/async_status.py index a29f3515a8a..26763341e2b 100644 --- a/lib/ansible/modules/async_status.py +++ b/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 diff --git a/lib/ansible/modules/async_wrapper.py b/lib/ansible/modules/async_wrapper.py index 7c2fb257f38..e69ced89ef1 100644 --- a/lib/ansible/modules/async_wrapper.py +++ b/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 diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 6e01885bc7f..5b98d74d235 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/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) diff --git a/lib/ansible/plugins/action/async_status.py b/lib/ansible/plugins/action/async_status.py index a0fe11eb59d..676fc9324ec 100644 --- a/lib/ansible/plugins/action/async_status.py +++ b/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 diff --git a/lib/ansible/plugins/action/gather_facts.py b/lib/ansible/plugins/action/gather_facts.py index cb0e8c741f6..f5185161909 100644 --- a/lib/ansible/plugins/action/gather_facts.py +++ b/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): diff --git a/lib/ansible/plugins/test/core.py b/lib/ansible/plugins/test/core.py index 37fc2173552..1fd743383dd 100644 --- a/lib/ansible/plugins/test/core.py +++ b/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 diff --git a/lib/ansible/plugins/test/finished.yml b/lib/ansible/plugins/test/finished.yml index c83c5a30031..6d582e31e89 100644 --- a/lib/ansible/plugins/test/finished.yml +++ b/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 diff --git a/test/integration/targets/async/tasks/main.yml b/test/integration/targets/async/tasks/main.yml index 66dac18b26b..bdce3dcba9f 100644 --- a/test/integration/targets/async/tasks/main.yml +++ b/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 diff --git a/test/support/windows-integration/plugins/modules/async_status.ps1 b/test/support/windows-integration/plugins/modules/async_status.ps1 index 1ce3ff40f3a..10a8942c348 100644 --- a/test/support/windows-integration/plugins/modules/async_status.ps1 +++ b/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 diff --git a/test/units/plugins/action/test_action.py b/test/units/plugins/action/test_action.py index f99332d334b..5891be5c647 100644 --- a/test/units/plugins/action/test_action.py +++ b/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)