From e0381606af35367f57d1896cdf310904c40e70fd Mon Sep 17 00:00:00 2001 From: David Wilson Date: Wed, 4 Apr 2018 14:05:57 +0100 Subject: [PATCH] Ensure remote_tmp is respected everywhere. Logic is still somewhat different from Ansible: we don't have to care about sudo/non-sudo cases, etc. --- ansible_mitogen/mixins.py | 21 ++++++++++++++------- ansible_mitogen/planner.py | 6 +++++- ansible_mitogen/runner.py | 25 +++++++++++++++++++------ docs/ansible.rst | 2 -- 4 files changed, 38 insertions(+), 16 deletions(-) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index 56eaa281..094c4510 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -177,6 +177,18 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): """ assert False, "_is_pipelining_enabled() should never be called." + def _get_remote_tmp(self): + """ + Mitogen-only: return the 'remote_tmp' setting. + """ + try: + s = self._connection._shell.get_option('remote_tmp') + except AttributeError: + # Required for <2.4.x. + s = '~/.ansible' + + return self._remote_expand_user(s) + def _make_tmp_path(self, remote_user=None): """ Replace the base implementation's use of shell to implement mkdtemp() @@ -184,18 +196,12 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): """ LOG.debug('_make_tmp_path(remote_user=%r)', remote_user) - try: - remote_tmp = self._connection._shell.get_option('remote_tmp') - except AttributeError: - # Required for <2.4.x. - remote_tmp = '~/.ansible' - # _make_tmp_path() is basically a global stashed away as Shell.tmpdir. # The copy action plugin violates layering and grabs this attribute # directly. self._connection._shell.tmpdir = self.call( ansible_mitogen.helpers.make_temp_directory, - base_dir=self._remote_expand_user(remote_tmp), + base_dir=self._get_remote_tmp(), ) LOG.debug('Temporary directory: %r', self._connection._shell.tmpdir) self._cleanup_remote_tmp = True @@ -313,6 +319,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): connection=self._connection, module_name=mitogen.utils.cast(module_name), module_args=mitogen.utils.cast(module_args), + remote_tmp=mitogen.utils.cast(self._get_remote_tmp()), task_vars=task_vars, templar=self._templar, env=mitogen.utils.cast(env), diff --git a/ansible_mitogen/planner.py b/ansible_mitogen/planner.py index 8d216af1..93461584 100644 --- a/ansible_mitogen/planner.py +++ b/ansible_mitogen/planner.py @@ -92,7 +92,7 @@ class Invocation(object): helpers.run_module() or helpers.run_module_async() in the target context. """ def __init__(self, action, connection, module_name, module_args, - task_vars, templar, env, wrap_async): + remote_tmp, task_vars, templar, env, wrap_async): #: ActionBase instance invoking the module. Required to access some #: output postprocessing methods that don't belong in ActionBase at #: all. @@ -104,6 +104,9 @@ class Invocation(object): self.module_name = module_name #: Final module arguments. self.module_args = module_args + #: Value of 'remote_tmp' parameter, to allow target to create temporary + #: files in correct location. + self.remote_tmp = remote_tmp #: Task variables, needed to extract ansible_*_interpreter. self.task_vars = task_vars #: Templar, needed to extract ansible_*_interpreter. @@ -179,6 +182,7 @@ class BinaryPlanner(Planner): 'path': invocation.module_path, 'args': invocation.module_args, 'env': invocation.env, + 'remote_tmp': invocation.remote_tmp, } diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index 6cfa4166..475e1b6d 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -40,6 +40,7 @@ import cStringIO import json import logging import os +import shutil import sys import tempfile import types @@ -81,16 +82,25 @@ class Runner(object): Subclasses may override `_run`()` and extend `setup()` and `revert()`. """ - def __init__(self, module, raw_params=None, args=None, env=None): + def __init__(self, module, remote_tmp, raw_params=None, args=None, env=None): if args is None: args = {} if raw_params is not None: args['_raw_params'] = raw_params self.module = module + self.remote_tmp = os.path.expanduser(remote_tmp) self.raw_params = raw_params self.args = args self.env = env + self._temp_dir = None + + def get_temp_dir(self): + if not self._temp_dir: + self._temp_dir = ansible_mitogen.helpers.make_temp_directory( + self.remote_tmp, + ) + return self._temp_dir def setup(self): """ @@ -105,6 +115,8 @@ class Runner(object): implementation simply restores the original environment. """ self._env.revert() + if self._temp_dir: + shutil.rmtree(self._temp_dir) def _run(self): """ @@ -195,9 +207,9 @@ class ProgramRunner(Runner): Create a temporary file containing the program code. The code is fetched via :meth:`_get_program`. """ - self.program_fp = tempfile.NamedTemporaryFile( - prefix='ansible_mitogen', - suffix='-binary', + self.program_fp = open( + os.path.join(self.get_temp_dir(), self.module), + 'wb' ) self.program_fp.write(self._get_program()) self.program_fp.flush() @@ -224,8 +236,8 @@ class ProgramRunner(Runner): """ Delete the temporary program file. """ - super(ProgramRunner, self).revert() self.program_fp.close() + super(ProgramRunner, self).revert() def _run(self): try: @@ -260,6 +272,7 @@ class ArgsFileRunner(Runner): self.args_fp = tempfile.NamedTemporaryFile( prefix='ansible_mitogen', suffix='-args', + dir=self.get_temp_dir(), ) self.args_fp.write(self._get_args_contents()) self.args_fp.flush() @@ -282,8 +295,8 @@ class ArgsFileRunner(Runner): """ Delete the temporary argument file. """ - super(ArgsFileRunner, self).revert() self.args_fp.close() + super(ArgsFileRunner, self).revert() class BinaryRunner(ArgsFileRunner, ProgramRunner): diff --git a/docs/ansible.rst b/docs/ansible.rst index a5a09eb6..d0b309f3 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -161,8 +161,6 @@ Low Risk * Only the ``sudo`` become method is available, however adding new methods is straightforward, and eventually at least ``su`` will be included. -* In some cases ``remote_tmp`` may not be respected. - * The extension's performance benefits do not scale perfectly linearly with the number of targets. This is a subject of ongoing investigation and improvements will appear in time.