Add template for HttpCheck alert configuration #499
Conversation
| @@ -48,7 +48,7 @@ spec: | |||
| {{- if .Values.ingress.enabled }} | |||
| - alert: HttpCheck | |||
| expr: probe_success{instance="https://{{ .Values.ingress.host }}/admin/healthcheck"} != 1 | |||
| for: 1m | |||
| for: "{{ "{{ if (((.Values.alerts).httpCheck).for) }}" }}{{ .Values.alerts.httpCheck.for }}{{ "{{ else }}" }}1m{{ "{{ end }}" }}" | |||
There was a problem hiding this comment.
I'm not sure but wouldn't something simpler like this work?
{{ default "1m" .Values.alerts.httpCheck.for | quote }}
There was a problem hiding this comment.
I did try this at first see bc991ee, but the tests kept failing with a parse error for "1m", written with and without quotes as well as with pipe to quotes and without.
There was a problem hiding this comment.
Hmmm I think you should try the way I've proposed (I've used that for rocketchat-deploy therefore I'm pretty sure it works). I'm not convinced that the way you tried does the same thing.
| @@ -92,7 +92,9 @@ metrics: | |||
| repository: sscaling/jmx-prometheus-exporter | |||
| tag: 0.12.0 | |||
| pullPolicy: IfNotPresent | |||
|
|
|||
| alerts: | |||
There was a problem hiding this comment.
This will apply the "30m" value to all environments that use this helm chart. I don't think this is what we want?
This value should only be set in the repo which invokes this chart.
There was a problem hiding this comment.
That's true, done.
|
Merging all the commits into one would also be nice. |
|
I figured I would squash all into one when actually merging. |
What this PR does / why we need it:
Checklist
make docschart/<chart-name>[
bug,enhancement,documentation,change,breaking,dependency]