Repository navigation
fix: Replace a test container whose port Docker did not publish, and terminate every container that failed to start. - #188
Merged
Conversation
…terminate every container that failed to start. Under load, Docker Desktop sometimes fails to publish the host port it chose for a container, because something on the host took that port in the meantime. The container keeps running, but the database in it can never be reached, so Start ran into its timeout, however long it was. Start now terminates such a container and starts a new one, up to three attempts in total, and its error says that Docker did not publish the port if all of them fail. It makes no new attempt once the context has ended. Every other failure is returned at once, as before. Start also terminates every container that failed to start, so that they no longer pile up, and reports if terminating one fails. Each attempt builds a request of its own, as starting a container reads the signing key file of its request to the end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TCdPte6nY6ToUVd2prb9GM
…an inspect it. A container is an interface, so a failed start may hand out a nil pointer to a concrete container, such as a nil *DockerContainer, which is not nil as an interface. Inspecting it to tell whether Docker published its port panicked. Such a container now counts as no container, as testcontainers.TerminateContainer checks it, so Start returns the error of the attempt at once, as it is. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TCdPte6nY6ToUVd2prb9GM
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a flaky start of the test container on macOS.
The problem. Docker Desktop picks a random host port for the container, and opens it on the Mac only afterwards. If something on the Mac takes that port in between, publishing it fails silently: Docker Desktop logs "bind: address already in use".
Startfails after its 10-second timeout withstart container: … get state … context deadline exceeded.The fix.
Startterminates it and starts a new one, up to 3 attempts. Every other error is returned at once and unchanged, so this reacts only to that one state, not to failures in general.Tests. The tests produce the unpublished state on purpose: with
Networks: ["none"], which reproduces the real hang on Docker Desktop, and with only another port published. They cover:All new cases fail on main. 23 mutations of the new code are all caught.
make qaand-raceare green.Not in this PR: the other flaky start,
wait for reaper …: unexpected container status "created". It is a bug in testcontainers-go, which upstream PR testcontainers/testcontainers-go#3868 fixes. The version will be bumped once that fix is released.🤖 Generated with Claude Code
https://claude.ai/code/session_01TCdPte6nY6ToUVd2prb9GM