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(Helm): Support for Zone Awareness and rollout-operator #11404

Closed
wants to merge 21 commits into from

Conversation

alex5517
Copy link
Contributor

@alex5517 alex5517 commented Dec 7, 2023

What this PR does / why we need it:
Adds support for zone awareness and use of rollout-operator
It also makes some changes to labeling and annotations so that it is done in a similar way to how the mimir-distributed helm chart does it. - A few other "alignments" have been done with the way mimir-distributed helm chart does it.
Which issue(s) this PR fixes:
Fixes #N/A

Special notes for your reviewer:
I will be adding docs for how to migrate from single zone to zone-aware replication, and i know it is a large PR but i have have tried to keep it familiar by implementing it the "same" way as it is done for mimir-distributed helm chart, i have also used the "guide" (https://grafana.com/docs/loki/latest/operations/zone-ingesters/) for the jsonnet deployment to make the migration possible in almost the same way.

Checklist

  • Reviewed the CONTRIBUTING.md guide (required)
  • Documentation added
  • Tests updated
  • CHANGELOG.md updated
    • If the change is worth mentioning in the release notes, add add-to-release-notes label
  • Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
  • For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
  • If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

@alex5517 alex5517 requested a review from a team as a code owner December 7, 2023 11:38
@CLAassistant
Copy link

CLAassistant commented Dec 7, 2023

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added area/helm type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories labels Dec 7, 2023
Copy link
Contributor

github-actions bot commented Dec 7, 2023

Trivy scan found the following vulnerabilities:

@alex5517
Copy link
Contributor Author

I have added migration guide to migrate from single zone --> zone-aware, i have not written anything about how it could be done with downtime, let me know if this is something that is wanted.

@alex5517
Copy link
Contributor Author

@JStickler or @MichelHollands

Could you let me know if this PR contains to many changes/to large for it to have a chance of being merged?
If it does not have a chance i can also make a smaller PR where it is implemented by just hacking it into production/helm/loki/templates/write/_helpers-write.tpl

@alex5517
Copy link
Contributor Author

alex5517 commented Jan 9, 2024

Hi @MichelHollands or @JStickler,

Are you able to review this PR, or am i missing something before this is possible?

@MichelHollands
Copy link
Contributor

Hi @MichelHollands or @JStickler,

Are you able to review this PR, or am i missing something before this is possible?

@alex5517 Sorry for the late reply. Due to holidays we had a backlog. This PR is a bit too big to merge as is, as you alluded to. Is it possible to split this into multiple smaller PRs? There's too much of a risk of breaking other installs with large PRs. Thanks for the contribution though, this is really impressive work.

@alex5517
Copy link
Contributor Author

alex5517 commented Jan 29, 2024

@MichelHollands,

I understand.
The big changes in the PR is the change to how labeling is done, so i could possible split this PR into 3 seperate PRs

  1. Labeling Change: Focused on alterations to the labeling mechanism.
  2. Minor Adjustments to Statically Defined Elements: small changes.
    3- Zone-Awareness Support: Dedicated to the integration of zone-awareness.

Do you believe this approach would be feasible? While the labeling PR might still appear substantial, it primarily results from identical changes applied across all resources. However, examining the YAML generated (old vs new) should facilitate easier validation.

Are you comfortable with reviewing and merging three distinct pull requests? Alternatively, I could create a pull request that quickly(hackish) integrates it into the helper.tpl file. Let me know your thoughts.

@alex5517
Copy link
Contributor Author

@MichelHollands,

Have you had time to consider my previous comment?

@alex5517
Copy link
Contributor Author

@MichelHollands

Have you had time to consider my previous comment?

@alex5517 alex5517 closed this Sep 23, 2024
@alex5517 alex5517 deleted the feat/zone-aware-helm branch September 23, 2024 05:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/helm size/XXL type/docs Issues related to technical documentation; the Docs Squad uses this label across many repositories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants