fix: [alert] OpenBaoReplicaNotReady (openbao) (#808) #811

Closed
issue-fix-bot wants to merge 1 commits from alert-fix/issue-808 into main
Member

What this PR does

On 2026-09-07 all three OpenBao replicas were sealed at once during a rolling
reboot of all three cluster nodes, raft lost quorum for ~14 minutes, the
openbao ClusterSecretStore went InvalidProviderConfig, and 18
ExternalSecrets plus 24 Flux kustomizations stalled behind it. The underlying
cause — shamir seal, no auto-unseal, so every restart leaves a replica sealed
until a human unseals it — is a recorded, deliberate design decision in this
repo (kubernetes/MIGRATION-sops-openbao.md:59: "auto-unseal needs a second
trust root (transit/KMS) and is explicitly out of scope"
), and reversing it is
not something an automated fix agent should do. So this PR does not touch that.

What it does fix is a separate, verifiable defect the issue did not find: the
two OpenBao alerts fired ~14 min and ~6.5 min later than their for: windows
specify
, because their expressions were bare kube_statefulset_status_replicas_ready{...}
selectors. kube-state-metrics stamps instance/pod/endpoint on every series
it exports, Prometheus tracks the for: timer per output series, and
kube-state-metrics itself was rescheduled twice during this incident (it lives on
the same three nodes that were rebooting) — each move produced a new series and
reset the pending alert to inactive. Wrapping both expressions in
max by (namespace, statefulset) (...) gives a single series whose identity does
not depend on which kube-state-metrics pod is currently scraped, so the timer
runs continuously. This makes the alerts fire sooner and more reliably — it
is the opposite of silencing.

Changes

  • kubernetes/apps/monitoring-rules/prometheusrule-openbao.yaml — wrap the
    OpenBaoReplicaNotReady (< 3) and OpenBaoQuorumLost (< 2) expressions in
    max by (namespace, statefulset) (...) so the alert series survives a
    kube-state-metrics restart, plus a comment block recording why. Thresholds,
    for: windows, severities and annotations are unchanged.
    OpenBaoMetricsMissing is left alone — absent() behaves identically either
    way, and it is the guard for the other two.

Verification

Everything below was observed this run via ./.ci-diag/kubectl.

Current state is healthy (the incident is over):

  • kubectl get pods -n openbao --show-labels → openbao-0/1/2 all 1/1 Running,
    openbao-sealed=false, openbao-initialized=true, openbao-active=true on
    openbao-1. Ages 170m/136m/136m.
  • kubectl get clustersecretstore openbao -o jsonpath='{.status.conditions}' →
    {"lastTransitionTime":"2026-09-07T05:32:57Z","message":"store validated","reason":"Valid","status":"True","type":"Ready"}.

The late-firing defect (the thing this PR fixes) — a query_range on
kube_statefulset_status_replicas_ready{namespace="openbao",statefulset="openbao"}
over 04:30–05:50 UTC returned three separate series, one per
kube-state-metrics pod:

kube-state-metrics pod (instance) covers
...-74b9f67964-d7dct (10.244.2.181) → 05:00
...-7d8cbb584f-hzmx6 (10.244.2.218) 05:03 – 05:06
...-7d8cbb584f-87lln (10.244.4.35) 05:06 → now

The same range aggregated (max(...), step 60s) is continuous and shows the
real transitions: 3 → 2 at 04:53, 2 → 1 at 05:00, 1 → 0 at 05:08.
Against those: OpenBaoReplicaNotReady (for: 10m, <3) should have fired
~05:03 but the issue's ALERTS query has it at 05:17:37; OpenBaoQuorumLost
(for: 5m, <2) should have fired ~05:05, actual 05:11:37. A
count(...)-over-time check at 30s step shows the series count never dropped to
0 (it briefly reads 2 during the 05:06–05:07:30 handoff), so the aggregated
series would have been gap-free — i.e. the fix genuinely would have held the
timer.

The new expression was executed against live Prometheus before committing:
max by (namespace, statefulset) (kube_statefulset_status_replicas_ready{namespace="openbao", statefulset="openbao"})
→ exactly one sample, {"namespace":"openbao","statefulset":"openbao"} = 3; with
< 3 appended → empty vector (correctly not firing right now).

Reviewer check after Flux reconciles: the rules should load without a
PrometheusRuleFailures bump; Prometheus /rules should show the two
expressions in aggregated form. Next time a replica is sealed, the warning
should arrive 10m after the drop rather than drifting, and the alert labels will
carry only namespace/statefulset (the job=kube-state-metrics,
instance, pod labels are intentionally gone). I checked the Alertmanager
route in kubernetes/apps/monitoring/helmrelease-prometheus.yaml:287-316 —
its matchers are on alertname and source only, so dropping those labels does
not change routing.

Notes

Two things the issue got wrong, both checked with a command:

  1. The trigger was not a HelmRelease reconcile. The issue states a
    HelmRelease reconcile landed at ~04:52 and produced a new StatefulSet
    revision. kubectl get helmrelease openbao -n openbao -o jsonpath='{.status.history}'
    shows the current release (version 5, chart 0.28.6, app v2.6.1) was
    lastDeployed 2026-08-26T20:51:18Z — twelve days earlier. Nothing helm
    did today touched openbao. The actual trigger was a rolling reboot of all
    three nodes
    : kubectl get nodes Ready lastTransitionTime is
    k8s-1 04:56:00, k8s-2 05:00:37, k8s-3 05:06:15 — ~4–5 minutes apart, not
    just k8s-3 as the issue says. Because the StatefulSet is OnDelete, the
    pending Aug-26 revision (openbao-b8c9c77d) simply rode along as each pod was
    recreated; it did not cause the roll.

  2. A PDB does exist, and it is not irrelevant. The issue says "a
    PodDisruptionBudget does not help here". The chart already creates one —
    kube-state-metrics reports poddisruptionbudget="openbao" with
    desired_healthy=2, expected_pods=3 (i.e. maxUnavailable: 1), currently
    current_healthy=3. That PDB would have blocked the second and third node
    drains while openbao-0 was sealed and not-Ready, which is exactly the guard
    the incident needed. That it did not block means the reboots did not go
    through the eviction API (--force / --disable-eviction, or a plain VM
    reboot). That is an operator-tooling matter, outside this repo — the PDB
    itself needs no change. (I could not get pdb directly: the CI diagnostics
    ServiceAccount is Forbidden on policy resources, so this is from
    kube-state-metrics via the Prometheus proxy.)

  3. The issue's KubeStatefulSetUpdateNotRolledOut side note checks out —
    updateStrategy is {"type":"OnDelete"}, currentRevision=openbao-69c46558cc
    vs updateRevision=openbao-b8c9c77d with updatedReplicas=3/3. No action, as
    the issue concluded.

What is still not fixed, and needs a human. This PR improves detection
latency; it does not remove the manual unseal. The recurrence guard is
procedural, and stays procedural:

  1. Before rolling node maintenance (talosctl upgrade, VM reboots), take a raft
    snapshot and have pass show homelab/openbao/unseal-key to hand.
  2. Reboot one node. Wait for its openbao replica to come back and unseal it:
    kubectl -n openbao exec -it openbao-<N> -- bao operator unseal (same key for
    all three).
  3. Confirm kubectl get pods -n openbao shows 3/3 Ready — then move to the
    next node. Do not reboot the second node on a 4-minute cadence as happened
    here.
  4. Use the eviction API (do not pass --force/--disable-eviction); the
    existing maxUnavailable: 1 PDB will then refuse the second drain for you.

The one repo change that would remove this class of incident entirely is
auto-unseal (transit or KMS seal in server.ha.raft.config). I deliberately did
not do it: it adds an external trust root and a new dependency, it is a
larger-than-surgical change, and the repo has already recorded the decision to
accept manual unseal instead (kubernetes/MIGRATION-sops-openbao.md:53-61,
kubernetes/infrastructure/openbao/helmrelease.yaml:56-65). Reversing a
documented architectural decision is an owner call, not an alert-fix call. If
the owner does want it, install.disableWait / upgrade.disableWait
(helmrelease.yaml:27-38) should be revisited in the same change, as the issue
correctly notes.

Nothing in the issue asked me to touch CI config, credentials, or anything
outside the edit scope.

Closes #808

## What this PR does On 2026-09-07 all three OpenBao replicas were sealed at once during a rolling reboot of all three cluster nodes, raft lost quorum for ~14 minutes, the `openbao` ClusterSecretStore went `InvalidProviderConfig`, and 18 ExternalSecrets plus 24 Flux kustomizations stalled behind it. The underlying cause — shamir seal, no auto-unseal, so every restart leaves a replica sealed until a human unseals it — is a **recorded, deliberate design decision** in this repo (`kubernetes/MIGRATION-sops-openbao.md:59`: *"auto-unseal needs a second trust root (transit/KMS) and is explicitly out of scope"*), and reversing it is not something an automated fix agent should do. So this PR does not touch that. What it does fix is a separate, verifiable defect the issue did not find: the two OpenBao alerts **fired ~14 min and ~6.5 min later than their `for:` windows specify**, because their expressions were bare `kube_statefulset_status_replicas_ready{...}` selectors. kube-state-metrics stamps `instance`/`pod`/`endpoint` on every series it exports, Prometheus tracks the `for:` timer per output series, and kube-state-metrics itself was rescheduled twice during this incident (it lives on the same three nodes that were rebooting) — each move produced a new series and reset the pending alert to inactive. Wrapping both expressions in `max by (namespace, statefulset) (...)` gives a single series whose identity does not depend on which kube-state-metrics pod is currently scraped, so the timer runs continuously. This makes the alerts fire **sooner and more reliably** — it is the opposite of silencing. ## Changes - `kubernetes/apps/monitoring-rules/prometheusrule-openbao.yaml` — wrap the `OpenBaoReplicaNotReady` (`< 3`) and `OpenBaoQuorumLost` (`< 2`) expressions in `max by (namespace, statefulset) (...)` so the alert series survives a kube-state-metrics restart, plus a comment block recording why. Thresholds, `for:` windows, severities and annotations are unchanged. `OpenBaoMetricsMissing` is left alone — `absent()` behaves identically either way, and it is the guard for the other two. ## Verification Everything below was observed this run via `./.ci-diag/kubectl`. **Current state is healthy (the incident is over):** - `kubectl get pods -n openbao --show-labels` → `openbao-0/1/2` all `1/1 Running`, `openbao-sealed=false`, `openbao-initialized=true`, `openbao-active=true` on `openbao-1`. Ages 170m/136m/136m. - `kubectl get clustersecretstore openbao -o jsonpath='{.status.conditions}'` → `{"lastTransitionTime":"2026-09-07T05:32:57Z","message":"store validated","reason":"Valid","status":"True","type":"Ready"}`. **The late-firing defect (the thing this PR fixes)** — a `query_range` on `kube_statefulset_status_replicas_ready{namespace="openbao",statefulset="openbao"}` over 04:30–05:50 UTC returned **three separate series**, one per kube-state-metrics pod: | kube-state-metrics pod (`instance`) | covers | |---|---| | `...-74b9f67964-d7dct` (`10.244.2.181`) | → 05:00 | | `...-7d8cbb584f-hzmx6` (`10.244.2.218`) | 05:03 – 05:06 | | `...-7d8cbb584f-87lln` (`10.244.4.35`) | 05:06 → now | The same range aggregated (`max(...)`, step 60s) is continuous and shows the real transitions: **3 → 2 at 04:53**, **2 → 1 at 05:00**, **1 → 0 at 05:08**. Against those: `OpenBaoReplicaNotReady` (`for: 10m`, `<3`) should have fired ~05:03 but the issue's ALERTS query has it at 05:17:37; `OpenBaoQuorumLost` (`for: 5m`, `<2`) should have fired ~05:05, actual 05:11:37. A `count(...)`-over-time check at 30s step shows the series count never dropped to 0 (it briefly reads 2 during the 05:06–05:07:30 handoff), so the aggregated series would have been **gap-free** — i.e. the fix genuinely would have held the timer. **The new expression was executed against live Prometheus before committing:** `max by (namespace, statefulset) (kube_statefulset_status_replicas_ready{namespace="openbao", statefulset="openbao"})` → exactly one sample, `{"namespace":"openbao","statefulset":"openbao"} = 3`; with `< 3` appended → empty vector (correctly not firing right now). **Reviewer check after Flux reconciles:** the rules should load without a `PrometheusRuleFailures` bump; Prometheus `/rules` should show the two expressions in aggregated form. Next time a replica is sealed, the warning should arrive 10m after the drop rather than drifting, and the alert labels will carry only `namespace`/`statefulset` (the `job=kube-state-metrics`, `instance`, `pod` labels are intentionally gone). I checked the Alertmanager route in `kubernetes/apps/monitoring/helmrelease-prometheus.yaml:287-316` — its matchers are on `alertname` and `source` only, so dropping those labels does not change routing. ## Notes **Two things the issue got wrong, both checked with a command:** 1. **The trigger was not a HelmRelease reconcile.** The issue states a HelmRelease reconcile landed at ~04:52 and produced a new StatefulSet revision. `kubectl get helmrelease openbao -n openbao -o jsonpath='{.status.history}'` shows the current release (version 5, chart `0.28.6`, app `v2.6.1`) was `lastDeployed` **2026-08-26T20:51:18Z** — twelve days earlier. Nothing helm did today touched openbao. The actual trigger was a **rolling reboot of all three nodes**: `kubectl get nodes` Ready `lastTransitionTime` is `k8s-1 04:56:00`, `k8s-2 05:00:37`, `k8s-3 05:06:15` — ~4–5 minutes apart, not just `k8s-3` as the issue says. Because the StatefulSet is `OnDelete`, the pending Aug-26 revision (`openbao-b8c9c77d`) simply rode along as each pod was recreated; it did not cause the roll. 2. **A PDB does exist, and it is not irrelevant.** The issue says "a PodDisruptionBudget does not help here". The chart already creates one — kube-state-metrics reports `poddisruptionbudget="openbao"` with `desired_healthy=2`, `expected_pods=3` (i.e. `maxUnavailable: 1`), currently `current_healthy=3`. That PDB *would* have blocked the second and third node drains while `openbao-0` was sealed and not-Ready, which is exactly the guard the incident needed. That it did not block means the reboots did not go through the eviction API (`--force` / `--disable-eviction`, or a plain VM reboot). That is an operator-tooling matter, outside this repo — the PDB itself needs no change. *(I could not `get pdb` directly: the CI diagnostics ServiceAccount is Forbidden on `policy` resources, so this is from kube-state-metrics via the Prometheus proxy.)* 3. The issue's `KubeStatefulSetUpdateNotRolledOut` side note checks out — `updateStrategy` is `{"type":"OnDelete"}`, `currentRevision=openbao-69c46558cc` vs `updateRevision=openbao-b8c9c77d` with `updatedReplicas=3/3`. No action, as the issue concluded. **What is still not fixed, and needs a human.** This PR improves detection latency; it does not remove the manual unseal. The recurrence guard is procedural, and stays procedural: 1. Before rolling node maintenance (`talosctl upgrade`, VM reboots), take a raft snapshot and have `pass show homelab/openbao/unseal-key` to hand. 2. Reboot **one** node. Wait for its openbao replica to come back and unseal it: `kubectl -n openbao exec -it openbao-<N> -- bao operator unseal` (same key for all three). 3. Confirm `kubectl get pods -n openbao` shows 3/3 Ready — *then* move to the next node. Do not reboot the second node on a 4-minute cadence as happened here. 4. Use the eviction API (do not pass `--force`/`--disable-eviction`); the existing `maxUnavailable: 1` PDB will then refuse the second drain for you. The one repo change that would remove this class of incident entirely is auto-unseal (transit or KMS seal in `server.ha.raft.config`). I deliberately did **not** do it: it adds an external trust root and a new dependency, it is a larger-than-surgical change, and the repo has already recorded the decision to accept manual unseal instead (`kubernetes/MIGRATION-sops-openbao.md:53-61`, `kubernetes/infrastructure/openbao/helmrelease.yaml:56-65`). Reversing a documented architectural decision is an owner call, not an alert-fix call. If the owner does want it, `install.disableWait` / `upgrade.disableWait` (`helmrelease.yaml:27-38`) should be revisited in the same change, as the issue correctly notes. Nothing in the issue asked me to touch CI config, credentials, or anything outside the edit scope. Closes #808
issue-fix-bot added 1 commit 2026-09-07 07:48:25 +00:00
fix: [alert] OpenBaoReplicaNotReady (openbao) (#808)
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
7c00ecf43c
review-bot left a comment
Member

🤖 Automated review — model claude-haiku-4-5-20251001 (complexity: trivial), commit 7c00ecf43c315a79aa476afa502cdb8d2b0a2b59.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

This PR fixes a subtle timing bug in OpenBao alerting where alerts were firing 6–14 minutes later than their for: windows specified. The root cause: bare metric selectors produce different series each time kube-state-metrics is rescheduled (due to instance/pod labels), and Prometheus tracks the for: timer per series, so rescheduling resets pending alerts. The fix wraps both alert expressions in max by (namespace, statefulset) (...) to produce a single stable series. The change is well-motivated, properly tested against live Prometheus, and does not affect downstream alert routing.

🟢 Nits

  1. Comment length (lines 31–43): The explanation is lengthy but justified—it documents a non-obvious Prometheus behavior that caused a real incident. Worth keeping as written, since the subtlety deserves documentation that future maintainers will actually read.

  2. Series aggregation labels: The aggregation removes instance, pod, and endpoint labels from the alert output. The PR body verifies this won't affect Alertmanager routing (which only matches on alertname and source), and checking kubernetes/apps/monitoring/helmrelease-prometheus.yaml:287–316 confirms this is correct. ✓

Tests

The repository has no automated prometheus-rule validation in CI (no promtool or similar lint step detected). The PR body documents manual verification against live Prometheus:

  • Tested the new expression directly: max by (namespace, statefulset) (kube_statefulset_status_replicas_ready{...}) returned exactly one series with value 3.
  • Validated that the aggregated series would have been gap-free during the incident (no drops to 0 series).

After merge: Flux will deploy the PrometheusRule. Verify:

  1. No PrometheusRuleFailures appear in the cluster.
  2. Prometheus /rules UI shows the two expressions in aggregated form.
  3. Alertmanager alert instances have only namespace and statefulset labels (no instance, pod, endpoint).

Recommendation

Looks reasonable — no blocking issues. The fix is sound, well-tested, and the comment block is appropriate for the subtlety being addressed.

🤖 **Automated review** — model `claude-haiku-4-5-20251001` (complexity: trivial), commit `7c00ecf43c315a79aa476afa502cdb8d2b0a2b59`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR fixes a subtle timing bug in OpenBao alerting where alerts were firing 6–14 minutes later than their `for:` windows specified. The root cause: bare metric selectors produce different series each time kube-state-metrics is rescheduled (due to instance/pod labels), and Prometheus tracks the `for:` timer per series, so rescheduling resets pending alerts. The fix wraps both alert expressions in `max by (namespace, statefulset) (...)` to produce a single stable series. The change is well-motivated, properly tested against live Prometheus, and does not affect downstream alert routing. ## 🟢 Nits 1. **Comment length** (lines 31–43): The explanation is lengthy but justified—it documents a non-obvious Prometheus behavior that caused a real incident. Worth keeping as written, since the subtlety deserves documentation that future maintainers will actually read. 2. **Series aggregation labels**: The aggregation removes `instance`, `pod`, and `endpoint` labels from the alert output. The PR body verifies this won't affect Alertmanager routing (which only matches on `alertname` and `source`), and checking `kubernetes/apps/monitoring/helmrelease-prometheus.yaml:287–316` confirms this is correct. ✓ ## Tests The repository has no automated prometheus-rule validation in CI (no promtool or similar lint step detected). The PR body documents manual verification against live Prometheus: - Tested the new expression directly: `max by (namespace, statefulset) (kube_statefulset_status_replicas_ready{...})` returned exactly one series with value `3`. - Validated that the aggregated series would have been gap-free during the incident (no drops to 0 series). **After merge**: Flux will deploy the PrometheusRule. Verify: 1. No `PrometheusRuleFailures` appear in the cluster. 2. Prometheus `/rules` UI shows the two expressions in aggregated form. 3. Alertmanager alert instances have only `namespace` and `statefulset` labels (no `instance`, `pod`, `endpoint`). ## Recommendation Looks reasonable — no blocking issues. The fix is sound, well-tested, and the comment block is appropriate for the subtlety being addressed. <!-- pr-review:sha=7c00ecf43c315a79aa476afa502cdb8d2b0a2b59 -->
k.renner closed this pull request 2026-09-07 11:40:10 +00:00

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.