From 8dfb3966dfbe809855ddd8c1069174aa12bc1737 Mon Sep 17 00:00:00 2001 From: David Wilson Date: Mon, 12 Aug 2019 15:32:48 +0100 Subject: [PATCH 1/2] issue #558, #582: preserve remote tmpdir if caller did not supply one The undocumented 'tmp' parameter controls whether _execute_module() would delete anything on 2.3, so mimic that. This means _execute_remove_stat() calls will not blow away the temp directory, which broke the unarchive plugin. --- ansible_mitogen/mixins.py | 5 ++--- docs/changelog.rst | 10 +++++++++- tests/ansible/regression/all.yml | 1 + .../regression/issue_558_unarchive_failed.yml | 11 +++++++++++ tests/data/unarchive_test.tar | Bin 0 -> 10240 bytes 5 files changed, 23 insertions(+), 4 deletions(-) create mode 100644 tests/ansible/regression/issue_558_unarchive_failed.yml create mode 100644 tests/data/unarchive_test.tar diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index 3a5d4c93..eee1ecd7 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -360,11 +360,10 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): ) ) - if ansible.__version__ < '2.5' and delete_remote_tmp and \ - getattr(self._connection._shell, 'tmpdir', None) is not None: + if tmp and ansible.__version__ < '2.5' and delete_remote_tmp: # Built-in actions expected tmpdir to be cleaned up automatically # on _execute_module(). - self._remove_tmp_path(self._connection._shell.tmpdir) + self._remove_tmp_path(tmp) return result diff --git a/docs/changelog.rst b/docs/changelog.rst index 37cab2e3..31605315 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -93,6 +93,11 @@ Mitogen for Ansible default soft limit, allowing *"too many open files"* errors to be avoided more often in large runs without user configuration. +* `#558 `_, + `#582 `_: on Ansible 2.3 a remote + directory was unconditionally deleted after the first module belonging to an + action plug-in had executed, causing the ``unarchive`` module to fail. + * `#578 `_: the extension could crash while rendering an error message, due to an incorrect format string. @@ -223,6 +228,7 @@ bug reports, testing, features and fixes in this release contributed by `Dave Cottlehuber `_, `Denis Krienbühl `_, `El Mehdi CHAOUKI `_, +`Florent Dutheil `_, `James Hogarth `_, `Marc Hartmayer `_, `Nigel Metheringham `_, @@ -235,8 +241,10 @@ bug reports, testing, features and fixes in this release contributed by `Yuki Nishida `_, `@alexhexabeam `_, `@DavidVentura `_, +`@dbiegunski `_, `@ghp-rr `_, -`@rizzly `_, and +`@rizzly `_, +`@SQGE `_, and `@tho86 `_. diff --git a/tests/ansible/regression/all.yml b/tests/ansible/regression/all.yml index 32852942..f75a050c 100644 --- a/tests/ansible/regression/all.yml +++ b/tests/ansible/regression/all.yml @@ -8,5 +8,6 @@ - include: issue_154__module_state_leaks.yml - include: issue_177__copy_module_failing.yml - include: issue_332_ansiblemoduleerror_first_occurrence.yml +- include: issue_558_unarchive_failed.yml - include: issue_590__sys_modules_crap.yml - include: issue_591__setuptools_cwd_crash.yml diff --git a/tests/ansible/regression/issue_558_unarchive_failed.yml b/tests/ansible/regression/issue_558_unarchive_failed.yml new file mode 100644 index 00000000..1f073d70 --- /dev/null +++ b/tests/ansible/regression/issue_558_unarchive_failed.yml @@ -0,0 +1,11 @@ +# _execute_module() would unconditionally delete shell.tmpdir without +# respecting the passed in 'tmp' parameter on Ansible 2.3. + +- name: regression/issue_558_unarchive_failed.yml + hosts: test-targets + tasks: + - file: state=absent path=/tmp/foo + - file: state=directory path=/tmp/foo + - unarchive: + src: "{{git_basedir}}/tests/data/unarchive_test.tar" + dest: /tmp/foo diff --git a/tests/data/unarchive_test.tar b/tests/data/unarchive_test.tar new file mode 100644 index 0000000000000000000000000000000000000000..97c36407ca2e73bccf7ccd020908e5e7afdf5434 GIT binary patch literal 10240 zcmeIvF$#b%3_#J$o}wp^Myp=IYX}Msil7xezH!n)aB>iSmXxFo;kECEVM^m3<-70} zWe8$PtHR&&*|I1_^CAnGbt@IGYWKzY?^e1duS;xZ=cFCCtN3tw>pwjCL#aOhKl#I^ z?1%sY2q1s}0tg_000IagfB*srAb Date: Mon, 12 Aug 2019 15:51:35 +0100 Subject: [PATCH 2/2] issue #558: disable test on OSX to cope with boundless mediocrity --- tests/ansible/regression/issue_558_unarchive_failed.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/ansible/regression/issue_558_unarchive_failed.yml b/tests/ansible/regression/issue_558_unarchive_failed.yml index 1f073d70..c6b1c9f6 100644 --- a/tests/ansible/regression/issue_558_unarchive_failed.yml +++ b/tests/ansible/regression/issue_558_unarchive_failed.yml @@ -9,3 +9,5 @@ - unarchive: src: "{{git_basedir}}/tests/data/unarchive_test.tar" dest: /tmp/foo + # garbage doesn't work with BSD tar + when: ansible_system != 'Darwin'