testcontainers / testcontainers-java

Testcontainers is a Java library that supports JUnit tests, providing lightweight, throwaway instances of common databases, Selenium web browsers, or anything else that can run in a Docker container.
https://testcontainers.org
MIT License
7.96k stars 1.64k forks source link

InternalCommandPortListeningCheck always fails for containers built without `/bin/sh` #3317

Open rnorth opened 3 years ago

rnorth commented 3 years ago

Relates to #2984

I'm surprised we've not come across this earlier, but the default TCP port listening detector InternalCommandPortListeningCheck does not work for containers that do not have /bin/sh included.

This predominantly affects images built upon scratch.

The workaround/solution is to use a more application specific WaitStrategy - e.g. an HTTP wait strategy.

However the log messages don't make this easy to realise.

Given that we do both the internal and external port checks for a good reason, I don't think we can just rely on the external port check. Still perhaps we can raise a very specific error message if /bin/sh is not available inside the container.

vcvitaly commented 3 years ago

I can take it.

vcvitaly commented 3 years ago

@rnorth I noticed that Testcontainers heavily uses static classes, for example:

try {
            ExecResult result = ExecInContainerPattern.execInContainer(waitStrategyTarget

or

default Container.ExecResult execInContainer(Charset outputCharset, String... command) throws UnsupportedOperationException, IOException, InterruptedException {
        return ExecInContainerPattern.execInContainer(getContainerInfo(), outputCharset, command);
    }

which makes it hard to unit test/mock some things. Is it done on purpose or have there been any ideas to change that?

Asking that because fixing this issue would require to run an integration test which is heavier and I read in some issue that it's not desirable to increase build running time with heavy tests.

bsideup commented 3 years ago

@vcvitaly overall, we prefer integration tests over unit tests. We do recommend avoiding unnecessary load on our CI but we don't mind it if it is needed.

Also, Testcontainers' nature is context-less, to make it easier to use in tests, hence the heavy usage of static objects.

vcvitaly commented 3 years ago

If an error is thrown, HostPortWaitStrategy will be running Unreliables.retryUntilTrue until the timeout is reached. I think in such cases as this it should be aborted immediately on some specific exception, however at the moment it's not possible with Unreliables.

I described the issue in https://github.com/rnorth/duct-tape/issues/11

bedla commented 1 year ago

Hi, I have create this PR https://github.com/testcontainers/testcontainers-java/pull/6942 pls take a look and say what do you think? Thx