Rooba
/
mitogen
mirror of https://github.com/mitogen-hq/mitogen.git
1
1
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'origin/dmw' 

* origin/dmw:
  issue #554: mitogen_action_script fix
  issue #554: fix Ansible 2.4 compatibility
  issue #554: don't rely on tmp_path autoremoval in test.
  issue #554: track and remove multiple make_tmp_path() calls.
  docs: update Changelog.
  docs: drastically simplify install/changelog.
  issue #552: include process identity in log messages.
  issue #550: update Changelog.
This commit is contained in:
David Wilson 2019-03-06 13:43:56 +00:00
parent a7df9cdb64 30b8172573
commit 58c853dbd1
13 changed files with 236 additions and 217 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
ansible_mitogen/connection.py
View File

@ -33,7 +33,6 @@ import errno
import logging
import os
import pprint
import random
import stat
import sys
import time
@ -703,35 +702,6 @@ class Connection(ansible.plugins.connection.ConnectionBase):
self._connect()
return self.init_child_result['good_temp_dir']
def _generate_tmp_path(self):
return os.path.join(
self.get_good_temp_dir(),
'ansible_mitogen_action_%016x' % (
random.getrandbits(8*8),
)
)
def _make_tmp_path(self):
assert getattr(self._shell, 'tmpdir', None) is None
self._shell.tmpdir = self._generate_tmp_path()
LOG.debug('Temporary directory: %r', self._shell.tmpdir)
self.get_chain().call_no_reply(os.mkdir, self._shell.tmpdir)
return self._shell.tmpdir
def _reset_tmp_path(self):
"""
Called by _mitogen_reset(); ask the remote context to delete any
temporary directory created for the action. CallChain is not used here
to ensure exception is logged by the context on failure, since the
CallChain itself is about to be destructed.
"""
if getattr(self._shell, 'tmpdir', None) is not None:
self.context.call_no_reply(
ansible_mitogen.target.prune_tree,
self._shell.tmpdir,
)
self._shell.tmpdir = None
def _connect(self):
"""
Establish a connection to the master process's UNIX listener socket,
@ -763,7 +733,6 @@ class Connection(ansible.plugins.connection.ConnectionBase):
if not self.context:
return
self._reset_tmp_path()
self.chain.reset()
self.parent.call_service(
service_name='ansible_mitogen.services.ContextService',

26
ansible_mitogen/logging.py
View File

@ -40,6 +40,25 @@ except ImportError:
display = Display()
#: The process name set via :func:`set_process_name`.
_process_name = None
#: The PID of the process that last called :func:`set_process_name`, so its
#: value can be ignored in unknown fork children.
_process_pid = None
def set_process_name(name):
"""
Set a name to adorn log messages with.
"""
global _process_name
_process_name = name
global _process_pid
_process_pid = os.getpid()
class Handler(logging.Handler):
"""
Use Mitogen's log format, but send the result to a Display method.
@ -65,7 +84,12 @@ class Handler(logging.Handler):
if mitogen_name in self.NOISY_LOGGERS and record.levelno >= logging.WARNING:
record.levelno = logging.DEBUG
s = '[pid %d] %s' % (os.getpid(), self.format(record))
if _process_pid == os.getpid():
process_name = _process_name
else:
process_name = '?'
s = '[%-4s %d] %s' % (process_name, os.getpid(), self.format(record))
if record.levelno >= logging.ERROR:
display.error(s, wrap_text=False)
elif record.levelno >= logging.WARNING:

55
ansible_mitogen/mixins.py
View File

