fix(alert-tuning): [SPRE-5878] suppress EtcdHighNumberOfLeaderChanges during cluster upgrades

[ay-garg]

Summary

Resolves SPRE-5878 — Alert Review & Tuning: EtcdHighNumberOfLeaderChanges

Suppresses the alert per-cluster during active OCP upgrades using a recording
rule as the inhibition signal, while preserving full sensitivity (> 2)
during normal operation.

---

EtcdHighNumberOfLeaderChanges generated ~109 false positive firings across
two consecutive 14-day investigation windows with 0% actionable signal and
zero associated PagerDuty incidents or user-impacting events.

Every firing occurred strictly within scheduled weekly OCP upgrade windows.
During a rolling upgrade, the MachineConfigOperator drains and reboots control
plane nodes one at a time — each restart forces a Raft leader re-election. A
standard 3-node etcd cluster produces 2–3 elections per upgrade (~70-minute
window), mathematically guaranteeing a crossing of the > 2 threshold on every
upgrade cycle.

---

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 leader 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 the Prometheus
unless operator with a dedicated recording rule as the inhibition signal:

(increase(etcd_server_leader_changes_seen_total{namespace="openshift-etcd"}[10m]) > 2)
unless on (source_cluster)
cluster_upgrade_ongoing

Why a recording rule instead of ALERTS{} directly

The unless clause references cluster_upgrade_ongoing (a recording rule)
rather than ALERTS{alertname="ClusterUpgradeOngoing"} directly, for two
reasons:

1. alertstate ambiguity — 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 within its for: 1m window.
2. Stability — ALERTS{} is an internal Prometheus construct. A named
recording rule produces a stable, well-defined, reusable time series that
other rules can consume in the future.

Label alignment

cluster_name on aus_cluster_upgrade_policy_info maps 1:1 to
source_cluster on etcd_server_leader_changes_seen_total across all RHTAP
clusters (verified in production RHOBS — short-name format, e.g.
kflux-prd-rh03). A label_replace() on the ClusterUpgradeOngoing alert
expression copies cluster_name → source_cluster, enabling the per-cluster
join.

---
Files changed

┌─────────────────────────────────────────────────────────────────────────┬──────────────────────────────────────────────────────┐
│                                  File                                   │                        Change                        │
├─────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│                                                                         │ New — recording rule that materialises               │
│ rhobs/recording/cluster_upgrade_recording_rules.yaml                    │ cluster_upgrade_ongoing{source_cluster}=1 only when  │
│                                                                         │ ClusterUpgradeOngoing is alertstate="firing"         │
├─────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│                                                                         │ Wrapped expression in label_replace() to expose      │
│ rhobs/alerting/data_plane/prometheus.cluster_ongoing_updrade_alert.yaml │ source_cluster from cluster_name, enabling           │
│                                                                         │ per-cluster join via unless on (source_cluster)      │
├─────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│                                                                         │ Added unless on (source_cluster)                     │
│ rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml       │ cluster_upgrade_ongoing inhibition; threshold        │
│                                                                         │ remains > 2                                          │
├─────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│ test/promql/tests/data_plane/cluster_upgrade_ongoing_alert_test.yaml    │ Added source_cluster label assertion (now present    │
│                                                                         │ due to label_replace)                                │
├─────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│                                                                         │ Fixed pre-existing vacuous test (missing namespace   │
│ test/promql/tests/data_plane/cluster_capacity_test.yaml                 │ label); added suppression, per-cluster specificity,  │
│                                                                         │ and stable-counter test cases                        │
├─────────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────┤
│                                                                         │ New — unit tests for recording rule:                 │
│ test/promql/tests/recording/cluster_upgrade_recording_rules_test.yaml   │ alertstate="firing" → signal=1; alertstate="pending" │
│                                                                         │  → no output                                         │
└─────────────────────────────────────────────────────────────────────────┴──────────────────────────────────────────────────────┘

---
Test results

promtool test rules:
  cluster_capacity_test.yaml                  SUCCESS
  cluster_upgrade_ongoing_alert_test.yaml     SUCCESS
  cluster_upgrade_recording_rules_test.yaml   SUCCESS

Validated scenarios:
- Leader changes > 2, no upgrade in progress → alert fires ✅
- Leader changes > 2, upgrade ongoing on same cluster → suppressed ✅
- Leader changes > 2, upgrade ongoing on different cluster → alert fires ✅
- Counter stable (no leader changes) → no alert ✅

---

[SPRE-5878]: https://redhat.atlassian.net/browse/SPRE-5878?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

[qodo-for-redhat-appstudio]

PR Summary by Qodo

Tune EtcdHighNumberOfLeaderChanges alert threshold (>2 → >4)

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

