windows async changes and tests (#17400)

lib/ansible/plugins/action/__init__.py

@@ -254,10 +254,15 @@ class ActionBase(with_metaclass(ABCMeta, object)):
 
         return rc
 
+    def _should_remove_tmp_path(self, tmp_path):
+        '''Determine if temporary path should be deleted or kept by user request/config'''
+
+        return tmp_path and self._cleanup_remote_tmp and not C.DEFAULT_KEEP_REMOTE_FILES and "-tmp-" in tmp_path
+
     def _remove_tmp_path(self, tmp_path):
         '''Remove a temporary path we created. '''
 
-        if tmp_path and self._cleanup_remote_tmp and not C.DEFAULT_KEEP_REMOTE_FILES and "-tmp-" in tmp_path:
+        if self._should_remove_tmp_path(tmp_path):
             cmd = self._connection._shell.remove(tmp_path, recurse=True)
             # If we have gotten here we have a working ssh configuration.
             # If ssh breaks we could leave tmp directories out on the remote system.

lib/ansible/plugins/action/async.py

@@ -46,8 +46,8 @@ class ActionModule(ActionBase):
             self._cleanup_remote_tmp=True
 
         module_name = self._task.action
-        async_module_path  = self._connection._shell.join_path(tmp, 'async_wrapper')
+
-        remote_module_path = self._connection._shell.join_path(tmp, module_name)
+
 
         env_string = self._compute_environment_string()
 
@@ -57,14 +57,18 @@ class ActionModule(ActionBase):
 
         # configure, upload, and chmod the target module
         (module_style, shebang, module_data, module_path) = self._configure_module(module_name=module_name, module_args=module_args, task_vars=task_vars)
+        remote_module_filename = self._connection._shell.get_remote_filename(module_path)
+        remote_module_path = self._connection._shell.join_path(tmp, remote_module_filename)
         if module_style == 'binary':
             self._transfer_file(module_path, remote_module_path)
         else:
             self._transfer_data(remote_module_path, module_data)
 
         # configure, upload, and chmod the async_wrapper module
-        (async_module_style, shebang, async_module_data, _) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars)
+        (async_module_style, shebang, async_module_data, async_module_path) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars)
-        self._transfer_data(async_module_path, async_module_data)
+        async_module_remote_filename = self._connection._shell.get_remote_filename(async_module_path)
+        remote_async_module_path = self._connection._shell.join_path(tmp, async_module_remote_filename)
+        self._transfer_data(remote_async_module_path, async_module_data)
 
         argsfile = None
         if module_style in ('non_native_want_json', 'binary'):
@@ -75,7 +79,7 @@ class ActionModule(ActionBase):
                 args_data += '%s="%s" ' % (k, pipes.quote(to_unicode(v)))
             argsfile = self._transfer_data(self._connection._shell.join_path(tmp, 'arguments'), args_data)
 
-        remote_paths = tmp, remote_module_path, async_module_path
+        remote_paths = tmp, remote_module_path, remote_async_module_path
 
         # argsfile doesn't need to be executable, but this saves an extra call to the remote host
         if argsfile:
@@ -86,25 +90,33 @@ class ActionModule(ActionBase):
         async_limit = self._task.async
         async_jid   = str(random.randint(0, 999999999999))
 
-        async_cmd = [env_string, async_module_path, async_jid, async_limit, remote_module_path]
+        async_cmd = [env_string, remote_async_module_path, async_jid, async_limit, remote_module_path]
         if argsfile:
             async_cmd.append(argsfile)
         else:
             # maintain a fixed number of positional parameters for async_wrapper
             async_cmd.append('_')
+
+        if not self._should_remove_tmp_path(tmp):
+            async_cmd.append("-preserve_tmp")
+
         async_cmd = " ".join([to_unicode(x) for x in async_cmd])
         result.update(self._low_level_execute_command(cmd=async_cmd))
 
-        # clean up after
-        self._remove_tmp_path(tmp)
-
         result['changed'] = True
 
