From d5a36b8734077de524c1bb090d6179278c1187d6 Mon Sep 17 00:00:00 2001 From: Martin Krizek Date: Mon, 17 Apr 2023 16:23:03 +0200 Subject: Change ansible_job_id format (#79951) (#80282) * Change ansible_job_id format ...to be something that does not resemble float or other type which value could be changed by literal_eval that jinja2_native uses. Specifically the format of '%d.%d' is converted from str to float and then back to float which may result in truncating the number resulting in the job not being found because the job id does not exist. (cherry picked from commit 09e0fb3516a793009a1b2ac8bfcc758016a10c6d) --- changelogs/fragments/fix_jinja_native_async.yml | 2 ++ lib/ansible/executor/powershell/module_manifest.py | 2 +- lib/ansible/plugins/action/__init__.py | 2 +- test/integration/targets/async/tasks/main.yml | 8 ++++---- test/integration/targets/async_fail/tasks/main.yml | 2 +- test/integration/targets/callback_default/runme.sh | 4 ++-- .../integration/targets/win_async_wrapper/tasks/main.yml | 16 ++++++++-------- 7 files changed, 19 insertions(+), 17 deletions(-) create mode 100644 changelogs/fragments/fix_jinja_native_async.yml diff --git a/changelogs/fragments/fix_jinja_native_async.yml b/changelogs/fragments/fix_jinja_native_async.yml new file mode 100644 index 0000000000..5072400390 --- /dev/null +++ b/changelogs/fragments/fix_jinja_native_async.yml @@ -0,0 +1,2 @@ +bugfixes: + - "jinja2_native - fix intermittent 'could not find job' failures when a value of ``ansible_job_id`` from a result of an async task was inadvertently changed during execution; to prevent this a format of ``ansible_job_id`` was changed." diff --git a/lib/ansible/executor/powershell/module_manifest.py b/lib/ansible/executor/powershell/module_manifest.py index 970e8489e4..87e2ce0ad0 100644 --- a/lib/ansible/executor/powershell/module_manifest.py +++ b/lib/ansible/executor/powershell/module_manifest.py @@ -319,7 +319,7 @@ def _create_powershell_wrapper(b_module_data, module_path, module_args, exec_manifest["actions"].insert(0, 'async_watchdog') exec_manifest["actions"].insert(0, 'async_wrapper') - exec_manifest["async_jid"] = str(random.randint(0, 999999999999)) + exec_manifest["async_jid"] = f'j{random.randint(0, 999999999999)}' exec_manifest["async_timeout_sec"] = async_timeout exec_manifest["async_startup_timeout"] = C.config.get_config_value("WIN_ASYNC_STARTUP_TIMEOUT", variables=task_vars) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index d199207cce..436fb65ae8 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -1125,7 +1125,7 @@ class ActionBase(ABC): remote_files.append(remote_async_module_path) async_limit = self._task.async_val - async_jid = str(random.randint(0, 999999999999)) + async_jid = f'j{random.randint(0, 999999999999)}' # call the interpreter for async_wrapper directly # this permits use of a script for an interpreter on non-Linux platforms diff --git a/test/integration/targets/async/tasks/main.yml b/test/integration/targets/async/tasks/main.yml index 05c789e6c9..f5e5c992e7 100644 --- a/test/integration/targets/async/tasks/main.yml +++ b/test/integration/targets/async/tasks/main.yml @@ -122,7 +122,7 @@ - name: assert task failed correctly assert: that: - - async_result.ansible_job_id is match('\d+\.\d+') + - async_result.ansible_job_id is match('j\d+\.\d+') - async_result.finished == 1 - async_result is finished - async_result is not changed @@ -140,7 +140,7 @@ - name: validate response assert: that: - - async_result.ansible_job_id is match('\d+\.\d+') + - async_result.ansible_job_id is match('j\d+\.\d+') - async_result.finished == 1 - async_result is finished - async_result.changed == false @@ -159,7 +159,7 @@ - name: validate response assert: that: - - async_result.ansible_job_id is match('\d+\.\d+') + - async_result.ansible_job_id is match('j\d+\.\d+') - async_result.finished == 1 - async_result is finished - async_result.changed == true @@ -176,7 +176,7 @@ - name: validate response assert: that: - - async_result.ansible_job_id is match('\d+\.\d+') + - async_result.ansible_job_id is match('j\d+\.\d+') - async_result.finished == 1 - async_result is finished - async_result.changed == true diff --git a/test/integration/targets/async_fail/tasks/main.yml b/test/integration/targets/async_fail/tasks/main.yml index 40f72e105d..24cea1d581 100644 --- a/test/integration/targets/async_fail/tasks/main.yml +++ b/test/integration/targets/async_fail/tasks/main.yml @@ -28,7 +28,7 @@ - name: validate that by the end of the retry interval, we succeeded assert: that: - - async_result.ansible_job_id is match('\d+\.\d+') + - async_result.ansible_job_id is match('j\d+\.\d+') - async_result.finished == 1 - async_result is finished - async_result is changed diff --git a/test/integration/targets/callback_default/runme.sh b/test/integration/targets/callback_default/runme.sh index 0ee4259dde..a815132a5c 100755 --- a/test/integration/targets/callback_default/runme.sh +++ b/test/integration/targets/callback_default/runme.sh @@ -135,8 +135,8 @@ run_test default test.yml # Check for async output # NOTE: regex to match 1 or more digits works for both BSD and GNU grep ansible-playbook -i inventory test_async.yml 2>&1 | tee async_test.out -grep "ASYNC OK .* jid=[0-9]\{1,\}" async_test.out -grep "ASYNC FAILED .* jid=[0-9]\{1,\}" async_test.out +grep "ASYNC OK .* jid=j[0-9]\{1,\}" async_test.out +grep "ASYNC FAILED .* jid=j[0-9]\{1,\}" async_test.out rm -f async_test.out # Hide skipped diff --git a/test/integration/targets/win_async_wrapper/tasks/main.yml b/test/integration/targets/win_async_wrapper/tasks/main.yml index 91b45846ed..0fc64d8c5c 100644 --- a/test/integration/targets/win_async_wrapper/tasks/main.yml +++ b/test/integration/targets/win_async_wrapper/tasks/main.yml @@ -12,12 +12,12 @@ - name: validate response assert: that: - - asyncresult.ansible_job_id is match('\d+\.\d+') + - asyncresult.ansible_job_id is match('j\d+\.\d+') - asyncresult.started == 1 - asyncresult is started - asyncresult.finished == 0 - asyncresult is not finished - - asyncresult.results_file is search('\.ansible_async.+\d+\.\d+') + - asyncresult.results_file is search('\.ansible_async.+j\d+\.\d+') # ensure that async is actually async- this test will fail if # hosts > forks or if the target host is VERY slow - (lookup('pipe', 'date +%s') | int) - (start_timestamp | int) < 15 @@ -31,7 +31,7 @@ - name: validate response assert: that: - - asyncresult.ansible_job_id is match('\d+\.\d+') + - asyncresult.ansible_job_id is match('j\d+\.\d+') - asyncresult.finished == 1 - asyncresult is finished - asyncresult is changed @@ -69,7 +69,7 @@ - name: validate response assert: that: - - asyncresult.ansible_job_id is match('\d+\.\d+') + - asyncresult.ansible_job_id is match('j\d+\.\d+') - asyncresult.finished == 1 - asyncresult is finished - asyncresult is changed @@ -107,7 +107,7 @@ - name: validate response assert: that: - - asyncresult.ansible_job_id is match('\d+\.\d+') + - asyncresult.ansible_job_id is match('j\d+\.\d+') - asyncresult.finished == 1 - asyncresult is finished - asyncresult is not changed @@ -125,7 +125,7 @@ - name: validate response assert: that: - - asyncresult.ansible_job_id is match('\d+\.\d+') + - asyncresult.ansible_job_id is match('j\d+\.\d+') - asyncresult.finished == 1 - asyncresult is finished - asyncresult is changed @@ -143,7 +143,7 @@ - name: validate response assert: that: - - asyncresult.ansible_job_id is match('\d+\.\d+') + - asyncresult.ansible_job_id is match('j\d+\.\d+') - asyncresult.finished == 1 - asyncresult is finished - asyncresult is not changed @@ -231,7 +231,7 @@ # - item is finished # - item.slept_sec == 3 # - item is changed -# - item.ansible_job_id is match('\d+\.\d+') +# - item.ansible_job_id is match('j\d+\.\d+') # with_items: "{{ asyncout.results }}" # this part of the test is flaky- Windows PIDs are reused aggressively, so this occasionally fails due to a new process with the same ID -- cgit v1.2.1