[Bug]: Latest Ryuk changed the log to "starting" but Ryuk container expects "Started" #9369
Labels
Comments
|
I would recommend waiting for port instead of logs, logs are always fragile and can lead to race cases which this would be as starting is output before it's actually able to accept connections. While this is likely to work it could fail. |
|
what does I think next time we should consider that there are users updating the image via |
@Kehrlann could you share the analysis you did so we can learn more about how to verify this kind of regressions beforehand? 🙏 In any case, we released a patch release in Ryuk keeping the old behaviour in the log, so I think we can close this one. |
|
Repro case was a simple testcontainers test, running against The test: import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.testcontainers.containers.GenericContainer;
import java.net.URI;
import java.net.http.HttpClient;
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import static org.assertj.core.api.Assertions.assertThat;
class TestcontainersTest {
private static final boolean DEBUG = false;
@Test
void test() throws Exception {
try (GenericContainer<?> nginx = new GenericContainer<>("nginx:1-alpine-slim")) {
nginx.withExposedPorts(80).start();
HttpClient httpClient = HttpClient.newBuilder().build();
String url = String.format("http://%s:%d", nginx.getHost(), nginx.getFirstMappedPort());
HttpResponse<String> response = httpClient.send(HttpRequest.newBuilder().GET().uri(URI.create(url)).build(), HttpResponse.BodyHandlers.ofString());
assertThat(response.statusCode()).isEqualTo(200);
assertThat(response.body()).contains("<h1>Welcome to nginx!</h1>");
}
}
}
ryuk.container.image=testcontainers/ryuk:0.10.0Tested in Testcontainers 1.17.1The test succeeds and works as expected. Tested in Testcontainers 1.19.8The test fails after a timeout, with log (full log at the end): AnalysisThe issue is that In ConclusionI agree with @stevenh , waiting on specific logs is brittle. In the meantime, @mdelapenya has mitigated this issue by releasing ryuk:0.10.1 which re-introduces the Appendix: full stack trace in TC 1.19.8 |
That absolutely depends on the image, for some images, a log based wait strategy is proven to be the most stable integration point for the actual wait behavior (PostgreSQL is such an example, that will have the port open considerable time before actual readiness and the database engine restart means, that log message based wait strategy is unfortunately the best indicator of non-flaky readiness). @eddumelendez is also right with avoiding a breaking change for users upgrading the image manually. So reverting the breaking change in an integration point in Ryuk is the right thing to do here. I'll close this issue, since it is not a bug in tc-java and we did revert the breaking change already in Ryuk. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Module
Core
Testcontainers version
Latest
Using the latest Testcontainers version?
Yes
Host OS
Any
Host Arch
Any
Docker version
Client:
Version: 27.2.1-rd
API version: 1.43 (downgraded from 1.47)
Go version: go1.22.7
Git commit: cc0ee3e
Built: Tue Sep 10 15:41:09 2024
OS/Arch: darwin/arm64
Context: tcd
Server: Testcontainers Cloud
Engine:
Version: 82+testcontainerscloud
API version: 1.46 (minimum version 1.24)
Go version: go1.21.12
Git commit: cc13f952511154a2866bddbb7dddebfe9e83b801
Built: Thu Aug 1 16:00:49 2024
OS/Arch: linux/amd64
Experimental: false
containerd:
Version: 1.7.12
GitCommit:
runc:
Version: 1.1.12-0ubuntu2~22.04.1
GitCommit:
docker-init:
Version: 0.19.0
GitCommit:
What happened?
testcontainers/moby-ryuk#141 changed the log format to be more structured and used lowercase
Relevant log output
Starting Ryuk:
Additional Information
1.17 works but latest does not
The text was updated successfully, but these errors were encountered: