From e9f9bcae6a8f44dd98c6e4c69f13cef07acded95 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Tue, 12 Mar 2019 10:41:30 -0500 Subject: [PATCH] Don't raise AnsibleConnectionFailure if the ssh process has already died. (#53534) * Don't raise AnsibleConnectionFailure if the ssh_process has already died. Fixes #53487 * Better support for file not found messages * Add changelog fragment --- .../ssh-check-returncode-before-exception.yaml | 5 +++++ lib/ansible/plugins/action/__init__.py | 4 ++-- lib/ansible/plugins/connection/ssh.py | 16 ++++++++++++---- 3 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 changelogs/fragments/ssh-check-returncode-before-exception.yaml diff --git a/changelogs/fragments/ssh-check-returncode-before-exception.yaml b/changelogs/fragments/ssh-check-returncode-before-exception.yaml new file mode 100644 index 00000000000..ae6b48da723 --- /dev/null +++ b/changelogs/fragments/ssh-check-returncode-before-exception.yaml @@ -0,0 +1,5 @@ +bugfixes: +- ssh - Check the return code of the ssh process before raising AnsibleConnectionFailure, as the error message + for the ssh process will likely contain more useful information. This will improve the missing interpreter messaging + when using modules such as setup which have a larger payload to transfer when combined with pipelining. + (https://github.com/ansible/ansible/issues/53487) diff --git a/lib/ansible/plugins/action/__init__.py b/lib/ansible/plugins/action/__init__.py index 2c489b43245..b56438dd8ae 100644 --- a/lib/ansible/plugins/action/__init__.py +++ b/lib/ansible/plugins/action/__init__.py @@ -975,8 +975,8 @@ class ActionBase(with_metaclass(ABCMeta, object)): # try to figure out if we are missing interpreter if self._used_interpreter is not None: - match = '%s: No such file or directory' % self._used_interpreter.lstrip('!#') - if match in data['module_stderr'] or match in data['module_stdout']: + match = re.compile('%s: (?:No such file or directory|not found)' % self._used_interpreter.lstrip('!#')) + if match.search(data['module_stderr']) or match.search(data['module_stdout']): data['msg'] = "The module failed to execute correctly, you probably need to set the interpreter." # always append hint diff --git a/lib/ansible/plugins/connection/ssh.py b/lib/ansible/plugins/connection/ssh.py index 7b878601306..1531e41b03c 100644 --- a/lib/ansible/plugins/connection/ssh.py +++ b/lib/ansible/plugins/connection/ssh.py @@ -674,7 +674,7 @@ class Connection(ConnectionBase): return b_command - def _send_initial_data(self, fh, in_data): + def _send_initial_data(self, fh, in_data, ssh_process): ''' Writes initial data to the stdin filehandle of the subprocess and closes it. (The handle must be closed; otherwise, for example, "sftp -b -" will @@ -687,7 +687,15 @@ class Connection(ConnectionBase): fh.write(to_bytes(in_data)) fh.close() except (OSError, IOError): - raise AnsibleConnectionFailure('SSH Error: data could not be sent to remote host "%s". Make sure this host can be reached over ssh' % self.host) + # The ssh connection may have already terminated at this point, with a more useful error + # Only raise AnsibleConnectionFailure if the ssh process is still alive + time.sleep(0.001) + ssh_process.poll() + if getattr(ssh_process, 'returncode', None) is None: + raise AnsibleConnectionFailure( + 'SSH Error: data could not be sent to remote host "%s". Make sure this host can be reached ' + 'over ssh' % self.host + ) display.debug('Sent initial data (%d bytes)' % len(in_data)) @@ -865,7 +873,7 @@ class Connection(ConnectionBase): # If we can send initial data without waiting for anything, we do so # before we start polling if states[state] == 'ready_to_send' and in_data: - self._send_initial_data(stdin, in_data) + self._send_initial_data(stdin, in_data, p) state += 1 try: @@ -979,7 +987,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, p) state += 1 # Now we're awaiting_exit: has the child process exited? If it has,