Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve ssh error #56460

Merged
merged 3 commits into from May 23, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -0,0 +1,2 @@
bugfixes:
- preserve actual ssh error when we cannot connect.
@@ -413,7 +413,7 @@ def wrapped(self, *args, **kwargs):
break

# 5 = Invalid/incorrect password from sshpass
except AnsibleAuthenticationFailure as e:
except AnsibleAuthenticationFailure:
# Raising this exception, which is subclassed from AnsibleConnectionFailure, prevents further retries
raise

@@ -686,15 +686,15 @@ def _send_initial_data(self, fh, in_data, ssh_process):
try:
fh.write(to_bytes(in_data))
fh.close()
except (OSError, IOError):
except (OSError, IOError) as e:
# 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
'Data could not be sent to remote host "%s". Make sure this host can be reached '
This conversation was marked as resolved by bcoca

This comment has been minimized.

Copy link
@agaffney

agaffney May 22, 2019

Contributor

Why remove the SSH Error: prefix here, but not below? You should probably just leave it here instead of removing it

This comment has been minimized.

Copy link
@bcoca

bcoca May 22, 2019

Author Member

we already add an [ERROR] and 'connection name' ... seemed redundant, but i missed the other one

'over ssh: %s' % (self.host, to_native(e)), orig_exc=e
)

display.debug('Sent initial data (%d bytes)' % len(in_data))
@@ -1030,11 +1030,15 @@ def _bare_run(self, cmd, in_data, sudoable=True, checkrc=True):
# If we find a broken pipe because of ControlPersist timeout expiring (see #16731),
# we raise a special exception so that we can retry a connection.
controlpersist_broken_pipe = b'mux_client_hello_exchange: write packet: Broken pipe' in b_stderr
if p.returncode == 255 and controlpersist_broken_pipe:
raise AnsibleControlPersistBrokenPipeError('SSH Error: data could not be sent because of ControlPersist broken pipe.')
if p.returncode == 255:

additional = to_native(b_stderr)
if controlpersist_broken_pipe:
raise AnsibleControlPersistBrokenPipeError('Data could not be sent because of ControlPersist broken pipe: %s' % additional)

if p.returncode == 255 and in_data and checkrc:
raise AnsibleConnectionFailure('SSH Error: data could not be sent to remote host "%s". Make sure this host can be reached over ssh' % self.host)
elif in_data and checkrc:
raise AnsibleConnectionFailure('Data could not be sent to remote host "%s". Make sure this host can be reached over ssh: %s'
% (self.host, additional))

return (p.returncode, b_stdout, b_stderr)

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.