Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows become fails on tasks that ignore/catch any errors #30468

Closed
jborean93 opened this issue Sep 17, 2017 · 2 comments · Fixed by #31227
Closed

Windows become fails on tasks that ignore/catch any errors #30468

jborean93 opened this issue Sep 17, 2017 · 2 comments · Fixed by #31227
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Projects
Milestone

Comments

@jborean93
Copy link
Contributor

jborean93 commented Sep 17, 2017

ISSUE TYPE
  • Bug Report
COMPONENT NAME

win_become

ANSIBLE VERSION
2.4
CONFIGURATION

Default

OS / ENVIRONMENT

Ansible Controller - MacOS
Ansible Remote - Windows (Server 2016 PSv5 and 2012 R2 PSv4 confirmed with the issue)

SUMMARY

When running with become on Windows and the module being called throws any error that is caught or ignored with -ErrorAction SilentlyContinue it will still fail with an RC value of 1.

This seems to be due to the $ps.HadErrors at https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/shell/powershell.py#L138 is being set to true if any errors at all occurred in the execution.

According to https://msdn.microsoft.com/en-us/library/system.management.automation.powershell.haderrors(v=vs.85).aspx HasErrors means Gets a value that indicates whether an error occurred while executing the pipeline. It seems like this also include errors that are caught in try {} catch {} or cmdlet calls with -ErrorAction SilentlyContinue when I would have assumed they would ignore them.

In the example below I am using the setup module as it contains the following block where Get-Command will throw the exception but is ultimately caught will set HadErrors to true.

# See if Facter is on the System Path
Try {
    $facter_exe = Get-Command facter -ErrorAction Stop
    $facter_installed = $true
} Catch {
    $facter_installed = $false
}

You can also verify it manually by running the following code through powershell on a Windows box.

$ps = [Powershell]::Create()
$ps.AddStatement().AddScript("Get-Command invalid -ErrorAction SilentlyContinue") | Out-Null
$output = $ps.Invoke()

$ps.HadErrors

Lastly you can simply a test module to produce this situation with the following module code

#!powershell
#Requires -Module Ansible.ModuleUtils.Legacy

Get-Service -Name Test -ErrorAction SilentlyContinue

Exit-Json @{changed=$false}

It seems like a return code of 1 is being set for normal (non-become) situations but the return code value is not used for error detection here unless it cannot parse the output as a JSON. The become wrapper actually checks the return code from the process and throws an exception if it doesn't equal 0.

STEPS TO REPRODUCE

Run the following command and target a WIndows host
ansible windows -m setup --become -e ansible_become_method=runas -e ansible_become_user=Administrator -e ansible_become_password=Password01

EXPECTED RESULTS

A successful run without any errors

ACTUAL RESULTS

The module will fail with the error

An exception occurred during task execution. To see the full traceback, use -vvv. The error was: system":"Win32NT"}}
Server2016 | FAILED! => {
    "changed": false,
    "failed": true,
    "msg": "failed, rc was 1, stderr was \r\n, stdout was (insert output json here)

Note: the stdout value is the actual output json containing all the facts, has been omitted for brevity here.

PROPOSED SOLUTION

Use the following diff to only set the error code if the error steam contains at least 1 entry. I haven't had too much time to determine if this would have any other negative impacts but from an initial look it seems fine.

diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py
index 185697388..b8bf24490 100644
--- a/lib/ansible/plugins/shell/powershell.py
+++ b/lib/ansible/plugins/shell/powershell.py
@@ -135,7 +135,7 @@ Function Run($payload) {
     $output
 
     # PS3 doesn't properly set HadErrors in many cases, inspect the error stream as a fallback
-    If ($ps.HadErrors -or ($PSVersionTable.PSVersion.Major -lt 4 -and $ps.Streams.Error.Count -gt 0)) {
+    If ($ps.Streams.Error.Count -gt 0) {
         [System.Console]::Error.WriteLine($($ps.Streams.Error | Out-String))
         $exit_code = $ps.Runspace.SessionStateProxy.GetVariable("LASTEXITCODE")
         If(-not $exit_code) {
@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bug_report needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 17, 2017
@jborean93 jborean93 changed the title Windows become fails on tasks that throw any errors Windows become fails on tasks that throw any ignored/catch errors Sep 17, 2017
@jborean93 jborean93 added the windows Windows community label Sep 17, 2017
@jborean93 jborean93 changed the title Windows become fails on tasks that throw any ignored/catch errors Windows become fails on tasks that ignore/catch any errors Sep 17, 2017
@gundalow gundalow removed the needs_triage Needs a first human triage before being processed. label Sep 20, 2017
@jborean93
Copy link
Contributor Author

Seems to be related to PowerShell/PowerShell#4613.

@jborean93
Copy link
Contributor Author

Merged into devel and cherry-picked to stable-2.4 39196a7. Will be available in the 2.4.1 beta 2.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 bug This issue/PR relates to a bug. support:core This issue/PR relates to code supported by the Ansible Engineering Team. windows Windows community
Projects
No open projects
2.5
Done
Development

Successfully merging a pull request may close this issue.

3 participants