-
Notifications
You must be signed in to change notification settings - Fork 557
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
Docs: Enable cluster label verification in Helm #2865
Comments
This looks like a docs issue to me. The migration is possible right now, helm doesn't prevent it. We can document how to do the migration and then enable this by default and do a major release. |
Yes, I agree, I was thinking about how to write a migration path, but it's three steps, not super complicated. And the user can keep backwards compatibility with one setting in |
Please link to this from the (yet to be completed) "Running Helm in production" document |
@lamida said he's interested in picking this up, so I'm posting my notes to help pick this up Breaking changeThe reason for doing this migration is because making all changes in one go is a breaking change. The agreement with @krajorama is that we want to eventually have cluster label verification enabled by default in the chart. We have a deprecation policy of 2 major releases. I think it might make sense to follow the same period for this breaking change. Otherwise, a period of 6 months to a year sounds viable for this. I'm curious to hear what others think. Migration docBecause it's a breaking change we need a migration doc. I think it would make sense to have multiple parts for this doc. Would be nice to get feedback from @osg-grafana and other involved if you think this is viable.
I think most of the sections are self-explanatory. I have notes on two of them
This is an excerpt from the internal docs for this migration. Might be good to include it in some form or another
The actual migration has three parts: I've taken them from this blog post and the internal procedure for doing the migration
Helping with the migrationTo help users with the migration we can do step 1. and include it in a helm release. The change is small and non-breaking. I think we should do it. This way the mandatory migration with the major release will be shorter for people that have a relatively recent helm chart version. |
SMEs were @dimitarvdimitrov and @krajorama; SMEs will be @lamida and @francoposa. |
I just finished the first draft of the documentation in #7961 @krajorama @dimitarvdimitrov . However, I haven't put anything yet on stating step 1 will be part of our future Mimir helm release.
Do we still want to release a helm chart version to disable cluster label verification to remove one migration step if eventually in the future we will enable cluster label verification again by default in the chart? |
I feel like we're past this point. If we have the migration doc already, we can switch on cluster label verification on in the next release. |
About the cluster label itself, I propose cluster_label to include the namespace and the helm release name, because technically you can install the same release name in different namespaces. |
Describe the bug
Two separate memberlist gossip rings may join into one when IP addresses are reused between the two due to for example a kubernetes migration restarting a lot of PODs quickly.
To Reproduce
N/A incident RCA exists
Expected behavior
Gossip rings do not interact
Environment
Additional Context
#2349
Requires a three step migration, so major version bump and migration document needed.
The text was updated successfully, but these errors were encountered: