Skip to content
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.

Sign up for GitHub

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

WIP: configure alertmanager HA cluster mode for sts #460

Closed
wants to merge 1 commit into from
Closed

WIP: configure alertmanager HA cluster mode for sts #460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ci/test-deployment-values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ runtimeconfigmap:
annotations:
foo: bar
alertmanager:
replicas: 1
replicas: 3
statefulSet:
enabled: false
extraVolumes:
Expand Down
2 changes: 1 addition & 1 deletion ci/test-sts-values.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ runtimeconfigmap:
annotations:
foo: bar
alertmanager:
replicas: 1
replicas: 3
statefulSet:
enabled: true
extraVolumes:
Expand Down
169 changes: 0 additions & 169 deletions templates/alertmanager/alertmanager-dep.yaml
View file
Open in desktop

This file was deleted.

17 changes: 17 additions & 0 deletions templates/alertmanager/alertmanager-statefulset.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
{{- $svcClusterAddress := ((.Values.config.alertmanager.cluster).listen_address) | default "0.0.0.0:9094" }}
{{- $svcClusterPort := (split ":" $svcClusterAddress)._1 }}
{{- if .Values.alertmanager.enabled -}}
{{- if .Values.alertmanager.statefulSet.enabled -}}
apiVersion: apps/v1
Expand Down Expand Up @@ -152,6 +154,12 @@ spec:
args:
- "-target=alertmanager"
- "-config.file=/etc/cortex/cortex.yaml"
{{- 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 }}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@humblebundledore humblebundledore Jun 28, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 !

{{- end }}
{{- end }}
{{- range $key, $value := .Values.alertmanager.extraArgs }}
- "-{{ $key }}={{ $value }}"
{{- end }}
Expand All @@ -175,6 +183,15 @@ spec:
- name: gossip
containerPort: {{ .Values.config.memberlist.bind_port }}
protocol: TCP
- name: grpc
containerPort: {{ .Values.config.server.grpc_listen_port }}
protocol: TCP
- containerPort: {{ $svcClusterPort }}
name: alert-clu-tcp
protocol: TCP
- containerPort: {{ $svcClusterPort }}
name: alert-clu-udp
protocol: UDP
startupProbe:
{{- toYaml .Values.alertmanager.startupProbe | nindent 12 }}
livenessProbe:
Expand Down
8 changes: 8 additions & 0 deletions templates/alertmanager/alertmanager-svc-headless.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,14 @@ spec:
protocol: TCP
name: http-metrics
targetPort: http-metrics
- port: 9094
protocol: UDP
name: alert-clu-udp
targetPort: alert-clu-udp
- port: 9094
protocol: TCP
name: alert-clu-tcp
targetPort: alert-clu-tcp
selector:
{{- include "cortex.alertmanagerSelectorLabels" . | nindent 4 }}
{{- end -}}
Expand Down