Add method to automatically clean up after an action plugin (#65509)

* Use correct var, move cleanup for async
* Add changelog and tests. Fixes #65393. Fixes #65277.
* Kill off all long running async tasks from listen_ports_facts
* Update task to work with older jinja2

changelogs/fragments/action-plugin-always-cleanup.yml

@@ -0,0 +1,7 @@
+bugfixes:
+- ActionBase - Add new ``cleanup`` method that is explicitly run by the
+  ``TaskExecutor`` to ensure that the shell plugins ``tmpdir`` is always
+  removed. This change means that individual action plugins need not be
+  responsible for removing the temporary directory, which ensures that
+  we don't have code paths that accidentally leave behind the temporary
+  directory.

lib/ansible/executor/task_executor.py

@@ -649,6 +649,8 @@ class TaskExecutor:
                 return dict(failed=True, msg=to_text(e))
             except AnsibleConnectionFailure as e:
                 return dict(unreachable=True, msg=to_text(e))
+            finally:
+                self._handler.cleanup()
             display.debug("handler run complete")
 
             # preserve no log
@@ -853,6 +855,7 @@ class TaskExecutor:
             else:
                 return dict(failed=True, msg="async task produced unparseable results", async_result=async_result)
         else:
+            async_handler.cleanup(force=True)
             return async_result
 
     def _get_become(self, name):

lib/ansible/plugins/action/__init__.py

@@ -115,6 +115,18 @@ class ActionBase(with_metaclass(ABCMeta, object)):
 
         return result
 
+    def cleanup(self, force=False):
+        """Method to perform a clean up at the end of an action plugin execution
+
+        By default this is designed to clean up the shell tmpdir, and is toggled based on whether
+        async is in use
+
+        Action plugins may override this if they deem necessary, but should still call this method
+        via super
+        """
+        if force or not self._task.async_val:
+            self._remove_tmp_path(self._connection._shell.tmpdir)
+
     def get_plugin_option(self, plugin, option, default=None):
         """Helper to get an option from a plugin without having to use
         the try/except dance everywhere to set a default

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

@@ -77,3 +77,9 @@
   assert:
     that: 5555 in ansible_facts.udp_listen | map(attribute='port') | sort | list
   when: (ansible_os_family == "RedHat" and ansible_distribution_major_version|int >= 7) or ansible_os_family == "Debian"
+
+- name: kill all async commands
+  command: "kill -9 {{ item.pid }}"
+  loop: "{{ [tcp_listen, udp_listen]|flatten }}"
+  when: item.name == 'nc'
+  ignore_errors: true

test/integration/targets/remote_tmp/playbook.yml

@@ -24,3 +24,32 @@
         - name: clean up test user
           user: name=tmptest state=absent
           become_user: root
+
+- name: Test tempdir is removed
+  hosts: testhost
+  gather_facts: false
+  tasks:
+    - file:
+        state: touch
+        path: "/{{ output_dir }}/65393"
+
+    - copy:
+        src: "/{{ output_dir }}/65393"
+        dest: "/{{ output_dir }}/65393.2"
+        remote_src: true
+
+    - find:
+        path: "~/.ansible/tmp"
+        use_regex: yes
+        patterns: 'AnsiballZ_.+\.py'
+        recurse: true
+      register: result
+
+    - debug:
+        var: result
+
+    - assert:
+        that:
+          # Should only be AnsiballZ_find.py because find is actively running
+          - result.files|length == 1
+          - result.files[0].path.endswith('/AnsiballZ_find.py')

test/integration/targets/remote_tmp/runme.sh

@@ -2,4 +2,4 @@
 
 set -ux
 
-ansible-playbook -i ../../inventory playbook.yml -v "$@"
+ansible-playbook -i ../../inventory playbook.yml -e "output_dir=${OUTPUT_DIR}" -v "$@"