+        # the async_wrapper module returns dumped JSON via its stdout
+        # response, so we (attempt to) parse it here
+        parsed_result = self._parse_returned_data(result)
+
+        # Delete tmpdir from controller unless async_wrapper says something else will do it.
+        # Windows cannot request deletion of files/directories that are in use, so the async
+        # supervisory process has to be responsible for it.
+        if parsed_result.get("_suppress_tmpdir_delete", False) != True:
+            self._remove_tmp_path(tmp)
+
+        # just return the original result
         if 'skipped' in result and result['skipped'] or 'failed' in result and result['failed']:
             return result
 
-        # the async_wrapper module returns dumped JSON via its stdout
+        return parsed_result
-        # response, so we parse it here and replace the result
-        result = self._parse_returned_data(result)
-
-        return result

test/integration/roles/test_win_async_wrapper/library/async_test.ps1

@@ -0,0 +1,57 @@
+#!powershell
+# 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 <http://www.gnu.org/licenses/>.
+
+# WANT_JSON
+# POWERSHELL_COMMON
+
+$parsed_args = Parse-Args $args
+
+$sleep_delay_sec = Get-AnsibleParam $parsed_args "sleep_delay_sec" -default 0
+$fail_mode = Get-AnsibleParam $parsed_args "fail_mode" -default "success" -validateset "success","graceful","exception"
+
+If($fail_mode -isnot [array]) {
+    $fail_mode = @($fail_mode)
+}
+
+$result = @{changed=$true; module_pid=$pid; module_tempdir=$PSScriptRoot}
+
+If($sleep_delay_sec -gt 0) {
+    Sleep -Seconds $sleep_delay_sec
+    $result["slept_sec"] = $sleep_delay_sec
+}
+
+If($fail_mode -contains "leading_junk") {
+    Write-Output "leading junk before module output"
+}
+
+Try {
+
+    If($fail_mode -contains "graceful") {
+        Fail-Json $result "failed gracefully"
+    }
+
+    If($fail_mode -eq "exception") {
+        Throw "failing via exception"
+    }
+
+    Exit-Json $result
+}
+Finally
+{
+    If($fail_mode -contains "trailing_junk") {
+        Write-Output "trailing junk after module output"
+    }
+}

test/integration/roles/test_win_async_wrapper/tasks/main.yml