AI Description

• Raise EtcdHighNumberOfLeaderChanges firing threshold to avoid upgrade-driven false positives.
• Update alert annotation text to match the new threshold.
• Extend PromQL unit tests to cover both firing and non-firing scenarios under the new threshold.

Diagram

graph TD
  A[/"rhobs/alerting/..._alerts.yaml"/] --> B["EtcdHigh... rule"] --> C(["Prometheus eval"]) --> D(["Alertmanager route"]) --> E["Slack"]
  F[/"test/promql/..._test.yaml"/] --> G["PromQL unit tests"] --> B
  subgraph Legend
    direction LR
    _file[/"File"/] ~~~ _logic["Rule/Test"] ~~~ _svc(["Service"])
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Inhibit alert during upgrades

• ➕ Targets the known false-positive window (control-plane upgrades) without changing steady-state sensitivity
• ➕ Keeps low thresholds for truly abnormal leader churn outside upgrades
• ➖ Requires a reliable upgrade/progress signal (ClusterVersion/MCO) and adds rule complexity
• ➖ Risk of masking real etcd instability if it occurs during an upgrade

2. Change the query/window instead of the threshold

• ➕ A longer lookback or different aggregation could reduce sensitivity to expected short bursts
• ➕ May better align with 'actionable' leader churn patterns
• ➖ Harder to reason about and tune than a simple threshold change
• ➖ May delay detection for real high-frequency leader flapping

Recommendation: Raising the threshold is the most pragmatic fix for deterministic, upgrade-driven false positives and keeps the rule simple. Given the PR’s added test coverage (including a case that would have fired under the old threshold but not under the new one), this approach is low-risk. If noise persists or upgrade detection becomes important, consider adding an explicit upgrade-time inhibition as a follow-up.

Files changed (2) +15 / -5

Bug fix (1) +2 / -2

prometheus.cluster_capacity_alerts.yamlRaise EtcdHighNumberOfLeaderChanges threshold and annotation text +2/-2

Raise EtcdHighNumberOfLeaderChanges threshold and annotation text

• Updates the alert expression threshold from >2 to >4 leader changes over 10 minutes. Aligns the human-readable description annotation with the new threshold.

rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml

Tests (1) +13 / -3

cluster_capacity_test.yamlUpdate and extend PromQL tests for the new leader-change threshold +13/-3

Update and extend PromQL tests for the new leader-change threshold

• Adjusts expected alert annotations to reflect the >4 threshold. Adds a new non-firing test case where the increase is ~3 in 10 minutes (previously would have alerted). Clarifies test comments to match the new intent.

test/promql/tests/data_plane/cluster_capacity_test.yaml

[qodo-for-redhat-appstudio]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Misleading stable etcd test ✓ Resolved 🐞 Bug ⚙ Maintainability

Description

The newly added “Counter is stable” test block uses an input series without
namespace="openshift-etcd", so it can never match the alert expression and therefore doesn’t
actually test the no-alert behavior it claims. This reduces regression coverage for future changes
to EtcdHighNumberOfLeaderChanges by creating a false sense that a stable-case is validated.

Code

test/promql/tests/data_plane/cluster_capacity_test.yaml[175]

+      # Counter is stable — no leader changes, no alert.

Relevance

●●● Strong

Team often fixes promql tests that don’t match rule semantics/labels; misleading or ineffective
tests get corrected.

PR-#999
PR-#1257
PR-#1201

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The alert rule explicitly filters the metric by namespace="openshift-etcd", but the “stable
counter” test case uses etcd_server_leader_changes_seen_total{source_cluster="cluster01"} without
the namespace label. As a result, that test’s series cannot be selected by the alert expression and
cannot validate the intended behavior.

rhobs/alerting/data_plane/prometheus.cluster_capacity_alerts.yaml[80-83]
test/promql/tests/data_plane/cluster_capacity_test.yaml[173-182]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The promtool unit test case commented as “Counter is stable — no leader changes, no alert.” uses a time series that lacks `namespace="openshift-etcd"`. The alert expression filters on `namespace="openshift-etcd"`, so this series is filtered out and the test does not exercise the rule.

### Issue Context
The alert `EtcdHighNumberOfLeaderChanges` is defined as:
`increase(etcd_server_leader_changes_seen_total{namespace="openshift-etcd"}[10m]) > 4`

### Fix
Update the stable-counter test input series to include `namespace="openshift-etcd"` (and keep `source_cluster`) so it truly validates that a stable counter does not fire the alert.

### Fix Focus Areas
- test/promql/tests/data_plane/cluster_capacity_test.yaml[173-182]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[gcpsoares]

Check if we can inhibit the alert during ongoing upgrades.