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

helm: build unique rollout-group names per chart release #8646

Closed
wants to merge 3 commits into from

Conversation

narqo
Copy link
Contributor

@narqo narqo commented Jul 7, 2024

What this PR does

This PR updates the helm chart, allowing Mimir's components to form a release scoped rollout groups, and not clash with any components, that aren't part of the release. That is if Mimir, Loki, Tempo are installed in the same namespace; or when multiple Mimir versions are installed in the namespace.

TODO I'm not yet sure how thse changes will affect the existing deployments. One needs to test this somehow.

Which issue(s) this PR fixes or relates to

Fixes #8406

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX].
  • about-versioning.md updated with experimental features.

narqo added 3 commits July 7, 2024 14:56
Signed-off-by: Vladimir Varankin <[email protected]>
@narqo narqo marked this pull request as ready for review July 8, 2024 18:00
@narqo narqo requested a review from a team as a code owner July 8, 2024 18:00
@narqo
Copy link
Contributor Author

narqo commented Jul 8, 2024

TODO I'm not yet sure how thse changes will affect the existing deployments. One needs to test this somehow.

@lindeskar I don't believe I will get to extensive testing of this one in the near future, unfortunately. Maybe, if you have a testing stand, where you can verify that the changes here improve the problem you outlined in #8406 (and don't add more problems), this would be very helpful.


Update Also, a related discussion in Loki grafana/loki#13168 (comment) where folks mentioned why we don't want to support that.

@dimitarvdimitrov
Copy link
Contributor

TODO I'm not yet sure how thse changes will affect the existing deployments. One needs to test this somehow.

yeah that's crucial to understand before merging this. I suspect the problem might come if an ingester is unavailable and then the rollout of this change happens - the rollout-operator might not take into account the unavailable ingester and roll out other zones too.

That at least one edge case I can think of

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

Successfully merging this pull request may close these issues.

[mimir-distributed] Ingester rollout-group conflicts with other charts
2 participants