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

[bitnami/mongodb] Enable publishNotReadyAddresses for ClusterIP services #31145

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

janvotava
Copy link

Description of the change

When MongoDB replicaset hosts configuration points to services (rather than pods on headless service) for inter-cluster communication (through tunnel for example), a deadlock can occur when majority Pods fail at once since services won't publish addresses until pods are ready.

This change prevents deadlocks by allowing MongoDB instances to discover each other during startup and keeps it similar to NodePort/Loadbalancer logic in external mode.

Benefits

Makes ClusterIP external replicaset mode more resilient.

Possible drawbacks

Applicable issues

  • fixes #

Additional information

Checklist

  • Chart version bumped in Chart.yaml according to semver. This is not necessary when the changes only affect README.md files.
  • Variables are documented in the values.yaml and added to the README.md using readme-generator-for-helm
  • Title of the pull request follows this pattern [bitnami/<name_of_the_chart>] Descriptive title
  • All commits signed off and in agreement of Developer Certificate of Origin (DCO)

@carrodher
Copy link
Member

Thank you for initiating this pull request. We appreciate your effort. This is just a friendly reminder that signing your commits is important. Your signature certifies that you either authored the patch or have the necessary rights to contribute to the changes. You can find detailed information on how to do this in the “Sign your work” section of our contributing guidelines.

Feel free to reach out if you have any questions or need assistance with the signing process.

@janvotava
Copy link
Author

Thank you for initiating this pull request. We appreciate your effort. This is just a friendly reminder that signing your commits is important. Your signature certifies that you either authored the patch or have the necessary rights to contribute to the changes. You can find detailed information on how to do this in the “Sign your work” section of our contributing guidelines.

Feel free to reach out if you have any questions or need assistance with the signing process.

There's no correct sign-off?

@carrodher
Copy link
Member

Here you can see the failing GH Action and some tips about how to remediate it

When MongoDB replicaset hosts configuration points to services (rather
than pods on headless service) for inter-cluster communication (through
tunnel for example), a deadlock can occur when majority Pods fail at
once since services won't publish addresses until pods are ready.

This change prevents deadlocks by allowing MongoDB instances to discover
each other during startup and keeps it similar to NodePort/Loadbalancer
logic in external mode.

Signed-off-by: Jan Votava <[email protected]>
@janvotava janvotava force-pushed the mongodb-external-publish-not-ready branch from bc1a0b8 to d2531ff Compare December 23, 2024 18:52
@janvotava
Copy link
Author

Sorry, wrong email. 🫣 Should be fixed now.

Signed-off-by: Bitnami Containers <[email protected]>
@carrodher carrodher added verify Execute verification workflow for these changes in-progress labels Dec 23, 2024
@github-actions github-actions bot removed the triage Triage is needed label Dec 23, 2024
@github-actions github-actions bot removed the request for review from carrodher December 23, 2024 18:59
@github-actions github-actions bot requested a review from gongomgra December 23, 2024 18:59
Copy link

github-actions bot commented Jan 8, 2025

This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added the stale 15 days without activity label Jan 8, 2025
@janvotava
Copy link
Author

Please don't close this PR.

@github-actions github-actions bot removed the stale 15 days without activity label Jan 9, 2025
carrodher and others added 2 commits January 23, 2025 10:08
Signed-off-by: Bitnami Containers <[email protected]>
@@ -25,6 +25,7 @@ metadata:
{{- end }}
spec:
type: ClusterIP
publishNotReadyAddresses: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of always setting the parameter to true, can you parameterize it (with default to false)? This way we maintain the current behavior and it can be set in an easy way when required.

Copy link
Author

@janvotava janvotava Jan 23, 2025

Choose a reason for hiding this comment

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

I can do that. But I wonder, what's the use case for the current behavior - could changing the default false -> true potentially be a breaking change?

Copy link
Contributor

Choose a reason for hiding this comment

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

The thing is that we have not being reported with any issue related to current setup, so to be on the safe side I think it is worth it to keep it this way for now yet allowing users to customize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in-progress mongodb verify Execute verification workflow for these changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants