From 10f32af918423028be447f6c0520af45888f3348 Mon Sep 17 00:00:00 2001
From: Sasha Sheikin <myminitrue@gmail.com>
Date: Wed, 6 Nov 2024 05:33:37 +0100
Subject: [PATCH] Stop container that doesn't match wait strategy (#9474)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes #2877

---------

Co-authored-by: Piotr Findeisen <piotr.findeisen@gmail.com>
Co-authored-by: Jan Waś <jan.was@starburstdata.com>
Co-authored-by: Eddú Meléndez <eddu.melendez@gmail.com>
---
 .../containers/GenericContainer.java          |  1 +
 .../containers/GenericContainerTest.java      | 63 +++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/core/src/main/java/org/testcontainers/containers/GenericContainer.java b/core/src/main/java/org/testcontainers/containers/GenericContainer.java
index 52dede8792e..b2436b77286 100644
--- a/core/src/main/java/org/testcontainers/containers/GenericContainer.java
+++ b/core/src/main/java/org/testcontainers/containers/GenericContainer.java
@@ -550,6 +550,7 @@ private void tryStart() {
                 } else {
                     logger().error("There are no stdout/stderr logs available for the failed container");
                 }
+                stop();
             }
 
             throw new ContainerLaunchException("Could not create/start container", e);
diff --git a/core/src/test/java/org/testcontainers/containers/GenericContainerTest.java b/core/src/test/java/org/testcontainers/containers/GenericContainerTest.java
index 0ba06dd4ca8..acb24cf0d68 100644
--- a/core/src/test/java/org/testcontainers/containers/GenericContainerTest.java
+++ b/core/src/test/java/org/testcontainers/containers/GenericContainerTest.java
@@ -5,9 +5,12 @@
 import com.github.dockerjava.api.DockerClient;
 import com.github.dockerjava.api.command.InspectContainerResponse;
 import com.github.dockerjava.api.command.InspectContainerResponse.ContainerState;
+import com.github.dockerjava.api.model.Container;
 import com.github.dockerjava.api.model.ExposedPort;
 import com.github.dockerjava.api.model.Info;
 import com.github.dockerjava.api.model.Ports;
+import com.google.common.base.MoreObjects;
+import com.google.common.collect.ImmutableList;
 import lombok.RequiredArgsConstructor;
 import lombok.SneakyThrows;
 import lombok.experimental.FieldDefaults;
@@ -28,9 +31,12 @@
 import org.testcontainers.utility.DockerImageName;
 import org.testcontainers.utility.MountableFile;
 
+import java.time.Duration;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -273,6 +279,63 @@ public void shouldRespectWaitStrategy() {
         }
     }
 
+    @Test
+    public void testStartupAttemptsDoesNotLeaveContainersRunningWhenWrongWaitStrategyIsUsed() {
+        try (
+            GenericContainer<?> container = new GenericContainer<>(TestImages.TINY_IMAGE)
+                .withLabel("waitstrategy", "wrong")
+                .withStartupAttempts(3)
+                .waitingFor(
+                    Wait.forLogMessage("this text does not exist in logs", 1).withStartupTimeout(Duration.ofMillis(1))
+                )
+                .withCommand("tail", "-f", "/dev/null");
+        ) {
+            assertThatThrownBy(container::start).hasStackTraceContaining("Retry limit hit with exception");
+        }
+        assertThat(reportLeakedContainers()).isEmpty();
+    }
+
+    private static Optional<String> reportLeakedContainers() {
+        @SuppressWarnings("resource") // Throws when close is attempted, as this is a global instance.
+        DockerClient dockerClient = DockerClientFactory.lazyClient();
+
+        List<Container> containers = dockerClient
+            .listContainersCmd()
+            .withAncestorFilter(Collections.singletonList("alpine:3.17"))
+            .withLabelFilter(
+                Arrays.asList(
+                    DockerClientFactory.TESTCONTAINERS_SESSION_ID_LABEL + "=" + DockerClientFactory.SESSION_ID,
+                    "waitstrategy=wrong"
+                )
+            )
+            // ignore status "exited" - for example, failed containers after using `withStartupAttempts()`
+            .withStatusFilter(Arrays.asList("created", "restarting", "running", "paused"))
+            .exec()
+            .stream()
+            .collect(ImmutableList.toImmutableList());
+
+        if (containers.isEmpty()) {
+            return Optional.empty();
+        }
+
+        return Optional.of(
+            String.format(
+                "Leaked containers: %s",
+                containers
+                    .stream()
+                    .map(container -> {
+                        return MoreObjects
+                            .toStringHelper("container")
+                            .add("id", container.getId())
+                            .add("image", container.getImage())
+                            .add("imageId", container.getImageId())
+                            .toString();
+                    })
+                    .collect(Collectors.joining(", ", "[", "]"))
+            )
+        );
+    }
+
     static class NoopStartupCheckStrategy extends StartupCheckStrategy {
 
         @Override