Skip to content
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

feat: enable reuse for mongodb #6235

Merged

Conversation

tiboun
Copy link
Contributor

@tiboun tiboun commented Nov 24, 2022

When we reuse container with mongodb images, it fails the first time we reuse it because the replica is already set, hence the exit code of the replica set is 1 and not 0.

In order to avoid that, a check is made on the state of the current node.
If the command fails because there is no replica, it will fallback to the initialisation of the replica set like today.
Otherwise, it will skip.

In order to know if a replica is set or not, since we are working with one node, it must be a primary one.
We use replSetGetStatus to check its "myState" attribute.

@tiboun tiboun requested a review from a team as a code owner November 24, 2022 15:16
@tiboun tiboun force-pushed the feat/enable-reuse-for-mongodb branch from fda22d4 to d38dcb4 Compare November 24, 2022 15:17
@kiview
Copy link
Member

kiview commented Nov 25, 2022

Thanks for the PR @tiboun. We have a different containerIsStarted method that is reused aware (

protected void containerIsStarted(InspectContainerResponse containerInfo, boolean reused) {
). We haven't used it so far, but I think it is better and simpler approach.

@tiboun tiboun force-pushed the feat/enable-reuse-for-mongodb branch from d38dcb4 to 3911244 Compare December 1, 2022 12:03
@tiboun
Copy link
Contributor Author

tiboun commented Dec 1, 2022

Hi @kiview , I've updated my MR to use the method mentionned. Let me know if it is correctly implemented :)

@tiboun tiboun force-pushed the feat/enable-reuse-for-mongodb branch from 3911244 to b3a7d86 Compare December 1, 2022 12:24
@eddumelendez eddumelendez added this to the next milestone Dec 2, 2022
@eddumelendez eddumelendez merged commit 77a92c0 into testcontainers:main Dec 2, 2022
@eddumelendez
Copy link
Member

Thanks for your contribution @tiboun ! I added two commits in order to polish the contribution a little bit and fix a check. This is now in main branch

@tiboun
Copy link
Contributor Author

tiboun commented Dec 3, 2022

Thx @eddumelendez !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants