fix(MongoDb): Probe the server instead of counting log messages before initiating - #1735
Conversation
…e initiating The readiness check that runs before the replica set is initiated counts how often "Waiting for connections" appears in the log and compares that count for equality. The count only grows and the module does not control what else writes that text, so any additional occurrence pushes it past the expected value and the check can never match. The default wait strategy timeout is one hour, so the container start appears to hang. A stale occurrence can also satisfy the check before mongod is serving, which surfaces as ECONNREFUSED on the next command. Ask the server instead. rs.status() answers only once the final mongod is serving: the temporary mongod the official image forks during first-time initialization is not started with --replSet, so it never reports NotYetInitialized. That keeps the handover guarantee testcontainers#1656 added for testcontainers#1636 while removing the dependency on log content. Scoped to the replica set path, which is where testcontainers#1656 introduced this. The non-replica-set wait strategy still counts log messages and has the same weakness, but changing it affects long-standing behaviour for every MongoDb container and is better decided separately.
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughMongoDB replica-set startup now waits for ChangesMongoDB replica-set readiness
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Warning |
|
One more argument for this direction that I missed when opening the PR: testcontainers-go already gates replica set readiness on // modules/mongodb/mongodb.go
req.WaitingFor = wait.ForAll(
req.WaitingFor,
wait.ForExec(cli.eval("rs.status().ok")),
).WithDeadline(60 * time.Second)https://github.com/testcontainers/testcontainers-go/blob/main/modules/mongodb/mongodb.go#L207 So this is not a new idea, it brings the .NET module in line with how the Go one already handles the same problem. Same command, and the same reason: |
HofmeisterAn
added
module
2 participants
Fixes #1732
The problem
The readiness check that runs before the replica set is initiated counts occurrences of a log message and compares that count for equality:
_countis 1 or 2. The count only grows, and the module does not control what else writes that text, so the check fails in both directions:mongodis serving, and the next command fails withMongoNetworkError: connect ECONNREFUSED 127.0.0.1:27017.@xamir82's reproduction on the issue shows this needs no restart: extra markers written by an init script during a single run are enough, and whether it fails at all depends on when the once-per-second poll samples.
This arrived in 4.11.0. #1656 (
ec6b50f6) put this check on the replica set path; before that the path did not use it.The fix
Ask the server instead of reading its logs:
This still satisfies what #1656 needed for #1636. The official image forks a temporary
mongodduring first-time initialization to create the root user, and that process is not started with--replSet, so it never reportsNotYetInitialized. Only the final server does.Measured on
mongo:6.0.27with the module's own configuration (auth plus the generated keyfile), polling twice a second from container start:rs.status()NotYetInitializedA plain
pingis not sufficient on its own; against a container without the keyfile it succeeds during the bootstrap phase.rs.status()is what distinguishes the two processes.It is also idempotent for an already initiated set, which returns
ok: 1and exits zero.Scope
Limited to the replica set path, which is where #1656 introduced this.
Build()still uses the log-counting strategy for non-replica-set containers, and it has the same weakness, but changing it alters long-standing behaviour for every MongoDb container and seemed better decided separately. Happy to extend this PR if you would prefer both in one go.Testing
MongoDbReplicaSetReadinessTestfollows @xamir82's reproduction: a replica set container with an init script that writes the marker twice more.Testcontainers.MongoDb.Tests: 19 passed, 0 failed, including every replica set fixture, which is what covers the first-time initialization path from [Bug]: MongoDB stops while starting #1636.Separate from #1731, which is about
rs.initiatenot being idempotent. The two touch the same method but neither depends on the other.Summary by CodeRabbit
Bug Fixes
NotYetInitializedstate are handled correctly.Tests