Bug 1955489: enable hard-anti affinity and PDB for Alertmanager #1489

simonpasquier · 2021-11-22T16:17:16Z

This change introduces hard pod anti-affinity rules and pod disruption
budgets for Alertmanager to ensure the maximum availability for the
Alertmanager cluster in the event of nodes going down (either due to
upgrades or unexpected outages). The cluster monitoring operator updates
the Upgradeable condition to false when it detects that the pods
aren't correctly spread to ensure that an upgrade only happens in safe
configurations.

The change also decreases the number of Alertmanager replicas from 3 to
2 to be consistent with the other monitoring components as well as the
HA conventions stating that in general OpenShift component should run
with a replica count of 2 [1]. In addition with 3 replicas, it is
impossible to enable hard anti-affinity on nodes since 2 worker nodes is
a supported deployment for OCP.

The initial idea of running 3 replicas was to guarantee the replication
of data (silences + notifications) during pod roll-outs even if the user
didn't configure persistent storage. However given that no pod
disruption budget were defined, there was no guarantee that Kubernetes
would always keep one Alertmanager pod running. With hard anti-affinity
and PDB, we are now sure that at least one Alertmanager pod is kept
running. And we are also setting up a startup probe that waits for at
least 20 seconds meaning that Kubernetes should wait for about 20
seconds after a new Alertmanager pod is running before considering
rolling out the next one. This interval of time should be more than
enough for the new Alertmanager to synchronize its data from the older
peer.

I added CHANGELOG entry for this change.
No user facing changes, so no entry in CHANGELOG was needed.

openshift-ci · 2021-11-22T16:19:22Z

@simonpasquier: This pull request references Bugzilla bug 1955489, which is invalid:

expected the bug to target the "4.10.0" release, but it targets "---" instead

Comment /bugzilla refresh to re-evaluate validity if changes to the Bugzilla bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

[WIP] Bug 1955489: enable hard-anti affinity and PDB for Alertmanager

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

simonpasquier · 2021-11-23T07:53:39Z

/bugzilla refresh

openshift-ci · 2021-11-23T07:53:46Z

@simonpasquier: This pull request references Bugzilla bug 1955489, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @juzhao

In response to this:

/bugzilla refresh

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

juzhao · 2021-11-25T03:43:49Z

see from https://bugzilla.redhat.com/show_bug.cgi?id=1955489#c11, tested with the PR, Alertmanager Statefulsets now have 2 replicas and hard affinity set but pods can not be started

simonpasquier · 2021-11-25T09:31:56Z

@juzhao yes the PR is still WIP and the CI fails for the same reason as you've noticed.

simonpasquier · 2021-11-26T10:10:59Z

/test e2e-agnostic-operator

simonpasquier · 2021-11-26T12:57:10Z

/test e2e-agnostic-operator

assets/alertmanager/alertmanager.yaml

openshift-ci · 2021-12-01T12:35:46Z

@simonpasquier: This pull request references Bugzilla bug 1955489, which is valid.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target release (4.10.0) matches configured target release for branch (4.10.0)
bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, ON_DEV, POST, POST)

Requesting review from QA contact:
/cc @juzhao

In response to this:

[WIP] Bug 1955489: enable hard-anti affinity and PDB for Alertmanager

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

simonpasquier · 2021-12-02T10:47:09Z

/skip

