Skip to content

fix(mongodb): replica set initialization & connection handling #2984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ttruongatl
Copy link
Contributor

@ttruongatl ttruongatl commented Feb 15, 2025

What does this PR do?

This PR addresses critical issues in the MongoDB container module regarding replica set initialization, host IP detection, and connection string formation. In addition, it expands test coverage by verifying change stream functionality when a replica set is configured.

  1. Replica Set Initialization Reliability:
    The previous implementation initialized the replica set using the container’s internal IP and a hardcoded port (27017). This approach caused issues when custom host port mappings were applied (e.g., mapping container port 27017 to host port 52120) because the advertised address was incorrect. Furthermore, the old code did not reliably wait for MongoDB to be fully ready before issuing the rs.initiate command, leading to intermittent failures that went undetected by tests.
    This PR introduces a waitForMongoReady helper, which waits for MongoDB’s readiness (using a ping command) before retrieving the host and mapped port and issuing the replica set initialization command with the correct values.

  2. Accurate Host IP Detection:
    Previously, using Unix sockets defaulted to "localhost," which often broke replica set functionality since that address isn't externally reachable. This PR introduces a helper, getLocalNonLoopbackIP, to obtain a non-loopback IPv4 address—and if that fails, it falls back to "host.docker.internal"—ensuring the connection string advertises an externally accessible IP.

  3. Enhanced Connection String Formation:
    The ConnectionString method has been updated to dynamically build the connection URI. When a replica set is configured, it now appends the appropriate query parameter (e.g., ?replicaSet=rs0) so that clients can connect properly. Previously, the tests did not verify this because the connection string did not accurately reflect the replica set configuration.

  4. Expanded Testing with Change Streams:
    The test suite has been extended to include change stream tests for containers configured with a replica set. If the connection string contains a replica set parameter, the tests now create a change stream on a collection, insert a document, and verify that the change event reflects an insert operation. This ensures that the replica set functionality is not only initialized correctly but also operational in terms of change stream processing.

Why is it important?

Without these enhancements, the replica set configuration might not work correctly—especially in CI or Docker host environments where custom port mappings are common. The shortcomings of the old code meant that, even though tests passed, the replica set functionality (and thus features like change streams) was not truly exercised. This PR improves reliability and test coverage, ensuring that integration tests can now fully validate replica set behavior.

How to test this PR:

git clone [email protected]:ttruongatl/testcontainers-go.git
cd modules/mongodb
make test

@ttruongatl ttruongatl requested a review from a team as a code owner February 15, 2025 01:25
Copy link

netlify bot commented Feb 15, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit d4adb4d
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/67ed203c4288b30008a53a9a
😎 Deploy Preview https://deploy-preview-2984--testcontainers-go.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@ttruongatl ttruongatl changed the title [MongoDB] Improve Replica Set Initialization & Connection Handling fix(replica): Mongodb Replica Set Initialization & Connection Handling Feb 15, 2025
@ttruongatl ttruongatl changed the title fix(replica): Mongodb Replica Set Initialization & Connection Handling fix(replica): mongodb replica set initialization & connection handling Feb 15, 2025
Copy link
Contributor

@stevenh stevenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR, here's some initial feedback.

@ttruongatl ttruongatl requested a review from stevenh February 16, 2025 20:33
Copy link
Contributor

@stevenh stevenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the updates, just done another quick pass.

@ttruongatl ttruongatl requested a review from stevenh February 17, 2025 02:29
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just left a few non-blocking comments. Will approve once discussed, thanks!

Copy link
Contributor

@stevenh stevenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry a little miss-understanding on a previous suggestion, should be good to go after this.

@ttruongatl ttruongatl requested a review from stevenh February 17, 2025 16:36
@ttruongatl ttruongatl force-pushed the thanhtruong/improve-MongoDB-Replica-Set-Initialization-and-connection-handling branch from 7966110 to 37cb2c5 Compare February 17, 2025 19:34
stevenh
stevenh previously approved these changes Feb 17, 2025
Copy link
Contributor

@stevenh stevenh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@stevenh
Copy link
Contributor

