Add support for LocalStack v2 #994
Conversation
`HOSTNAME_EXTERNAL` env var is deprecated and will be replaced by `LOCALSTACK_HOST` in the upcoming v2.
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Co-authored-by: Manuel de la Peña <mdelapenya@gmail.com>
|
Localstack tests are failing. Could you take a look 🙏 ? |
| t.Fatal(err) | ||
| } | ||
| if c != 0 { | ||
| t.Fatalf("AWS CLI command was executed, expected return code 1, got %v", c) |
There was a problem hiding this comment.
The tests are currently failing here. I'm debugging them locally atm
There was a problem hiding this comment.
Double checked that it fails locally. Will try to understand why and propose a fix
There was a problem hiding this comment.
I'm able to reproduce it but under certain circumstances: debugging and waiting too much while debugging.
Have you though about adding a waitStrategy with the command to the cli container? Then we can waitFor the result after a number of retries/timeout
There was a problem hiding this comment.
@eddumelendez I'm sending a PR to provide wait.ForExec(...).WithResponseCodeMatcher(...), in the same way that we provide it for wait.ForHTTP
That will simplify this PR :)
There was a problem hiding this comment.
This is interesting! the same config works in tc-java. I was wondering if the network name was making those tests flaky but look like is not. So, if I understood correctly the suggestion is to add the wait strategies as part of the container request, right? Although, the check is against the cli output. I think we can store the result in a file and check it later but doesn't seem to simplify the pr
There was a problem hiding this comment.
I'd still create the CLI container, but instead of directly using the Exec method of the container, move it to a wait.ForExec(...).WithResponseMatcher(). If the container is not created without error, then the tests should fail
Does it sound good to you?
There was a problem hiding this comment.
I think we have the same issue 🤔 (locally)
There was a problem hiding this comment.
🤔 Could it be caused by the startup timeout not being enough depending on the machine?
I've seen some issues regarding that in the couchbase module in a non-deterministic manner. Restarting its CI build fixes it. Could it be the case for this?
|
Kudos, SonarCloud Quality Gate passed!
|
* main: Add support for LocalStack v2 (testcontainers#994) docs: document tc_host inside the networking section (testcontainers#1041)
HOSTNAME_EXTERNALenv var is deprecated and will be replaced byLOCALSTACK_HOSTin the upcoming v2.