This change introduces hard pod anti-affinity rules and pod disruption budgets for Alertmanager to ensure the maximum availability for the Alertmanager cluster in the event of nodes going down (either due to upgrades or unexpected outages). The cluster monitoring operator updates the `Upgradeable` condition to false when it detects that the pods aren't correctly spread to ensure that an upgrade only happens in safe configurations. The change also decreases the number of Alertmanager replicas from 3 to 2 to be consistent with the other monitoring components as well as the HA conventions stating that in general OpenShift component should run with a replica count of 2 [1]. In addition with 3 replicas, it is impossible to enable hard anti-affinity on nodes since 2 worker nodes is a supported deployment for OCP. The initial idea of running 3 replicas was to guarantee the replication of data (silences + notifications) during pod roll-outs even if the user didn't configure persistent storage. However given that no pod disruption budget were defined, there was no guarantee that Kubernetes would always keep one Alertmanager pod running. With hard anti-affinity and PDB, we are now sure that at least one Alertmanager pod is kept running. And we are also setting up a startup probe that waits for at least 20 seconds meaning that Kubernetes should wait for about 20 seconds after a new Alertmanager pod is running before considering rolling out the next one. This interval of time should be more than enough for the new Alertmanager to synchronize its data from the older peer. [1] https://github.com/openshift/enhancements/blob/master/CONVENTIONS.md#high-availability Signed-off-by: Simon Pasquier <spasquie@redhat.com>

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

simonpasquier · 2021-12-06T08:55:54Z

/skip
/retitle Bug 1955489: enable hard-anti affinity and PDB for Alertmanager

simonpasquier · 2021-12-06T14:36:42Z

/retest
/assign @jan--f

simonpasquier · 2021-12-06T14:37:50Z

/label tide/merge-method-squash

simonpasquier · 2021-12-06T14:38:59Z

/skip

openshift-ci · 2021-12-06T15:52:11Z

@simonpasquier: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/e2e-aws-single-node	`7f745e1`	link	false	`/test e2e-aws-single-node`

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fpetkovski · 2021-12-07T06:42:15Z

pkg/manifests/manifests.go