@ -30,6 +30,7 @@ from __future__ import absolute_import
import logging
import os
import pwd
import random
import traceback
try:
@ -173,26 +174,48 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase):
"""
assert False, "_is_pipelining_enabled() should never be called."
def _generate_tmp_path(self):
return os.path.join(
self._connection.get_good_temp_dir(),
'ansible_mitogen_action_%016x' % (
random.getrandbits(8*8),
)
)
def _generate_tmp_path(self):
return os.path.join(
self._connection.get_good_temp_dir(),
'ansible_mitogen_action_%016x' % (
random.getrandbits(8*8),
)
)
def _make_tmp_path(self, remote_user=None):
"""
Return the directory created by the Connection instance during
connection.
Create a temporary subdirectory as a child of the temporary directory
managed by the remote interpreter.
"""
LOG.debug('_make_tmp_path(remote_user=%r)', remote_user)
return self._connection._make_tmp_path()
path = self._generate_tmp_path()
LOG.debug('Temporary directory: %r', path)
self._connection.get_chain().call_no_reply(os.mkdir, path)
self._connection._shell.tmpdir = path
return path
def _remove_tmp_path(self, tmp_path):
"""
Stub out the base implementation's invocation of rm -rf, replacing it
with nothing, as the persistent interpreter automatically cleans up
after itself without introducing roundtrips.
Replace the base implementation's invocation of rm -rf, replacing it
with a pipelined call to :func:`ansible_mitogen.target.prune_tree`.
"""
# The actual removal is pipelined by Connection.close().
LOG.debug('_remove_tmp_path(%r)', tmp_path)
# Upstream _remove_tmp_path resets shell.tmpdir here, however
# connection.py uses that as the sole location of the temporary
# directory, if one exists.
# self._connection._shell.tmpdir = None
if tmp_path is None and ansible.__version__ > '2.6':
tmp_path = self._connection._shell.tmpdir # 06f73ad578d
if tmp_path is not None:
self._connection.get_chain().call_no_reply(
ansible_mitogen.target.prune_tree,
tmp_path,
)
self._connection._shell.tmpdir = None
def _transfer_data(self, remote_path, data):
"""
@ -331,7 +354,7 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase):
self._temp_file_gibberish(module_args, wrap_async)
self._connection._connect()
return ansible_mitogen.planner.invoke(
result = ansible_mitogen.planner.invoke(
ansible_mitogen.planner.Invocation(
action=self,
connection=self._connection,
@ -345,6 +368,14 @@ 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:
# Built-in actions expected tmpdir to be cleaned up automatically
# on _execute_module().
self._remove_tmp_path(self._connection._shell.tmpdir)
return result
def _postprocess_response(self, result):
"""
Apply fixups mimicking ActionBase._execute_module(); this is copied

6
ansible_mitogen/process.py
View File

@ -185,19 +185,21 @@ class MuxProcess(object):
cls.profiling = os.environ.get('MITOGEN_PROFILING') is not None
if cls.profiling:
mitogen.core.enable_profiling()
if _init_logging:
ansible_mitogen.logging.setup()
cls.original_env = dict(os.environ)
cls.child_pid = os.fork()
if _init_logging:
ansible_mitogen.logging.setup()
if cls.child_pid:
save_pid('controller')
ansible_mitogen.logging.set_process_name('top')
ansible_mitogen.affinity.policy.assign_controller()
cls.child_sock.close()
cls.child_sock = None
mitogen.core.io_op(cls.worker_sock.recv, 1)
else:
save_pid('mux')
ansible_mitogen.logging.set_process_name('mux')
ansible_mitogen.affinity.policy.assign_muxprocess()
cls.worker_sock.close()
cls.worker_sock = None

1
ansible_mitogen/strategy.py
View File

@ -108,6 +108,7 @@ def wrap_worker__run(*args, **kwargs):
if mitogen.core._profile_hook.__name__ != '_profile_hook':
signal.signal(signal.SIGTERM, signal.SIG_IGN)
ansible_mitogen.logging.set_process_name('task')
ansible_mitogen.affinity.policy.assign_worker()
return mitogen.core._profile_hook('WorkerProcess',
lambda: worker__run(*args, **kwargs)

8
ansible_mitogen/target.py
View File

@ -260,14 +260,6 @@ def prune_tree(path):
LOG.error('prune_tree(%r): %s', path, e)
def _on_broker_shutdown():
"""
Respond to broker shutdown (graceful termination by parent, or loss of
connection to parent) by deleting our sole temporary directory.
"""
prune_tree(temp_dir)
def is_good_temp_dir(path):
"""
Return :data:`True` if `path` can be used as a temporary directory, logging

105
docs/ansible.rst
View File

@ -57,7 +57,7 @@ write files.
Installation
------------
1. Thoroughly review :ref:`noteworthy_differences` and :ref:`known_issues`.
1. Review :ref:`noteworthy_differences`.
2. Download and extract |mitogen_url|.
3. Modify ``ansible.cfg``:
@ -143,13 +143,29 @@ Testimonials
Noteworthy Differences
----------------------
* Ansible 2.3-2.7 are supported along with Python 2.6, 2.7 or 3.6. Verify your
installation is running one of these versions by checking ``ansible
* Ansible 2.3-2.7 are supported along with Python 2.6, 2.7, 3.6 and 3.7. Verify
your installation is running one of these versions by checking ``ansible
--version`` output.
* The Ansible ``raw`` action executes as a regular Mitogen connection,
precluding its use for installing Python on a target. This will be addressed
soon.
* The ``raw`` action executes as a regular Mitogen connection, which requires
Python on the target, precluding its use for installing Python. This will be
addressed in a future release. For now, simply mix Mitogen and vanilla
Ansible strategies:
.. code-block:: yaml
- hosts: web-servers
strategy: linear
tasks:
- name: Install Python if necessary.
raw: test -e /usr/bin/python || apt install -y python-minimal
- hosts: web-servers
strategy: mitogen_linear
roles:
- nginx
- initech_app
- y2k_fix
* The ``doas``, ``su`` and ``sudo`` become methods are available. File bugs to
register interest in more.
@ -166,43 +182,70 @@ Noteworthy Differences
:ref:`mitogen_su <su>`, :ref:`mitogen_sudo <sudo>`, and :ref:`setns <setns>`
types. File bugs to register interest in others.
* Local commands execute in a reuseable interpreter created identically to
interpreters on targets. Presently one interpreter per ``become_user``
exists, and so only one local action may execute simultaneously.
* Actions are single-threaded for each `(host, user account)` combination,
including actions that execute on the local machine. Playbooks may experience
slowdown compared to vanilla Ansible if they employ long-running
``local_action`` or ``delegate_to`` tasks delegating many target hosts to a
single machine and user account.
Ansible usually permits up to ``forks`` simultaneous local actions. Any
long-running local actions that execute for every target will experience
artificial serialization, causing slowdown equivalent to `task_duration *
num_targets`. This will be fixed soon.
num_targets`. This will be addressed soon.
* "Module Replacer" style modules are not supported. These rarely appear in
practice, and light web searches failed to reveal many examples of them.
* The Ansible 2.7 `reboot
<https://docs.ansible.com/ansible/latest/modules/reboot_module.html>`_ module
may require a ``pre_reboot_delay`` on systemd hosts, as insufficient time
exists for the reboot command's exit status to be reported before necessary
processes are torn down.
* On OS X when a SSH password is specified and the default connection type of
``smart`` is used, Ansible may select the Paramiko plug-in rather than
Mitogen. If you specify a password on OS X, ensure ``connection: ssh``
appears in your playbook, ``ansible.cfg``, or as ``-c ssh`` on the
command-line.
* Ansible permits up to ``forks`` connections to be setup in parallel, whereas
in Mitogen this is handled by a fixed-size thread pool. Up to 32 connections
may be established in parallel by default, this can be modified by setting
the ``MITOGEN_POOL_SIZE`` environment variable.
* The ``ansible_python_interpreter`` variable is parsed using a restrictive
:mod:`shell-like <shlex>` syntax, permitting values such as ``/usr/bin/env
FOO=bar python``, which occur in practice. Ansible `documents this
<https://docs.ansible.com/ansible/latest/user_guide/intro_inventory.html#ansible-python-interpreter>`_
as an absolute path, however the implementation passes it unquoted through
the shell, permitting arbitrary code to be injected.
* Performance does not scale linearly with target count. This will improve over
* Performance does not scale cleanly with target count. This will improve over
time.
* SSH and ``become`` are treated distinctly when applying timeouts, and
timeouts apply up to the point when the new interpreter is ready to accept
messages. Ansible has two timeouts: ``ConnectTimeout`` for SSH, applying up
to when authentication completes, and a separate parallel timeout up to when
``become`` authentication completes.
* Performance on Python 3 is significantly worse than on Python 2. While this
has not yet been investigated, at least some of the regression appears to be
part of the core library, and should therefore be straightforward to fix as
part of 0.2.x.
For busy targets, Ansible may successfully execute a module where Mitogen
would fail without increasing the timeout. For sick targets, Ansible may hang
indefinitely after authentication without executing a command, for example
due to a stuck filesystem IO appearing in ``$HOME/.profile``.
..
* SSH and ``become`` are treated distinctly when applying timeouts, and
timeouts apply up to the point when the new interpreter is ready to accept
messages. Ansible has two timeouts: ``ConnectTimeout`` for SSH, applying up
to when authentication completes, and a separate parallel timeout up to
when ``become`` authentication completes.
For busy targets, Ansible may successfully execute a module where Mitogen
would fail without increasing the timeout. For sick targets, Ansible may
hang indefinitely after authentication without executing a command, for
example due to a stuck filesystem IO appearing in ``$HOME/.profile``.
..
* "Module Replacer" style modules are not supported. These rarely appear in
practice, and light web searches failed to reveal many examples of them.
..
* The ``ansible_python_interpreter`` variable is parsed using a restrictive
:mod:`shell-like <shlex>` syntax, permitting values such as ``/usr/bin/env
FOO=bar python``, which occur in practice. Ansible `documents this
<https://docs.ansible.com/ansible/latest/user_guide/intro_inventory.html#ansible-python-interpreter>`_
as an absolute path, however the implementation passes it unquoted through
the shell, permitting arbitrary code to be injected.
..
* Configurations will break that rely on the `hashbang argument splitting
behaviour <https://github.com/ansible/ansible/issues/15635>`_ of the
``ansible_python_interpreter`` setting, contrary to the Ansible
documentation. This will be addressed in a future 0.2 release.
New Features & Notes
@ -253,8 +296,8 @@ container.
``ansible_password``, or ``ansible_become_pass`` inventory variables.
* Automatic tunnelling of SSH-dependent actions, such as the
``synchronize`` module, is not yet supported. This will be added in the
0.3 series.
``synchronize`` module, is not yet supported. This will be addressed in a
future release.
To enable connection delegation, set ``mitogen_via=<inventory name>`` on the
command line, or as host and group variables.

128
docs/changelog.rst
View File

@ -15,116 +15,6 @@ Release Notes
</style>
.. _known_issues:
Known Issues
------------
Mitogen For Ansible
~~~~~~~~~~~~~~~~~~~
* The Ansible 2.7 `reboot
<https://docs.ansible.com/ansible/latest/modules/reboot_module.html>`_ module
may require a ``pre_reboot_delay`` on systemd hosts, as insufficient time
exists for the reboot command's exit status to be reported before necessary
processes are torn down.
* On OS X when a SSH password is specified and the default connection type of
``smart`` is used, Ansible may select the Paramiko plug-in rather than
Mitogen. If you specify a password on OS X, ensure ``connection: ssh``
appears in your playbook, ``ansible.cfg``, or as ``-c ssh`` on the
command-line.
* The ``raw`` action executes as a regular Mitogen connection, which requires
Python on the target, precluding its use for installing Python. This will be
addressed in a future 0.2 release. For now, simply mix Mitogen and vanilla
Ansible strategies in your playbook:
.. code-block:: yaml
- hosts: web-servers
strategy: linear
tasks:
- name: Install Python if necessary.
raw: test -e /usr/bin/python || apt install -y python-minimal
- hosts: web-servers
strategy: mitogen_linear
roles:
- nginx
- initech_app
- y2k_fix
.. * When running with ``-vvv``, log messages will be printed to the console
*after* the Ansible run completes, as connection multiplexer shutdown only
begins after Ansible exits. This is due to a lack of suitable shutdown hook
in Ansible, and is fairly harmless, albeit cosmetically annoying. A future
release may include a solution.
.. * Configurations will break that rely on the `hashbang argument splitting
behaviour <https://github.com/ansible/ansible/issues/15635>`_ of the
``ansible_python_interpreter`` setting, contrary to the Ansible
documentation. This will be addressed in a future 0.2 release.
* Performance does not scale linearly with target count. This requires
significant additional work, as major bottlenecks exist in the surrounding
Ansible code. Performance-related bug reports for any scenario remain
welcome with open arms.
* Performance on Python 3 is significantly worse than on Python 2. While this
has not yet been investigated, at least some of the regression appears to be
part of the core library, and should therefore be straightforward to fix as
part of 0.2.x.
* *Module Replacer* style Ansible modules are not supported.
* Actions are single-threaded for each `(host, user account)` combination,
including actions that execute on the local machine. Playbooks may experience
slowdown compared to vanilla Ansible if they employ long-running
``local_action`` or ``delegate_to`` tasks delegating many target hosts to a
single machine and user account.
* Connection Delegation remains in preview and has bugs around how it infers
connections. Connection establishment will remain single-threaded for the 0.2
series, however connection inference bugs will be addressed in a future 0.2
release.
* Connection Delegation does not support automatic tunnelling of SSH-dependent
actions, such as the ``synchronize`` module. This will be addressed in the
0.3 series.
Core Library
~~~~~~~~~~~~
* Serialization is still based on :mod:`pickle`. While there is high confidence
remote code execution is impossible in Mitogen's configuration, an untrusted
context may at least trigger disproportionately high memory usage injecting
small messages (*"billion laughs attack"*). Replacement is an important
future priority, but not critical for an initial release.
* Child processes are not reliably reaped, leading to a pileup of zombie
processes when a program makes many short-lived connections in a single
invocation. This does not impact Mitogen for Ansible, however it limits the
usefulness of the core library. A future 0.2 release will address it.
* Some races remain around :class:`mitogen.core.Broker <Broker>` destruction,
disconnection and corresponding file descriptor closure. These are only
problematic in situations where child process reaping is also problematic.
* The `fakessh` component does not shut down correctly and requires flow
control added to the design. While minimal fixes are possible, due to the
absence of flow control the original design is functionally incomplete.
* The multi-threaded :ref:`service` remains in a state of design flux and
should be considered obsolete, despite heavy use in Mitogen for Ansible. A
future replacement may be integrated more tightly with, or entirely replace
the RPC dispatcher on the main thread.
* Documentation is in a state of disrepair. This will be improved over the 0.2
series.
v0.2.6 (unreleased)
-------------------
@ -147,6 +37,17 @@ Fixes
* `#548 <https://github.com/dw/mitogen/issues/548>`_: `mitogen_via=` could fail
when the selected transport was set to ``smart``.
* `#550 <https://github.com/dw/mitogen/issues/550>`_: avoid some broken
TTY-related `ioctl()` calls on Windows Subsystem for Linux 2016 Anniversary
Update.
* `#554 <https://github.com/dw/mitogen/issues/554>`_: third party Ansible
action plug-ins that invoked :func:`_make_tmp_path` repeatedly could trigger
an assertion failure.
* `ffae0355 <https://github.com/dw/mitogen/commit/ffae0355>`_: needless
information was removed from the documentation and installation procedure.
Core Library
~~~~~~~~~~~~
@ -177,9 +78,12 @@ Thanks!
Mitogen would not be possible without the support of users. A huge thanks for
bug reports, testing, features and fixes in this release contributed by
`Fabian Arrotin <https://github.com/arrfab>`_,
`Matt Layman <https://github.com/mblayman>`_,
`Percy Grunwald <https://github.com/percygrunwald>`_,
`Petr Enkov <https://github.com/enkov>`_, and
`@elbunda <https://github.com/elbunda>`_.
`Petr Enkov <https://github.com/enkov>`_,
`Tony Finch <https://github.com/fanf2>`_,
`@elbunda <https://github.com/elbunda>`_, and
`@zyphermonkey <https://github.com/zyphermonkey>`_.

1
tests/ansible/integration/action/all.yml
View File

@ -2,6 +2,7 @@
- include: fixup_perms2__copy.yml
- include: low_level_execute_command.yml
- include: make_tmp_path.yml
- include: make_tmp_path__double.yml
- include: remote_expand_user.yml
- include: remote_file_exists.yml
- include: remove_tmp_path.yml

31
tests/ansible/integration/action/make_tmp_path.yml
View File

@ -19,22 +19,26 @@
#
- name: "Find regular temp path"
action_passthrough:
method: _make_tmp_path
mitogen_action_script:
script: |
path = self._make_tmp_path()
self._remove_tmp_path(path)
register: tmp_path
- name: "Find regular temp path (new task)"
action_passthrough:
method: _make_tmp_path
mitogen_action_script:
script: |
path = self._make_tmp_path()
self._remove_tmp_path(path)
register: tmp_path2
- name: "Find good temp path"
set_fact:
good_temp_path: "{{tmp_path.result|dirname}}"
good_temp_path: "{{tmp_path.path|dirname}}"
- name: "Find good temp path (new task)"
set_fact:
good_temp_path2: "{{tmp_path2.result|dirname}}"
good_temp_path2: "{{tmp_path2.path|dirname}}"
- name: "Verify common base path for both tasks"
assert:
@ -44,7 +48,7 @@
- name: "Verify different subdir for both tasks"
assert:
that:
- tmp_path.result != tmp_path2.result
- tmp_path.path != tmp_path2.path
#
# Verify subdirectory removal.
@ -52,12 +56,12 @@
- name: Stat temp path
stat:
path: "{{tmp_path.result}}"
path: "{{tmp_path.path}}"
register: stat1
- name: Stat temp path (new task)
stat:
path: "{{tmp_path2.result}}"
path: "{{tmp_path2.path}}"
register: stat2
- name: "Verify neither subdir exists any more"
@ -108,14 +112,17 @@
- name: "Find root temp path"
become: true
action_passthrough:
method: _make_tmp_path
mitogen_action_script:
script: |
path = self._make_tmp_path()
self._remove_tmp_path(path)
register: tmp_path_root
- name: "Verify root temp path differs from regular path"
assert:
that:
- tmp_path2.result != tmp_path_root.result
- tmp_path2.path != tmp_path_root.path
- tmp_path2.path|dirname != tmp_path_root.path|dirname
#
# readonly homedir

20
tests/ansible/integration/action/make_tmp_path__double.yml Normal file
View File

@ -0,0 +1,20 @@
# issue #554: double calls to make_tmp_path() fail with assertion error. Ensure
# they succeed and are cleaned up correctly.
- hosts: target
tasks:
- mitogen_action_script:
script: |
result['t1'] = self._make_tmp_path()
result['t2'] = self._make_tmp_path()
assert result['t1'] != result['t2']
assert self._remote_file_exists(result['t1'])
assert self._remote_file_exists(result['t2'])
self._remove_tmp_path(result['t1'])
self._remove_tmp_path(result['t2'])
register: out
- mitogen_action_script:
script: |
assert not self._remote_file_exists("{{ out.t1 }}")
assert not self._remote_file_exists("{{ out.t2 }}")

3
tests/ansible/integration/action/remove_tmp_path.yml
View File

@ -6,9 +6,6 @@
hosts: test-targets
any_errors_fatal: true
tasks:
- meta: end_play
when: not is_mitogen
#
# Use the copy module to cause a temporary directory to be created, and
# return a result with a 'src' attribute pointing into that directory.

38
tests/ansible/lib/action/mitogen_action_script.py
View File

@ -5,6 +5,21 @@ import sys
from ansible.plugins.action import ActionBase
try:
long
except NameError:
long = int
try:
unicode
except NameError:
unicode = str
try:
bytes
except NameError:
bytes = str
def execute(s, gbls, lcls):
if sys.version_info > (3,):
@ -16,12 +31,25 @@ def execute(s, gbls, lcls):
class ActionModule(ActionBase):
def run(self, tmp=None, task_vars=None):
super(ActionModule, self).run(tmp=tmp, task_vars=task_vars)
lcls = {
'self': self,
'result': {}
}
lcls = {}
# Capture keys to remove later, including any crap Python adds.
execute('pass', globals(), lcls)
lcls['self'] = self
# Old mitogen_action_script used explicit result dict.
lcls['result'] = lcls
pre_keys = list(lcls)
execute(self._task.args['script'], globals(), lcls)
return lcls['result']
for key in pre_keys:
del lcls[key]
for key in list(lcls):
if not isinstance(lcls[key],
(unicode, bytes, int, long, dict, list, tuple,
bool)):
del lcls[key]
return lcls
if __name__ == '__main__':