diff --git a/ansible_mitogen/connection.py b/ansible_mitogen/connection.py index bd0d79ff..a538f743 100644 --- a/ansible_mitogen/connection.py +++ b/ansible_mitogen/connection.py @@ -303,7 +303,7 @@ class Connection(ansible.plugins.connection.ConnectionBase): LOG.debug('Call %s%r took %d ms', func.func_name, args, 1000 * (time.time() - t0)) - def exec_command(self, cmd, in_data='', sudoable=True): + def exec_command(self, cmd, in_data='', sudoable=True, mitogen_chdir=None): """ Implement exec_command() by calling the corresponding ansible_mitogen.helpers function in the target. @@ -315,8 +315,20 @@ class Connection(ansible.plugins.connection.ConnectionBase): :returns: (return code, stdout bytes, stderr bytes) """ - return self.call(ansible_mitogen.helpers.exec_command, - cast(cmd), cast(in_data)) + emulate_tty = (not in_data and sudoable) + rc, stdout, stderr = self.call( + ansible_mitogen.helpers.exec_command, + cmd=cast(cmd), + in_data=cast(in_data), + chdir=mitogen_chdir, + emulate_tty=emulate_tty, + ) + + stderr += 'Shared connection to %s closed.%s' % ( + self._play_context.remote_addr, + ('\r\n' if emulate_tty else '\n'), + ) + return rc, stdout, stderr def fetch_file(self, in_path, out_path): """ diff --git a/ansible_mitogen/helpers.py b/ansible_mitogen/helpers.py index 20f69c19..167afbb5 100644 --- a/ansible_mitogen/helpers.py +++ b/ansible_mitogen/helpers.py @@ -176,7 +176,7 @@ def get_user_shell(): return pw_shell or '/bin/sh' -def exec_args(args, in_data='', chdir=None, shell=None): +def exec_args(args, in_data='', chdir=None, shell=None, emulate_tty=False): """ Run a command in a subprocess, emulating the argument handling behaviour of SSH. @@ -185,24 +185,36 @@ def exec_args(args, in_data='', chdir=None, shell=None): Argument vector. :param bytes in_data: Optional standard input for the command. + :param bool emulate_tty: + If :data:`True`, arrange for stdout and stderr to be merged into the + stdout pipe and for LF to be translated into CRLF, emulating the + behaviour of a TTY. :return: (return code, stdout bytes, stderr bytes) """ LOG.debug('exec_args(%r, ..., chdir=%r)', args, chdir) assert isinstance(args, list) + if emulate_tty: + stderr = subprocess.STDOUT + else: + stderr = subprocess.PIPE + proc = subprocess.Popen( args=args, stdout=subprocess.PIPE, - stderr=subprocess.PIPE, + stderr=stderr, stdin=subprocess.PIPE, cwd=chdir, ) stdout, stderr = proc.communicate(in_data) - return proc.returncode, stdout, stderr + + if emulate_tty: + stdout = stdout.replace('\n', '\r\n') + return proc.returncode, stdout, stderr or '' -def exec_command(cmd, in_data='', chdir=None, shell=None): +def exec_command(cmd, in_data='', chdir=None, shell=None, emulate_tty=False): """ Run a command in a subprocess, emulating the argument handling behaviour of SSH. @@ -220,6 +232,7 @@ def exec_command(cmd, in_data='', chdir=None, shell=None): in_data=in_data, chdir=chdir, shell=shell, + emulate_tty=emulate_tty, ) diff --git a/ansible_mitogen/mixins.py b/ansible_mitogen/mixins.py index f2b8aedc..2151e6b3 100644 --- a/ansible_mitogen/mixins.py +++ b/ansible_mitogen/mixins.py @@ -367,11 +367,11 @@ class ActionModuleMixin(ansible.plugins.action.ActionBase): if executable: cmd = executable + ' -c ' + commands.mkarg(cmd) - rc, stdout, stderr = self.call( - ansible_mitogen.helpers.exec_command, - cast(cmd), - cast(in_data), - chdir=cast(chdir), + rc, stdout, stderr = self._connection.exec_command( + cmd=cast(cmd), + in_data=cast(in_data), + sudoable=sudoable, + mitogen_chdir=cast(chdir), ) stdout_text = to_text(stdout, errors=encoding_errors) diff --git a/ansible_mitogen/runner.py b/ansible_mitogen/runner.py index 475e1b6d..ae6bd270 100644 --- a/ansible_mitogen/runner.py +++ b/ansible_mitogen/runner.py @@ -243,6 +243,7 @@ class ProgramRunner(Runner): try: rc, stdout, stderr = ansible_mitogen.helpers.exec_args( args=self._get_program_args(), + emulate_tty=True, ) except Exception, e: LOG.exception('While running %s', self._get_program_args()) diff --git a/docs/ansible.rst b/docs/ansible.rst index d0b309f3..fdccdbf1 100644 --- a/docs/ansible.rst +++ b/docs/ansible.rst @@ -165,28 +165,10 @@ Low Risk number of targets. This is a subject of ongoing investigation and improvements will appear in time. -* Ansible defaults to requiring pseudo TTYs for most SSH invocations, in order - to allow it to handle ``sudo`` with ``requiretty`` enabled, however it - disables pseudo TTYs for certain commands where standard input is required or - ``sudo`` is not in use. Mitogen does not require this, as it can simply call - :py:func:`pty.openpty` from the SSH user account during ``sudo`` setup. - - A major downside to Ansible's default is that stdout and stderr of any - resulting executed command are merged, with additional carriage return - characters synthesized in the output by the TTY layer. Neither of these - problems are apparent using the Mitogen extension, which may break some - playbooks. - - A future version will emulate Ansible's behaviour, once it is clear precisely - what that behaviour is supposed to be. See `Ansible#14377`_ for related - discussion. - * "Module Replacer" style modules are not yet supported. These rarely appear in practice, and light Github code searches failed to reveal many examples of them. -.. _Ansible#14377: https://github.com/ansible/ansible/issues/14377 - Behavioural Differences ----------------------- @@ -212,10 +194,6 @@ Behavioural Differences captured and returned to the host machine, where it can be viewed as desired with ``-vvv``. -* Ansible with SSH multiplexing enabled causes a string like ``Shared - connection to host closed`` to appear in ``stderr`` output of every executed - command. This never manifests with the Mitogen extension. - * Local commands are executed in a reuseable Python interpreter created identically to interpreters used on remote hosts. At present only one such interpreter per ``become_user`` exists, and so only one action may be @@ -357,6 +335,30 @@ plug-ins are unlikely to attempt similar patches, so the risk to an established configuration should be minimal. +Standard IO +~~~~~~~~~~~ + +Ansible uses pseudo TTYs for most invocations, to allow it to handle typing +passwords interactively, however it disables pseudo TTYs for certain commands +where standard input is required or ``sudo`` is not in use. Additionally when +SSH multiplexing is enabled, a string like ``Shared connection to localhost +closed\r\n`` appears in ``stderr`` of every invocation. + +Mitogen does not naturally require either of these, as command output is +embedded within the SSH stream, and it can simply call :py:func:`pty.openpty` +in every location an interactive password must be typed. + +A major downside to Ansible's behaviour is that ``stdout`` and ``stderr`` are +merged together into a single ``stdout`` variable, with carriage returns +inserted in the output by the TTY layer. However ugly, the extension emulates +all of this behaviour precisely, to avoid breaking playbooks that expect +certain text to appear in certain variables with certain linefeed characters. + +See `Ansible#14377`_ for related discussion. + +.. _Ansible#14377: https://github.com/ansible/ansible/issues/14377 + + Flag Emulation ~~~~~~~~~~~~~~ diff --git a/tests/ansible/integration/action__low_level_execute_command.yml b/tests/ansible/integration/action__low_level_execute_command.yml index fd0217bc..7cb6c410 100644 --- a/tests/ansible/integration/action__low_level_execute_command.yml +++ b/tests/ansible/integration/action__low_level_execute_command.yml @@ -28,6 +28,7 @@ - debug: msg={{raw}} - name: Verify raw module output. assert: - that: - - 'raw.rc == 0' - - 'raw.stdout_lines == ["root"]' + that: | + raw.rc == 0 and + raw.stdout == "root\r\n" and + raw.stdout_lines == ["root"] diff --git a/tests/ansible/integration/runner__custom_binary_single_null.yml b/tests/ansible/integration/runner__custom_binary_single_null.yml index c521e3b2..4933a8ec 100644 --- a/tests/ansible/integration/runner__custom_binary_single_null.yml +++ b/tests/ansible/integration/runner__custom_binary_single_null.yml @@ -15,4 +15,10 @@ out.failed and out.results[0].failed and out.results[0].msg == 'MODULE FAILURE' and - out.results[0].rc == 126 + out.results[0].module_stdout.startswith('/bin/sh: ') and + out.results[0].module_stdout.endswith('/custom_binary_single_null: cannot execute binary file\r\n') + + +# Can't test this: Mitogen returns 126, 2.5.x returns 126, 2.4.x discarded the +# return value and always returned 0. +# out.results[0].rc == 126