Skip to content

Commit

Permalink
Use GenericContainer in RyukResourceReaper (#5358)
Browse files Browse the repository at this point in the history
* Experiment: Use `GenericContainer` in `RyukResourceReaper`

* reduce global `DockerClient` usage

* revert indentation

* Avoid bind check

* Fix `japicmp`

* Add `ResourceReaperTest`
  • Loading branch information
bsideup authored May 16, 2022
1 parent c0183f1 commit 98b60fb
Show file tree
Hide file tree
Showing 15 changed files with 257 additions and 146 deletions.
10 changes: 3 additions & 7 deletions core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,8 @@ tasks.japicmp {
classExcludes = []

methodExcludes = [
"org.testcontainers.utility.ResourceReaper#start(java.lang.String,com.github.dockerjava.api.DockerClient)",
"org.testcontainers.utility.ResourceReaper#start(com.github.dockerjava.api.DockerClient)",
"org.testcontainers.utility.ResourceReaper#registerNetworkForCleanup(java.lang.String)",
"org.testcontainers.utility.ResourceReaper#removeNetworks(java.lang.String)",
"org.testcontainers.images.builder.Transferable#of(java.lang.String)",
"org.testcontainers.images.builder.Transferable#updateChecksum(java.util.zip.Checksum)"
"org.testcontainers.containers.Container#getDockerClient()",
"org.testcontainers.containers.ContainerState#getDockerClient()",
]

fieldExcludes = []
Expand All @@ -62,7 +58,7 @@ configurations.all {
}

dependencies {
baseline 'org.testcontainers:testcontainers:1.16.3', {
baseline 'org.testcontainers:testcontainers:1.17.1', {
exclude group: "*", module: "*"
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
package org.testcontainers.containers;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.command.InspectContainerResponse;
import com.github.dockerjava.api.model.Container;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NonNull;
import org.testcontainers.DockerClientFactory;
import org.testcontainers.containers.wait.strategy.WaitStrategyTarget;

import java.util.ArrayList;
Expand All @@ -22,13 +22,15 @@ class ComposeServiceWaitStrategyTarget implements WaitStrategyTarget {

private final Container container;
private final GenericContainer proxyContainer;
private final DockerClient dockerClient;
@NonNull
private Map<Integer, Integer> mappedPorts;
@Getter(lazy=true)
private final InspectContainerResponse containerInfo = DockerClientFactory.instance().client().inspectContainerCmd(getContainerId()).exec();
private final InspectContainerResponse containerInfo = dockerClient.inspectContainerCmd(getContainerId()).exec();

ComposeServiceWaitStrategyTarget(Container container, GenericContainer proxyContainer,
ComposeServiceWaitStrategyTarget(DockerClient dockerClient, Container container, GenericContainer proxyContainer,
@NonNull Map<Integer, Integer> mappedPorts) {
this.dockerClient = dockerClient;
this.container = container;
this.proxyContainer = proxyContainer;
this.mappedPorts = new HashMap<>(mappedPorts);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ default SELF withClasspathResourceMapping(final String resourcePath, final Strin
* @param consumer consumer that the frames should be sent to
*/
default void followOutput(Consumer<OutputFrame> consumer) {
LogUtils.followOutput(DockerClientFactory.instance().client(), getContainerId(), consumer);
LogUtils.followOutput(getDockerClient(), getContainerId(), consumer);
}

/**
Expand All @@ -400,7 +400,7 @@ default void followOutput(Consumer<OutputFrame> consumer) {
* @param types types that should be followed (one or both of STDOUT, STDERR)
*/
default void followOutput(Consumer<OutputFrame> consumer, OutputFrame.OutputType... types) {
LogUtils.followOutput(DockerClientFactory.instance().client(), getContainerId(), consumer, types);
LogUtils.followOutput(getDockerClient(), getContainerId(), consumer, types);
}


Expand Down Expand Up @@ -439,8 +439,6 @@ default void followOutput(Consumer<OutputFrame> consumer, OutputFrame.OutputType
@Deprecated
Map<String, LinkableContainer> getLinkedContainers();

DockerClient getDockerClient();

void setExposedPorts(List<Integer> exposedPorts);

void setPortBindings(List<String> portBindings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ default String getContainerIpAddress() {
return getHost();
}

default DockerClient getDockerClient() {
return DockerClientFactory.lazyClient();
}

/**
* Get the host that this container may be reached on (may not be the local machine).
*
Expand Down Expand Up @@ -115,7 +119,7 @@ default boolean isHealthy() {
}

default InspectContainerResponse getCurrentContainerInfo() {
return DockerClientFactory.instance().client().inspectContainerCmd(getContainerId()).exec();
return getDockerClient().inspectContainerCmd(getContainerId()).exec();
}

/**
Expand Down Expand Up @@ -195,15 +199,15 @@ default List<Integer> getBoundPortNumbers() {
* @return all log output from the container from start until the current instant (both stdout and stderr)
*/
default String getLogs() {
return LogUtils.getOutput(DockerClientFactory.instance().client(), getContainerId());
return LogUtils.getOutput(getDockerClient(), getContainerId());
}

/**
* @param types log types to return
* @return all log output from the container from start until the current instant
*/
default String getLogs(OutputFrame.OutputType... types) {
return LogUtils.getOutput(DockerClientFactory.instance().client(), getContainerId(), types);
return LogUtils.getOutput(getDockerClient(), getContainerId(), types);
}

/**
Expand Down Expand Up @@ -234,7 +238,7 @@ default Container.ExecResult execInContainer(String... command) throws Unsupport
* @see ExecInContainerPattern#execInContainer(com.github.dockerjava.api.command.InspectContainerResponse, Charset, String...)
*/
default Container.ExecResult execInContainer(Charset outputCharset, String... command) throws UnsupportedOperationException, IOException, InterruptedException {
return ExecInContainerPattern.execInContainer(getContainerInfo(), outputCharset, command);
return ExecInContainerPattern.execInContainer(getDockerClient(), getContainerInfo(), outputCharset, command);
}

/**
Expand Down Expand Up @@ -279,7 +283,7 @@ default void copyFileToContainer(Transferable transferable, String containerPath
transferable.transferTo(tarArchive, containerPath);
tarArchive.finish();

DockerClientFactory.instance().client()
getDockerClient()
.copyArchiveToContainerCmd(getContainerId())
.withTarInputStream(new ByteArrayInputStream(byteArrayOutputStream.toByteArray()))
.withRemotePath("/")
Expand Down Expand Up @@ -316,7 +320,7 @@ default <T> T copyFileFromContainer(String containerPath, ThrowingFunction<Inpu
throw new IllegalStateException("copyFileFromContainer can only be used when the Container is created.");
}

DockerClient dockerClient = DockerClientFactory.instance().client();
DockerClient dockerClient = getDockerClient();
try (
InputStream inputStream = dockerClient.copyArchiveFromContainerCmd(getContainerId(), containerPath).exec();
TarArchiveInputStream tarInputStream = new TarArchiveInputStream(inputStream)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,15 +124,14 @@ public DockerComposeContainer(String identifier, File... composeFiles) {
}

public DockerComposeContainer(String identifier, List<File> composeFiles) {

this.composeFiles = composeFiles;
this.dockerComposeFiles = new DockerComposeFiles(composeFiles);

// Use a unique identifier so that containers created for this compose environment can be identified
this.identifier = identifier;
this.project = randomProjectId();

this.dockerClient = DockerClientFactory.instance().client();
this.dockerClient = DockerClientFactory.lazyClient();
}

@Override
Expand Down Expand Up @@ -269,7 +268,7 @@ private void waitUntilServiceStarted() {

private void createServiceInstance(Container container) {
String serviceName = getServiceNameFromContainer(container);
final ComposeServiceWaitStrategyTarget containerInstance = new ComposeServiceWaitStrategyTarget(container,
final ComposeServiceWaitStrategyTarget containerInstance = new ComposeServiceWaitStrategyTarget(dockerClient, container,
ambassadorContainer, ambassadorPortMappings.getOrDefault(serviceName, new HashMap<>()));

String containerId = containerInstance.getContainerId();
Expand Down Expand Up @@ -559,7 +558,7 @@ public Optional<ContainerState> getContainerByServiceName(String serviceName) {
}

private void followLogs(String containerId, Consumer<OutputFrame> consumer) {
LogUtils.followOutput(DockerClientFactory.instance().client(), containerId, consumer);
LogUtils.followOutput(dockerClient, containerId, consumer);
}

private SELF self() {
Expand Down Expand Up @@ -659,7 +658,7 @@ public void invoke() {

AuditLogger.doComposeLog(this.getCommandParts(), this.getEnv());

final Integer exitCode = this.dockerClient.inspectContainerCmd(getContainerId())
final Integer exitCode = getDockerClient().inspectContainerCmd(getContainerId())
.exec()
.getState()
.getExitCode();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import java.io.IOException;
import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;

/**
* Provides utility methods for executing commands in containers
Expand All @@ -22,24 +23,49 @@
@Slf4j
public class ExecInContainerPattern {


/**
*
* @deprecated use {@link #execInContainer(DockerClient, InspectContainerResponse, String...)}
*/
@Deprecated
public Container.ExecResult execInContainer(InspectContainerResponse containerInfo, String... command)
throws UnsupportedOperationException, IOException, InterruptedException {
DockerClient dockerClient = DockerClientFactory.instance().client();
return execInContainer(dockerClient, containerInfo, command);
}

/**
*
* @deprecated use {@link #execInContainer(DockerClient, InspectContainerResponse, Charset, String...)}
*/
@Deprecated
public Container.ExecResult execInContainer(InspectContainerResponse containerInfo, Charset outputCharset, String... command)
throws UnsupportedOperationException, IOException, InterruptedException {
DockerClient dockerClient = DockerClientFactory.instance().client();
return execInContainer(dockerClient, containerInfo, outputCharset, command);
}

/**
* Run a command inside a running container, as though using "docker exec", and interpreting
* the output as UTF8.
* <p></p>
* @param dockerClient the {@link DockerClient}
* @param containerInfo the container info
* @param command the command to execute
* @see #execInContainer(InspectContainerResponse, Charset, String...)
* @see #execInContainer(DockerClient, InspectContainerResponse, Charset, String...)
*/
public Container.ExecResult execInContainer(InspectContainerResponse containerInfo, String... command)
public Container.ExecResult execInContainer(DockerClient dockerClient, InspectContainerResponse containerInfo, String... command)
throws UnsupportedOperationException, IOException, InterruptedException {
return execInContainer(containerInfo, Charset.forName("UTF-8"), command);
return execInContainer(dockerClient, containerInfo, StandardCharsets.UTF_8, command);
}

/**
* Run a command inside a running container, as though using "docker exec".
* <p>
* This functionality is not available on a docker daemon running the older "lxc" execution driver. At
* the time of writing, CircleCI was using this driver.
* @param dockerClient the {@link DockerClient}
* @param containerInfo the container info
* @param outputCharset the character set used to interpret the output.
* @param command the parts of the command to run
Expand All @@ -48,7 +74,7 @@ public Container.ExecResult execInContainer(InspectContainerResponse containerIn
* @throws InterruptedException if the thread waiting for the response is interrupted
* @throws UnsupportedOperationException if the docker daemon you're connecting to doesn't support "exec".
*/
public Container.ExecResult execInContainer(InspectContainerResponse containerInfo, Charset outputCharset, String... command)
public Container.ExecResult execInContainer(DockerClient dockerClient, InspectContainerResponse containerInfo, Charset outputCharset, String... command)
throws UnsupportedOperationException, IOException, InterruptedException {
if (!TestEnvironment.dockerExecutionDriverSupportsExec()) {
// at time of writing, this is the expected result in CircleCI.
Expand All @@ -64,8 +90,6 @@ public Container.ExecResult execInContainer(InspectContainerResponse containerIn
String containerId = containerInfo.getId();
String containerName = containerInfo.getName();

DockerClient dockerClient = DockerClientFactory.instance().client();

log.debug("{}: Running \"exec\" command: {}", containerName, String.join(" ", command));
final ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(containerId)
.withAttachStdout(true).withAttachStderr(true).withCmd(command).exec();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ private void tryStart(Instant startedAt) {

if (!reusable) {
//noinspection deprecation
createCommand.getLabels().putAll(ResourceReaper.instance().getLabels());
createCommand = ResourceReaper.instance().register(this, createCommand);
}

if (!reused) {
Expand Down Expand Up @@ -469,7 +469,7 @@ private void tryStart(Instant startedAt) {
containerIsStarting(containerInfo, reused);

// Wait until the container has reached the desired running state
if (!this.startupCheckStrategy.waitUntilStartupSuccessful(dockerClient, containerId)) {
if (!this.startupCheckStrategy.waitUntilStartupSuccessful(this)) {
// Bail out, don't wait for the port to start listening.
// (Exception thrown here will be caught below and wrapped)
throw new IllegalStateException("Container did not start correctly.");
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.InspectContainerResponse;
import org.testcontainers.containers.GenericContainer;
import org.testcontainers.utility.DockerStatus;

/**
Expand All @@ -10,10 +11,24 @@
*/
public class IsRunningStartupCheckStrategy extends StartupCheckStrategy {

@SuppressWarnings("deprecation")
@Override
public boolean waitUntilStartupSuccessful(GenericContainer<?> container) {
// Optimization: container already has the initial "after start" state, check it first
if (checkState(container.getContainerInfo().getState()) == StartupStatus.SUCCESSFUL) {
return true;
}
return super.waitUntilStartupSuccessful(container);
}

@Override
public StartupStatus checkStartupState(DockerClient dockerClient, String containerId) {
InspectContainerResponse.ContainerState state = getCurrentState(dockerClient, containerId);
if (state.getRunning()) {
return checkState(state);
}

private StartupStatus checkState(InspectContainerResponse.ContainerState state) {
if (Boolean.TRUE.equals(state.getRunning())) {
return StartupStatus.SUCCESSFUL;
} else if (!DockerStatus.isContainerExitCodeSuccess(state)) {
return StartupStatus.FAILED;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import org.rnorth.ducttape.ratelimits.RateLimiter;
import org.rnorth.ducttape.ratelimits.RateLimiterBuilder;
import org.rnorth.ducttape.unreliables.Unreliables;
import org.testcontainers.containers.GenericContainer;

import java.time.Duration;
import java.util.concurrent.TimeUnit;
Expand All @@ -30,6 +31,15 @@ public <SELF extends StartupCheckStrategy> SELF withTimeout(Duration timeout) {
return (SELF) this;
}

/**
*
* @deprecated internal API
*/
@Deprecated
public boolean waitUntilStartupSuccessful(GenericContainer<?> container) {
return waitUntilStartupSuccessful(container.getDockerClient(), container.getContainerId());
}

public boolean waitUntilStartupSuccessful(DockerClient dockerClient, String containerId) {
final Boolean[] startedOK = {null};
Unreliables.retryUntilTrue((int) timeout.toMillis(), TimeUnit.MILLISECONDS, () -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public Boolean call() {

Instant before = Instant.now();
try {
ExecResult result = ExecInContainerPattern.execInContainer(waitStrategyTarget.getContainerInfo(), "/bin/sh", "-c", command.toString());
ExecResult result = ExecInContainerPattern.execInContainer(waitStrategyTarget.getDockerClient(), waitStrategyTarget.getContainerInfo(), "/bin/sh", "-c", command.toString());
log.trace("Check for {} took {}. Result code '{}', stdout message: '{}'", internalPorts, Duration.between(before, Instant.now()), result.getExitCode(), result.getStdout());
int exitCode = result.getExitCode();
if (exitCode != 0 && exitCode != 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class LogMessageWaitStrategy extends AbstractWaitStrategy {
protected void waitUntilReady() {
WaitingConsumer waitingConsumer = new WaitingConsumer();

LogContainerCmd cmd = DockerClientFactory.instance().client().logContainerCmd(waitStrategyTarget.getContainerId())
LogContainerCmd cmd = waitStrategyTarget.getDockerClient().logContainerCmd(waitStrategyTarget.getContainerId())
.withFollowStream(true)
.withSince(0)
.withStdOut(true)
Expand Down
12 changes: 12 additions & 0 deletions core/src/main/java/org/testcontainers/utility/ResourceReaper.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.testcontainers.utility;

import com.github.dockerjava.api.DockerClient;
import com.github.dockerjava.api.command.CreateContainerCmd;
import com.github.dockerjava.api.command.InspectContainerResponse;
import com.github.dockerjava.api.exception.NotFoundException;
import com.github.dockerjava.api.model.Network;
Expand All @@ -11,6 +12,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.testcontainers.DockerClientFactory;
import org.testcontainers.containers.GenericContainer;

import java.io.BufferedReader;
import java.io.IOException;
Expand Down Expand Up @@ -307,6 +309,16 @@ public Map<String, String> getLabels() {
return MARKER_LABELS;
}

/**
*
* @deprecated internal API
*/
@Deprecated
public CreateContainerCmd register(GenericContainer<?> container, CreateContainerCmd cmd) {
cmd.getLabels().putAll(getLabels());
return cmd;
}

/**
* @deprecated internal API
*/
Expand Down
Loading

0 comments on commit 98b60fb

Please sign in to comment.