+		// that 20 seconds is enough for a full synchronization (this is twice
+		// the time Alertmanager waits before declaring that it can start
+		// sending notfications).
+		a.Spec.Containers = append(a.Spec.Containers,


Should this change be made in the prometheus operator?

Yes it makes sense to follow up upstream. BTW I notice that we have no readiness probe for Alertmanager...

juzhao · 2021-12-07T08:06:12Z

assets/alertmanager/pod-disruption-budget.yaml

+  name: alertmanager-main
+  namespace: openshift-monitoring
+spec:
+  maxUnavailable: 1


maxUnavailable is fine, maybe minAvailable is better

# oc -n openshift-monitoring get pdb NAME MIN AVAILABLE MAX UNAVAILABLE ALLOWED DISRUPTIONS AGE alertmanager-main N/A 1 1 39m prometheus-adapter 1 N/A 1 50m prometheus-k8s 1 N/A 1 39m thanos-querier-pdb 1 N/A 1 38m

maxUnavailable comes from upstream (https://github.com/prometheus-operator/kube-prometheus/blob/6d013d4e4f980ba99cfdafa9432819d484e2f829/jsonnet/kube-prometheus/components/alertmanager.libsonnet#L154) and my understanding is that because kube-prometheus deploys 3 replicas of Alertmanager, the choice was either maxUnavailable: 1 or minAvailable: 2. Ideally we should settle on one field and automatically calculate the budget.

From https://kubernetes.io/docs/tasks/run-application/configure-pdb/

The use of maxUnavailable is recommended as it automatically responds to changes in the number of replicas of the corresponding controller.

I'll check with the workloads team if there's any strong recommendation.

👍 for using maxUnavailable here. The note about the number of replicas is relevant here imo.

simonpasquier · 2021-12-07T10:23:54Z

/skip

jan--f · 2021-12-07T12:04:34Z

/lgtm
This looks great!

openshift-ci · 2021-12-07T12:05:10Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jan--f, simonpasquier

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [jan--f,simonpasquier]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci · 2021-12-07T14:04:05Z

@simonpasquier: All pull requests linked via external trackers have merged:

openshift/cluster-monitoring-operator#1489

Bugzilla bug 1955489 has been moved to the MODIFIED state.

In response to this:

Bug 1955489: enable hard-anti affinity and PDB for Alertmanager

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

…shift#1489) * *: enable hard-anti affinity and PDB for Alertmanager This change introduces hard pod anti-affinity rules and pod disruption budgets for Alertmanager to ensure the maximum availability for the Alertmanager cluster in the event of nodes going down (either due to upgrades or unexpected outages). The cluster monitoring operator updates the `Upgradeable` condition to false when it detects that the pods aren't correctly spread to ensure that an upgrade only happens in safe configurations. The change also decreases the number of Alertmanager replicas from 3 to 2 to be consistent with the other monitoring components as well as the HA conventions stating that in general OpenShift component should run with a replica count of 2 [1]. In addition with 3 replicas, it is impossible to enable hard anti-affinity on nodes since 2 worker nodes is a supported deployment for OCP. The initial idea of running 3 replicas was to guarantee the replication of data (silences + notifications) during pod roll-outs even if the user didn't configure persistent storage. However given that no pod disruption budget were defined, there was no guarantee that Kubernetes would always keep one Alertmanager pod running. With hard anti-affinity and PDB, we are now sure that at least one Alertmanager pod is kept running. And we are also setting up a startup probe that waits for at least 20 seconds meaning that Kubernetes should wait for about 20 seconds after a new Alertmanager pod is running before considering rolling out the next one. This interval of time should be more than enough for the new Alertmanager to synchronize its data from the older peer. [1] https://github.com/openshift/enhancements/blob/master/CONVENTIONS.md#high-availability Signed-off-by: Simon Pasquier <spasquie@redhat.com> * assets: regenerate * jsonnet,pkg: configure startupProbe only when no storage * assets: regenerate * test/e2e: add TestAlertmanagerDataReplication test Signed-off-by: Simon Pasquier <spasquie@redhat.com>

openshift-ci bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Nov 22, 2021

openshift-ci bot requested review from arajkumar and fpetkovski November 22, 2021 16:18

openshift-ci bot added bugzilla/severity-high Referenced Bugzilla bug's severity is high for the branch this PR is targeting. bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Nov 22, 2021

openshift-ci bot added bugzilla/valid-bug Indicates that a referenced Bugzilla bug is valid for the branch this PR is targeting. and removed bugzilla/invalid-bug Indicates that a referenced Bugzilla bug is invalid for the branch this PR is targeting. labels Nov 23, 2021

openshift-ci bot requested a review from juzhao November 23, 2021 07:53

simonpasquier force-pushed the bz1955489 branch 3 times, most recently from 9c5435c to 6d848f7 Compare November 23, 2021 09:37

simonpasquier force-pushed the bz1955489 branch from 6d848f7 to 3dd7c1b Compare November 25, 2021 09:02

simonpasquier force-pushed the bz1955489 branch from 3dd7c1b to fa637b6 Compare November 25, 2021 15:08

simonpasquier mentioned this pull request Nov 26, 2021

Bug 2008540: remove alert HighlyAvailableWorkloadIncorrectlySpread #1488

Merged

2 tasks

simonpasquier force-pushed the bz1955489 branch from fa637b6 to 4fff5a0 Compare November 26, 2021 16:35

fpetkovski reviewed Nov 26, 2021

View reviewed changes

assets/alertmanager/alertmanager.yaml Show resolved Hide resolved

openshift-ci bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Nov 30, 2021

simonpasquier force-pushed the bz1955489 branch from 4fff5a0 to f86c588 Compare December 1, 2021 10:49

openshift-ci bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Dec 1, 2021

simonpasquier force-pushed the bz1955489 branch from 130683f to 3e8ffd2 Compare December 2, 2021 13:24

fpetkovski mentioned this pull request Dec 2, 2021

feat: deploy alertmanagers on different nodes rhobs/observability-operator#91

Merged

simonpasquier force-pushed the bz1955489 branch 3 times, most recently from 42a6c24 to c570f13 Compare December 3, 2021 12:46

simonpasquier added 4 commits December 3, 2021 16:37

assets: regenerate

62eb8c4

jsonnet,pkg: configure startupProbe only when no storage

a64697f

assets: regenerate

e3659c4

simonpasquier force-pushed the bz1955489 branch from c570f13 to d0fafd8 Compare December 3, 2021 15:37

test/e2e: add TestAlertmanagerDataReplication test

7f745e1

Signed-off-by: Simon Pasquier <spasquie@redhat.com>

simonpasquier force-pushed the bz1955489 branch from d0fafd8 to 7f745e1 Compare December 6, 2021 08:54

openshift-ci bot changed the title ~~[WIP] Bug 1955489: enable hard-anti affinity and PDB for Alertmanager~~ Bug 1955489: enable hard-anti affinity and PDB for Alertmanager Dec 6, 2021

openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 6, 2021

openshift-ci bot assigned jan--f Dec 6, 2021

openshift-ci bot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Dec 6, 2021

fpetkovski reviewed Dec 7, 2021

View reviewed changes

juzhao reviewed Dec 7, 2021

View reviewed changes

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Dec 7, 2021

openshift-merge-robot merged commit 4996004 into openshift:master Dec 7, 2021

simonpasquier deleted the bz1955489 branch December 7, 2021 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 1955489: enable hard-anti affinity and PDB for Alertmanager #1489

Bug 1955489: enable hard-anti affinity and PDB for Alertmanager #1489

simonpasquier commented Nov 22, 2021 •

edited

Loading

openshift-ci bot commented Nov 22, 2021

simonpasquier commented Nov 23, 2021

openshift-ci bot commented Nov 23, 2021

juzhao commented Nov 25, 2021

simonpasquier commented Nov 25, 2021

simonpasquier commented Nov 26, 2021

simonpasquier commented Nov 26, 2021

openshift-ci bot commented Dec 1, 2021

simonpasquier commented Dec 2, 2021

simonpasquier commented Dec 6, 2021

simonpasquier commented Dec 6, 2021

simonpasquier commented Dec 6, 2021

simonpasquier commented Dec 6, 2021

openshift-ci bot commented Dec 6, 2021 •

edited

Loading

fpetkovski Dec 7, 2021

simonpasquier Dec 7, 2021

juzhao Dec 7, 2021

simonpasquier Dec 7, 2021

jan--f Dec 7, 2021

simonpasquier commented Dec 7, 2021

jan--f commented Dec 7, 2021

openshift-ci bot commented Dec 7, 2021

openshift-ci bot commented Dec 7, 2021

Bug 1955489: enable hard-anti affinity and PDB for Alertmanager #1489

Bug 1955489: enable hard-anti affinity and PDB for Alertmanager #1489

Conversation

simonpasquier commented Nov 22, 2021 • edited Loading

openshift-ci bot commented Nov 22, 2021

simonpasquier commented Nov 23, 2021

openshift-ci bot commented Nov 23, 2021

juzhao commented Nov 25, 2021

simonpasquier commented Nov 25, 2021

simonpasquier commented Nov 26, 2021

simonpasquier commented Nov 26, 2021

openshift-ci bot commented Dec 1, 2021

simonpasquier commented Dec 2, 2021

simonpasquier commented Dec 6, 2021

simonpasquier commented Dec 6, 2021

simonpasquier commented Dec 6, 2021

simonpasquier commented Dec 6, 2021

openshift-ci bot commented Dec 6, 2021 • edited Loading

fpetkovski Dec 7, 2021

Choose a reason for hiding this comment

simonpasquier Dec 7, 2021

Choose a reason for hiding this comment

juzhao Dec 7, 2021

Choose a reason for hiding this comment

simonpasquier Dec 7, 2021

Choose a reason for hiding this comment

jan--f Dec 7, 2021

Choose a reason for hiding this comment

simonpasquier commented Dec 7, 2021

jan--f commented Dec 7, 2021

openshift-ci bot commented Dec 7, 2021

openshift-ci bot commented Dec 7, 2021

simonpasquier commented Nov 22, 2021 •

edited

Loading

openshift-ci bot commented Dec 6, 2021 •

edited

Loading