Skip to content
This repository has been archived by the owner on Mar 21, 2022. It is now read-only.

Commit

Permalink
#446 make default docker client thread safe (#744)
Browse files Browse the repository at this point in the history
* make default docker client thread safe

* reapplied fix after merge

* checkstyle

* checkstyle and imports
  • Loading branch information
unicolet authored and mattnworb committed May 18, 2017
1 parent d331a82 commit cde07b4
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 9 deletions.
18 changes: 9 additions & 9 deletions src/main/java/com/spotify/docker/client/DefaultDockerClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ public void progress(ProgressMessage message) throws DockerException {
}

}

/**
* Hack: this {@link ProgressHandler} is meant to capture the image names
* of an image being loaded. Weirdly enough, Docker returns the name of a newly
Expand Down Expand Up @@ -257,7 +257,7 @@ public void progress(ProgressMessage message) throws DockerException {
if (streamMatcher.matches()) {
imageNames.add(streamMatcher.group("image"));
}

}
}

Expand All @@ -275,7 +275,7 @@ public void progress(ProgressMessage message) throws DockerException {
private static final long DEFAULT_READ_TIMEOUT_MILLIS = SECONDS.toMillis(30);
private static final int DEFAULT_CONNECTION_POOL_SIZE = 100;

private static final ClientConfig DEFAULT_CONFIG = new ClientConfig(
private final ClientConfig defaultConfig = new ClientConfig(
ObjectMapperProvider.class,
JacksonFeature.class,
LogsResponseReader.class,
Expand Down Expand Up @@ -327,7 +327,7 @@ public ClientBuilder get() {
private static final GenericType<List<Task>> TASK_LIST = new GenericType<List<Task>>() { };

private static final GenericType<List<Node>> NODE_LIST = new GenericType<List<Node>>() { };

private static final GenericType<List<Secret>> SECRET_LIST = new GenericType<List<Secret>>() { };

private final Client client;
Expand Down Expand Up @@ -409,7 +409,7 @@ protected DefaultDockerClient(final Builder builder) {
.setSocketTimeout((int) builder.readTimeoutMillis)
.build();

final ClientConfig config = DEFAULT_CONFIG
final ClientConfig config = defaultConfig
.connectorProvider(new ApacheConnectorProvider())
.property(ApacheClientProperties.CONNECTION_MANAGER, cm)
.property(ApacheClientProperties.REQUEST_CONFIG, requestConfig);
Expand Down Expand Up @@ -1103,10 +1103,10 @@ public Set<String> load(final InputStream imagePayload, final ProgressHandler ha
.path("images")
.path("load")
.queryParam("quiet", "false");

final LoadProgressHandler loadProgressHandler = new LoadProgressHandler(handler);
final Entity<InputStream> entity = Entity.entity(imagePayload, APPLICATION_OCTET_STREAM);

try (final ProgressStream load =
request(POST, ProgressStream.class, resource,
resource.request(APPLICATION_JSON_TYPE), entity)) {
Expand Down Expand Up @@ -1975,7 +1975,7 @@ public List<Node> listNodes() throws DockerException, InterruptedException {
WebTarget resource = resource().path("nodes");
return request(GET, NODE_LIST, resource, resource.request(APPLICATION_JSON_TYPE));
}

@Override
public void execResizeTty(final String execId,
final Integer height,
Expand Down Expand Up @@ -2079,7 +2079,7 @@ public List<Network> listNetworks(final ListNetworksParam... params)
resource = addParameters(resource, params);
return request(GET, NETWORK_LIST, resource, resource.request(APPLICATION_JSON_TYPE));
}

@Override
public Network inspectNetwork(String networkId) throws DockerException, InterruptedException {
final WebTarget resource = resource().path("networks").path(networkId);
Expand Down
105 changes: 105 additions & 0 deletions src/test/java/com/spotify/docker/client/ConnectionPoolTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*-
* -\-\-
* docker-client
* --
* Copyright (C) 2016 Spotify AB
* --
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
* -/-/-
*/

package com.spotify.docker.client;

import static java.lang.Long.toHexString;

import com.spotify.docker.client.exceptions.DockerCertificateException;
import com.spotify.docker.client.exceptions.DockerException;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.ThreadLocalRandom;
import org.hamcrest.MatcherAssert;
import org.hamcrest.Matchers;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;
import org.junit.rules.TestName;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* Checks thread safety of DefaultDockerClient implementation.
*
* @author Umberto Nicoletti ([email protected])
*/
public class ConnectionPoolTest {

private static final String BUSYBOX = "busybox";
private static final String BUSYBOX_LATEST = BUSYBOX + ":latest";
private static final String BUSYBOX_BUILDROOT_2013_08_1 = BUSYBOX + ":buildroot-2013.08.1";
private static final String MEMCACHED = "rohan/memcached-mini";
private static final String CIRROS_PRIVATE = "dxia/cirros-private";

private static final Logger log = LoggerFactory.getLogger(ConnectionPoolTest.class);

@Rule
public final ExpectedException exception = ExpectedException.none();

@Rule
public final TestName testName = new TestName();

private final String nameTag = toHexString(ThreadLocalRandom.current().nextLong());

/**
* Checks that running a parallel operation does not break DefaultDockerClient.
* Fixes issue #446.
*
* @throws Exception on error.
*/
@Test
public void testParallelOperation() throws Exception {
final ExecutorService executor = Executors.newFixedThreadPool(5);
List<Future<Exception>> tasks = new ArrayList<>(20);
for (int i = 0; i < 20; i++) {
tasks.add(executor.submit(
new Callable<Exception>() {
@Override
public Exception call() throws Exception {
try (DockerClient docker = DefaultDockerClient.fromEnv().build()) {
docker.pull(ConnectionPoolTest.BUSYBOX_LATEST);
docker.pull(ConnectionPoolTest.BUSYBOX_BUILDROOT_2013_08_1);
final String name = "test-repo/tag-force:sometag";
docker.tag(ConnectionPoolTest.BUSYBOX_LATEST, name);
docker.tag(ConnectionPoolTest.BUSYBOX_BUILDROOT_2013_08_1, name, true);
} catch (InterruptedException | DockerException | DockerCertificateException e) {
ConnectionPoolTest.log.error(
"Error running task: {}", e.getMessage(), e
);
return e;
}
return null;
}
}
)
);
}
executor.shutdown();
while (!executor.isTerminated()) {};
for (final Future<Exception> task : tasks) {
MatcherAssert.assertThat(task.get(), Matchers.nullValue());
}
}
}

0 comments on commit cde07b4

Please sign in to comment.