stevenh commented Feb 17, 2025

@ttruongatl looks like the file needs a format, likely just a spacing change from one of my suggestions.

@ttruongatl
Copy link
Contributor Author

ttruongatl commented Feb 17, 2025

@stevenh I've fixed the lint check issues. PTAL! Thank you.

@ttruongatl ttruongatl requested a review from stevenh February 17, 2025 22:40
stevenh
stevenh previously approved these changes Feb 18, 2025
@mdelapenya mdelapenya changed the title fix(replica): mongodb replica set initialization & connection handling fix(mongodb): replica set initialization & connection handling Apr 2, 2025
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mdelapenya mdelapenya merged commit 675acc3 into testcontainers:main Apr 2, 2025
17 checks passed
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Apr 2, 2025
* main:
  fix(mongodb): replica set initialization & connection handling (testcontainers#2984)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Apr 3, 2025
* main:
  chore: bump golangci-lint to v2 (testcontainers#3082)
  chore(gcloud): deprecate old gcp containers, creating subpackages for them (testcontainers#3063)
  fix(mongodb): replica set initialization & connection handling (testcontainers#2984)
  chore(deps): bump docker/setup-docker-action from 4.2.0 to 4.3.0 (testcontainers#3077)
  chore(deps): bump github/codeql-action from 3.28.12 to 3.28.13 (testcontainers#3078)
  chore(deps): bump tj-actions/changed-files from 45.0.4 to 46.0.3 (testcontainers#3076)
  docs: add dependabot configuration (testcontainers#3074)
  chore(deps): replace `golang.org/x/exp/slices` with stdlib (testcontainers#3075)
  fix(dind): use docker image load (testcontainers#3073)
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Apr 3, 2025
* main:
  fix(mssql): reduce flakiness in tests (testcontainers#3084)
  chore: bump golangci-lint to v2 (testcontainers#3082)
  chore(gcloud): deprecate old gcp containers, creating subpackages for them (testcontainers#3063)
  fix(mongodb): replica set initialization & connection handling (testcontainers#2984)
@mdelapenya mdelapenya added the bug An issue with the library label Apr 3, 2025
mdelapenya added a commit to mdelapenya/testcontainers-go that referenced this pull request Apr 14, 2025
* main: (91 commits)
  chore(deps): bump github/codeql-action from 3.28.13 to 3.28.15 (testcontainers#3097)
  chore(deps): bump golang.org/x/crypto from 0.31.0 to 0.37.0 (testcontainers#3098)
  feat(aerospike): add Aerospike module (testcontainers#3094)
  security(compose): upgrade github.com/docker/compose/v2 to fix security vulnerability (testcontainers#3095)
  feat: add more functional options to the modules API (testcontainers#3070)
  chore(deps): bump golang.org/x/net in /modules/arangodb (testcontainers#3087)
  feat: add arangodb module (testcontainers#3083)
  chore(deps): bump actions/upload-artifact from 4.6.0 to 4.6.2 (testcontainers#3086)
  chore(deps): bump SonarSource/sonarqube-scan-action from 5.0.0 to 5.1.0 (testcontainers#3085)
  feat: add socat container (testcontainers#3071)
  fix(mssql): reduce flakiness in tests (testcontainers#3084)
  chore: bump golangci-lint to v2 (testcontainers#3082)
  chore(gcloud): deprecate old gcp containers, creating subpackages for them (testcontainers#3063)
  fix(mongodb): replica set initialization & connection handling (testcontainers#2984)
  chore(deps): bump docker/setup-docker-action from 4.2.0 to 4.3.0 (testcontainers#3077)
  chore(deps): bump github/codeql-action from 3.28.12 to 3.28.13 (testcontainers#3078)
  chore(deps): bump tj-actions/changed-files from 45.0.4 to 46.0.3 (testcontainers#3076)
  docs: add dependabot configuration (testcontainers#3074)
  chore(deps): replace `golang.org/x/exp/slices` with stdlib (testcontainers#3075)
  fix(dind): use docker image load (testcontainers#3073)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants