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

[WIP] Switch to shared docker client instance #193

Merged
merged 1 commit into from
Aug 6, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
3 changes: 2 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