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

Waiter until ssh connection can be estabilished #230

Merged
merged 1 commit into from
Jun 3, 2015
Merged

Waiter until ssh connection can be estabilished #230

merged 1 commit into from
Jun 3, 2015

Conversation

lanwen
Copy link
Member

@lanwen lanwen commented May 24, 2015

also:

  • add tests for all (except sshd successful connection)

Review on Reviewable

@lanwen lanwen changed the title Tests for PortUtils + javadoc + code cleanup Wait until ssh connection can be estabilished (wip #150) May 25, 2015
@KostyaSha
Copy link
Member

This conflicts with my work where i'm cleanuping DockerTemplate/Base/Slave/launcher and other classes. I'm unbundling launchers. So i wouldn't merge this change.

@lanwen lanwen changed the title Wait until ssh connection can be estabilished (wip #150) Wait until ssh connection can be estabilished (continue of #150) May 25, 2015
@KostyaSha KostyaSha mentioned this pull request May 25, 2015
21 tasks
LOGGER.info("Try to connect to sshd on {}:{}", host, port);

waitForSshOn(host, port);
super.launch(computer, listener);
Copy link
Member

Choose a reason for hiding this comment

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

I think after #234 it will be possible just to have retries as additional option for DockerSSHComputerLauncher, because it already wrapping sshlauncher/sshconnector to have ability to choose/configure launcher in UI.

@KostyaSha KostyaSha self-assigned this May 27, 2015
* @param time number of units
* @param units to convert to millis
*/
private static void sleepFor(int time, TimeUnit units) {
Copy link
Member

Choose a reason for hiding this comment

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

Please make it public

@KostyaSha
Copy link
Member

Could you send in separate PR PortUtils changes without removing/adding ssh launcher?

@KostyaSha
Copy link
Member

And could you unhardcode timeout values?

@lanwen lanwen changed the title Wait until ssh connection can be estabilished (continue of #150) Waiter until ssh connection can be estabilished Jun 3, 2015
- also remove unused method
- add javadocs
- every timeout settable
socket = new Socket(host, port);
available = true;
private int retries = 10;
private int sshTimeoutMillis = (int) SECONDS.toMillis(2);
Copy link
Member

Choose a reason for hiding this comment

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

mm... maybe better long?

Copy link
Member Author

Choose a reason for hiding this comment

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

ConnectionInfo connect(ServerHostKeyVerifier verifier, int connectTimeout, int readTimeout, int kexTimeout) - that's the main cause of int cast

KostyaSha added a commit that referenced this pull request Jun 3, 2015
Waiter until ssh connection can be estabilished
@KostyaSha KostyaSha merged commit 45bdf08 into jenkinsci:master Jun 3, 2015
@lanwen lanwen deleted the portutils branch June 3, 2015 21:10
}

/**
* @param host hostname to connect to
Copy link
Member

Choose a reason for hiding this comment

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

Some description about what kind of connection required: tcp/udp/ssh?

Copy link
Member Author

Choose a reason for hiding this comment

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

only tcp. Will add it next time

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.

2 participants