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

`containerInfo` is queried until all ports expected by Testcontainers are mapped.
Introduces Awaitility for retry logic.

Removing `withPublishAllPorts` is especially important because else this can lead to publishing of excluded ports on Windows

Co-authored-by: Sergei Egorov <[email protected]>

Author: kiview

core/build.gradle

@@ -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') {

core/src/main/java/org/testcontainers/containers/GenericContainer.java

@@ -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;
@@ -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;
@@ -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;
 
 /**
@@ -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();
 
     @Nullable
     private Map<String, String> tmpFsMapping;
@@ -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)
+                            .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

core/src/main/java/org/testcontainers/utility/DynamicPollInterval.java

@@ -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;
+    }
+}

core/src/main/java/org/testcontainers/utility/ResourceReaper.java

@@ -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;
@@ -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;
@@ -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.
  */
@@ -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)
@@ -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() {

core/src/test/java/org/testcontainers/containers/GenericContainerTest.java

@@ -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;
@@ -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 {
 
@@ -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

core/src/test/java/org/testcontainers/containers/ReusabilityUnitTests.java

@@ -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;
@@ -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;
@@ -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));
             };