From f488de85997079f480d504f73537e3e33ff2495b Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Thu, 5 Nov 2015 17:31:31 +0530 Subject: [PATCH] Make sudo+requiretty and ANSIBLE_PIPELINING work together MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pipelining is a *significant* performance benefit, because each task can be completed with a single SSH connection (vs. one ssh connection at the start to mkdir, plus one sftp and one ssh per task). Pipelining is disabled by default in Ansible because it conflicts with the use of sudo if 'Defaults requiretty' is set in /etc/sudoers (as it is on Red Hat) and su (which always requires a tty). We can (and already do) make sudo/su happy by using "ssh -t" to allocate a tty, but then the python interpreter goes into interactive mode and is unhappy with module source being written to its stdin, per the following comment from connections/ssh.py: # we can only use tty when we are not pipelining the modules. # piping data into /usr/bin/python inside a tty automatically # invokes the python interactive-mode but the modules are not # compatible with the interactive-mode ("unexpected indent" # mainly because of empty lines) Instead of the (current) drastic solution of turning off pipelining when we use a tty, we can instead use a tty but suppress the behaviour of the Python interpreter to switch to interactive mode. The easiest way to do this is to make its stdin *not* be a tty, e.g. with cat|python. This works, but there's a problem: ssh will ignore -t if its input isn't really a tty. So we could open a pseudo-tty and use that as ssh's stdin, but if we then write Python source into it, it's all echoed back to us (because we're a tty). So we have to use -tt to force tty allocation; in that case, however, ssh puts the tty into "raw" mode (~ICANON), so there is no good way for the process on the other end to detect EOF on stdin. So if we do: echo -e "print('hello world')\n"|ssh -tt someho.st "cat|python" …it hangs forever, because cat keeps on reading input even after we've closed our pipe into ssh's stdin. We can get around this by writing a special __EOF__ marker after writing in_data, and doing this: echo -e "print('hello world')\n__EOF__\n"|ssh -tt someho.st "sed -ne '/__EOF__/q' -e p|python" This works fine, but in fact I use a clever python one-liner by mgedmin to achieve the same effect without depending on sed (at the expense of a much longer command line, alas; Python really isn't one-liner-friendly). We also enable pipelining by default as a consequence. --- lib/ansible/constants.py | 2 +- lib/ansible/plugins/action/__init__.py | 6 ++-- lib/ansible/plugins/connection/ssh.py | 43 +++++++------------------ lib/ansible/plugins/shell/powershell.py | 2 +- lib/ansible/plugins/shell/sh.py | 11 +++++-- 5 files changed, 25 insertions(+), 39 deletions(-) diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py index 6ecaaac0b3b..fa215efe995 100644 --- a/lib/ansible/constants.py +++ b/lib/ansible/constants.py @@ -237,7 +237,7 @@ DEFAULT_NULL_REPRESENTATION = get_config(p, DEFAULTS, 'null_representation', # CONNECTION RELATED ANSIBLE_SSH_ARGS = get_config(p, 'ssh_connection', 'ssh_args', 'ANSIBLE_SSH_ARGS', '-o ControlMaster=auto -o ControlPersist=60s') ANSIBLE_SSH_CONTROL_PATH = get_config(p, 'ssh_connection', 'control_path', 'ANSIBLE_SSH_CONTROL_PATH', "%(directory)s/ansible-ssh-%%h-%%p-%%r") -ANSIBLE_SSH_PIPELINING = get_config(p, 'ssh_connection', 'pipelining', 'ANSIBLE_SSH_PIPELINING', False, boolean=True) +ANSIBLE_SSH_PIPELINING = get_config(p, 'ssh_connection', 'pipelining', 'ANSIBLE_SSH_PIPELINING', True, boolean=True) ANSIBLE_SSH_RETRIES = get_config(p, 'ssh_connection', 'retries', 'ANSIBLE_SSH_RETRIES', 0, integer=True) PARAMIKO_RECORD_HOST_KEYS = get_config(p, 'paramiko_connection', 'record_host_keys', 'ANSIBLE_PARAMIKO_RECORD_HOST_KEYS', True, boolean=True) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index e8f08ca3445..e1dec686d50 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -177,7 +177,7 @@ class ActionBase(with_metaclass(ABCMeta, object)): if tmp and "tmp" in tmp: # tmp has already been created return False - if not self._connection.has_pipelining or not self._play_context.pipelining or C.DEFAULT_KEEP_REMOTE_FILES or self._play_context.become_method == 'su': + if not self._connection.has_pipelining or not self._play_context.pipelining or C.DEFAULT_KEEP_REMOTE_FILES: # tmp is necessary to store the module source code # or we want to keep the files on the target system return True @@ -438,7 +438,9 @@ class ActionBase(with_metaclass(ABCMeta, object)): # not sudoing or sudoing to root, so can cleanup files in the same step rm_tmp = tmp - cmd = self._connection._shell.build_module_command(environment_string, shebang, cmd, arg_path=args_file_path, rm_tmp=rm_tmp) + python_interp = task_vars.get('ansible_python_interpreter', 'python') + + cmd = self._connection._shell.build_module_command(environment_string, shebang, cmd, arg_path=args_file_path, rm_tmp=rm_tmp, python_interpreter=python_interp) cmd = cmd.strip() sudoable = True diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index debe36bd320..607dcd667fe 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -241,7 +241,7 @@ class Connection(ConnectionBase): return self._command - def _send_initial_data(self, fh, in_data): + def _send_initial_data(self, fh, in_data, tty=False): ''' Writes initial data to the stdin filehandle of the subprocess and closes it. (The handle must be closed; otherwise, for example, "sftp -b -" will @@ -252,6 +252,8 @@ class Connection(ConnectionBase): try: fh.write(in_data) + if tty: + fh.write("__EOF__942d747a0772c3284ffb5920e234bd57__\n") fh.close() except (OSError, IOError): raise AnsibleConnectionFailure('SSH Error: data could not be sent to the remote host. Make sure this host can be reached over ssh') @@ -314,7 +316,7 @@ class Connection(ConnectionBase): return ''.join(output), remainder - def _run(self, cmd, in_data, sudoable=True): + def _run(self, cmd, in_data, sudoable=True, tty=False): ''' Starts the command and communicates with it until it ends. ''' @@ -322,25 +324,10 @@ class Connection(ConnectionBase): display_cmd = map(pipes.quote, cmd[:-1]) + [cmd[-1]] display.vvv('SSH: EXEC {0}'.format(' '.join(display_cmd)), host=self.host) - # Start the given command. If we don't need to pipeline data, we can try - # to use a pseudo-tty (ssh will have been invoked with -tt). If we are - # pipelining data, or can't create a pty, we fall back to using plain - # old pipes. + # Start the given command. - p = None - if not in_data: - try: - # Make sure stdin is a proper pty to avoid tcgetattr errors - master, slave = pty.openpty() - p = subprocess.Popen(cmd, stdin=slave, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdin = os.fdopen(master, 'w', 0) - os.close(slave) - except (OSError, IOError): - p = None - - if not p: - p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) - stdin = p.stdin + p = subprocess.Popen(cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE) + stdin = p.stdin # If we are using SSH password authentication, write the password into # the pipe we opened in _build_command. @@ -403,7 +390,7 @@ class Connection(ConnectionBase): # before we call select. if states[state] == 'ready_to_send' and in_data: - self._send_initial_data(stdin, in_data) + self._send_initial_data(stdin, in_data, tty) state += 1 while True: @@ -501,7 +488,7 @@ class Connection(ConnectionBase): if states[state] == 'ready_to_send': if in_data: - self._send_initial_data(stdin, in_data) + self._send_initial_data(stdin, in_data, tty) state += 1 # Now we're awaiting_exit: has the child process exited? If it has, @@ -557,17 +544,9 @@ class Connection(ConnectionBase): display.vvv("ESTABLISH SSH CONNECTION FOR USER: {0}".format(self._play_context.remote_user), host=self._play_context.remote_addr) - # we can only use tty when we are not pipelining the modules. piping - # data into /usr/bin/python inside a tty automatically invokes the - # python interactive-mode but the modules are not compatible with the - # interactive-mode ("unexpected indent" mainly because of empty lines) + cmd = self._build_command('ssh', '-tt', self.host, cmd) - if in_data: - cmd = self._build_command('ssh', self.host, cmd) - else: - cmd = self._build_command('ssh', '-tt', self.host, cmd) - - (returncode, stdout, stderr) = self._run(cmd, in_data, sudoable=sudoable) + (returncode, stdout, stderr) = self._run(cmd, in_data, sudoable=sudoable, tty=True) return (returncode, stdout, stderr) diff --git a/lib/ansible/plugins/shell/powershell.py b/lib/ansible/plugins/shell/powershell.py index ead0c02fa6f..5d7d2a07be3 100644 --- a/lib/ansible/plugins/shell/powershell.py +++ b/lib/ansible/plugins/shell/powershell.py @@ -103,7 +103,7 @@ class ShellModule(object): ''' % dict(path=path) return self._encode_script(script) - def build_module_command(self, env_string, shebang, cmd, arg_path=None, rm_tmp=None): + def build_module_command(self, env_string, shebang, cmd, arg_path=None, rm_tmp=None, python_interpreter=None): cmd_parts = shlex.split(to_bytes(cmd), posix=False) cmd_parts = map(to_unicode, cmd_parts) if shebang and shebang.lower() == '#!powershell': diff --git a/lib/ansible/plugins/shell/sh.py b/lib/ansible/plugins/shell/sh.py index 54d2b57c8fe..9d7fb034dd0 100644 --- a/lib/ansible/plugins/shell/sh.py +++ b/lib/ansible/plugins/shell/sh.py @@ -134,12 +134,17 @@ class ShellModule(object): cmd = "%s; %s || (echo \'0 \'%s)" % (test, cmd, shell_escaped_path) return cmd - def build_module_command(self, env_string, shebang, cmd, arg_path=None, rm_tmp=None): + def build_module_command(self, env_string, shebang, cmd, arg_path=None, rm_tmp=None, python_interpreter='python'): # don't quote the cmd if it's an empty string, because this will # break pipelining mode - if cmd.strip() != '': + env = env_string.strip() + exe = shebang.replace("#!", "").strip() + if cmd.strip() == '': + reader = "%s -uc 'import sys; [sys.stdout.write(s) for s in iter(sys.stdin.readline, \"__EOF__942d747a0772c3284ffb5920e234bd57__\\n\")]'|" % python_interpreter + cmd_parts = [env, reader, env, exe] + else: cmd = pipes.quote(cmd) - cmd_parts = [env_string.strip(), shebang.replace("#!", "").strip(), cmd] + cmd_parts = [env, exe, cmd] if arg_path is not None: cmd_parts.append(arg_path) new_cmd = " ".join(cmd_parts)