Enhancement: (re-)Add "Read Timeout" to DockerCloud. #610
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's possible for a docker API endpoint to seize up, accepting incoming connections but never replying. To cope with this situation, we need the docker-plugin to timeout and fail instead of just hanging.
pjdarton
commented
Jan 23, 2018
| client = DockerClientBuilder.getInstance( | ||
| new DefaultDockerClientConfig.Builder() | ||
| .withDockerHost(dockerHost.getUri()) | ||
| .withCustomSslConfig(toSSlConfig(dockerHost.getCredentialsId())) | ||
| ) | ||
| .withDockerCmdExecFactory(new NettyDockerCmdExecFactory() | ||
| .withConnectTimeout(connectTimeout)) |
There was a problem hiding this comment.
Note: I'm pretty sure that this old code was buggy - withConnectTimeout expects to be given milliseconds (or null for "no timeout"), but connectTimeout is in seconds.
pjdarton
commented
Jan 23, 2018
| final Version v = vc.exec(); | ||
| final String actualVersion = v.getVersion(); | ||
| final String actualApiVersion = v.getApiVersion(); | ||
| return FormValidation.ok("Version = " + actualVersion + ", API Version = " + actualApiVersion); |
There was a problem hiding this comment.
Splitting the test code up so that we only get one method call per line means that, in the event of a failure, the exception trace will tell us exactly where the problem came from.
e.g. If everything is on one line and we get a NullPointerException then it's impossible to see which bit failed.
1 task
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's possible for a docker API endpoint to seize up, accepting incoming connections but never replying. This causes docker-plugin's calls to the docker API to hang, causing Jenkins' call to the docker-plugin to hang, causing Jenkins' cloud functionality to hang, crippling Jenkins.
To handle this, the docker-plugin must timeout so that it won't wait forever for the docker-api to return, and treat a timeout as a failure. This will help mitigate #594.
This code change re-introduces the functionality as follows:
Note: This functionality was present in older versions of the plugin but was lost due to later versions of the docker-java library missing this functionality. There's a separate issue to put that back again. Once the official docker-java release is enhanced to support readTimeout, this plugin can remove its own enhanced NettyDockerCmdFactory and go back to using the docker-java version directly.