Conversation
charts/operator/README.md carried a `### operator SLO (slo.*)` section
describing `slo.operatorNamespace` and `slo.alerting.enabled`, complete with the
six Secrets the receivers need and advice to enable it per-environment in
production. Chart.yaml's description said the same, three lines above the entry
recording that 0.6.0 removed the PrometheusRule, the AlertmanagerConfig and the
namespace that held them.
Until 0.6.4 that was merely false, in the way Helm makes cheap: an unknown value
is absorbed in silence, so a reader who followed the README got no alerting and
no error either. 0.6.4 shipped values.schema.json, which closes the top level.
Following this README now fails outright:
helm template ... --set slo.enabled=true
Error: values don't meet the specifications of the schema(s)
- at '': additional properties 'slo' not allowed
So the documentation instructed a user to do something the artifact rejects.
Both spellings are corrected, and the README now states where the operator's
alert rules actually live — eks-gitops, as Grafana-managed rules evaluated
against AMP — and distinguishes the `reconcilers.slo` key that does exist and
toggles the SLOPolicy reconciler in the binary.
The version moves to 0.6.5 because README.md is packaged content and the
published 0.6.4 would otherwise be overwritten with different bytes under the
same tag. This was caught by check-chart-version-bump.py rather than by me, on
its first real change since it landed, and it discriminated correctly:
Chart.yaml's edit is comment-only and helm drops comments on package, so only
README.md was reported as differing.
No template, CRD, default or appVersion changed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The claim
charts/operator/README.mdcarried a full### operator SLO (slo.*)section —slo.operatorNamespace,slo.alerting.enabled, the six Secrets its receivers need, and advice to "enable per-env in production once provisioned."Chart.yaml's description said the same thing three lines above the entry recording that 0.6.0 removed the PrometheusRule, the AlertmanagerConfig and the namespace that held them.Why it got worse, not better
Until 0.6.4 this was merely false in the way Helm makes cheap: an unknown value is absorbed in silence, so a reader who followed the README got no alerting and no error either.
0.6.4 shipped
values.schema.json, which closes the top level. Following this README now fails outright:Documentation instructing a user to do something the artifact rejects.
The fix
Both spellings corrected. The README now says where the operator's alert rules actually live — eks-gitops, as Grafana-managed rules evaluated against AMP, not a prometheus-operator in-cluster — and distinguishes the
reconcilers.slokey that does exist and toggles the SLOPolicy reconciler in the binary.The version bump, and who caught it
0.6.4 → 0.6.5, because
README.mdis packaged content and the published 0.6.4 would otherwise be overwritten with different bytes under the same tag.I did not notice that.
check-chart-version-bump.pydid, on its first real change since it landed this morning — and it discriminated correctly:Only README.md.
Chart.yaml's edit is comment-only andhelm packagedrops comments, which is exactly the case the gate's self-test holds green.No template, CRD, default or appVersion changed.
Related
This is half of what keeps regenerating eks-gitops#33. The other half — whether
severity: pagerules have any contact point behind them — can't be answered from source and is commented there separately.