-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add Alertmanager config and templates in Helm chart #188
base: main
Are you sure you want to change the base?
Conversation
http_config: | ||
proxy_url: {{ .alertmanager.proxyURL }} | ||
{{- end }} | ||
{{- if .alertmanager.slackAPIToken }} |
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.
Is there a CAPA installation where we do not use slackAPIToken?
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.
I think not, but it doesn't hurt to keep it for now, I'd rather be safe.
send_resolved: true | ||
actions: *slack-actions | ||
|
||
- name: team_turtles_slack |
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.
This maybe should go away
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.
True
@@ -0,0 +1,18 @@ | |||
{{` | |||
{{ define "__alerturl" }} | |||
`}}{{ .alertmanager.grafanaAddress }}{{`/alerting/Mimir/{{ .CommonLabels.alertname }}/find |
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.
I think we should be able to replace this with the grafanaExploreURL or this queryFromGeneratorURL instead https://grafana.com/docs/mimir/latest/references/architecture/components/alertmanager/#templating right?
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.
Yes I'll look into this later
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.
I'm trying to see if we can get rid of this whole file. that would make things simpler imo
helm/observability-operator/templates/alertmanager/_helpers.tpl
Outdated
Show resolved
Hide resolved
helm/observability-operator/files/alertmanager/url-template.tmpl.helm-template
Show resolved
Hide resolved
helm/observability-operator/files/alertmanager/notification-template.tmpl
Show resolved
Hide resolved
|
||
# Link to related PMs | ||
{{ define "__alert_linked_postmortems" -}} | ||
https://github.com/giantswarm/giantswarm/issues?q=is%3Aissue+is%3Aopen+label%3Apostmortem+label%3Aalert%2F{{ .CommonLabels.alertname }} |
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.
Same remark but that's not that urgent
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.
This is used as a link on Slack
4b4c161
to
11abd5f
Compare
- Add secret resource, embedding raw and templated alertmanager files - Expose alertmanager templates values as helm chart values - Remove all Mimir related conditions - Split template into url and notification templates - Drop template directive, dynamically set by the operator - Escape template in template - Re-use slack actions
…asepath from values inside tpl function
This fixes the infamous: error calling tpl: cannot retrieve Template.Basepath from values inside tpl function It does use .Values in templates to access values and pass $ root context to tpl
Co-authored-by: Quentin Bisson <[email protected]>
9d0c548
to
5d320b9
Compare
alerting: | ||
alertmanagerURL: "" | ||
grafanaAddress: "" | ||
proxyURL: "" |
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.
proxy url should be coming from an env variable right?
resolve_timeout: 5m | ||
{{- if .Values.alerting.proxyURL }} | ||
http_config: | ||
proxy_url: {{ .Values.alerting.proxyURL }} |
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.
I think this is not useful anymore with the env variables automatically set on all pods. Do you think you could test without it on goat? Because I'm not sure we can get this value anyway in config as it is set by kyverno policies
Apart from my proxy url questions, I'm fine with this for now but I'd rather we remove the url mumbo jumbo later on |
This reverts commit 283c100.
* Team: {{ (index .Alerts 0).Labels.team }} | ||
* Area: {{ (index .Alerts 0).Labels.area }} / {{ (index .Alerts 0).Labels.topic }} | ||
* Instances:{{ range .Alerts.Firing }} | ||
🔥 {{ if .Labels.instance }}{{ .Labels.instance }}: {{ end }}{{ .Annotations.description }}{{ end }} | ||
{{- end }} | ||
|
||
# This builds the silence URL. We exclude the alertname in the range |
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.
I think we need to remove this silence URL right?
Towards: giantswarm/roadmap#3746
This PR does couple of things to get the Alertmanager into a Secret in the Helm chart:
How I generated the new Alertmanager config and notification template
Alertmanager config
Notification template
I would like some opinions before I continue in this direction, because I feel there are a lot of workaround here to get this config into a Secret and it could be easier to have it directly in code. Also does anyone remember what the
ProxyURL
is used for ? It seems to be OpsGenie related but I gladly ignored it and I have a feeling this could be important.