From f8d8d84f05a6dda2811077ec37e4a475c0191a17 Mon Sep 17 00:00:00 2001 From: Jordan Borean Date: Fri, 23 Feb 2018 09:05:13 +1000 Subject: [PATCH] Win feature error cp (#36559) * win_feature: better error handling to make it easier to debug issues (#36491) * win_feature: better error handling to make it easier to debug issues * removed ignroed pslint rules that are no longer needed (cherry picked from commit ef4f8851dc46429622465d9fc24912abc1303271) * Added changelog for win_feature error handling fix --- .../fragments/win_feature-better-errors.yaml | 2 + lib/ansible/modules/windows/win_feature.ps1 | 183 +++++++----------- lib/ansible/modules/windows/win_feature.py | 51 ++--- .../targets/win_feature/tasks/main.yml | 6 +- test/sanity/pslint/ignore.txt | 3 - 5 files changed, 91 insertions(+), 154 deletions(-) create mode 100644 changelogs/fragments/win_feature-better-errors.yaml diff --git a/changelogs/fragments/win_feature-better-errors.yaml b/changelogs/fragments/win_feature-better-errors.yaml new file mode 100644 index 00000000000..ed3b9647b53 --- /dev/null +++ b/changelogs/fragments/win_feature-better-errors.yaml @@ -0,0 +1,2 @@ +bugfixes: +- win_feature - will display a more helpful error when it fails during execution (https://github.com/ansible/ansible/pull/36491) diff --git a/lib/ansible/modules/windows/win_feature.ps1 b/lib/ansible/modules/windows/win_feature.ps1 index e7feabd3307..214560b6878 100644 --- a/lib/ansible/modules/windows/win_feature.ps1 +++ b/lib/ansible/modules/windows/win_feature.ps1 @@ -1,25 +1,12 @@ #!powershell -# This file is part of Ansible. -# -# Copyright 2014, Paul Durivage -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . - -# WANT_JSON -# POWERSHELL_COMMON - -Import-Module Servermanager +# This file is part of Ansible + +# Copyright: (c) 2014, Paul Durivage +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +#Requires -Module Ansible.ModuleUtils.Legacy + +Import-Module -Name ServerManager $result = @{ changed = $false @@ -28,141 +15,107 @@ $result = @{ $params = Parse-Args $args -supports_check_mode $true $check_mode = Get-AnsibleParam -obj $params -name "_ansible_check_mode" -type "bool" -default $false -$name = Get-AnsibleParam -obj $params -name "name" -type "str" -failifempty $true +$name = Get-AnsibleParam -obj $params -name "name" -type "list" -failifempty $true $state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present","absent" # DEPRECATED 2.4, potential removal in 2.6+ $restart = Get-AnsibleParam -obj $params -name "restart" -type "bool" -default $false -$includesubfeatures = Get-AnsibleParam -obj $params -name "include_sub_features" -type "bool" -default $false -$includemanagementtools = Get-AnsibleParam -obj $params -name "include_management_tools" -type "bool" -default $false +$include_sub_features = Get-AnsibleParam -obj $params -name "include_sub_features" -type "bool" -default $false +$include_management_tools = Get-AnsibleParam -obj $params -name "include_management_tools" -type "bool" -default $false $source = Get-AnsibleParam -obj $params -name "source" -type "str" -$name = $name -split ',' | % { $_.Trim() } - If ($restart) { Add-DeprecationWarning -obj $result -message "The 'restart' parameter causes instability. Use the 'win_reboot' action conditionally on 'reboot_required' return value instead" -version 2.6 } -# Determine which cmdlets we need to work with. Then we can set options appropriate for the cmdlet -$installWF= $false -$addWF = $false - -try { - # We can infer uninstall/remove if install/add cmdlets exist - if (Get-Command "Install-WindowsFeature" -ErrorAction SilentlyContinue) { - $addCmdlet = "Install-WindowsFeature" - $removeCmdlet = "Uninstall-WindowsFeature" - $installWF = $true - } - elseif (Get-Command "Add-WindowsFeature" -ErrorAction SilentlyContinue) { - $addCmdlet = "Add-WindowsFeature" - $removeCmdlet = "Remove-WindowsFeature" - $addWF = $true - } - else { - throw [System.Exception] "Not supported on this version of Windows" - } -} -catch { - Fail-Json $result $_.Exception.Message +$install_cmdlet = $false +if (Get-Command -Name Install-WindowsFeature -ErrorAction SilentlyContinue) { + Set-Alias -Name Install-AnsibleWindowsFeature -Value Install-WindowsFeature + Set-Alias -Name Uninstall-AnsibleWindowsFeature -Value Uninstall-WindowsFeature + $install_cmdlet = $true +} elseif (Get-Command -Name Add-WindowsFeature -ErrorAction SilentlyContinue) { + Set-Alias -Name Install-AnsibleWindowsFeature -Value Add-WindowsFeature + Set-Alias -Name Uninstall-AnsibleWindowsFeature -Value Remove-WindowsFeature +} else { + Fail-Json -obj $result -message "This version of Windows does not support the cmdlets Install-WindowsFeature or Add-WindowsFeature" } - -If ($state -eq "present") { - - # Base params to cover both Add/Install-WindowsFeature - $InstallParams = @{ +if ($state -eq "present") { + $install_args = @{ Name = $name Restart = $restart - IncludeAllSubFeature = $includesubfeatures - ErrorAction = "SilentlyContinue" + IncludeAllSubFeature = $include_sub_features WhatIf = $check_mode + ErrorAction = "Stop" } - # IncludeManagementTools and source are options only for Install-WindowsFeature - if ($installWF) { - + if ($install_cmdlet) { + $install_args.IncludeManagementTools = $include_management_tools + $install_args.Confirm = $false if ($source) { if (-not (Test-Path -Path $source)) { - Fail-Json $result "Failed to find source path $source" + Fail-Json -obj $result -message "Failed to find source path $source for feature install" } - - $InstallParams.add("Source",$source) - } - - if ($IncludeManagementTools) { - $InstallParams.add("IncludeManagementTools",$includemanagementtools) + $install_args.Source = $source } } try { - $featureresult = Invoke-Expression "$addCmdlet @InstallParams" - } - catch { - Fail-Json $result $_.Exception.Message + $action_results = Install-AnsibleWindowsFeature @install_args + } catch { + Fail-Json -obj $result -message "Failed to install Windows Feature: $($_.Exception.Message)" } -} -ElseIf ($state -eq "absent") { - - $UninstallParams = @{ +} else { + $uninstall_args = @{ Name = $name Restart = $restart - ErrorAction = "SilentlyContinue" WhatIf = $check_mode + ErrorAction = "Stop" + } + if ($install_cmdlet) { + $uninstall_args.IncludeManagementTools = $include_management_tools } try { - $featureresult = Invoke-Expression "$removeCmdlet @UninstallParams" - } - catch { - Fail-Json $result $_.Exception.Message + $action_results = Uninstall-AnsibleWindowsFeature @uninstall_args + } catch { + Fail-Json -obj $result -message "Failed to uninstall Windows Feature: $($_.Exception.Message)" } } # Loop through results and create a hash containing details about # each role/feature that is installed/removed -$installed_features = @() -#$featureresult.featureresult is filled if anything was changed -If ($featureresult.FeatureResult) -{ - ForEach ($item in $featureresult.FeatureResult) { - $message = @() - ForEach ($msg in $item.Message) { - $message += @{ - message_type = $msg.MessageType.ToString() - error_code = $msg.ErrorCode - text = $msg.Text - } - } - $installed_features += @{ - id = $item.Id - display_name = $item.DisplayName - message = $message - reboot_required = $item.RestartNeeded.ToString() | ConvertTo-Bool - skip_reason = $item.SkipReason.ToString() - success = $item.Success.ToString() | ConvertTo-Bool - - # DEPRECATED 2.4, potential removal in 2.6+ (standardize naming to "reboot_required") - restart_needed = $item.RestartNeeded.ToString() | ConvertTo-Bool +# $action_results.FeatureResult is not empty if anything was changed +$feature_results = @() +foreach ($action_result in $action_results.FeatureResult) { + $message = @() + foreach ($msg in $action_result.Message) { + $message += @{ + message_type = $msg.MessageType.ToString() + error_code = $msg.ErrorCode + text = $msg.Text } } + + $feature_results += @{ + id = $action_result.Id + display_name = $action_result.DisplayName + message = $message + reboot_required = ConvertTo-Bool -obj $action_result.RestartNeeded + skip_reason = $action_result.SkipReason.ToString() + success = ConvertTo-Bool -obj $action_result.Success + restart_needed = ConvertTo-Bool -obj $action_result.RestartNeeded + } $result.changed = $true } - -$result.feature_result = $installed_features -$result.success = ($featureresult.Success.ToString() | ConvertTo-Bool) -$result.exitcode = $featureresult.ExitCode.ToString() -$result.reboot_required = ($featureresult.RestartNeeded.ToString() | ConvertTo-Bool) +$result.feature_result = $feature_results +$result.success = ConvertTo-Bool -obj $action_results.Success +$result.exitcode = $action_results.ExitCode.ToString() +$result.reboot_required = ConvertTo-Bool -obj $action_results.RestartNeeded +# controls whether Ansible will fail or not +$result.failed = (-not $action_results.Success) # DEPRECATED 2.4, potential removal in 2.6+ (standardize naming to "reboot_required") $result.restart_needed = $result.reboot_required -If ($result.success) { - Exit-Json $result -} -ElseIf ($state -eq "present") { - Fail-Json $result "Failed to add feature" -} -Else { - Fail-Json $result "Failed to remove feature" -} +Exit-Json -obj $result diff --git a/lib/ansible/modules/windows/win_feature.py b/lib/ansible/modules/windows/win_feature.py index 66164acd72d..fce6b65f1df 100644 --- a/lib/ansible/modules/windows/win_feature.py +++ b/lib/ansible/modules/windows/win_feature.py @@ -1,22 +1,10 @@ #!/usr/bin/python # -*- coding: utf-8 -*- -# (c) 2014, Paul Durivage , Trond Hindenes and others -# # This file is part of Ansible -# -# Ansible is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# Ansible is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Ansible. If not, see . + +# (c) 2014, Paul Durivage , Trond Hindenes and others +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) # this is a windows documentation stub. actual code lives in the .ps1 # file of the same name @@ -32,16 +20,16 @@ module: win_feature version_added: "1.7" short_description: Installs and uninstalls Windows Features on Windows Server description: - - Installs or uninstalls Windows Roles or Features on Windows Server. This module uses the Add/Remove-WindowsFeature Cmdlets on Windows 2008 + - Installs or uninstalls Windows Roles or Features on Windows Server. This module uses the Add/Remove-WindowsFeature Cmdlets on Windows 2008 R2 and Install/Uninstall-WindowsFeature Cmdlets on Windows 2012, which are not available on client os machines. options: name: description: - - Names of roles or features to install as a single feature or a comma-separated list of features + - Names of roles or features to install as a single feature or a comma-separated list of features. required: true state: description: - - State of the features or roles on the system + - State of the features or roles on the system. choices: - present - absent @@ -51,27 +39,24 @@ options: - Restarts the computer automatically when installation is complete, if restarting is required by the roles or features installed. - DEPRECATED in Ansible 2.4, as unmanaged reboots cause numerous issues under Ansible. Check the C(reboot_required) return value from this module to determine if a reboot is necessary, and if so, use the M(win_reboot) action to perform it. - choices: - - yes - - no + type: bool + default: 'no' include_sub_features: description: - - Adds all subfeatures of the specified feature - choices: - - yes - - no + - Adds all subfeatures of the specified feature. + type: bool + default: 'no' include_management_tools: description: - Adds the corresponding management tools to the specified feature. - - Not supported in Windows 2008. If present when using Windows 2008 this option will be ignored. - choices: - - yes - - no + - Not supported in Windows 2008 R2 and will be ignored. + type: bool + default: 'no' source: description: - Specify a source to install the feature from. - - Not supported in Windows 2008. If present when using Windows 2008 this option will be ignored. - choices: [ ' {driveletter}:\sources\sxs', ' {IP}\Share\sources\sxs' ] + - Not supported in Windows 2008 R2 and will be ignored. + - Can either be C({driveletter}:\sources\sxs) or C(\\{IP}\share\sources\sxs). version_added: "2.1" author: - "Paul Durivage (@angstwad)" @@ -86,7 +71,9 @@ EXAMPLES = r''' - name: Install IIS (Web-Server and Web-Common-Http) win_feature: - name: Web-Server,Web-Common-Http + name: + - Web-Server + - Web-Common-Http state: present - name: Install NET-Framework-Core from file diff --git a/test/integration/targets/win_feature/tasks/main.yml b/test/integration/targets/win_feature/tasks/main.yml index 806f30f95be..b8f3f55b7cd 100644 --- a/test/integration/targets/win_feature/tasks/main.yml +++ b/test/integration/targets/win_feature/tasks/main.yml @@ -127,8 +127,7 @@ that: - "win_feature_install_invalid_result is failed" - "win_feature_install_invalid_result is not changed" - - "win_feature_install_invalid_result.msg" - - "win_feature_install_invalid_result.exitcode == 'InvalidArgs'" + - "'The name was not found' in win_feature_install_invalid_result.msg" when: win_feature_has_servermanager is successful - name: try to remove an invalid feature name @@ -144,6 +143,5 @@ that: - "win_feature_remove_invalid_result is failed" - "win_feature_remove_invalid_result is not changed" - - "win_feature_remove_invalid_result.msg" - - "win_feature_remove_invalid_result.exitcode == 'InvalidArgs'" + - "'The name was not found' in win_feature_remove_invalid_result.msg" when: win_feature_has_servermanager is successful diff --git a/test/sanity/pslint/ignore.txt b/test/sanity/pslint/ignore.txt index 7f54725681b..06abd52e9e4 100644 --- a/test/sanity/pslint/ignore.txt +++ b/test/sanity/pslint/ignore.txt @@ -64,9 +64,6 @@ lib/ansible/modules/windows/win_dsc.ps1 PSAvoidUsingEmptyCatchBlock lib/ansible/modules/windows/win_dsc.ps1 PSPossibleIncorrectComparisonWithNull lib/ansible/modules/windows/win_dsc.ps1 PSUseApprovedVerbs lib/ansible/modules/windows/win_eventlog.ps1 PSUseDeclaredVarsMoreThanAssignments -lib/ansible/modules/windows/win_feature.ps1 PSAvoidUsingCmdletAliases -lib/ansible/modules/windows/win_feature.ps1 PSAvoidUsingInvokeExpression -lib/ansible/modules/windows/win_feature.ps1 PSUseDeclaredVarsMoreThanAssignments lib/ansible/modules/windows/win_file.ps1 PSPossibleIncorrectComparisonWithNull lib/ansible/modules/windows/win_file_version.ps1 PSUseBOMForUnicodeEncodedFile lib/ansible/modules/windows/win_find.ps1 PSAvoidUsingEmptyCatchBlock