-
Notifications
You must be signed in to change notification settings - Fork 326
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
k8s-stack: check notifier.blackhole
flag if no notifiers are set in vmalert
#1518
Conversation
{{- if not (or (hasKey $spec "notifier") (hasKey $spec "notifiers") (hasKey $spec "notifierConfigRef")) }} | ||
{{- fail "`notifier`, `notifiers` or `notifierConfigRef` should be set for vmalert"}} | ||
{{- end }} |
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.
Wdyt about enforcing user to set notifier.blackhole
if there are no other notifiers set?
This allows to make sure that user explicitly wants to blackhole all notifications and did not forget to set notifier by acccident.
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.
updated PR
8c77a00
to
9ba5d3f
Compare
notifier.blackhole
if no notifiers are set
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
See a suggestion for a changelog entry, it should make it easier to understand for the end users.
@@ -3,6 +3,7 @@ | |||
- Added VMAuth to k8s stack. See [this issue](https://github.com/VictoriaMetrics/helm-charts/issues/829) | |||
- Fixed ETCD dashboard | |||
- Use path prefix from args as a default path prefix for ingress. Related [issue](https://github.com/VictoriaMetrics/helm-charts/issues/1260) | |||
- Force setting blackhole notifier if no notifiers are set |
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.
- Force setting blackhole notifier if no notifiers are set | |
- Allow using vmalert without notifiers configuration. Note that it is required to use `.vmalert.spec.extraArgs["notifiers.blackhole"]: true` in order to start vmalert with a blackhole configuration. |
9ba5d3f
to
fdac4c9
Compare
updated error in helm and changelog, please approve again |
notifier.blackhole
if no notifiers are setnotifier.blackhole
flag if no notifiers are set in vmalert
https://victoriametrics.slack.com/archives/CTC488T1P/p1726776490053099?thread_ts=1725540517.623349&cid=CTC488T1P