The cleanup is missing between the startup attempts

[vmassol]

When BrowserWebDriverContainer#containerIsStarted() fails to construct the WebDriver object, TC tries to restart it, up to 3 times, leading to several containers running.

This is because GenericContainer#doStart()does:

            Unreliables.retryUntilSuccess(startupAttempts, () -> {
                logger().debug("Trying to start container: {} (attempt {}/{})", getDockerImageName(), attempt.incrementAndGet(), startupAttempts);
                tryStart(startedAt);
                return true;
            });

And BrowserWebDriverContainer has a retry set to 3 (see https://github.com/testcontainers/testcontainers-java/blob/master/modules/selenium/src/main/java/org/testcontainers/containers/BrowserWebDriverContainer.java#L191).

So if containerIsStarted() fails then tryStart() fails and thus it's called again.

Leading to seeing the following for example, where you can see 2 BrowserWebDriverContainer containers:

$ sudo docker ps -a
CONTAINER ID        IMAGE                                        COMMAND                   CREATED             STATUS              PORTS                                              NAMES
cff188572636        testcontainersofficial/vnc-recorder:1.1.0    "/bin/sh -c 'echo 'c…"    24 seconds ago      Up 19 seconds                                                          xenodochial_galileo
03f13169c6c4        selenium/standalone-firefox-debug:3.141.59   "/opt/bin/entry_poin…"    5 minutes ago       Up 5 minutes        0.0.0.0:37893->4444/tcp, 0.0.0.0:37892->5900/tcp   compassionate_leavitt
ceef840e2bc0        selenium/standalone-firefox-debug:3.141.59   "/opt/bin/entry_poin…"    6 minutes ago       Up 5 minutes        0.0.0.0:37889->4444/tcp, 0.0.0.0:37888->5900/tcp   wizardly_bell
ac259d89690e        testcontainersofficial/sshd:1.0.0            "sh -c 'echo \"root:$…"   8 minutes ago       Up 8 minutes        0.0.0.0:37885->22/tcp                              wonderful_visvesvaraya
37a11f5e2228        testcontainersofficial/ryuk:0.3.0            "/app"                    8 minutes ago       Up 8 minutes        0.0.0.0:37884->8080/tcp                            testcontainers-ryuk-4e2447ec-fcf9-409a-827f-3c35953890d3
b1f5c0da7be5        xwiki/build:jessie                           "setup-xwiki-ssh /bi…"    About an hour ago   Up About an hour    22/tcp                                             modest_heisenberg

It could be better that TC stops it before it retries to start it to use less resource on the machine. TBH I don't know how much of it is a problem but it seems not right somehow.

WDYT?

Thanks

[bsideup]

After looking at the issue, I get a feeling that setStartupAttempts makes very little sense (or should be reworked and perform a cleanup between the attempts)

I've changed the scope of the issue to a broader one, about the startup attempts 👍

[fknittel]

We appear to be running into this issue. In case many tests fail to bring up their browser, we end up with many docker containers using up resources. This in turn causes our ephemeral docker host to grind to a halt. As this failure mode is slow, the feedback we get is quite delayed and the follow-up errors due to the resource starved environment further muddy the water.

Of course the problem only occurs if something goes wrong. Unfortunately we're in the process of migrating to testcontainers, so the number of known configuration problems is not zero yet. This behaviour didn't exactly help us quickly iron out the remaining issues.

Our work-around: We override BrowserWebDriverContainer.configure() to force startupAttempts back to 1:

    public static class SingleRetryBrowserWebDriverContainer<SELF extends SingleRetryBrowserWebDriverContainer<SELF>> extends BrowserWebDriverContainer<SELF> {
        @Override
        protected void configure() {
            super.configure();

            // Force single start-up attempt to avoid potential container leak during retries.
            setStartupAttempts(1);
        }
    }

[jacek-lewandowski]

I don't know if it is expected result of this change but since
10f32af calls stop on exception, the container id and other information about the container is gone if the startup fails (https://github.com/testcontainers/testcontainers-java/blame/main/core/src/main/java/org/testcontainers/containers/GenericContainer.java#L660). This is painful in a situation when the container is expected to fail and we need to investigate logs or perform some cleanup (for example it attempted to register in some ETCD as a service and we need its IP to clean up). I know there is still a way to workaround that by implementing hooks like containerIsStopping, but since there is no listener registry, doing that would require refactoring all tests which create GenericContainer to create a custom one.