@@ -0,0 +1,187 @@
+- name: async fire and forget
+  async_test:
+    sleep_delay_sec: 5
+  async: 20
+  poll: 0
+  register: asyncresult
+
+- name: validate response
+  assert:
+    that:
+    - asyncresult.ansible_job_id is match('\d+\.\d+')
+    - asyncresult.started == 1
+    - asyncresult.finished == 0
+    - asyncresult.results_file is search('\.ansible_async.+\d+\.\d+')
+    - asyncresult._suppress_tmpdir_delete == true
+
+- name: async poll immediate success
+  async_test:
+    sleep_delay_sec: 0
+  async: 10
+  poll: 1
+  register: asyncresult
+
+- name: validate response
+  assert:
+    that:
+    - asyncresult.ansible_job_id is match('\d+\.\d+')
+    - asyncresult.finished == 1
+    - asyncresult.changed == true
+    - asyncresult.ansible_async_watchdog_pid is number
+    - asyncresult.module_tempdir is search('ansible-tmp-')
+    - asyncresult.module_pid is number
+
+# 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
+# FUTURE: consider having the test module hook to a kernel object we can poke at that gets signaled/released on exit
+#- name: ensure that watchdog and module procs have exited
+#  raw: Get-Process | Where { $_.Id -in ({{ asyncresult.ansible_async_watchdog_pid }}, {{ asyncresult.module_pid }}) }
+#  register: proclist
+#
+#- name: validate no running watchdog/module processes were returned
+#  assert:
+#    that:
+#    - proclist.stdout.strip() == ''
+
+- name: ensure that module_tempdir was deleted
+  raw: Test-Path {{ asyncresult.module_tempdir }}
+  register: tempdircheck
+
+- name: validate tempdir response
+  assert:
+    that:
+    - tempdircheck.stdout | search('False')
+
+- name: async poll retry
+  async_test:
+    sleep_delay_sec: 5
+  async: 10
+  poll: 1
+  register: asyncresult
+
+- name: validate response
+  assert:
+    that:
+    - asyncresult.ansible_job_id is match('\d+\.\d+')
+    - asyncresult.finished == 1
+    - asyncresult.changed == true
+    - asyncresult.module_tempdir is search('ansible-tmp-')
+    - asyncresult.module_pid is number
+
+# 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
+# FUTURE: consider having the test module hook to a kernel object we can poke at that gets signaled/released on exit
+#- name: ensure that watchdog and module procs have exited
+#  raw: Get-Process | Where { $_.Id -in ({{ asyncresult.ansible_async_watchdog_pid }}, {{ asyncresult.module_pid }}) }
+#  register: proclist
+#
+#- name: validate no running watchdog/module processes were returned
+#  assert:
+#    that:
+#    - proclist.stdout.strip() == ''
+
+- name: ensure that module_tempdir was deleted
+  raw: Test-Path {{ asyncresult.module_tempdir }}
+  register: tempdircheck
+
+- name: validate tempdir response
+  assert:
+    that:
+    - tempdircheck.stdout | search('False')
+
+- name: async poll timeout
+  async_test:
+    sleep_delay_sec: 5
+  async: 3
+  poll: 1
+  register: asyncresult
+  ignore_errors: true
+
+- name: validate response
+  assert:
+    that:
+    - asyncresult.ansible_job_id is match('\d+\.\d+')
+    - asyncresult.finished == 1
+    - asyncresult.changed == false
+    - asyncresult | failed == true
+    - asyncresult.msg is search('timed out')
+
+- name: async poll graceful module failure
+  async_test:
+    fail_mode: graceful
+  async: 5
+  poll: 1
+  register: asyncresult
+  ignore_errors: true
+
+- name: validate response
+  assert:
+    that:
+    - asyncresult.ansible_job_id is match('\d+\.\d+')
+    - asyncresult.finished == 1
+    - asyncresult.changed == false
+    - asyncresult | failed == true
+    - asyncresult.msg == 'failed gracefully'
+
+- name: async poll exception module failure
+  async_test:
+    fail_mode: exception
+  async: 5
+  poll: 1
+  register: asyncresult
+  ignore_errors: true
+
+- name: validate response
+  assert:
+    that:
+    - asyncresult.ansible_job_id is match('\d+\.\d+')
+    - asyncresult.finished == 1
+    - asyncresult.changed == false
+    - asyncresult | failed == true
+    - asyncresult.msg is search('failing via exception')
+
+- name: loop async success
+  async_test:
+    sleep_delay_sec: 3
+  async: 10
+  poll: 0
+  with_sequence: start=1 end=4
+  register: async_many
+
+- name: wait for completion
+  async_status:
+    jid: "{{ item }}"
+  register: asyncout
+  until: asyncout.finished == 1
+  retries: 10
+  delay: 1
+  with_items: "{{ async_many.results | map(attribute='ansible_job_id') | list }}"
+
+- name: validate results
+  assert:
+    that:
+    - item.finished == 1
+    - item.slept_sec == 3
+    - item.changed == true
+    - item.ansible_job_id is match('\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
+# FUTURE: consider having the test module hook to a kernel object we can poke at that gets signaled/released on exit
+#- name: ensure that all watchdog and module procs have exited
+#  raw: Get-Process | Where { $_.Id -in ({{ asyncout.results | join(',', attribute='ansible_async_watchdog_pid') }}, {{ asyncout.results | join(',', attribute='module_pid') }}) }
+#  register: proclist
+#
+#- name: validate no processes were returned
+#  assert:
+#    that:
+#    - proclist.stdout.strip() == ""
+
+# FUTURE: test junk before/after JSON
+# FUTURE: verify tempdir stays through module exec
+# FUTURE: verify tempdir is deleted after module exec
+# FUTURE: verify tempdir is permanent with ANSIBLE_KEEP_REMOTE_FILES=1 (how?)
+# FUTURE: verify binary modules work
+
+# FUTURE: test status/return
+# FUTURE: test status/cleanup
+# FUTURE: test reboot/connection failure
+# FUTURE: figure out how to ensure that processes and tempdirs are cleaned up in all exceptional cases

test/integration/test_win_group3.yml

@@ -4,3 +4,4 @@
     - { role: test_win_service, tags: test_win_service }
     - { role: test_win_feature, tags: test_win_feature }
     - { role: test_win_user, tags: test_win_user }
+    - { role: test_win_async_wrapper, tags: test_win_async_wrapper }