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

Add auth_timeout parameter when supported by paramiko #50448

Merged
merged 3 commits into from Jan 9, 2019

Conversation

orgito
Copy link
Contributor

@orgito orgito commented Jan 2, 2019

Paramiko 2.2 introduces the auth_timeout parameter. This will set the
parameter to the same value of the timeout parameter to prevent
"Authentication timeout" errors.

SUMMARY

If the paramiko version installed is at least 2.2 include the auth_timeout parameter to the connect method with the same value used for the timeout method. This will prevent "Authentication timeout" errors when a slow authentication step (>30s) happens with a host. This is common with some network devices.

Fixes #42596

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

paramiko_ssh connection plugin

ADDITIONAL INFORMATION

I noticed that problem when using the network_cli connection plugin, which in turn uses the paramiko_ssh connection plugin.

@orgito orgito changed the title Add auth_timeout parameter when supported Add auth_timeout parameter when supported by paramiko Jan 2, 2019
@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. core_review In order to be merged, this PR must follow the core review workflow. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jan 2, 2019
Paramiko 2.2 introduces the auth_timeout parameter. This will set the
parameter to the same value of the timeout parameter to prevent
"Authentication timeout" errors.
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 2, 2019
Renamed sock_kwarg to ssh_connect_kwargs and conditionally added the
auth_timeout parameter based on the installed paramiko version.
@dagwieers
Copy link
Member

I am wondering whether there would ever be a need to set a different auth_timeout in paramiko ?
Other transports have separate values for connection_timeout/connect_timeout, read_timeout or operation_timeout, even when they default to another timeout value if unset.

@orgito
Copy link
Contributor Author

orgito commented Jan 8, 2019

The advantage of using the same value is that it retains current expected behavior when using a version of paramiko older than 2.2. So I don't think it is necessary to handle an extra parameter here.

Copy link
Member

@dagwieers dagwieers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@samdoran samdoran removed the needs_triage Needs a first human triage before being processed. label Jan 8, 2019
@samdoran
Copy link
Contributor

samdoran commented Jan 8, 2019

Please create a changelog fragment. See this fragment as an example.

@ansibot ansibot added the support:community This issue/PR relates to code supported by the Ansible community. label Jan 8, 2019
@ansibot

This comment has been minimized.

@ansibot ansibot added ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jan 8, 2019
@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. and removed ci_verified Changes made in this PR are causing tests to fail. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 8, 2019
@orgito
Copy link
Contributor Author

orgito commented Jan 9, 2019

Please create a changelog fragment. See this fragment as an example.

Thank you. It is done

@dagwieers
Copy link
Member

rebuild_merge

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed core_review In order to be merged, this PR must follow the core review workflow. labels Jan 9, 2019
@dagwieers dagwieers merged commit 6f9bca9 into ansible:devel Jan 9, 2019
@orgito orgito deleted the paramiko_add_auth_timeout branch January 9, 2019 01:14
orgito added a commit to orgito/ansible that referenced this pull request Jan 9, 2019
Add auth_timeout parameter when supported

Paramiko 2.2 introduces the auth_timeout parameter. This will set the
parameter to the same value of the timeout parameter to prevent
"Authentication timeout" errors.

(cherry picked from commit e7f21dd)

Conditionally add auth_timeout to ssh.connect

Renamed sock_kwarg to ssh_connect_kwargs and conditionally added the
auth_timeout parameter based on the installed paramiko version.

(cherry picked from commit 6c41e97)

Add changelog fragment

(cherry picked from commit 7679a92)
abadger pushed a commit that referenced this pull request Jan 9, 2019
Add auth_timeout parameter when supported

Paramiko 2.2 introduces the auth_timeout parameter. This will set the
parameter to the same value of the timeout parameter to prevent
"Authentication timeout" errors.

(cherry picked from commit e7f21dd)

Conditionally add auth_timeout to ssh.connect

Renamed sock_kwarg to ssh_connect_kwargs and conditionally added the
auth_timeout parameter based on the installed paramiko version.

(cherry picked from commit 6c41e97)

Add changelog fragment

(cherry picked from commit 7679a92)
kbreit pushed a commit to kbreit/ansible that referenced this pull request Jan 11, 2019
* Add auth_timeout parameter when supported

Paramiko 2.2 introduces the auth_timeout parameter. This will set the
parameter to the same value of the timeout parameter to prevent
"Authentication timeout" errors.

* Conditionally add auth_timeout to ssh.connect

Renamed sock_kwarg to ssh_connect_kwargs and conditionally added the
auth_timeout parameter based on the installed paramiko version.

* Add changelog fragment
knumskull pushed a commit to knumskull/ansible that referenced this pull request Jan 21, 2019
* Add auth_timeout parameter when supported

Paramiko 2.2 introduces the auth_timeout parameter. This will set the
parameter to the same value of the timeout parameter to prevent
"Authentication timeout" errors.

* Conditionally add auth_timeout to ssh.connect

Renamed sock_kwarg to ssh_connect_kwargs and conditionally added the
auth_timeout parameter based on the installed paramiko version.

* Add changelog fragment
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 bug This issue/PR relates to a bug. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Paramiko ignores timeout setting
5 participants