Skip to content
This repository has been archived by the owner on Oct 30, 2018. It is now read-only.

Be explicit about specifying the ssh port if it was user specified #4302

Merged
merged 1 commit into from Aug 8, 2016

Conversation

abadger
Copy link
Contributor

@abadger abadger commented Aug 2, 2016

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

synchronize.py

ANSIBLE VERSION
devel

Probably affects all 2.x versions and possibly earlier
SUMMARY

Previously, if the port specified by the user or inventory was 22, then
the ssh client port would be used instead.

Fixes #3895

Previously, if the port specified by the user or inventory was 22, then
the ssh client port would be used instead.

Fixes ansible#3895
@abadger
Copy link
Contributor Author

abadger commented Aug 2, 2016

@tima, @dvzubarev see if this fixes the problem for you.

Note -- I changed the documentation to match what the code was doing when the dest_port is unspecified. I didn't want to change the behaviour of the code too much because people might be relying on synchronize to pick up ssh client configuration when they didn't specify any port setting explicitly.

@gregdek
Copy link
Contributor

gregdek commented Aug 2, 2016

Thanks @abadger. To the current maintainers, @tima please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with text 'shipit', 'needs_revision' or 'close_me' as appropriate.

[This message brought to you by your friendly Ansibull-bot.]

@tima
Copy link
Contributor

tima commented Aug 3, 2016

Looks right to me based on what we discussed. @dvzubarev?

@gregdek
Copy link
Contributor

gregdek commented Aug 3, 2016

Thanks @abadger for this PR. This module is maintained by the Ansible core team, so it can take a while for patches to be reviewed. Thanks for your patience.

Core team: please review according to guidelines (http://docs.ansible.com/ansible/developing_modules.html#module-checklist) and comment with 'needs_revision' or merge as appropriate.

[This message brought to you by your friendly Ansibull-bot.]

@tima
Copy link
Contributor

tima commented Aug 8, 2016

@dvzubarev confirmed that this fixed the issue they reported in #3895.

shipit

@abadger abadger merged commit 5d7b46e into ansible:devel Aug 8, 2016
@abadger abadger deleted the synchronize-fix-default-dest-port branch August 8, 2016 15:59
abadger added a commit that referenced this pull request Aug 8, 2016
…4302)

Previously, if the port specified by the user or inventory was 22, then
the ssh client port would be used instead.

Fixes #3895
@abadger
Copy link
Contributor Author

abadger commented Aug 8, 2016

Merged to devel and cherry-picked to the stable-2.1 branch

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Synchronize: a problem with a non-standard ssh port
3 participants