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

Remove withPublishAllPorts from Ryuk and stabilize containerInfo content on start #4263

Merged
merged 20 commits into from
Jul 20, 2021
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
2 changes: 2 additions & 0 deletions core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ dependencies {
exclude(group: 'org.jetbrains', module: 'annotations')
}

shaded 'org.awaitility:awaitility:4.1.0'

api "com.github.docker-java:docker-java-api:3.2.11"

shaded ('com.github.docker-java:docker-java-core:3.2.11') {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
import org.testcontainers.utility.DockerImageName;
import org.testcontainers.utility.DockerLoggerFactory;
import org.testcontainers.utility.DockerMachineClient;
import org.testcontainers.utility.DynamicPollInterval;
import org.testcontainers.utility.MountableFile;
import org.testcontainers.utility.PathUtils;
import org.testcontainers.utility.ResourceReaper;
Expand Down Expand Up @@ -84,6 +85,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ExecutionException;
Expand All @@ -97,6 +99,7 @@
import java.util.zip.Checksum;

import static com.google.common.collect.Lists.newArrayList;
import static org.awaitility.Awaitility.await;
import static org.testcontainers.utility.CommandLine.runShellCommand;

/**
Expand Down Expand Up @@ -216,10 +219,10 @@ public class GenericContainer<SELF extends GenericContainer<SELF>>

private static final Set<String> AVAILABLE_IMAGE_NAME_CACHE = new HashSet<>();
private static final RateLimiter DOCKER_CLIENT_RATE_LIMITER = RateLimiterBuilder
.newBuilder()
.withRate(1, TimeUnit.SECONDS)
.withConstantThroughput()
.build();
.newBuilder()
.withRate(1, TimeUnit.SECONDS)
.withConstantThroughput()
.build();
Comment on lines +222 to +225
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, sorry for the autoformat here.


@Nullable
private Map<String, String> tmpFsMapping;
Expand Down Expand Up @@ -425,8 +428,30 @@ private void tryStart(Instant startedAt) {
// For all registered output consumers, start following as close to container startup as possible
this.logConsumers.forEach(this::followOutput);

// Wait until inspect container returns the mapped ports
containerInfo = await()
.atMost(5, TimeUnit.SECONDS)
.pollInterval(DynamicPollInterval.ofMillis(50))
.pollInSameThread()
.until(
() -> dockerClient.inspectContainerCmd(containerId).exec(),
inspectContainerResponse -> {
Set<Integer> exposedAndMappedPorts = inspectContainerResponse
.getNetworkSettings()
.getPorts()
.getBindings()
.entrySet()
.stream()
.filter(it -> Objects.nonNull(it.getValue())) // filter out exposed but not yet mapped
.map(Entry::getKey)
.map(ExposedPort::getPort)
bsideup marked this conversation as resolved.
Show resolved Hide resolved
.collect(Collectors.toSet());

return exposedAndMappedPorts.containsAll(exposedPorts);
}
);

// Tell subclasses that we're starting
containerInfo = dockerClient.inspectContainerCmd(containerId).exec();
containerIsStarting(containerInfo, reused);

// Wait until the container has reached the desired running state
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package org.testcontainers.utility;

import org.awaitility.pollinterval.PollInterval;

import java.time.Duration;
import java.time.Instant;

/**
* Awaitility {@link org.awaitility.pollinterval.PollInterval} that takes execution time into consideration,
* to allow a constant poll-interval, as opposed to Awaitility's default poll-delay behaviour.
*
* @deprecated For internal usage only.
*/
@Deprecated
public class DynamicPollInterval implements PollInterval {

final Duration interval;
Instant lastTimestamp;

private DynamicPollInterval(Duration interval) {
this.interval = interval;
lastTimestamp = Instant.now();
}

public static DynamicPollInterval of(Duration duration) {
return new DynamicPollInterval(duration);
}

public static DynamicPollInterval ofMillis(long millis) {
return DynamicPollInterval.of(Duration.ofMillis(millis));
}

@Override
public Duration next(int pollCount, Duration previousDuration) {
Instant now = Instant.now();
Duration executionDuration = Duration.between(lastTimestamp, now);

Duration result = interval.minusMillis(Math.min(interval.toMillis(), executionDuration.toMillis()));
lastTimestamp = now.plus(result);
return result;
}
}
34 changes: 30 additions & 4 deletions core/src/main/java/org/testcontainers/utility/ResourceReaper.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import com.github.dockerjava.api.model.Frame;
import com.github.dockerjava.api.model.HostConfig;
import com.github.dockerjava.api.model.Network;
import com.github.dockerjava.api.model.PortBinding;
import com.github.dockerjava.api.model.Ports;
import com.github.dockerjava.api.model.Volume;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Throwables;
Expand All @@ -32,11 +34,13 @@
import java.net.Socket;
import java.net.URLEncoder;
import java.nio.charset.StandardCharsets;
import java.time.Duration;
import java.util.AbstractMap.SimpleEntry;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.CountDownLatch;
Expand All @@ -45,6 +49,8 @@
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static org.awaitility.Awaitility.await;

/**
* Component that responsible for container removal and automatic cleanup of dead containers at JVM shutdown.
*/
Expand Down Expand Up @@ -96,10 +102,14 @@ public static String start(DockerClient client) {
List<Bind> binds = new ArrayList<>();
binds.add(new Bind(DockerClientFactory.instance().getRemoteDockerUnixSocketPath(), new Volume("/var/run/docker.sock")));

ExposedPort ryukExposedPort = ExposedPort.tcp(8080);
String ryukContainerId = client.createContainerCmd(ryukImage)
.withHostConfig(new HostConfig().withAutoRemove(true))
.withExposedPorts(new ExposedPort(8080))
.withPublishAllPorts(true)
.withHostConfig(
new HostConfig()
.withAutoRemove(true)
.withPortBindings(new PortBinding(Ports.Binding.empty(), ryukExposedPort))
)
.withExposedPorts(ryukExposedPort)
.withName("testcontainers-ryuk-" + DockerClientFactory.SESSION_ID)
.withLabels(Collections.singletonMap(DockerClientFactory.TESTCONTAINERS_LABEL, "true"))
.withBinds(binds)
Expand All @@ -125,7 +135,23 @@ public void onNext(Frame frame) {

ContainerState containerState = new ContainerState() {

final InspectContainerResponse inspectedContainer = client.inspectContainerCmd(ryukContainerId).exec();
// inspect container response might initially not contain the mapped port
final InspectContainerResponse inspectedContainer = await()
.atMost(5, TimeUnit.SECONDS)
.pollInterval(DynamicPollInterval.ofMillis(50))
.pollInSameThread()
.until(
() -> client.inspectContainerCmd(ryukContainerId).exec(),
inspectContainerResponse -> {
return inspectContainerResponse
.getNetworkSettings()
.getPorts()
.getBindings()
.values()
.stream()
.anyMatch(Objects::nonNull);
}
);

@Override
public List<Integer> getExposedPorts() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.rnorth.ducttape.unreliables.Unreliables;
import org.testcontainers.DockerClientFactory;
import org.testcontainers.TestImages;
import org.testcontainers.containers.startupcheck.OneShotStartupCheckStrategy;
import org.testcontainers.containers.startupcheck.StartupCheckStrategy;
import org.testcontainers.containers.wait.strategy.AbstractWaitStrategy;
import org.testcontainers.images.builder.ImageFromDockerfile;
Expand All @@ -29,9 +30,11 @@
import java.util.stream.Collectors;

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.rnorth.visibleassertions.VisibleAssertions.assertEquals;
import static org.rnorth.visibleassertions.VisibleAssertions.assertThrows;
import static org.rnorth.visibleassertions.VisibleAssertions.assertTrue;
import static org.testcontainers.TestImages.TINY_IMAGE;

public class GenericContainerTest {

Expand Down Expand Up @@ -119,6 +122,30 @@ public void shouldOnlyPublishExposedPorts() {
}
}

@Test
public void shouldWaitUntilExposedPortIsMapped() {

ImageFromDockerfile image = new ImageFromDockerfile("publish-multiple")
.withDockerfileFromBuilder(builder ->
builder
.from("testcontainers/helloworld:1.1.0")
.expose(8080, 8081) // one additional port exposed in image
.build()
);

try (
GenericContainer container = new GenericContainer<>(image)
.withExposedPorts(8080)
.withCreateContainerCmdModifier(it -> it.withExposedPorts(ExposedPort.tcp(8082))) // another port exposed by modifier
) {

container.start();

assertEquals("Only withExposedPort should be exposed", 1, container.getExposedPorts().size());
assertTrue("withExposedPort should be exposed", container.getExposedPorts().contains(8080));
}
}

static class NoopStartupCheckStrategy extends StartupCheckStrategy {

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.github.dockerjava.api.command.ListContainersCmd;
import com.github.dockerjava.api.command.StartContainerCmd;
import com.github.dockerjava.api.model.Container;
import com.github.dockerjava.api.model.NetworkSettings;
import com.github.dockerjava.core.command.CreateContainerCmdImpl;
import com.github.dockerjava.core.command.InspectContainerCmdImpl;
import com.github.dockerjava.core.command.ListContainersCmdImpl;
Expand All @@ -22,6 +23,8 @@
import org.junit.runner.RunWith;
import org.junit.runners.BlockJUnit4ClassRunner;
import org.junit.runners.Parameterized;
import org.mockito.Answers;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.stubbing.Answer;
import org.rnorth.visibleassertions.VisibleAssertions;
Expand Down Expand Up @@ -520,7 +523,9 @@ protected Answer<StartContainerCmd> startContainerAnswer() {
protected Answer<InspectContainerCmd> inspectContainerAnswer() {
return invocation -> {
InspectContainerCmd.Exec exec = command -> {
return new InspectContainerResponse();
InspectContainerResponse stubResponse = Mockito.mock(InspectContainerResponse.class, Answers.RETURNS_DEEP_STUBS);
when(stubResponse.getNetworkSettings().getPorts().getBindings()).thenReturn(Collections.emptyMap());
return stubResponse;
};
return new InspectContainerCmdImpl(exec, invocation.getArgument(0));
};
Expand Down