From f239e1e61f71e23300ce64e320c7962a1ba2ad8b Mon Sep 17 00:00:00 2001 From: Matt Davis Date: Tue, 6 Sep 2016 17:38:12 -0700 Subject: [PATCH] windows async changes and tests (#17400) --- lib/ansible/plugins/action/__init__.py | 7 +- lib/ansible/plugins/action/async.py | 40 ++-- .../library/async_test.ps1 | 57 ++++++ .../test_win_async_wrapper/tasks/main.yml | 187 ++++++++++++++++++ test/integration/test_win_group3.yml | 1 + 5 files changed, 277 insertions(+), 15 deletions(-) create mode 100644 test/integration/roles/test_win_async_wrapper/library/async_test.ps1 create mode 100644 test/integration/roles/test_win_async_wrapper/tasks/main.yml diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index e6ed4aa80f7..ef9920033d2 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/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. diff --git a/lib/ansible/plugins/action/async.py b/lib/ansible/plugins/action/async.py index 5d2b821ca18..5e1bc467d24 100644 --- a/lib/ansible/plugins/action/async.py +++ b/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) - self._transfer_data(async_module_path, async_module_data) + (async_module_style, shebang, async_module_data, async_module_path) = self._configure_module(module_name='async_wrapper', module_args=dict(), task_vars=task_vars) + 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 - # response, so we parse it here and replace the result - result = self._parse_returned_data(result) - - return result + return parsed_result diff --git a/test/integration/roles/test_win_async_wrapper/library/async_test.ps1 b/test/integration/roles/test_win_async_wrapper/library/async_test.ps1 new file mode 100644 index 00000000000..3b891b9fac1 --- /dev/null +++ b/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 . + +# 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" + } +} diff --git a/test/integration/roles/test_win_async_wrapper/tasks/main.yml b/test/integration/roles/test_win_async_wrapper/tasks/main.yml new file mode 100644 index 00000000000..d3d24bb753a --- /dev/null +++ b/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 diff --git a/test/integration/test_win_group3.yml b/test/integration/test_win_group3.yml index f8bcdcd780e..7ade0cb1229 100644 --- a/test/integration/test_win_group3.yml +++ b/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 }