-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
fix(helm): Split ingester HPA when zoneAwareReplication is enabled #14565
fix(helm): Split ingester HPA when zoneAwareReplication is enabled #14565
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank you!
This comment has been minimized.
This comment has been minimized.
4 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Looks like |
This comment has been minimized.
This comment has been minimized.
@trevorwhitney From what I can tell, the CI fails once the branch is out of date. But when I rebase I'm not seeing any failures. I sense I'm missing something simple here, sorry 😅 |
Line in question: https://github.com/prometheus-community/helm-charts/blob/kube-prometheus-stack-65.5.1/charts/kube-prometheus-stack/templates/prometheus/prometheus.yaml#L262 Looking at other open PRs, it appears that ~all helm chart changes are failing with the same error |
@Kellen275 yeah, it's breaking everything helm related at the moment, including releases |
I've created an issue, this is affecting multiple PRs and all helm releases #14799 |
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The helm linting failure seems to be related to the submodule in the operator folder, and submitting a PR from a fork. I've asked for some help looking into it. |
the helm linting here has been fixed if you rebase with |
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
Kubernetes Manifest Diff SummaryScenario: default-single-binary-values (Added: 0, Modified: 0, Removed: 0)
Summary:
Added FilesNo added files Modified FilesNo modified files Removed FilesNo removed files Scenario: default-values (Added: 0, Modified: 0, Removed: 0)
Summary:
Added FilesNo added files Modified FilesNo modified files Removed FilesNo removed files Scenario: ingress-values (Added: 0, Modified: 0, Removed: 0)
Summary:
Added FilesNo added files Modified FilesNo modified files Removed FilesNo removed files Scenario: legacy-monitoring-values (Added: 0, Modified: 0, Removed: 0)
Summary:
Added FilesNo added files Modified FilesNo modified files Removed FilesNo removed files Scenario: simple-scalable-aws-kube-irsa-values (Added: 0, Modified: 0, Removed: 0)
Summary:
Added FilesNo added files Modified FilesNo modified files Removed FilesNo removed files |
What this PR does / why we need it:
When running a distributed Loki with zone aware replication. The
ingester
HPA is not configured to properly locate the 3 zoned ingester StatefulSets. This MR adds zoned HPAs similar to how the existing ingester headless service and statefulsets are handled.Which issue(s) this PR fixes:
Fixes #14021
Special notes for your reviewer:
This MR simply copies the existing
hpa.yaml
into 3 zoned yamls, and updates 3 linesmetadata.name
to append the-zone-{a|b|c}
suffixspec.scaleTargetRef.name
to append the-zone-{a|b|c}
suffixWe also update the
hpa.yaml
to not deploy when zoneAwareReplication is enabled (following the same pattern seen inservice-ingester-headless.yaml
Checklist
CONTRIBUTING.md
guide (required)feat
PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.docs/sources/setup/upgrade/_index.md
deprecated-config.yaml
anddeleted-config.yaml
files respectively in thetools/deprecated-config-checker
directory. Example PR