win_wait_for_process: Fixes and integration tests (#44801)

* win_wait_for_process: Add integration tests

* Disable reporting changes

* Added more tests checking PID

* Various improvements

This PR includes:
- Use Get-Process instead of CIM Win32_Process
- Rewrite of process filter logic (speedup)
- Fix error messages
- Fixes to documentation, examples and return output

* win_wait_for_process: Limit to PowerShell 4 and higher

* Improve RESULT documentation

* Last minute fixes for CI

* Catch Powershell exceptions

* Increase timeout to make tests more stable

lib/ansible/modules/windows/win_wait_for_process.ps1

@@ -1,60 +1,65 @@
 #!powershell
-# This file is part of Ansible
 
-# Copyright (c) 2017 Ansible Project
+# Copyright: (c) 2017, Ansible Project
+# Copyright: (c) 2018, Dag Wieers (@dagwieers) <dag@wieers.com>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 #Requires -Module Ansible.ModuleUtils.Legacy
-#Requires -Module Ansible.ModuleUtils.FileUtil
+#Requires -Module Ansible.ModuleUtils.SID
 
 $ErrorActionPreference = "Stop"
 
+# NOTE: Ensure we get proper debug information when things fall over
+trap {
+    if ($null -eq $result) { $result = @{} }
+    $result.exception = "$($_ | Out-String)`r`n$($_.ScriptStackTrace)"
+    Fail-Json -obj $result -message "Uncaught exception: $($_.Exception.Message)"
+}
+
 $params = Parse-Args -arguments $args -supports_check_mode $true
 
-$process_name_exact = Get-AnsibleParam -obj $params -name "process_name_exact" -type "list" 
+$process_name_exact = Get-AnsibleParam -obj $params -name "process_name_exact" -type "list"
-$process_name_pattern = Get-AnsibleParam -obj $params -name "process_name_pattern" -type "str" 
+$process_name_pattern = Get-AnsibleParam -obj $params -name "process_name_pattern" -type "str"
-$process_id = Get-AnsibleParam -obj $params -name "pid" -type "int" -default 0 #pid is a reserved variable in PowerShell.  use process_id instead.
+$process_id = Get-AnsibleParam -obj $params -name "pid" -type "int" -default 0  # pid is a reserved variable in PowerShell, using process_id instead.
 $owner = Get-AnsibleParam -obj $params -name "owner" -type "str"
 $sleep = Get-AnsibleParam -obj $params -name "sleep" -type "int" -default 1
 $pre_wait_delay = Get-AnsibleParam -obj $params -name "pre_wait_delay" -type "int" -default 0
 $post_wait_delay = Get-AnsibleParam -obj $params -name "post_wait_delay" -type "int" -default 0
 $process_min_count = Get-AnsibleParam -obj $params -name "process_min_count" -type "int" -default 1
-$state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "present","absent"
+$state = Get-AnsibleParam -obj $params -name "state" -type "str" -default "present" -validateset "absent","present"
 $timeout = Get-AnsibleParam -obj $params -name "timeout" -type "int" -default 300
 
 $result = @{
     changed = $false
+    elapsed = 0
+    matched_processes = @()
 }
 
-# validate the input
+# Validate the input
-if ($state -eq "absent" -and $sleep -ne 1)
+if ($state -eq "absent" -and $sleep -ne 1) {
-{
+    Add-Warning -obj $result -message "Parameter 'sleep' has no effect when waiting for a process to stop."
-    Add-Warning $result "sleep parameter has no effect when waiting for a process to stop."
 }
 
-if ($state -eq "absent" -and $process_min_count -ne 1)
+if ($state -eq "absent" -and $process_min_count -ne 1) {
-{
+    Add-Warning -obj $result -message "Parameter 'process_min_count' has no effect when waiting for a process to stop."
-    Add-Warning $result "process_min_count parameter has no effect when waiting for a process to stop."
 }
 
-if (($process_name_exact -or $process_name_pattern) -and $process_id)
+if (($process_name_exact -or $process_name_pattern) -and $process_id) {
-{
+    Fail-Json -obj $result -message "Parameter 'pid' may not be used with process_name_exact or process_name_pattern."
-    Fail-json $result "process_id may not be used with process_name_exact or process_name_pattern."
 }
-if ($process_name_exact -and $process_name_pattern)
+if ($process_name_exact -and $process_name_pattern) {
-{
+    Fail-Json -obj $result -message "Parameter 'process_name_exact' and 'process_name_pattern' may not be used at the same time."
-    Fail-json $result "process_name_exact and process_name_pattern may not be used at the same time."
 }
 
-if (-not ($process_name_exact -or $process_name_pattern -or $process_id -or $owner))
+if (-not ($process_name_exact -or $process_name_pattern -or $process_id -or $owner)) {
-{
+    Fail-Json -obj $result -message "At least one of 'process_name_exact', 'process_name_pattern', 'pid' or 'owner' must be supplied."
-    Fail-json $result "at least one of: process_name_exact, process_name_pattern, process_id, or owner must be supplied."
 }
 
-$module_start = Get-Date
+if ($owner -and ("IncludeUserName" -notin (Get-Command -Name Get-Process).Parameters.Keys)) {
+    Fail-Json -obj $result -message "This version of Powershell does not support filtering processes by 'owner'."
+}
 
-#Get-Process doesn't actually return a UserName value, so get it from WMI.
+Function Get-FilteredProcesses {
-Function Get-ProcessMatchesFilter {
     [cmdletbinding()]
     Param(
         [String]
@@ -64,99 +69,106 @@ Function Get-ProcessMatchesFilter {
         [int]
         $ProcessId
     )
-    
+
-    $CIMProcesses = Get-CimInstance Win32_Process
+    $FilteredProcesses = @()
-    foreach ($CIMProcess in $CIMProcesses)
+
-    {
+    try {
-        $include = $true
+        $Processes = Get-Process -IncludeUserName
-        if(-not [String]::IsNullOrEmpty($ProcessNamePattern))
+        $SupportsUserNames = $true
-        {
+    } catch [System.Management.Automation.ParameterBindingException] {
-            #if a process name was specified in the filter, validate that here.
+        $Processes = Get-Process
-            $include = $include -and ($CIMProcess.ProcessName -match $ProcessNamePattern)
+        $SupportsUserNames = $false
-        }
+    }
-        if($ProcessNameExact -is [Array] -or (-not [String]::IsNullOrEmpty($ProcessNameExact)))
+
-        {
+    foreach ($Process in $Processes) {
-            #if a process name was specified in the filter, validate that here.
+
-            if ($ProcessNameExact -is [Array] )
+        # If a process name was specified in the filter, validate that here.
-            {
+        if ($ProcessNamePattern) {
-                $include = $include -and ($ProcessNameExact -contains $CIMProcess.ProcessName)
+            if ($Process.ProcessName -notmatch $ProcessNamePattern) {
-            }   
+                continue
-            else {
-                $include = $include -and ($ProcessNameExact -eq $CIMProcess.ProcessName)
             }
         }
-        if ($ProcessId -and $ProcessId -ne 0)
+
-        {
+        # If a process name was specified in the filter, validate that here.
-            # if a PID was specified in the filger, validate that here.
+        if ($ProcessNameExact -is [Array]) {
-            $include = $include -and ($CIMProcess.ProcessId -eq $ProcessId)
+            if ($ProcessNameExact -notcontains $Process.ProcessName) {
+                continue
+            }
+        } elseif ($ProcessNameExact) {
+            if ($ProcessNameExact -ne $Process.ProcessName) {
+                continue
+            }
         }
-        if (-not [String]::IsNullOrEmpty($Owner) )
+
-        {
+        # If a PID was specified in the filter, validate that here.
-            # if an owner was specified in the filter, validate that here.
+        if ($ProcessId -and $ProcessId -ne 0) {
-            $include = $include -and ($($(Invoke-CimMethod -InputObject $CIMProcess -MethodName GetOwner).User) -eq $Owner)
+            if ($ProcessId -ne $Process.Id) {
+                continue
+            }
         }
-        
+
-        if ($include)
+        # If an owner was specified in the filter, validate that here.
-        {
+        if ($Owner) {
-            $CIMProcess | Select-Object -Property ProcessId, ProcessName, @{name="Owner";Expression={$($(Invoke-CimMethod -InputObject $CIMProcess -MethodName GetOwner).User)}}
+            if (-not $Process.UserName) {
+                continue
+            } elseif ((Convert-ToSID($Owner)) -ne (Convert-ToSID($Process.UserName))) {  # NOTE: This is rather expensive
+                continue
+            }
+        }
+
+        if ($SupportsUserNames -eq $true) {
+            $FilteredProcesses += @{ name = $Process.ProcessName; pid = $Process.Id; owner = $Process.UserName }
+        } else {
+            $FilteredProcesses += @{ name = $Process.ProcessName; pid = $Process.Id }
         }
     }
+
+    return ,$FilteredProcesses
 }
 
+$module_start = Get-Date
 Start-Sleep -Seconds $pre_wait_delay
+
 if ($state -eq "present" ) {
-    #wait for a process to start
+
-    $Processes = @()
+    # Wait for a process to start
-    $attempts = 0
+    do {
-    Do {
+
-        if (((Get-Date) - $module_start).TotalSeconds -gt $timeout)
+        $Processes = Get-FilteredProcesses -Owner $owner -ProcessNameExact $process_name_exact -ProcessNamePattern $process_name_pattern -ProcessId $process_id
-        {
+        $result.matched_processes = $Processes
-            $result.elapsed = ((Get-Date) - $module_start).TotalSeconds
+
-            Fail-Json $result "timeout while waiting for $process_name to start.  waited $timeout seconds"
+        if ($Processes.count -ge $process_min_count) {
+            break
         }
-       
+
-        $Processes = Get-ProcessMatchesFilter -Owner $owner -ProcessNameExact $process_name_exact -ProcessNamePattern $process_name_pattern -ProcessId $process_id
+        if (((Get-Date) - $module_start).TotalSeconds -gt $timeout) {
+            $result.elapsed = ((Get-Date) - $module_start).TotalSeconds
+            Fail-Json -obj $result -message "Timed out while waiting for process(es) to start"
+        }
+
         Start-Sleep -Seconds $sleep
-        $attempts ++
-        $ProcessCount = $null
-        if ($Processes -is [array]) { 
-            $ProcessCount = $Processes.count 
-        } 
-        elseif ($null -ne $Processes) { 
-            $ProcessCount = 1 
-        }
-        else {
-            $ProcessCount = 0
-        }
-    } While ($ProcessCount -lt $process_min_count)
 
-    if ($attempts -gt 0)
+    } while ($true)
-    {
+
-        $result.changed = $true
+} elseif ($state -eq "absent") {
-    }
+
-    $result.matched_processess = $Processes
+    # Wait for a process to stop
-}
+    $Processes = Get-FilteredProcesses -Owner $owner -ProcessNameExact $process_name_exact -ProcessNamePattern $process_name_pattern -ProcessId $process_id
-elseif ($state -eq "absent") {
-    #wait for a process to stop
-    $Processes = Get-ProcessMatchesFilter -Owner $owner -ProcessNameExact $process_name_exact -ProcessNamePattern $process_name_pattern -ProcessId $process_id 
     $result.matched_processes = $Processes
-    $ProcessCount = $(if ($Processes -is [array]) { $Processes.count } elseif ($Processes){ 1 } else {0})
-    if ($ProcessCount -gt 0 )
-    {
-        try { 
-            Wait-Process -Id $($Processes | Select-Object -ExpandProperty ProcessId) -Timeout $timeout -ErrorAction Stop
-            $result.changed = $true
-        }
-        catch {
-            $result.elapsed = ((Get-Date) - $module_start).TotalSeconds
-            Fail-Json $result "$($_.Exception.Message). timeout while waiting for $process_name to stop.  waited $timeout seconds"
-        }
-    }
-    else{
-        $result.changed = $false
 
+    if ($Processes.count -gt 0 ) {
+        try {
+            # This may randomly fail when used on specially protected processes (think: svchost)
+            Wait-Process -Id $Processes.pid -Timeout $timeout
+        } catch [System.TimeoutException] {
+            $result.elapsed = ((Get-Date) - $module_start).TotalSeconds
+            Fail-Json -obj $result -message "Timeout while waiting for process(es) to stop"
+        }
     }
+
 }
+
 Start-Sleep -Seconds $post_wait_delay
 $result.elapsed = ((Get-Date) - $module_start).TotalSeconds
-Exit-Json $result
+
+Exit-Json -obj $result

lib/ansible/modules/windows/win_wait_for_process.py

@@ -17,29 +17,50 @@ module: win_wait_for_process
 version_added: '2.7'
 short_description: Waits for a process to exist or not exist before continuing.
 description:
-- Waiting for a process to start or stop is useful when Windows services
+- Waiting for a process to start or stop.
-  behave poorly and do not enumerate external dependencies in their
+- This is useful when Windows services behave poorly and do not enumerate external dependencies in their manifest.
-  manifest.
 options:
   process_name_exact:
     description:
-      - The name of the process(es) for which to wait.
+    - The name of the process(es) for which to wait.
-      - Must inclue the file extension of the process binary (.exe)
+    type: str
   process_name_pattern:
-     description:
+    description:
-      - RegEx pattern matching desired process(es)
+    - RegEx pattern matching desired process(es).
+    type: str
   sleep:
     description:
     - Number of seconds to sleep between checks.
     - Only applies when waiting for a process to start.  Waiting for a process to start
       does not have a native non-polling mechanism. Waiting for a stop uses native PowerShell
       and does not require polling.
+    type: int
     default: 1
   process_min_count:
     description:
       - Minimum number of process matching the supplied pattern to satisfy C(present) condition.
       - Only applies to C(present).
+    type: int
     default: 1
+  pid:
+    description:
+    - The PID of the process.
+    type: int
+  owner:
+    description:
+    - The owner of the process.
+    - Requires PowerShell version 4.0 or newer.
+    type: str
+  pre_wait_delay:
+    description:
+    - Seconds to wait before checking processes.
+    type: int
+    default: 0
+  post_wait_delay:
+    description:
+    - Seconds to wait after checking for processes.
+    type: int
+    default: 0
   state:
     description:
     - When checking for a running process C(present) will block execution
@@ -52,12 +73,14 @@ options:
     - If, while waiting for C(absent), new processes matching the supplied
       pattern are started, these new processes will not be included in the
       action.
+    type: str
     default: present
-    choices: [ absent,  present ]
+    choices: [ absent, present ]
   timeout:
     description:
     - The maximum number of seconds to wait for a for a process to start or stop
       before erroring out.
+    type: int
     default: 300
 author:
 - Charles Crossan (@crossan007)
@@ -66,34 +89,43 @@ author:
 EXAMPLES = r'''
 - name: Wait 300 seconds for all Oracle VirtualBox processes to stop. (VBoxHeadless, VirtualBox, VBoxSVC)
   win_wait_for_process:
-    process_name: "v(irtual)?box(headless|svc)?"
+    process_name: 'v(irtual)?box(headless|svc)?'
     state: absent
     timeout: 500
 
-
 - name: Wait 300 seconds for 3 instances of cmd to start, waiting 5 seconds between each check
   win_wait_for_process:
-    process_name: "cmd\\.exe"
+    process_name_exact: cmd
     state: present
     timeout: 500
     sleep: 5
     process_min_count: 3
-
 '''
 
 RETURN = r'''
 elapsed:
-  description: The elapsed seconds between the start of poll and the end of the
+  description: The elapsed seconds between the start of poll and the end of the module.
-    module.
   returned: always
   type: float
   sample: 3.14159265
-changed:
-  description: True if a process was started or stopped during the module execution.
-  returned: always
-  type: bool
 matched_processes:
-  description: Count of processes stopped or started.
+  description: List of matched processes (either stopped or started)
   returned: always
-  type: int
+  type: complex
+  contains:
+    name:
+      description: The name of the matched process
+      returned: always
+      type: str
+      sample: svchost
+    owner:
+      description: The owner of the matched process
+      returned: when supported by PowerShell
+      type: str
+      sample: NT AUTHORITY\SYSTEM
+    pid:
+      description: The PID of the matched process
+      returned: always
+      type: int
+      sample: 7908
 '''

test/integration/targets/win_wait_for_process/aliases

@@ -0,0 +1 @@
+shippable/windows/group4

test/integration/targets/win_wait_for_process/tasks/main.yml

@@ -0,0 +1,200 @@
+---
+- name: Get powershell version
+  win_shell: $PSVersionTable.PSVersion.Major
+  register: powershell_version
+
+- name: Ensure Spooler service is started
+  win_service:
+    name: Spooler
+    state: started
+
+- name: Wait for non-existing process to not exist
+  win_wait_for_process:
+    process_name_exact:
+    - ansible_foobar
+    timeout: 30
+    state: absent
+  register: absent_nonexisting_process
+
+- assert:
+    that:
+    - absent_nonexisting_process is success
+    - absent_nonexisting_process is not changed
+    - absent_nonexisting_process.elapsed > 0
+    - absent_nonexisting_process.elapsed < 30
+    - absent_nonexisting_process.matched_processes|length == 0
+
+- name: Wait for non-existing process until timeout
+  win_wait_for_process:
+    process_name_exact: ansible_foobar
+    timeout: 30
+    state: present
+  ignore_errors: yes
+  register: present_nonexisting_process
+
+- assert:
+    that:
+    - present_nonexisting_process is failed
+    - present_nonexisting_process is not changed
+    - present_nonexisting_process.elapsed > 30
+    - present_nonexisting_process.msg == 'Timed out while waiting for process(es) to start'
+    - present_nonexisting_process.matched_processes|length == 0
+
+- name: Wait for existing process to exist
+  win_wait_for_process:
+    process_name_exact: spoolsv
+    timeout: 30
+    state: present
+  register: present_existing_process
+
+- assert:
+    that:
+    - present_existing_process is success
+    - present_existing_process is not changed
+    - present_existing_process.elapsed > 0
+    - present_existing_process.elapsed < 30
+    - present_existing_process.matched_processes|length > 0
+
+- name: Wait for existing process until timeout
+  win_wait_for_process:
+    process_name_exact:
+    - spoolsv
+    timeout: 30
+    state: absent
+  ignore_errors: yes
+  register: absent_existing_process
+
+- assert:
+    that:
+    - absent_existing_process is failed
+    - absent_existing_process is not changed
+    - absent_existing_process.elapsed > 30
+    - absent_existing_process.matched_processes|length > 0
+    - absent_existing_process.msg == 'Timeout while waiting for process(es) to stop'
+
+- name: Wait for existing process to exist (using owner)
+  win_wait_for_process:
+    process_name_exact: spoolsv
+    owner: SYSTEM
+    timeout: 30
+    state: present
+  ignore_errors: yes
+  register: present_existing_owner_process
+
+- assert:
+    that:
+    - present_existing_owner_process is success
+    - present_existing_owner_process is not changed
+    - present_existing_owner_process.elapsed > 0
+    - present_existing_owner_process.elapsed < 30
+    - present_existing_owner_process.matched_processes|length > 0
+  when: powershell_version.stdout_lines[0]|int >= 4
+
+- assert:
+    that:
+    - present_existing_owner_process is failed
+    - present_existing_owner_process is not changed
+    - present_existing_owner_process.elapsed == 0
+    - present_existing_owner_process.matched_processes|length == 0
+    - present_existing_owner_process.msg ==  "This version of Powershell does not support filtering processes by 'owner'."
+  when: powershell_version.stdout_lines[0]|int < 4
+
+- name: Wait for Spooler service to stop
+  win_wait_for_process:
+    process_name_exact:
+    - spoolsv
+    timeout: 60
+    state: absent
+  async: 30
+  poll: 0
+  register: spoolsv_process
+
+- name: Stop the Spooler service
+  win_service:
+    name: Spooler
+    force_dependent_services: yes
+    state: stopped
+
+- name: Check on async task
+  async_status:
+    jid: '{{ spoolsv_process.ansible_job_id }}'
+  until: absent_spoolsv_process is finished
+  retries: 20
+  register: absent_spoolsv_process
+
+- assert:
+    that:
+    - absent_spoolsv_process is success
+    - absent_spoolsv_process is not changed
+    - absent_spoolsv_process is finished
+    - absent_spoolsv_process.elapsed > 0
+    - absent_spoolsv_process.elapsed < 30
+    - absent_spoolsv_process.matched_processes|length == 1
+
+- name: Wait for Spooler service to start
+  win_wait_for_process:
+    process_name_exact: spoolsv
+    timeout: 60
+    state: present
+  async: 60
+  poll: 0
+  register: spoolsv_process
+
+- name: Start the spooler service
+  win_service:
+    name: Spooler
+    force_dependent_services: yes
+    state: started
+
+- name: Check on async task
+  async_status:
+    jid: '{{ spoolsv_process.ansible_job_id }}'
+  until: present_spoolsv_process is finished
+  retries: 10
+  register: present_spoolsv_process
+
+- assert:
+    that:
+    - present_spoolsv_process is success
+    - present_spoolsv_process is not changed
+    - present_spoolsv_process is finished
+    - present_spoolsv_process.elapsed > 0
+    - present_spoolsv_process.elapsed < 60
+    - present_spoolsv_process.matched_processes|length == 1
+
+- name: Start a new long-running process
+  win_shell: |
+    Start-Sleep -Seconds 15
+  async: 40
+  poll: 0
+  register: sleep_pid
+
+- name: Wait for PID to start
+  win_wait_for_process:
+    pid: '{{ sleep_pid.ansible_async_watchdog_pid }}'
+    timeout: 20
+    state: present
+  register: present_sleep_pid
+
+- assert:
+    that:
+    - present_sleep_pid is success
+    - present_sleep_pid is not changed
+    - present_sleep_pid.elapsed > 0
+    - present_sleep_pid.elapsed < 15
+    - present_sleep_pid.matched_processes|length == 1
+
+- name: Wait for PID to stop
+  win_wait_for_process:
+    pid: '{{ sleep_pid.ansible_async_watchdog_pid }}'
+    timeout: 20
+    state: absent
+  register: absent_sleep_pid
+
+- assert:
+    that:
+    - absent_sleep_pid is success
+    - absent_sleep_pid is not changed
+    - absent_sleep_pid.elapsed > 0
+    - absent_sleep_pid.elapsed < 15
+    - absent_sleep_pid.matched_processes|length == 1