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.

Sign up for GitHub

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

Jump to bottom

Add a timeout to the sshDialer to prevent indefinite hangs. #23843

Merged
merged 1 commit into from
Apr 5, 2016
Merged

Add a timeout to the sshDialer to prevent indefinite hangs. #23843

merged 1 commit into from
Apr 5, 2016

Conversation

cjcullen
Copy link
Member

@cjcullen cjcullen commented Apr 4, 2016

Prevents the SSH Dialer from hanging forever. Fixes a problem where SSH Tunnels get stuck trying to open.

Addresses #23835.

@cjcullen
Copy link
Member Author

cjcullen commented Apr 4, 2016

@a-robinson @roberthbailey

@k8s-cherrypick-bot
Copy link

Removing label cherrypick-candidate because no release milestone was set. This is an invalid state and thus this PR is not being considered for cherry-pick to any release branch. Please add an appropriate release milestone and then re-add the label.

@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Apr 4, 2016
@cjcullen cjcullen added this to the v1.2 milestone Apr 4, 2016
@a-robinson a-robinson added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Apr 4, 2016
a-robinson
a-robinson reviewed Apr 4, 2016
View reviewed changes
pkg/ssh/ssh.go
@@ -163,11 +163,38 @@ func (d *realSSHDialer) Dial(network, addr string, config *ssh.ClientConfig) (*s
return ssh.Dial(network, addr, config)
}

// timeoutDialer wraps an sshDialer with a timeout around Dial().
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a comment either here or up above where it's used with a little more context on why this is needed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@a-robinson
Copy link
Contributor

LGTM

@a-robinson a-robinson added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 4, 2016
@k8s-bot
Copy link

k8s-bot commented Apr 4, 2016

GCE e2e build/test passed for commit 0f113d3bdd17c156c94d708b1f4994e11e73fb16.

@roberthbailey
Copy link
Contributor

LGTM.

@k8s-bot
Copy link

k8s-bot commented Apr 5, 2016

GCE e2e build/test passed for commit cde4f6d.

@cjcullen
Copy link
Member Author

cjcullen commented Apr 5, 2016

@k8s-bot unit test this issue: #21640

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Apr 5, 2016

GCE e2e build/test passed for commit cde4f6d.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 52276bc into kubernetes:master Apr 5, 2016
@cjcullen cjcullen mentioned this pull request Apr 6, 2016
Merged
@roberthbailey roberthbailey added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Apr 6, 2016
zmerlynn added a commit that referenced this pull request Apr 6, 2016
@zmerlynn
Merge pull request #23901 from cjcullen/automated-cherry-pick-of-#238…
8099c6a 
…43-upstream-release-1.2

Automated cherry pick of #23843
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

@cjcullen cjcullen mentioned this pull request Apr 12, 2016
Closed
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@zmerlynn
Merge pull request kubernetes#23901 from cjcullen/automated-cherry-pi…
c819e1f 
…ck-of-#23843-upstream-release-1.2

Automated cherry pick of kubernetes#23843
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
@zmerlynn
Merge pull request kubernetes#23901 from cjcullen/automated-cherry-pi…
eb5efc4 
…ck-of-#23843-upstream-release-1.2

Automated cherry pick of kubernetes#23843
@PengTaoWW PengTaoWW mentioned this pull request Dec 29, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@a-robinson a-robinson

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.
8 participants
@cjcullen @k8s-cherrypick-bot @k8s-github-robot @a-robinson @k8s-bot @roberthbailey @k8s-teamcity-mesosphere @googlebot