WIP: configure alertmanager HA cluster mode for sts #460
Conversation
Signed-off-by: Niclas Schad <[email protected]>
| {{- if gt (int .Values.alertmanager.replicas) 1}} | ||
| {{- $fullName := include "cortex.alertmanagerFullname" . }} | ||
| {{- range $i := until (int .Values.alertmanager.replicas) }} | ||
| - "-alertmanager.cluster.peers={{ $fullName }}-{{ $i }}.{{ $fullName }}-headless.{{ $.Release.Namespace }}.svc.cluster.local:{{ $svcClusterPort }}" |
There was a problem hiding this comment.
Not sure if this even technically correct. Since according to the documentation this is supposed to be comma seperated list which is different from alertmanager configuration where you have to repeat the flag for each peer.
There was a problem hiding this comment.
Not sure if this even technically correct. Since according to the documentation this is supposed to be comma seperated list which is different from alertmanager configuration where you have to repeat the flag for each peer.
I see you are referencing L88
AlertmanagerClusterFlags = func(peers string) map[string]string {
return map[string]string{
"-alertmanager.cluster.listen-address": "0.0.0.0:9094", // This is the default, but let's be explicit.
"-alertmanager.cluster.peers": peers,
"-alertmanager.cluster.peer-timeout": "2s",
}
}
but @nschad where do you see that the flag need to be repeated for each peer ?
There was a problem hiding this comment.
alertmanager.cluster.peers
I don't that why I said it should be single string and comma separated (Doc: https://cortexmetrics.io/docs/configuration/configuration-file/)
There was a problem hiding this comment.
alertmanager.cluster.peers
I don't that why I said it should be single string and comma separated (Doc: https://cortexmetrics.io/docs/configuration/configuration-file/)
I misunderstood your message so, sorry.
There was a problem hiding this comment.
I have a commit waiting to be pushed to this branch to change to a comma separated peers :
commit 9c245d488cc916df1e76be1afb66a593dabccdd8 (HEAD -> feature/alertmanager-cluster)
Author: AlexandreRoux <[email protected]>
Date: Thu Jun 29 17:28:18 2023 +0200
configure alertmanager cluster peers as comma seperated list
Signed-off-by: AlexandreRoux <[email protected]>
diff --git a/templates/alertmanager/alertmanager-statefulset.yaml b/templates/alertmanager/alertmanager-statefulset.yaml
index 41b592b..b5d5043 100644
--- a/templates/alertmanager/alertmanager-statefulset.yaml
+++ b/templates/alertmanager/alertmanager-statefulset.yaml
@@ -156,9 +156,12 @@ spec:
- "-config.file=/etc/cortex/cortex.yaml"
{{- if gt (int .Values.alertmanager.replicas) 1}}
{{- $fullName := include "cortex.alertmanagerFullname" . }}
+ {{- $peers := list }}
{{- range $i := until (int .Values.alertmanager.replicas) }}
- - "-alertmanager.cluster.peers={{ $fullName }}-{{ $i }}.{{ $fullName }}-headless.{{ $.Release.Namespace }}.svc.cluster.local:{{ $svcClusterPort }}"
+ {{- $peer := printf "%s-%d.%s-headless.%s.svc.cluster.local:%s" $fullName $i $fullName $.Release.Namespace $svcClusterPort }}
+ {{- $peers = append $peers $peer }}
{{- end }}
+ - "-alertmanager.cluster.peers={{ join "," $peers }}"
{{- end }}
{{- range $key, $value := .Values.alertmanager.extraArgs }}
- "-{{ $key }}={{ $value }}"
but it seems I have no access to push to the branch, is this expected @nschad ?
Is there any preferred way to get it here (then I will merge this upstream / branch to my code in #435.
There was a problem hiding this comment.
Uh I don't know if I can enable this after the fact. I can close this PR and you can just edit your PR #435?
There was a problem hiding this comment.
Uh I don't know if I can enable this after the fact. I can close this PR and you can just edit your PR #435?
sounds good !
|
This issue has been automatically marked as stale because it has not had any activity in the past 30 days. It will be closed in 15 days if no further activity occurs. Thank you for your contributions. |
What this PR does:
Add's support for automatically adding peers for alertmanager.
I've disabled alertmanager deployment mode as I don't see how we could realistically configure the cluster mode for it.
(deployment names are not predictable/consistent). But I don't have a strong opinion about it, I just think it's unneeded.
CC: @kd7lxl @AlexandreRoux
Note: the ci test values are increased for testing purposes. They will be reverted back if we ever merge this PR
TODO:
Which issue(s) this PR fixes:
Fixes #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]