From 3b6433761145e5f4218dd9f0e8fa3ad746c609e8 Mon Sep 17 00:00:00 2001 From: Ayush Garg Date: Thu, 6 Aug 2026 22:48:26 +0530 Subject: [PATCH] fix(alert-tuning): [SPRE-5878] suppress EtcdHighNumberOfLeaderChanges during cluster upgrades MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem EtcdHighNumberOfLeaderChanges was generating ~109 false positive firings across two consecutive 14-day investigation windows with 0% actionable signal. Every firing fell strictly within scheduled OCP upgrade windows. Root cause: during a rolling upgrade, the MachineConfigOperator drains and reboots control plane nodes one at a time. Each restart stops the local etcd member, forcing a Raft leader re-election. A 3-node cluster produces 2-3 elections per upgrade (~70-minute window), consistently crossing the > 2 threshold. The alert is mathematically guaranteed to fire on every upgrade. ## Why not raise the threshold Raising the threshold from > 2 to > 4 was considered and rejected. The maximum observed increase() value during any upgrade was 3.158 — raising to > 4 would eliminate all false positives but creates a blind spot: genuine instability (e.g. disk pressure, network blip) causing 3-4 elections during normal operation would go silently undetected. Preserving full sensitivity (> 2) is the correct behaviour. ## Resolution Suppress the alert per-cluster during active upgrades using a recording rule derived from ClusterUpgradeOngoing as the inhibition signal: (increase(etcd_server_leader_changes_seen_total{namespace="openshift-etcd"}[10m]) > 2) unless on (source_cluster) cluster_upgrade_ongoing This preserves full sensitivity during normal operation and suppresses only when an upgrade is confirmed firing on the same cluster. An upgrade on one cluster does not affect alerting on others. ## Why a recording rule instead of ALERTS{} directly The unless clause references cluster_upgrade_ongoing (a recording rule) rather than ALERTS{alertname="ClusterUpgradeOngoing"} directly, per reviewer feedback: 1. ALERTS{} silently matches both alertstate="firing" and alertstate="pending". The recording rule explicitly filters alertstate="firing" — suppression only activates once the upgrade is confirmed, not while it is still pending. 2. ALERTS{} is an internal Prometheus construct. A named recording rule produces a stable, well-defined, reusable time series. Known tradeoff: the recording rule adds one evaluation cycle (~1 min) of additional lag before suppression activates. This is negligible given the ~70-minute upgrade window and the etcd alert's own for: 1m gate. Label alignment verified in production RHOBS: cluster_name in aus_cluster_upgrade_policy_info matches source_cluster on etcd_server_leader_changes_seen_total across all RHTAP clusters. ## Changes rhobs/recording/cluster_upgrade_recording_rules.yaml (new): Recording rule that materialises cluster_upgrade_ongoing{source_cluster}=1 only when ClusterUpgradeOngoing is alertstate="firing". Uses max by (source_cluster) to produce a clean per-cluster signal. rhobs/alerting/data_plane/prometheus.cluster_ongoing_updrade_alert.yaml: Wrap expression in label_replace() to copy cluster_name into source_cluster, enabling per-cluster join with etcd metrics via unless on (source_cluster). rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml: Add unless on (source_cluster) cluster_upgrade_ongoing inhibition to EtcdHighNumberOfLeaderChanges. Threshold stays at > 2. test/promql/tests/data_plane/cluster_upgrade_ongoing_alert_test.yaml: Assert source_cluster label is present in the fired alert. test/promql/tests/data_plane/cluster_capacity_test.yaml: - Fix pre-existing bug: stable-counter test was missing namespace="openshift-etcd", causing vacuous pass - Add suppression test: same-cluster upgrade suppresses the alert - Add per-cluster specificity test: different-cluster upgrade does not - Add stable-counter negative test test/promql/tests/recording/cluster_upgrade_recording_rules_test.yaml (new): - alertstate="firing" → cluster_upgrade_ongoing{source_cluster}=1 - alertstate="pending" → no output ## Test results promtool test rules (quay.io/prometheus/prometheus): cluster_capacity_test.yaml SUCCESS cluster_upgrade_ongoing_alert_test.yaml SUCCESS cluster_upgrade_recording_rules_test.yaml SUCCESS Jira: https://redhat.atlassian.net/browse/SPRE-5878 --- .../prometheus.cluster_capacity_alerts.yaml | 4 +- ...metheus.cluster_ongoing_updrade_alert.yaml | 9 ++-- .../cluster_upgrade_recording_rules.yaml | 16 +++++++ .../data_plane/cluster_capacity_test.yaml | 44 +++++++++++++++++-- .../cluster_upgrade_ongoing_alert_test.yaml | 1 + .../cluster_upgrade_recording_rules_test.yaml | 29 ++++++++++++ 6 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 rhobs/recording/cluster_upgrade_recording_rules.yaml create mode 100644 test/promql/tests/recording/cluster_upgrade_recording_rules_test.yaml diff --git a/rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml b/rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml index 46d0b47fb..43212408c 100644 --- a/rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml +++ b/rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml @@ -79,7 +79,9 @@ spec: - alert: EtcdHighNumberOfLeaderChanges expr: | - increase(etcd_server_leader_changes_seen_total{namespace="openshift-etcd"}[10m]) > 2 + (increase(etcd_server_leader_changes_seen_total{namespace="openshift-etcd"}[10m]) > 2) + unless on (source_cluster) + cluster_upgrade_ongoing for: 1m labels: severity: warning diff --git a/rhobs/alerting/data_plane/prometheus.cluster_ongoing_updrade_alert.yaml b/rhobs/alerting/data_plane/prometheus.cluster_ongoing_updrade_alert.yaml index ac2cae86a..14de831fa 100644 --- a/rhobs/alerting/data_plane/prometheus.cluster_ongoing_updrade_alert.yaml +++ b/rhobs/alerting/data_plane/prometheus.cluster_ongoing_updrade_alert.yaml @@ -12,9 +12,12 @@ spec: rules: - alert: ClusterUpgradeOngoing expr: >- - (aus_cluster_version_remaining_soak_days <= -2 >= -3) - * on(cluster_uuid) group_left(cluster_name, current_version) - aus_cluster_upgrade_policy_info{org_name="RHTAP"} + label_replace( + (aus_cluster_version_remaining_soak_days <= -2 >= -3) + * on(cluster_uuid) group_left(cluster_name, current_version) + aus_cluster_upgrade_policy_info{org_name="RHTAP"}, + "source_cluster", "$1", "cluster_name", "(.*)" + ) for: 1m labels: severity: info diff --git a/rhobs/recording/cluster_upgrade_recording_rules.yaml b/rhobs/recording/cluster_upgrade_recording_rules.yaml new file mode 100644 index 000000000..54391c075 --- /dev/null +++ b/rhobs/recording/cluster_upgrade_recording_rules.yaml @@ -0,0 +1,16 @@ +apiVersion: monitoring.coreos.com/v1 +kind: PrometheusRule +metadata: + name: rhtap-cluster-upgrade + labels: + tenant: rhtap +spec: + groups: + - name: cluster_upgrade + interval: 1m + rules: + - record: cluster_upgrade_ongoing + expr: | + max by (source_cluster) ( + ALERTS{alertname="ClusterUpgradeOngoing", alertstate="firing"} + ) diff --git a/test/promql/tests/data_plane/cluster_capacity_test.yaml b/test/promql/tests/data_plane/cluster_capacity_test.yaml index 8c025ee36..3a8316521 100644 --- a/test/promql/tests/data_plane/cluster_capacity_test.yaml +++ b/test/promql/tests/data_plane/cluster_capacity_test.yaml @@ -140,7 +140,7 @@ tests: - interval: 1m input_series: - # Etcd No Leader failures within threshold limit, so it will be alerted. + # Leader changes exceed threshold, no upgrade in progress — alert fires. - series: 'etcd_server_leader_changes_seen_total{namespace="openshift-etcd",source_cluster="cluster01"}' values: '3+2x60' @@ -162,8 +162,46 @@ tests: - interval: 1m input_series: - # Etcd has Leader within threshold limit, so it will not be alerted. - - series: 'etcd_server_leader_changes_seen_total{source_cluster="cluster01"}' + # Leader changes exceed threshold BUT a cluster upgrade is ongoing for the same cluster. + # The unless operator suppresses the alert — upgrade-induced re-elections are expected behaviour. + - series: 'etcd_server_leader_changes_seen_total{namespace="openshift-etcd",source_cluster="cluster01"}' + values: '3+2x60' + - series: 'cluster_upgrade_ongoing{source_cluster="cluster01"}' + values: '1+0x60' + + alert_rule_test: + - eval_time: 11m + alertname: EtcdHighNumberOfLeaderChanges + + - interval: 1m + input_series: + # Leader changes exceed threshold on cluster01, but the upgrade is ongoing on a DIFFERENT cluster (cluster02). + # Suppression is per-cluster — cluster01 alert must still fire. + - series: 'etcd_server_leader_changes_seen_total{namespace="openshift-etcd",source_cluster="cluster01"}' + values: '3+2x60' + - series: 'cluster_upgrade_ongoing{source_cluster="cluster02"}' + values: '1+0x60' + + alert_rule_test: + - eval_time: 11m + alertname: EtcdHighNumberOfLeaderChanges + exp_alerts: + - exp_labels: + component: etcd + namespace: openshift-etcd + severity: warning + source_cluster: cluster01 + exp_annotations: + summary: 'Etcd high number of leader changes' + description: 'Etcd leader changed more than 2 times during 10 minutes in cluster cluster01.' + alert_routing_key: spreandinfra + team: o11y + slo: 'false' + + - interval: 1m + input_series: + # Counter is stable — no leader changes, no alert. + - series: 'etcd_server_leader_changes_seen_total{namespace="openshift-etcd",source_cluster="cluster01"}' values: '1+0x60' alert_rule_test: diff --git a/test/promql/tests/data_plane/cluster_upgrade_ongoing_alert_test.yaml b/test/promql/tests/data_plane/cluster_upgrade_ongoing_alert_test.yaml index ff1060c76..1b3769609 100644 --- a/test/promql/tests/data_plane/cluster_upgrade_ongoing_alert_test.yaml +++ b/test/promql/tests/data_plane/cluster_upgrade_ongoing_alert_test.yaml @@ -22,6 +22,7 @@ tests: cluster_uuid: "uuid-1" cluster_name: "prod-cluster" current_version: "4.14.1" + source_cluster: "prod-cluster" alertname: ClusterUpgradeOngoing exp_annotations: summary: "Cluster prod-cluster is currently undergoing an upgrade." diff --git a/test/promql/tests/recording/cluster_upgrade_recording_rules_test.yaml b/test/promql/tests/recording/cluster_upgrade_recording_rules_test.yaml new file mode 100644 index 000000000..2e8b623d1 --- /dev/null +++ b/test/promql/tests/recording/cluster_upgrade_recording_rules_test.yaml @@ -0,0 +1,29 @@ +evaluation_interval: 1m + +rule_files: + - cluster_upgrade_recording_rules.yaml + +tests: + - name: ClusterUpgradeOngoingSignalFiring + interval: 1m + input_series: + - series: 'ALERTS{alertname="ClusterUpgradeOngoing",alertstate="firing",source_cluster="cluster01",severity="info"}' + values: '1+0x10' + + promql_expr_test: + - expr: cluster_upgrade_ongoing + eval_time: 5m + exp_samples: + - labels: '{__name__="cluster_upgrade_ongoing", source_cluster="cluster01"}' + value: 1 + + - name: ClusterUpgradeOngoingSignalPendingNotCaptured + interval: 1m + input_series: + - series: 'ALERTS{alertname="ClusterUpgradeOngoing",alertstate="pending",source_cluster="cluster01",severity="info"}' + values: '1+0x10' + + promql_expr_test: + - expr: cluster_upgrade_ongoing + eval_time: 5m + exp_samples: []