Skip to content

Commit

Permalink
Switch to shared docker client instance (netty client should be threa…
Browse files Browse the repository at this point in the history
…d-safe)
  • Loading branch information
rnorth committed Jul 31, 2016
1 parent 662f0af commit e2301eb
Show file tree
Hide file tree
Showing 10 changed files with 38 additions and 50 deletions.
10 changes: 6 additions & 4 deletions core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -204,11 +204,13 @@
<phase>package</phase>
<configuration>
<target>
<echo message="unjar" />
<unzip src="${project.build.directory}/${project.artifactId}-${project.version}.jar" dest="${project.build.directory}/exploded/" />
<echo message="unjar"/>
<unzip src="${project.build.directory}/${project.artifactId}-${project.version}.jar"
dest="${project.build.directory}/exploded/"/>
<move file="${project.build.directory}/exploded/META-INF/native/libnetty-transport-native-epoll.so"
tofile="${project.build.directory}/exploded/META-INF/native/liborg-testcontainers-shaded-netty-transport-native-epoll.so" />
<jar destfile="${project.build.directory}/${project.artifactId}-${project.version}.jar" basedir="${project.build.directory}/exploded" />
tofile="${project.build.directory}/exploded/META-INF/native/liborg-testcontainers-shaded-netty-transport-native-epoll.so"/>
<jar destfile="${project.build.directory}/${project.artifactId}-${project.version}.jar"
basedir="${project.build.directory}/exploded"/>
</target>
</configuration>
<goals>
Expand Down
25 changes: 6 additions & 19 deletions core/src/main/java/org/testcontainers/DockerClientFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -70,22 +70,14 @@ public synchronized static DockerClientFactory instance() {
*
* @return a new initialized Docker client
*/
public DockerClient client() {
return client(true);
}

/**
*
* @param failFast fail if client fails to ping Docker daemon
* @return a new initialized Docker client
*/
@Synchronized
public DockerClient client(boolean failFast) {
public DockerClient client() {

if (strategy == null) {
strategy = DockerClientProviderStrategy.getFirstValidStrategy(CONFIGURATION_STRATEGIES);
if (strategy != null) {
return strategy.getClient();
}

strategy = DockerClientProviderStrategy.getFirstValidStrategy(CONFIGURATION_STRATEGIES);
DockerClient client = strategy.getClient();

if (!preconditionsChecked) {
Expand All @@ -104,11 +96,6 @@ public DockerClient client(boolean failFast) {
preconditionsChecked = true;
}

if (failFast) {
// Ping, to fail fast if our docker environment has gone away
client.pingCmd().exec();
}

return client;
}

Expand Down Expand Up @@ -197,7 +184,7 @@ private void checkDiskSpace(DockerClient client) {
*/
public String getActiveApiVersion() {
if (!preconditionsChecked) {
client(true);
client();
}
return activeApiVersion;
}
Expand All @@ -207,7 +194,7 @@ public String getActiveApiVersion() {
*/
public String getActiveExecutionDriver() {
if (!preconditionsChecked) {
client(true);
client();
}
return activeExecutionDriver;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,12 +261,6 @@ public void stop() {
}

ResourceReaper.instance().stopAndRemoveContainer(containerId, imageName);

try {
dockerClient.close();
} catch (IOException e) {
logger().debug("Failed to close docker client");
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
*/
public abstract class DockerClientProviderStrategy {

protected DockerClient client;
protected DockerClientConfig config;

private static final RateLimiter PING_RATE_LIMITER = RateLimiterBuilder.newBuilder()
Expand Down Expand Up @@ -93,7 +94,7 @@ public static DockerClientProviderStrategy getFirstValidStrategy(List<DockerClie
* @return a usable, tested, Docker client configuration for the host system environment
*/
public DockerClient getClient() {
return getClientForConfig(config);
return client;
}

protected DockerClient getClientForConfig(DockerClientConfig config) {
Expand All @@ -114,7 +115,7 @@ protected void ping(DockerClient client, int timeoutInSeconds) {
}

public String getDockerHostIpAddress() {
return DockerClientConfigUtils.getDockerHostIpAddress(config);
return DockerClientConfigUtils.getDockerHostIpAddress(this.config);
}


Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.testcontainers.dockerclient;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.core.DockerClientConfig;
import org.testcontainers.utility.CommandLine;
import org.testcontainers.utility.DockerMachineClient;
Expand All @@ -18,8 +17,6 @@ public class DockerMachineClientProviderStrategy extends DockerClientProviderStr
@Override
public void test() throws InvalidConfigurationException {

DockerClient client;

try {
boolean installed = DockerMachineClient.instance().isInstalled();
checkArgument(installed, "docker-machine executable was not found on PATH (" + Arrays.toString(CommandLine.getSystemPath()) + ")");
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.testcontainers.dockerclient;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.core.DockerClientConfig;

/**
Expand All @@ -15,7 +14,7 @@ public void test() throws InvalidConfigurationException {
try {
// Try using environment variables
config = DockerClientConfig.createDefaultConfigBuilder().build();
DockerClient client = getClientForConfig(config);
client = getClientForConfig(config);

ping(client, 1);
} catch (Exception e) {
Expand All @@ -38,13 +37,11 @@ private String stringRepresentation(DockerClientConfig config) {
}

return " dockerHost=" + config.getDockerHost() + "\n" +
" dockerCertPath='" + config.getDockerCertPath() + "'\n" +
" dockerTlsVerify='" + config.getDockerTlsVerify() + "'\n" +
" apiVersion='" + config.getApiVersion() + "'\n" +
" registryUrl='" + config.getRegistryUrl() + "'\n" +
" registryUsername='" + config.getRegistryUsername() + "'\n" +
" registryPassword='" + config.getRegistryPassword() + "'\n" +
" registryEmail='" + config.getRegistryEmail() + "'\n" +
" dockerConfig='" + config.getDockerConfig() + "'\n";
" dockerConfig='" + config.toString() + "'\n";
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package org.testcontainers.dockerclient;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.core.DockerClientConfig;
import org.jetbrains.annotations.NotNull;

Expand Down Expand Up @@ -46,12 +45,11 @@ protected DockerClientConfig tryConfiguration(String dockerHost) {
throw new InvalidConfigurationException("Found docker unix domain socket but file mode was not as expected (expected: srwxr-xr-x). This problem is possibly due to occurrence of this issue in the past: https://github.com/docker/docker/issues/13121");
}

DockerClientConfig config;
config = new DockerClientConfig.DockerClientConfigBuilder()
config = DockerClientConfig.createDefaultConfigBuilder()
.withDockerHost(dockerHost)
.withDockerTlsVerify(false)
.build();
DockerClient client = getClientForConfig(config);
client = getClientForConfig(config);

ping(client, 3);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.testcontainers.images;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.exception.DockerClientException;
import com.github.dockerjava.api.model.Image;
import com.github.dockerjava.core.command.PullImageResultCallback;
import lombok.NonNull;
Expand All @@ -12,7 +13,6 @@
import org.testcontainers.utility.DockerLoggerFactory;
import org.testcontainers.utility.LazyFuture;

import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
Expand Down Expand Up @@ -40,7 +40,8 @@ protected final String resolve() {
profiler.setLogger(logger);

Profiler nested = profiler.startNested("Obtaining client");
try (DockerClient dockerClient = DockerClientFactory.instance().client(false)) {
DockerClient dockerClient = DockerClientFactory.instance().client();
try {
nested.stop();

profiler.start("Check local images");
Expand Down Expand Up @@ -88,7 +89,7 @@ protected final String resolve() {
}

return dockerImageName;
} catch (IOException e) {
} catch (DockerClientException e) {
throw new ContainerFetchException("Failed to get Docker client for " + dockerImageName, e);
} finally {
profiler.stop().log();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.command.BuildImageCmd;
import com.github.dockerjava.api.exception.DockerClientException;
import com.github.dockerjava.api.model.BuildResponseItem;
import com.github.dockerjava.core.command.BuildImageResultCallback;
import com.google.common.collect.Sets;
Expand Down Expand Up @@ -39,7 +40,8 @@ public class ImageFromDockerfile extends LazyFuture<String> implements

static {
Runtime.getRuntime().addShutdownHook(new Thread(() -> {
try (DockerClient dockerClientForCleaning = DockerClientFactory.instance().client(false)) {
DockerClient dockerClientForCleaning = DockerClientFactory.instance().client();
try {
for (String dockerImageName : imagesToDelete) {
log.info("Removing image tagged {}", dockerImageName);
try {
Expand All @@ -48,7 +50,7 @@ public class ImageFromDockerfile extends LazyFuture<String> implements
log.warn("Unable to delete image " + dockerImageName, e);
}
}
} catch (IOException e) {
} catch (DockerClientException e) {
throw new RuntimeException(e);
}
}));
Expand Down Expand Up @@ -91,7 +93,8 @@ protected final String resolve() {
Profiler profiler = new Profiler("Rule creation - build image");
profiler.setLogger(logger);

try (DockerClient dockerClient = DockerClientFactory.instance().client(false)) {
DockerClient dockerClient = DockerClientFactory.instance().client();
try {
if (deleteOnExit) {
imagesToDelete.add(dockerImageName);
}
Expand Down
10 changes: 9 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>

<groupId>org.testcontainers</groupId>
Expand Down Expand Up @@ -188,4 +189,11 @@
<url>https://api.bintray.com/maven/richnorth/maven/test-containers</url>
</repository>
</distributionManagement>

<repositories>
<repository>
<id>jitpack.io</id>
<url>https://jitpack.io</url>
</repository>
</repositories>
</project>

0 comments on commit e2301eb

Please sign in to comment.