Skip to content

Commit 978ac88

Browse files
KostyaShaKostyaSha
authored
Add timeouts, cleanup (docker-java#1336)
* Add timeouts, cleanup * Enlarge timeout * restore value ffor tests * Mac ttypos * Configurable options with default * allign * Comment * Unformat
1 parent fe0ce66 commit 978ac88

File tree

6 files changed

+68
-60
lines changed

6 files changed

+68
-60
lines changed

‎docker-java-core/src/main/java/com/github/dockerjava/core/AbstractDockerCmdExecFactory.java‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,9 @@ public abstract class AbstractDockerCmdExecFactory implements DockerCmdExecFacto
150150

151151
private DockerClientConfig dockerClientConfig;
152152

153+
protected Integer connectTimeout;
154+
protected Integer readTimeout;
155+
153156
protected DockerClientConfig getDockerClientConfig() {
154157
checkNotNull(dockerClientConfig,
155158
"Factor not initialized, dockerClientConfig not set. You probably forgot to call init()!");
@@ -172,6 +175,22 @@ public CopyArchiveToContainerCmd.Exec createCopyArchiveToContainerCmdExec() {
172175
return new CopyArchiveToContainerCmdExec(getBaseResource(), getDockerClientConfig());
173176
}
174177

178+
/**
179+
* Configure connection timeout in milliseconds
180+
*/
181+
public AbstractDockerCmdExecFactory withConnectTimeout(Integer connectTimeout) {
182+
this.connectTimeout = connectTimeout;
183+
return this;
184+
}
185+
186+
/**
187+
* Configure read timeout in milliseconds
188+
*/
189+
public AbstractDockerCmdExecFactory withReadTimeout(Integer readTimeout) {
190+
this.readTimeout = readTimeout;
191+
return this;
192+
}
193+
175194
@Override
176195
public AuthCmd.Exec createAuthCmdExec() {
177196
return new AuthCmdExec(getBaseResource(), getDockerClientConfig());

‎docker-java-transport-jersey/src/main/java/com/github/dockerjava/jaxrs/JerseyDockerCmdExecFactory.java‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,6 @@ public class JerseyDockerCmdExecFactory extends AbstractDockerCmdExecFactory {
5050

5151
private JerseyWebTarget baseResource;
5252

53-
private Integer readTimeout = null;
54-
55-
private Integer connectTimeout = null;
56-
5753
private Integer maxTotalConnections = null;
5854

5955
private Integer maxPerRouteConnections = null;
@@ -262,16 +258,6 @@ public void close() throws IOException {
262258
connManager.close();
263259
}
264260

265-
public JerseyDockerCmdExecFactory withReadTimeout(Integer readTimeout) {
266-
this.readTimeout = readTimeout;
267-
return this;
268-
}
269-
270-
public JerseyDockerCmdExecFactory withConnectTimeout(Integer connectTimeout) {
271-
this.connectTimeout = connectTimeout;
272-
return this;
273-
}
274-
275261
public JerseyDockerCmdExecFactory withMaxTotalConnections(Integer maxTotalConnections) {
276262
this.maxTotalConnections = maxTotalConnections;
277263
return this;

‎docker-java-transport-netty/src/main/java/com/github/dockerjava/netty/NettyDockerCmdExecFactory.java‎

Lines changed: 4 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.github.dockerjava.netty;
22

33
import static com.google.common.base.Preconditions.checkNotNull;
4+
import static java.util.Objects.nonNull;
45

56
import java.io.IOException;
67
import java.net.InetAddress;
@@ -58,7 +59,7 @@
5859
* @see https://docs.docker.com/engine/reference/api/docker_remote_api_v1.21/#attach-to-a-container
5960
* @see https://docs.docker.com/engine/reference/api/docker_remote_api_v1.21/#exec-start
6061
*/
61-
public class NettyDockerCmdExecFactory extends AbstractDockerCmdExecFactory implements DockerCmdExecFactory {
62+
public class NettyDockerCmdExecFactory extends AbstractDockerCmdExecFactory {
6263

6364
private static String threadPrefix = "dockerjava-netty";
6465

@@ -88,10 +89,6 @@ public DuplexChannel getChannel() {
8889
}
8990
};
9091

91-
private Integer connectTimeout = null;
92-
93-
private Integer readTimeout = null;
94-
9592
@Override
9693
public void init(DockerClientConfig dockerClientConfig) {
9794
super.init(dockerClientConfig);
@@ -292,29 +289,13 @@ public void close() throws IOException {
292289
eventLoopGroup.shutdownGracefully();
293290
}
294291

295-
/**
296-
* Configure connection timeout in milliseconds
297-
*/
298-
public NettyDockerCmdExecFactory withConnectTimeout(Integer connectTimeout) {
299-
this.connectTimeout = connectTimeout;
300-
return this;
301-
}
302-
303-
/**
304-
* Configure read timeout in milliseconds
305-
*/
306-
public NettyDockerCmdExecFactory withReadTimeout(Integer readTimeout) {
307-
this.readTimeout = readTimeout;
308-
return this;
309-
}
310-
311292
private <T extends Channel> T configure(T channel) {
312293
ChannelConfig channelConfig = channel.config();
313294

314-
if (connectTimeout != null) {
295+
if (nonNull(connectTimeout)) {
315296
channelConfig.setConnectTimeoutMillis(connectTimeout);
316297
}
317-
if (readTimeout != null) {
298+
if (nonNull(readTimeout)) {
318299
channel.pipeline().addLast("readTimeoutHandler", new ReadTimeoutHandler());
319300
}
320301

‎docker-java-transport-okhttp/src/main/java/com/github/dockerjava/okhttp/OkHttpDockerCmdExecFactory.java‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,45 @@
2020
import java.util.Collections;
2121
import java.util.concurrent.TimeUnit;
2222

23+
import static java.util.Objects.nonNull;
24+
2325
public class OkHttpDockerCmdExecFactory extends AbstractDockerCmdExecFactory {
2426

2527
private static final String SOCKET_SUFFIX = ".socket";
2628

2729
private ObjectMapper objectMapper;
2830

2931
private OkHttpClient okHttpClient;
32+
private Boolean retryOnConnectionFailure;
3033

3134
private HttpUrl baseUrl;
3235

36+
public OkHttpDockerCmdExecFactory setRetryOnConnectionFailure(Boolean retryOnConnectionFailure) {
37+
this.retryOnConnectionFailure = retryOnConnectionFailure;
38+
return this;
39+
}
40+
3341
@Override
3442
public void init(DockerClientConfig dockerClientConfig) {
3543
super.init(dockerClientConfig);
3644

37-
OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder()
38-
.readTimeout(0, TimeUnit.SECONDS)
39-
.retryOnConnectionFailure(true);
45+
OkHttpClient.Builder clientBuilder = new OkHttpClient.Builder();
46+
if (nonNull(readTimeout)) {
47+
clientBuilder.readTimeout(readTimeout, TimeUnit.MILLISECONDS);
48+
} else {
49+
// default is too small for most docker commands, set default like in jersey/netty
50+
clientBuilder.readTimeout(0, TimeUnit.MILLISECONDS);
51+
}
52+
53+
if (nonNull(connectTimeout)) {
54+
clientBuilder.connectTimeout(connectTimeout, TimeUnit.MILLISECONDS);
55+
}
56+
57+
if (nonNull(retryOnConnectionFailure)) {
58+
clientBuilder.retryOnConnectionFailure(retryOnConnectionFailure);
59+
} else {
60+
clientBuilder.retryOnConnectionFailure(true);
61+
}
4062

4163
URI dockerHost = dockerClientConfig.getDockerHost();
4264
switch (dockerHost.getScheme()) {
@@ -45,11 +67,9 @@ public void init(DockerClientConfig dockerClientConfig) {
4567
String socketPath = dockerHost.getPath();
4668

4769
if ("unix".equals(dockerHost.getScheme())) {
48-
clientBuilder
49-
.socketFactory(new UnixSocketFactory(socketPath));
70+
clientBuilder.socketFactory(new UnixSocketFactory(socketPath));
5071
} else {
51-
clientBuilder
52-
.socketFactory(new NamedPipeSocketFactory(socketPath));
72+
clientBuilder.socketFactory(new NamedPipeSocketFactory(socketPath));
5373
}
5474

5575
clientBuilder
@@ -72,8 +92,7 @@ public void init(DockerClientConfig dockerClientConfig) {
7292
SSLContext sslContext = sslConfig.getSSLContext();
7393
if (sslContext != null) {
7494
isSSL = true;
75-
clientBuilder
76-
.sslSocketFactory(sslContext.getSocketFactory(), new TrustAllX509TrustManager());
95+
clientBuilder.sslSocketFactory(sslContext.getSocketFactory(), new TrustAllX509TrustManager());
7796
}
7897
} catch (Exception e) {
7998
throw new RuntimeException(e);
@@ -88,14 +107,14 @@ public void init(DockerClientConfig dockerClientConfig) {
88107
case "unix":
89108
case "npipe":
90109
baseUrlBuilder = new HttpUrl.Builder()
91-
.scheme("http")
92-
.host("docker" + SOCKET_SUFFIX);
110+
.scheme("http")
111+
.host("docker" + SOCKET_SUFFIX);
93112
break;
94113
case "tcp":
95114
baseUrlBuilder = new HttpUrl.Builder()
96-
.scheme(isSSL ? "https" : "http")
97-
.host(dockerHost.getHost())
98-
.port(dockerHost.getPort());
115+
.scheme(isSSL ? "https" : "http")
116+
.host(dockerHost.getHost())
117+
.port(dockerHost.getPort());
99118
break;
100119
default:
101120
baseUrlBuilder = HttpUrl.get(dockerHost.toString()).newBuilder();

‎docker-java/src/test/java/com/github/dockerjava/cmd/CmdIT.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,19 @@ public enum FactoryType {
2323
NETTY(true) {
2424
@Override
2525
public DockerCmdExecFactory createExecFactory() {
26-
return new NettyDockerCmdExecFactory().withConnectTimeout(10 * 1000);
26+
return new NettyDockerCmdExecFactory().withConnectTimeout(30 * 1000);
2727
}
2828
},
2929
JERSEY(false) {
3030
@Override
3131
public DockerCmdExecFactory createExecFactory() {
32-
return new JerseyDockerCmdExecFactory().withConnectTimeout(10 * 1000);
32+
return new JerseyDockerCmdExecFactory().withConnectTimeout(30 * 1000);
3333
}
3434
},
3535
OKHTTP(true) {
3636
@Override
3737
public DockerCmdExecFactory createExecFactory() {
38-
return new OkHttpDockerCmdExecFactory();
38+
return new OkHttpDockerCmdExecFactory().withConnectTimeout(30 * 1000);
3939
}
4040
};
4141

‎docker-java/src/test/java/com/github/dockerjava/cmd/StatsCmdIT.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package com.github.dockerjava.cmd;
22

3+
import com.github.dockerjava.api.async.ResultCallbackTemplate;
34
import com.github.dockerjava.api.command.CreateContainerResponse;
45
import com.github.dockerjava.api.model.Statistics;
5-
import com.github.dockerjava.core.async.ResultCallbackTemplate;
66
import org.junit.Test;
77
import org.slf4j.Logger;
88
import org.slf4j.LoggerFactory;
@@ -28,8 +28,10 @@ public void testStatsStreaming() throws InterruptedException, IOException {
2828
dockerRule.getClient().startContainerCmd(container.getId()).exec();
2929

3030
boolean gotStats = false;
31-
try (StatsCallbackTest statsCallback = dockerRule.getClient().statsCmd(container.getId()).exec(
32-
new StatsCallbackTest(countDownLatch))) {
31+
try (StatsCallbackTest statsCallback = dockerRule.getClient()
32+
.statsCmd(container.getId())
33+
.exec(new StatsCallbackTest(countDownLatch))) {
34+
3335
assertTrue(countDownLatch.await(10, TimeUnit.SECONDS));
3436
gotStats = statsCallback.gotStats();
3537

@@ -52,8 +54,9 @@ public void testStatsNoStreaming() throws InterruptedException, IOException {
5254

5355
dockerRule.getClient().startContainerCmd(container.getId()).exec();
5456

55-
try (StatsCallbackTest statsCallback = dockerRule.getClient().statsCmd(container.getId()).withNoStream(true).exec(
56-
new StatsCallbackTest(countDownLatch))) {
57+
try (StatsCallbackTest statsCallback = dockerRule.getClient().statsCmd(container.getId())
58+
.withNoStream(true)
59+
.exec(new StatsCallbackTest(countDownLatch))) {
5760
countDownLatch.await(5, TimeUnit.SECONDS);
5861

5962
LOG.info("Stop stats collection");
@@ -67,7 +70,7 @@ public void testStatsNoStreaming() throws InterruptedException, IOException {
6770
assertEquals("Expected stats called only once", countDownLatch.getCount(), NUM_STATS - 1);
6871
}
6972

70-
private class StatsCallbackTest extends ResultCallbackTemplate<StatsCallbackTest, Statistics> {
73+
private static class StatsCallbackTest extends ResultCallbackTemplate<StatsCallbackTest, Statistics> {
7174
private final CountDownLatch countDownLatch;
7275

7376
private Boolean gotStats = false;

0 commit comments

Comments
 (0)