diff options
5 files changed, 69 insertions, 1 deletions
diff --git a/changelogs/fragments/powershell-module-error-handling.yml b/changelogs/fragments/powershell-module-error-handling.yml new file mode 100644 index 0000000000..cdc40e36ef --- /dev/null +++ b/changelogs/fragments/powershell-module-error-handling.yml @@ -0,0 +1,4 @@ +bugfixes: +- >- + powershell modules - Only set an rc of 1 if the PowerShell pipeline signaled an error occurred AND there are error + records present. Previously it would do so only if the error signal was present without checking the error count. diff --git a/lib/ansible/executor/powershell/module_wrapper.ps1 b/lib/ansible/executor/powershell/module_wrapper.ps1 index 20a967731b..1cfaf3ceae 100644 --- a/lib/ansible/executor/powershell/module_wrapper.ps1 +++ b/lib/ansible/executor/powershell/module_wrapper.ps1 @@ -207,7 +207,10 @@ if ($null -ne $rc) { # with the trap handler that's now in place, this should only write to the output if # $ErrorActionPreference != "Stop", that's ok because this is sent to the stderr output # for a user to manually debug if something went horribly wrong -if ($ps.HadErrors -or ($PSVersionTable.PSVersion.Major -lt 4 -and $ps.Streams.Error.Count -gt 0)) { +if ( + $ps.Streams.Error.Count -and + ($ps.HadErrors -or $PSVersionTable.PSVersion.Major -lt 4) +) { Write-AnsibleLog "WARN - module had errors, outputting error info $ModuleName" "module_wrapper" # if the rc wasn't explicitly set, we return an exit code of 1 if ($null -eq $rc) { diff --git a/test/integration/targets/win_exec_wrapper/action_plugins/test_rc_1.py b/test/integration/targets/win_exec_wrapper/action_plugins/test_rc_1.py new file mode 100644 index 0000000000..60cffde96b --- /dev/null +++ b/test/integration/targets/win_exec_wrapper/action_plugins/test_rc_1.py @@ -0,0 +1,35 @@ +# Copyright: (c) 2023, Ansible Project +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +import json + +from ansible.plugins.action import ActionBase + + +class ActionModule(ActionBase): + + def run(self, tmp=None, task_vars=None): + super().run(tmp, task_vars) + del tmp + + exec_command = self._connection.exec_command + + def patched_exec_command(*args, **kwargs): + rc, stdout, stderr = exec_command(*args, **kwargs) + + new_stdout = json.dumps({ + "rc": rc, + "stdout": stdout.decode(), + "stderr": stderr.decode(), + "failed": False, + "changed": False, + }).encode() + + return (0, new_stdout, b"") + + try: + # This is done to capture the raw rc/stdio from the module exec + self._connection.exec_command = patched_exec_command + return self._execute_module(task_vars=task_vars) + finally: + self._connection.exec_command = exec_command diff --git a/test/integration/targets/win_exec_wrapper/library/test_rc_1.ps1 b/test/integration/targets/win_exec_wrapper/library/test_rc_1.ps1 new file mode 100644 index 0000000000..a9879548fa --- /dev/null +++ b/test/integration/targets/win_exec_wrapper/library/test_rc_1.ps1 @@ -0,0 +1,17 @@ +#!powershell + +# This scenario needs to use Legacy, the same HadErrors won't be set if using +# Ansible.Basic +#Requires -Module Ansible.ModuleUtils.Legacy + +# This will set `$ps.HadErrors` in the running pipeline but with no error +# record written. We are testing that it won't set the rc to 1 for this +# scenario. +try { + Write-Error -Message err -ErrorAction Stop +} +catch { + Exit-Json @{} +} + +Fail-Json @{} "This should not be reached" diff --git a/test/integration/targets/win_exec_wrapper/tasks/main.yml b/test/integration/targets/win_exec_wrapper/tasks/main.yml index 8fc54f7ca8..f1342c480b 100644 --- a/test/integration/targets/win_exec_wrapper/tasks/main.yml +++ b/test/integration/targets/win_exec_wrapper/tasks/main.yml @@ -272,3 +272,12 @@ assert: that: - ps_log_count.stdout | int == 0 + +- name: test module that sets HadErrors with no error records + test_rc_1: + register: module_had_errors + +- name: assert test module that sets HadErrors with no error records + assert: + that: + - module_had_errors.rc == 0 |