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

ssh: only rely on timeout/retries if one is set #116

Merged
merged 1 commit into from Jul 27, 2022

Conversation

lbajolet-hashicorp
Copy link
Contributor

In a configuration, when either the SSH timeout or the SSH handshake
retries are set, the other attribute would also be set to its default
value.

This in turn could cause a problem where a user expects his choice to be
the one that makes the final decision to abort, but in the end this
choice could cause the build to be cancelled independently as the other
setting may cause a premature cancellation.

To avoid this, we clarify the documentation, and ensure that when one is
set, the other will be ignored.

Closes: #87

@lbajolet-hashicorp lbajolet-hashicorp requested a review from a team as a code owner July 11, 2022 20:31
@lbajolet-hashicorp lbajolet-hashicorp changed the title ssh: only rely on timeout/retries is one is set ssh: only rely on timeout/retries if one is set Jul 12, 2022
Copy link
Member

@nywilken nywilken left a comment

Choose a reason for hiding this comment

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

This looks good to me. One nit.

communicator/step_connect_ssh.go Outdated Show resolved Hide resolved
@lbajolet-hashicorp
Copy link
Contributor Author

Good call on the nit! I'll merge this once tests pass

In a configuration, when either the SSH timeout or the SSH handshake
retries are set, the other attribute would also be set to its default
value.

This in turn could cause a problem where a user expects his choice to be
the one that makes the final decision to abort, but in the end this
choice could cause the build to be cancelled independently as the other
setting may cause a premature cancellation.

To avoid this, we clarify the documentation, and ensure that when one is
set, the other will be ignored.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ssh_timeout not working windows and wsl
2 participants