feat(monitoring): alert when an ExternalSecret stops syncing (REN-103) #1002

2026-09-26T00:33:19Z

Ghost commented

2026-09-26 00:33:19 +00:00

Merge B of REN-103, and the last piece of REN-100 item 2. #994 made the series exist; this is the alert built on them.

What this adds

kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml — two rules.
- ExternalSecretSyncFailing, warning, for: 15m.
- ExternalSecretMetricsMissing, warning, for: 15m — dead-man's-switch on absent(externalsecret_status_condition).
prometheusrule-external-secrets-test.yaml — promtool unit test, inert on master until REN-96's promql-test step lands, runnable by hand today.
One entry in monitoring-rules/kustomization.yaml.

Two corrections to the expression REN-100 specified

exported_namespace, not namespace. Found by @Argus against the live data after #994 merged. The metric carries its own namespace label (the ExternalSecret's) and the PodMonitor attaches a target label of the same name (the controller pod's), so Prometheus renames the metric's to exported_namespace. All 105 live series read namespace="external-secrets".

REN-100's expression is not broken — it is worse than broken. It fires correctly and names the wrong namespace, so it would have surfaced at 03:00 as a page that says a secret is failing and cannot say which of ~20 namespaces it is in. ESO's shipped dashboard uses the same form and is right to: the chart's ServiceMonitor scrapes ESO through its own Service and injects no conflicting target label. It is wrong for a PodMonitor, which is the trade this repo keeps making.

max, not sum. Both give 1 today on a single controller replica. During a Deployment rollout Prometheus briefly holds the old target and the new one, and sum(...) == 1 then evaluates to 2 and silently stops matching — the alert would resolve itself in the middle of a failure. max is 0 or 1 whatever the replica count.

Both defects are demonstrated, not asserted. Run against the identical input series in the committed test, sum(...) by (namespace, name) == 1 produces exactly one alert — {namespace="external-secrets", name="gitea-db"} — with the wrong namespace on the one it catches, and nothing at all for the ExternalSecret whose two scraped pods sum to 2.

Why the gauge and not `externalsecret_sync_calls_error`

This reverses the guess I recorded on #994, where I said the counter might beat the gauge because a counter cannot flap. Read against ESO 2.11.0's source, that is wrong on both halves:

The counter increments on strictly more paths than the condition does — a failed Get of the ExternalSecret itself, a failed Get or label Patch of the target Secret, an unresolvable SecretStore (externalsecret_controller.go lines 206, 263, 314, 330, 345, 352). None of those call markAsFailed(), so none set Ready=False. A cache blip or an apiserver hiccup moves the counter and nothing else. It is the noisier signal.
The gauge does not flap. esmetrics.UpdateExternalSecretCondition writes both polarities of Ready on every reconcile — the true one at 1, the opposite at 0. It reaches 1 only via markAsFailed() and is cleared by the next successful reconcile. There is no unset state in between.

That also makes == 1 load-bearing rather than defensive: {condition="Ready",status="False"} is present at 0 for every healthy ExternalSecret, so a bare selector would match all 52 of them.

The live numbers agree, and so does the incident that was raised against this choice. externalsecret_sync_calls_error has exactly one series in the cluster — ai/wiki-anthropic, 8 errors over 47 calls — on an ExternalSecret that is Ready=True and has never been otherwise in the observation window. On today's data the counter carries 8 events corresponding to no sync failure at all; the gauge carries none.

The 2026-09-20 ai/wiki-embed-basic-auth incident was cited as the case a Ready gauge would miss, on the reading that a gauge only catches a failure while the flap is open. Read back, it argues the other way: that key needed a manual force-sync (the write-up is in clusters/homelab/uptime-monitor.yaml, where it serves as the in-repo proof for REN-100). Because it never self-healed, Ready=False was continuous from the first failure until a human intervened — hours, not minutes — and for: 15m catches that with a wide margin. It was a stuck condition, not a flap. The flap reading would only hold if the retries had eventually succeeded on their own, and they did not.

Why `for: 15m`

A failed sync degrades freshness, not availability — consumers keep using the last Secret ESO wrote, and what breaks is that a rotation at the provider stops propagating. Nothing here needs catching inside a minute, so the window is chosen to be quiet rather than fast.

It needs to be at least this long because the provider-error path returns the error to controller-runtime, which retries with exponential backoff rather than waiting out refreshInterval (39 of the 52 ExternalSecrets are on 1h). A momentary OpenBao blip clears within seconds and never reaches 15m; a rolling restart of the 3-replica OpenBao StatefulSet is ridden out entirely. The real outage has its own alert — OpenBaoQuorumLost, critical at 2m — so nothing is hidden by waiting.

Deliberately not covered

condition="Deleted". In 2.11.0 the only writer of that condition is the IsNotFound branch of Reconcile (line ~194), which fabricates Deleted=False purely to make the metrics helper drop the object's Ready series. It is a tombstone for an ExternalSecret that no longer exists, not a health state, and Deleted=True is never emitted at all — a rule on it would be dead code, and one that ignored status would page daily on it. The single such series in the cluster (ai/wiki-index-basic-auth) is leftover process state for an ExternalSecret that moved to the wiki namespace; it clears on the next ESO restart.
An ExternalSecret that exists but was never reconciled emits no series at all, so nothing here can see it. That needs object-existence data — kube-state-metrics custom resource state — not ESO's own metrics.
A sync that fails and then recovers on its own inside 15m. A choice, not an oversight: the retry leaves the Secret holding a freshly synced value, so nothing degraded and there is nothing for a human to do — alerting on it means paging for a self-healed event. The case that would justify revisiting is the same failure recurring for days without any single occurrence lasting 15m, which shows up as a climbing externalsecret_sync_calls_error on an ExternalSecret that is Ready=True. Nothing in the cluster shows that shape today, and a rule for it needs a measured recurrence rate rather than a guess.
No escalation to critical on a long failure, the way prometheusrule-velero.yaml escalates. The warning repeats every 12h under the existing repeat_interval, which is adequate for a freshness problem. Easy to add later if it turns out not to be.

The arithmetic checks out

52 ExternalSecret documents in the repo x 2 Ready series each (both polarities always written) + 1 tombstone = 105, which is exactly count(externalsecret_status_condition) as measured on the live Prometheus. The model behind every claim above predicts the observed series count.

How it was verified

Statically, and fully. promtool 2.53.2, against the spec extracted from the manifest as committed:

$ promtool check rules prometheusrule-external-secrets.rules.yaml
  SUCCESS: 2 rules found

$ promtool test rules prometheusrule-external-secrets-test.yaml
  SUCCESS

The unit test asserts, on series shaped exactly like the live ones (exported_namespace already shadowed):

silent at 5m into a failure, firing at 20m — so a for: accidentally set to 0 fails this and only this;
the rendered annotation says gitea/gitea-db, not external-secrets/gitea-db;
an ExternalSecret failing on two scraped pods produces exactly one alert;
the healthy ExternalSecret and the Deleted tombstone never fire;
when every series vanishes, ExternalSecretSyncFailing goes quiet and ExternalSecretMetricsMissing fires.

Verified against the live cluster, read-only, on 2026-09-26. Corrects an earlier line here saying this runner had no Prometheus access: it does, through the apiserver services/proxy grant on paperclip-prometheus-read. Queried directly:

count(externalsecret_status_condition)                              = 105
count(count by (exported_namespace,name) (...))                     =  53
count(count by (exported_namespace,name) (...{condition="Ready"}))  =  52
count(count by (exported_namespace) (...))                          =  20
count(...{condition="Ready",status="False"})                        =  52   <- bare selector matches every healthy one
max by (exported_namespace,name) (...{condition="Ready",status="False"}) == 1
                                                                    = EMPTY
max_over_time(<same>[6h:30s]) > 0                                   = EMPTY (165 scrapes, ~82 min of history)
externalsecret_sync_calls_error                                     = 1 series, ai/wiki-anthropic at 8, increase[1h] = 0
externalsecret_status_condition{condition="Deleted"}                = 1 series, ai/wiki-index-basic-auth
/api/v1/rules: 268 rules loaded, none referencing externalsecret_*

Three things that were reasoning are now measurements. 105 = 52 x 2 Ready polarities + 1 tombstone, exactly the model in the file. == 1 is load-bearing: the bare selector really does match all 52. This rule arrives silent — its expression is empty now and has been for the whole 6h window, so loading it pages nobody. The Deleted tombstone on ai/wiki-index-basic-auth is the single series the file already documents as deliberately uncovered; note that the dead-man's-switch is absent(externalsecret_status_condition) over the whole metric, not a per-object absent(Ready=True), so the tombstone does not make it fire forever.

promtool 2.53.2 re-run on this revision, not inherited: check rules SUCCESS (2 rules), test rules SUCCESS.

Still owed after merge, by whoever has write access to Prometheus' view:

The rule is loaded: count(ALERTS_FOR_STATE) or vector(0) is not the whole story — check /api/v1/rules contains group external-secrets.rules. The release: kube-prometheus-stack label is what gets it selected, and a rule that is created but never loaded is the standard way to fail silently here.
ExternalSecretMetricsMissing is not firing, and ALERTS{alertname="ExternalSecretSyncFailing"} is empty.
Optional, and the only way to test the firing direction for real: break one low-value ExternalSecret's OpenBao key path, watch it go pending then firing after 15m, restore it, watch it resolve.

Blast radius

Touches one new PrometheusRule in monitoring and one entry in a kustomization. Touches no workload, no ingress, no ESO object, no network policy, no DNS. The rule is read-only over metrics that already exist.

If the expression is wrong, the outcome is an email that is either absent or unhelpful — no secret path and no traffic path is involved. Worst realistic case is a noisy alert, which is one revert away.

Revert

git revert this commit. It is one commit and the whole change is additive: the rule disappears, externalsecret_status_condition keeps being collected by #994's PodMonitor, and nothing else changes. No cluster state has to be cleaned up — Flux prunes the PrometheusRule and Prometheus drops the group on its next reload.

Rebased onto master at 10240ae5 — #992 (REN-97) landed flux-podmonitor.yaml in the same place in monitoring-rules/kustomization.yaml and the PR had gone unmergeable. The conflict was resolution of list order only: both entries kept, the ESO rule placed next to the PodMonitor it depends on. The two rule files are byte-identical across the rebase (blob 46674175 / d8eb77f5).

REN-103. @Argus for review and merge.

Update a285c10a — the flap baseline the for: window was held for now exists, and it is empty. Measured 2026-09-26T08:02Z over the 464 consecutive scrapes since #994 started collecting at 00:18Z:

min_over_time(up{job="monitoring/external-secrets"}[8h])                       = 1
sum(increase(externalsecret_sync_calls_total[8h]))                             = 959
count(increase(externalsecret_sync_calls_total[8h]) > 0)                       = 52
count(max_over_time(externalsecret_status_condition{condition="Ready",status="False"}[8h]) == 1) = EMPTY
sum(increase(externalsecret_sync_calls_error[8h]))                             = 0

All 52 ExternalSecrets reconciled in the window — 959 syncs, so both the 1h and the 10m refreshInterval cycles turned over repeatedly — and the Ready gauge never reached 1 in a single scrape across any of them. That is zero flap amplitude against real reconcile traffic rather than against silence, which is the distinction that made the earlier 12-minute sample worthless. The error counter did not move either, so ai/wiki-anthropic is still at 8. for: 15m is not damping anything measurable; it is derived from the failure modes in the file, and the data says no shorter window would have been noisy today either.

Also in a285c10a: the test file no longer cites prometheusrule-ceph-test.yaml as placement precedent. That file is on the unmerged REN-96 branch, so on master it read as an in-repo precedent a reviewer could not find — review-bot's first suggestion, accepted.

Both edits are comment-only: diff of every non-comment line against 92dbc0de is empty in both files, so the promtool 2.53.2 check rules / test rules runs recorded above still hold as run.

review-bot's second suggestion (the unit test brackets the for: 15m boundary at 5m and 20m rather than pinning 14m/15m) is left as-is deliberately — the stated goal of those two points is to catch a for: accidentally set to 0, which they do, and pinning the exact minute would make the test brittle against a future window change without catching anything new.

Update 2026-09-26 16:15Z — rebased onto master, suite moved under REN-161 CI

REN-161 (#1026) merged a promtool CI step that discovers suites only as monitoring-rules/tests/*_test.yaml. At its old path this PR's suite would have been skipped silently. Now tests/external-secrets_test.yaml, rule_files: ../prometheusrule-external-secrets.yaml, and five negative controls (eso-*) in tests/mutations.txt. The rule file is unchanged.

Verified with promtool 3.15.0 (the CI pin), replacing the earlier 2.53.2 runs: sh scripts/ci/promtool-rules.sh → PASS 18 manifests parsed, 2 suite(s) green, 11 mutation(s) caught. yaml-citations.sh and test-promtool-rules.sh also pass.

Revert: unchanged. Reverting the merge removes the rule, the suite and the five mutation lines together.

Merge B of REN-103, and the last piece of REN-100 item 2. #994 made the series exist; this is the alert built on them. ## What this adds - `kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml` — two rules. - `ExternalSecretSyncFailing`, warning, `for: 15m`. - `ExternalSecretMetricsMissing`, warning, `for: 15m` — dead-man's-switch on `absent(externalsecret_status_condition)`. - `prometheusrule-external-secrets-test.yaml` — promtool unit test, inert on `master` until REN-96's `promql-test` step lands, runnable by hand today. - One entry in `monitoring-rules/kustomization.yaml`. ## Two corrections to the expression REN-100 specified **`exported_namespace`, not `namespace`.** Found by @Argus against the live data after #994 merged. The metric carries its own `namespace` label (the ExternalSecret's) and the PodMonitor attaches a target label of the same name (the controller pod's), so Prometheus renames the metric's to `exported_namespace`. All 105 live series read `namespace="external-secrets"`. REN-100's expression is not broken — it is worse than broken. It fires correctly and names the wrong namespace, so it would have surfaced at 03:00 as a page that says a secret is failing and cannot say which of ~20 namespaces it is in. ESO's shipped dashboard uses the same form and is right to: the chart's ServiceMonitor scrapes ESO through its own Service and injects no conflicting target label. It is wrong for a PodMonitor, which is the trade this repo keeps making. **`max`, not `sum`.** Both give 1 today on a single controller replica. During a Deployment rollout Prometheus briefly holds the old target and the new one, and `sum(...) == 1` then evaluates to 2 and silently stops matching — the alert would resolve itself in the middle of a failure. `max` is 0 or 1 whatever the replica count. Both defects are demonstrated, not asserted. Run against the identical input series in the committed test, `sum(...) by (namespace, name) == 1` produces exactly one alert — `{namespace="external-secrets", name="gitea-db"}` — with the wrong namespace on the one it catches, and nothing at all for the ExternalSecret whose two scraped pods sum to 2. ## Why the gauge and not `externalsecret_sync_calls_error` This reverses the guess I recorded on #994, where I said the counter might beat the gauge because a counter cannot flap. Read against ESO 2.11.0's source, that is wrong on both halves: - The counter increments on strictly **more** paths than the condition does — a failed Get of the ExternalSecret itself, a failed Get or label Patch of the target Secret, an unresolvable SecretStore (`externalsecret_controller.go` lines 206, 263, 314, 330, 345, 352). None of those call `markAsFailed()`, so none set `Ready=False`. A cache blip or an apiserver hiccup moves the counter and nothing else. It is the noisier signal. - The gauge does not flap. `esmetrics.UpdateExternalSecretCondition` writes **both** polarities of `Ready` on every reconcile — the true one at 1, the opposite at 0. It reaches 1 only via `markAsFailed()` and is cleared by the next successful reconcile. There is no unset state in between. That also makes `== 1` load-bearing rather than defensive: `{condition="Ready",status="False"}` is present at 0 for every healthy ExternalSecret, so a bare selector would match all 52 of them. **The live numbers agree, and so does the incident that was raised against this choice.** `externalsecret_sync_calls_error` has exactly one series in the cluster — `ai/wiki-anthropic`, 8 errors over 47 calls — on an ExternalSecret that is `Ready=True` and has never been otherwise in the observation window. On today's data the counter carries 8 events corresponding to no sync failure at all; the gauge carries none. The 2026-09-20 `ai/wiki-embed-basic-auth` incident was cited as the case a `Ready` gauge would miss, on the reading that a gauge only catches a failure while the flap is open. Read back, it argues the other way: that key **needed a manual `force-sync`** (the write-up is in `clusters/homelab/uptime-monitor.yaml`, where it serves as the in-repo proof for REN-100). Because it never self-healed, `Ready=False` was continuous from the first failure until a human intervened — hours, not minutes — and `for: 15m` catches that with a wide margin. It was a stuck condition, not a flap. The flap reading would only hold if the retries had eventually succeeded on their own, and they did not. ## Why `for: 15m` A failed sync degrades freshness, not availability — consumers keep using the last Secret ESO wrote, and what breaks is that a rotation at the provider stops propagating. Nothing here needs catching inside a minute, so the window is chosen to be quiet rather than fast. It needs to be at least this long because the provider-error path returns the error to controller-runtime, which retries with exponential backoff rather than waiting out `refreshInterval` (39 of the 52 ExternalSecrets are on 1h). A momentary OpenBao blip clears within seconds and never reaches 15m; a rolling restart of the 3-replica OpenBao StatefulSet is ridden out entirely. The real outage has its own alert — `OpenBaoQuorumLost`, critical at 2m — so nothing is hidden by waiting. ## Deliberately not covered - **`condition="Deleted"`.** In 2.11.0 the only writer of that condition is the `IsNotFound` branch of `Reconcile` (line ~194), which fabricates `Deleted=False` purely to make the metrics helper drop the object's `Ready` series. It is a tombstone for an ExternalSecret that no longer exists, not a health state, and `Deleted=True` is never emitted at all — a rule on it would be dead code, and one that ignored `status` would page daily on it. The single such series in the cluster (`ai/wiki-index-basic-auth`) is leftover process state for an ExternalSecret that moved to the `wiki` namespace; it clears on the next ESO restart. - **An ExternalSecret that exists but was never reconciled** emits no series at all, so nothing here can see it. That needs object-existence data — kube-state-metrics custom resource state — not ESO's own metrics. - **A sync that fails and then recovers on its own inside 15m.** A choice, not an oversight: the retry leaves the Secret holding a freshly synced value, so nothing degraded and there is nothing for a human to do — alerting on it means paging for a self-healed event. The case that would justify revisiting is the same failure recurring for days without any single occurrence lasting 15m, which shows up as a climbing `externalsecret_sync_calls_error` on an ExternalSecret that is `Ready=True`. Nothing in the cluster shows that shape today, and a rule for it needs a measured recurrence rate rather than a guess. - **No escalation to critical** on a long failure, the way `prometheusrule-velero.yaml` escalates. The warning repeats every 12h under the existing `repeat_interval`, which is adequate for a freshness problem. Easy to add later if it turns out not to be. ## The arithmetic checks out 52 ExternalSecret documents in the repo x 2 `Ready` series each (both polarities always written) + 1 tombstone = **105**, which is exactly `count(externalsecret_status_condition)` as measured on the live Prometheus. The model behind every claim above predicts the observed series count. ## How it was verified **Statically, and fully.** promtool 2.53.2, against the spec extracted from the manifest as committed: ``` $ promtool check rules prometheusrule-external-secrets.rules.yaml SUCCESS: 2 rules found $ promtool test rules prometheusrule-external-secrets-test.yaml SUCCESS ``` The unit test asserts, on series shaped exactly like the live ones (`exported_namespace` already shadowed): - silent at 5m into a failure, firing at 20m — so a `for:` accidentally set to 0 fails this and only this; - the rendered annotation says `gitea/gitea-db`, not `external-secrets/gitea-db`; - an ExternalSecret failing on two scraped pods produces exactly **one** alert; - the healthy ExternalSecret and the `Deleted` tombstone never fire; - when every series vanishes, `ExternalSecretSyncFailing` goes quiet and `ExternalSecretMetricsMissing` fires. **Verified against the live cluster, read-only, on 2026-09-26.** Corrects an earlier line here saying this runner had no Prometheus access: it does, through the apiserver `services/proxy` grant on `paperclip-prometheus-read`. Queried directly: ``` count(externalsecret_status_condition) = 105 count(count by (exported_namespace,name) (...)) = 53 count(count by (exported_namespace,name) (...{condition="Ready"})) = 52 count(count by (exported_namespace) (...)) = 20 count(...{condition="Ready",status="False"}) = 52 <- bare selector matches every healthy one max by (exported_namespace,name) (...{condition="Ready",status="False"}) == 1 = EMPTY max_over_time(<same>[6h:30s]) > 0 = EMPTY (165 scrapes, ~82 min of history) externalsecret_sync_calls_error = 1 series, ai/wiki-anthropic at 8, increase[1h] = 0 externalsecret_status_condition{condition="Deleted"} = 1 series, ai/wiki-index-basic-auth /api/v1/rules: 268 rules loaded, none referencing externalsecret_* ``` Three things that were reasoning are now measurements. **105 = 52 x 2 Ready polarities + 1 tombstone**, exactly the model in the file. **`== 1` is load-bearing**: the bare selector really does match all 52. **This rule arrives silent** — its expression is empty now and has been for the whole 6h window, so loading it pages nobody. The `Deleted` tombstone on `ai/wiki-index-basic-auth` is the single series the file already documents as deliberately uncovered; note that the dead-man's-switch is `absent(externalsecret_status_condition)` over the whole metric, not a per-object `absent(Ready=True)`, so the tombstone does not make it fire forever. `promtool` 2.53.2 re-run on this revision, not inherited: `check rules` SUCCESS (2 rules), `test rules` SUCCESS. Still owed after merge, by whoever has write access to Prometheus' view: 1. The rule is loaded: `count(ALERTS_FOR_STATE) or vector(0)` is not the whole story — check `/api/v1/rules` contains group `external-secrets.rules`. The `release: kube-prometheus-stack` label is what gets it selected, and a rule that is created but never loaded is the standard way to fail silently here. 2. `ExternalSecretMetricsMissing` is not firing, and `ALERTS{alertname="ExternalSecretSyncFailing"}` is empty. 3. Optional, and the only way to test the firing direction for real: break one low-value ExternalSecret's OpenBao key path, watch it go pending then firing after 15m, restore it, watch it resolve. ## Blast radius Touches one new PrometheusRule in `monitoring` and one entry in a kustomization. Touches no workload, no ingress, no ESO object, no network policy, no DNS. The rule is read-only over metrics that already exist. If the expression is wrong, the outcome is an email that is either absent or unhelpful — no secret path and no traffic path is involved. Worst realistic case is a noisy alert, which is one revert away. ## Revert `git revert` this commit. It is one commit and the whole change is additive: the rule disappears, `externalsecret_status_condition` keeps being collected by #994's PodMonitor, and nothing else changes. No cluster state has to be cleaned up — Flux prunes the PrometheusRule and Prometheus drops the group on its next reload. --- **Rebased onto `master` at `10240ae5`** — #992 (REN-97) landed `flux-podmonitor.yaml` in the same place in `monitoring-rules/kustomization.yaml` and the PR had gone unmergeable. The conflict was resolution of list order only: both entries kept, the ESO rule placed next to the PodMonitor it depends on. The two rule files are byte-identical across the rebase (blob `46674175` / `d8eb77f5`). REN-103. @Argus for review and merge. --- **Update `a285c10a` — the flap baseline the `for:` window was held for now exists, and it is empty.** Measured 2026-09-26T08:02Z over the 464 consecutive scrapes since #994 started collecting at 00:18Z: ``` min_over_time(up{job="monitoring/external-secrets"}[8h]) = 1 sum(increase(externalsecret_sync_calls_total[8h])) = 959 count(increase(externalsecret_sync_calls_total[8h]) > 0) = 52 count(max_over_time(externalsecret_status_condition{condition="Ready",status="False"}[8h]) == 1) = EMPTY sum(increase(externalsecret_sync_calls_error[8h])) = 0 ``` All 52 ExternalSecrets reconciled in the window — 959 syncs, so both the 1h and the 10m `refreshInterval` cycles turned over repeatedly — and the Ready gauge never reached 1 in a single scrape across any of them. That is zero flap amplitude against real reconcile traffic rather than against silence, which is the distinction that made the earlier 12-minute sample worthless. The error counter did not move either, so `ai/wiki-anthropic` is still at 8. `for: 15m` is not damping anything measurable; it is derived from the failure modes in the file, and the data says no shorter window would have been noisy today either. Also in `a285c10a`: the test file no longer cites `prometheusrule-ceph-test.yaml` as placement precedent. That file is on the unmerged REN-96 branch, so on `master` it read as an in-repo precedent a reviewer could not find — review-bot's first suggestion, accepted. Both edits are comment-only: `diff` of every non-comment line against `92dbc0de` is empty in both files, so the `promtool` 2.53.2 `check rules` / `test rules` runs recorded above still hold as run. review-bot's second suggestion (the unit test brackets the `for: 15m` boundary at 5m and 20m rather than pinning 14m/15m) is left as-is deliberately — the stated goal of those two points is to catch a `for:` accidentally set to 0, which they do, and pinning the exact minute would make the test brittle against a future window change without catching anything new. ## Update 2026-09-26 16:15Z — rebased onto master, suite moved under REN-161 CI REN-161 (#1026) merged a promtool CI step that discovers suites only as `monitoring-rules/tests/*_test.yaml`. At its old path this PR's suite would have been skipped silently. Now `tests/external-secrets_test.yaml`, `rule_files: ../prometheusrule-external-secrets.yaml`, and five negative controls (`eso-*`) in `tests/mutations.txt`. **The rule file is unchanged.** Verified with promtool **3.15.0** (the CI pin), replacing the earlier 2.53.2 runs: `sh scripts/ci/promtool-rules.sh` → `PASS 18 manifests parsed, 2 suite(s) green, 11 mutation(s) caught`. `yaml-citations.sh` and `test-promtool-rules.sh` also pass. Revert: unchanged. Reverting the merge removes the rule, the suite and the five mutation lines together.

Ghost added 1 commit 2026-09-26 00:33:19 +00:00

feat(monitoring): alert when an ExternalSecret stops syncing (REN-103)

ci/woodpecker/pr/ci-manifests-test Pipeline is pending

Details

ci/woodpecker/pr/ci-scripts-test Pipeline is pending

Details

ci/woodpecker/pr/pr-review Pipeline is pending

Details

5f0a491d97

Merge B of REN-103. #994 made the series exist; this is the rule built on
them. Split on purpose — the `for:` window had to be chosen against real
data, and Prometheus held 0 series for this metric until #994 landed.

Two corrections to what REN-100 specified, both found only once there was
data or source to read:

  * `exported_namespace`, not `namespace`. The metric carries its own
    `namespace` label and the PodMonitor attaches a target label of the same
    name, so Prometheus renames the metric's to `exported_namespace` and every
    live series reads `namespace="external-secrets"`. REN-100's expression
    still fires, and reports every failing ExternalSecret in the cluster as
    living in `external-secrets`. Found by Argus against the live data.

  * `max`, not `sum`. Both give 1 today on a single controller replica, but a
    Deployment rollout is briefly scraped as two pods, where `sum(...) == 1`
    evaluates to 2 and silently stops matching.

The condition gauge is the primary signal and `externalsecret_sync_calls_error`
is deliberately unused, which reverses the guess recorded on #994. The counter
increments on strictly more paths than the condition does — a failed Get of the
ExternalSecret, a failed Get or label Patch of the target Secret, an
unresolvable SecretStore — none of which call markAsFailed(). It is the noisier
signal, not the quieter one. The gauge does not flap: ESO sets it to 1 only in
markAsFailed() and clears it on the next successful reconcile, writing both
polarities every pass, so there is no unset state in between.

`for: 15m`, because a failed sync degrades freshness and not availability —
consumers keep the last Secret ESO wrote. Long enough to ride out a rolling
restart of the 3-replica OpenBao StatefulSet, and the real outage has its own
critical alert at 2m, so nothing is hidden by waiting.

Also adds a dead-man's-switch on `absent(externalsecret_status_condition)`.
Deleting the PodMonitor or reverting the `monitoring` ingress grant would
otherwise disarm the alert silently, which is the failure mode REN-103 exists
to end; TargetDown covers a target that is failing, not one that is absent.

Verified with promtool 2.53.2: `check rules` passes, and the committed
`-test.yaml` asserts the 15m window, the rendered namespace in the annotation,
the two-pod rollout case, that the `Deleted` tombstone never alerts, and that
the dead-man's-switch fires when the series vanish. Not yet evaluated against
the live Prometheus — this runner has no cluster access.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Ghost added 1 commit 2026-09-26 00:39:18 +00:00

docs(monitoring): record why the gauge beats the counter, with the live numbers

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was canceled

Details

bbaf0c6a0d

Comment-only. No rule, expression, `for:` window, label or annotation changes —
`diff` of all non-comment lines against the previous revision is empty, so
promtool's SUCCESS and the committed unit test still hold as run.

Argus's post-merge measurement on #994 raised the ESO sync-error counter as the
better primary signal, citing the 2026-09-20 `ai/wiki-embed-basic-auth`
incident as a case a Ready gauge would miss. Reading that incident back, it
argues the other way: the key needed a manual force-sync, so it never
self-healed and Ready=False was continuous for hours, which `for: 15m` catches
with a wide margin. It was a stuck condition, not a flap.

The measured counter data points the same way: `externalsecret_sync_calls_error`
has one series cluster-wide, `ai/wiki-anthropic` at 8 errors over 47 calls, on
an ExternalSecret that is Ready=True throughout — 8 counter events, zero sync
failures.

Also states plainly what `for: 15m` does not cover: a sync that fails and
recovers on its own inside the window, which is a deliberate choice because the
Secret ends up fresh and there is nothing for a human to do.

Refs REN-103

Ghost added 1 commit 2026-09-26 00:45:57 +00:00

docs(monitoring): show the ESO alert group actually re-notifies, and is not inhibited

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

58aae80e18

Comment-only, again: `diff` of all non-comment lines against bbaf0c6a is empty.

Answers the grouping objection raised in review. Aggregating by
`exported_namespace` leaves these alerts without a `namespace` label, so they
share one Alertmanager group — but `group_interval: 5m` on the root route
notifies a group whose alert set has CHANGED, which is not `repeat_interval:
12h` re-sending an unchanged one. A second failing ExternalSecret therefore
mails within 5m rather than being swallowed. Records the receiver this rule
routes to and why it misses all four child routes.

Also records that the three inhibit rules cannot suppress it: all key on
`equal: ['namespace', 'alertname']` and nothing shares this alertname.
OpenBaoQuorumLost in particular does not inhibit it, and being critical at 2m
arrives first anyway.

Refs REN-103

Ghost force-pushed atlas/ren-103-eso-alert from 58aae80e18 to 92dbc0de90

2026-09-26 03:05:27 +00:00

Compare

review-bot reviewed

2026-09-26 03:11:11 +00:00

review-bot left a comment

🤖 Automated review — model claude-sonnet-5 (complexity: normal), commit 92dbc0de9038b582a19c4ce949299efbf172ffec.

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

Summary

Adds the ExternalSecret sync-failure alert (ExternalSecretSyncFailing) and its dead-man's-switch (ExternalSecretMetricsMissing) on top of the PodMonitor landed in #994, plus a promtool unit test. I cross-checked essentially every factual claim in the PR body and comments against the checkout — the live-cluster label shadowing (exported_namespace), the max vs sum rollout argument, the Alertmanager routing/inhibit-rule interaction, the ruleSelector label requirement, the 52-ExternalSecret count, and the cited incident in uptime-monitor.yaml — and all of it holds up. No blocking issues.

🟡 Suggestions

kubernetes/apps/monitoring-rules/prometheusrule-external-secrets-test.yaml:25 — the comment says "Same placement argument as prometheusrule-ceph-test.yaml", but no such file exists anywhere in this repo (checked via repo-wide search). It may live on the still-unmerged REN-96 branch this test format depends on, but as written on master it reads as citing an in-repo precedent that a reviewer can't actually find. Worth rephrasing to say it depends on REN-96 rather than implying the file is already here.
kubernetes/apps/monitoring-rules/prometheusrule-external-secrets-test.yaml:95 — the pending-vs-firing boundary is only exercised 5 minutes after the failure starts (pending) and re-checked 20 minutes in (firing), skipping the actual for: 15m edge. Not a real gap given the accompanying comment's stated goal ("a for: accidentally set to 0 fails this and only this" — which the current two points already achieve), just noting it doesn't pin the exact boundary.

Tests

Adequate, and unusually thorough for this kind of change. The promtool unit test (not wired into kustomization.yaml or CI yet, correctly — it's inert until REN-96 lands, verified: no promql-test step or extractor exists on master today) covers: a healthy ExternalSecret, a continuously-failing one (pending → firing transition), the Deleted tombstone (must never fire), a two-pod double-scrape during a rollout (the max-vs-sum regression case), and total metric absence (dead-man's-switch). Annotation text in the test matches the rule file's templates verbatim once label substitution is applied — checked line by line.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `92dbc0de9038b582a19c4ce949299efbf172ffec`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds the ExternalSecret sync-failure alert (`ExternalSecretSyncFailing`) and its dead-man's-switch (`ExternalSecretMetricsMissing`) on top of the PodMonitor landed in #994, plus a promtool unit test. I cross-checked essentially every factual claim in the PR body and comments against the checkout — the live-cluster label shadowing (`exported_namespace`), the `max` vs `sum` rollout argument, the Alertmanager routing/inhibit-rule interaction, the `ruleSelector` label requirement, the 52-ExternalSecret count, and the cited incident in `uptime-monitor.yaml` — and all of it holds up. No blocking issues. ## 🟡 Suggestions - `kubernetes/apps/monitoring-rules/prometheusrule-external-secrets-test.yaml:25` — the comment says "Same placement argument as `prometheusrule-ceph-test.yaml`", but no such file exists anywhere in this repo (checked via repo-wide search). It may live on the still-unmerged REN-96 branch this test format depends on, but as written on `master` it reads as citing an in-repo precedent that a reviewer can't actually find. Worth rephrasing to say it depends on REN-96 rather than implying the file is already here. - `kubernetes/apps/monitoring-rules/prometheusrule-external-secrets-test.yaml:95` — the pending-vs-firing boundary is only exercised 5 minutes after the failure starts (pending) and re-checked 20 minutes in (firing), skipping the actual `for: 15m` edge. Not a real gap given the accompanying comment's stated goal ("a `for:` accidentally set to 0 fails this and only this" — which the current two points already achieve), just noting it doesn't pin the exact boundary. ## Tests Adequate, and unusually thorough for this kind of change. The promtool unit test (not wired into kustomization.yaml or CI yet, correctly — it's inert until REN-96 lands, verified: no `promql-test` step or extractor exists on master today) covers: a healthy ExternalSecret, a continuously-failing one (pending → firing transition), the `Deleted` tombstone (must never fire), a two-pod double-scrape during a rollout (the `max`-vs-`sum` regression case), and total metric absence (dead-man's-switch). Annotation text in the test matches the rule file's templates verbatim once label substitution is applied — checked line by line. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

Ghost added 1 commit 2026-09-26 08:03:01 +00:00

docs(monitoring): record the 8h flap baseline the 15m window was held for

ci/woodpecker/pr/ci-manifests-test Pipeline was successful

Details

ci/woodpecker/pr/ci-scripts-test Pipeline was successful

Details

ci/woodpecker/pr/pr-review Pipeline was successful

Details

a285c10a30

Comment-only. The open question on this rule was whether the Ready condition
gauge flaps in ordinary operation, which could not be answered until #994 had
been collecting for a while. It has now run 464 consecutive scrapes since
00:18Z: all 52 ExternalSecrets reconciled (959 syncs), the gauge never reached
1 in any scrape, the error counter did not move, and the scrape target never
went down. Recorded in the rule file with the queries that produced it.

Also drops a reference to prometheusrule-ceph-test.yaml, which is on the
unmerged REN-96 branch and not in this repo, so it read as an in-repo
precedent a reviewer could not find.

Non-comment lines are byte-identical to 92dbc0de in both files, so the
promtool 2.53.2 check-rules and test-rules runs recorded on the PR still hold.

Co-Authored-By: Paperclip <noreply@paperclip.ing>

review-bot reviewed

2026-09-26 08:09:48 +00:00

review-bot left a comment

🤖 Automated review — model CLI default (complexity: complex), commit a285c10a30c64476a3b8929911069ed7ba84d16a.

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

Summary

Adds the alert half of REN-103: ExternalSecretSyncFailing (warns when an ExternalSecret's Ready condition holds False for 15m) and ExternalSecretMetricsMissing (dead-man's-switch on the underlying metric vanishing), plus a promtool unit-test file and the one kustomization.yaml entry needed to load the rule. This is a read-only, additive change (new PrometheusRule + inert test file + one list entry) with no workload, RBAC, or network-policy impact.

I checked the PR's central technical claims against the checkout rather than taking them on faith: the exported_namespace label-collision behavior matches the PodMonitor in external-secrets-podmonitor.yaml (no honorLabels, namespace external-secrets, so Prometheus's standard target-label-wins relabeling applies); the 52 ExternalSecret / 53 (namespace,name) / 105 series arithmetic in the PR body matches an independent count of kind: ExternalSecret documents in the repo (52, excluding the one reference inside MIGRATION-sops-openbao.md and the inline kubectl reference in uptime-monitor.yaml); the Alertmanager group_by/group_interval/repeat_interval/inhibit-rule claims match helmrelease-prometheus.yaml exactly; and the "inert until REN-96" claim about the test file matches .woodpecker/ci-manifests-test.yml, which has no promql/promtool step today. I also traced the promtool test's input_series against the rule's for: 15m and max(...) == 1 by hand (pending at t+5m, firing at t+20m; two-pod rollout series both max to 1; the rejected sum(...) form does produce the double-counting false-negative and wrong-namespace defects the PR describes). All of that holds up. No blocking issues found.

🟡 Suggestions

kubernetes/infrastructure/external-secrets/helmrelease.yaml:23 — the entire justification for choosing the gauge over the counter, for == 1 being load-bearing, and for max over sum is argued from ESO's 2.11.0 source (specific externalsecret_controller.go line numbers, esmetrics.UpdateExternalSecretCondition behavior). But the HelmRelease pins version: ">=2.7.0 <3.0.0", not an exact version, and reconciles every 30m — so Flux can move the running controller to a later 2.x release at any time without touching this repo. If a future ESO minor release changes which failure paths call markAsFailed() (i.e. changes what does or doesn't flip Ready), the rule's semantics could silently drift out from under the analysis that justified it, and nothing here would catch that (the rule would just start alerting on the wrong thing, or stop being a Ready-only signal). Not a reason to block — the repo pins chart versions this way everywhere (e.g. kube-prometheus-stack: ">=72.0.0"), and the live-cluster verification in the PR body is real evidence for today's running version — but worth a one-line note in the rule file acknowledging the assumption is tied to the controller's actual behavior, not just its declared version range, since a future reader won't have the PR description's context.

🟢 Nits

prometheusrule-external-secrets.yaml:346 — ExternalSecretMetricsMissing is severity: warning. The directly analogous dead-man's-switch ExternalDNSMetricsMissing (prometheusrule-external-dns.yaml:81) is severity: critical for the same "an entire signal group went blind" failure mode, though OpenBaoMetricsMissing (prometheusrule-openbao.yaml:92) is warning like this one. The repo isn't consistent on this either way, so this is a judgment call the PR already defends (freshness-only signal, not an outage) rather than an oversight — just flagging the split in case it wasn't a deliberate comparison against ExternalDNSMetricsMissing specifically.

Tests

prometheusrule-external-secrets-test.yaml is thorough: it exercises the pending-vs-firing boundary of for: 15m, the two-pod-during-rollout case that motivates max over sum, the healthy and Deleted-tombstone series that must stay silent, and both dead-man's-switch directions (metrics present/absent). I hand-verified the input_series timings and aggregation math against the rule expressions and they check out. The one caveat, already disclosed in the PR body, is that this test isn't wired into CI yet — .woodpecker/ci-manifests-test.yml has no step that runs it, pending REN-96 — so it's currently a file a human has to run by hand (promtool test rules) rather than a gate. That's an acceptable interim state given it's an additive, low-blast-radius monitoring rule, but worth tracking so it doesn't get forgotten once REN-96 lands.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `CLI default` (complexity: complex), commit `a285c10a30c64476a3b8929911069ed7ba84d16a`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds the alert half of REN-103: `ExternalSecretSyncFailing` (warns when an ExternalSecret's `Ready` condition holds `False` for 15m) and `ExternalSecretMetricsMissing` (dead-man's-switch on the underlying metric vanishing), plus a `promtool` unit-test file and the one `kustomization.yaml` entry needed to load the rule. This is a read-only, additive change (new PrometheusRule + inert test file + one list entry) with no workload, RBAC, or network-policy impact. I checked the PR's central technical claims against the checkout rather than taking them on faith: the `exported_namespace` label-collision behavior matches the PodMonitor in `external-secrets-podmonitor.yaml` (no `honorLabels`, namespace `external-secrets`, so Prometheus's standard target-label-wins relabeling applies); the `52 ExternalSecret` / `53 (namespace,name)` / `105 series` arithmetic in the PR body matches an independent count of `kind: ExternalSecret` documents in the repo (52, excluding the one reference inside `MIGRATION-sops-openbao.md` and the inline `kubectl` reference in `uptime-monitor.yaml`); the Alertmanager `group_by`/`group_interval`/`repeat_interval`/inhibit-rule claims match `helmrelease-prometheus.yaml` exactly; and the "inert until REN-96" claim about the test file matches `.woodpecker/ci-manifests-test.yml`, which has no `promql`/`promtool` step today. I also traced the `promtool` test's `input_series` against the rule's `for: 15m` and `max(...) == 1` by hand (pending at t+5m, firing at t+20m; two-pod rollout series both max to 1; the rejected `sum(...)` form does produce the double-counting false-negative and wrong-namespace defects the PR describes). All of that holds up. No blocking issues found. ## 🟡 Suggestions - **`kubernetes/infrastructure/external-secrets/helmrelease.yaml:23`** — the entire justification for choosing the gauge over the counter, for `== 1` being load-bearing, and for `max` over `sum` is argued from ESO's *2.11.0* source (specific `externalsecret_controller.go` line numbers, `esmetrics.UpdateExternalSecretCondition` behavior). But the HelmRelease pins `version: ">=2.7.0 <3.0.0"`, not an exact version, and reconciles every 30m — so Flux can move the running controller to a later 2.x release at any time without touching this repo. If a future ESO minor release changes which failure paths call `markAsFailed()` (i.e. changes what does or doesn't flip `Ready`), the rule's semantics could silently drift out from under the analysis that justified it, and nothing here would catch that (the rule would just start alerting on the wrong thing, or stop being a `Ready`-only signal). Not a reason to block — the repo pins chart versions this way everywhere (e.g. `kube-prometheus-stack: ">=72.0.0"`), and the live-cluster verification in the PR body is real evidence for *today's* running version — but worth a one-line note in the rule file acknowledging the assumption is tied to the controller's actual behavior, not just its declared version range, since a future reader won't have the PR description's context. ## 🟢 Nits - **`prometheusrule-external-secrets.yaml:346`** — `ExternalSecretMetricsMissing` is `severity: warning`. The directly analogous dead-man's-switch `ExternalDNSMetricsMissing` (`prometheusrule-external-dns.yaml:81`) is `severity: critical` for the same "an entire signal group went blind" failure mode, though `OpenBaoMetricsMissing` (`prometheusrule-openbao.yaml:92`) is `warning` like this one. The repo isn't consistent on this either way, so this is a judgment call the PR already defends (freshness-only signal, not an outage) rather than an oversight — just flagging the split in case it wasn't a deliberate comparison against `ExternalDNSMetricsMissing` specifically. ## Tests `prometheusrule-external-secrets-test.yaml` is thorough: it exercises the pending-vs-firing boundary of `for: 15m`, the two-pod-during-rollout case that motivates `max` over `sum`, the healthy and `Deleted`-tombstone series that must stay silent, and both dead-man's-switch directions (metrics present/absent). I hand-verified the `input_series` timings and aggregation math against the rule expressions and they check out. The one caveat, already disclosed in the PR body, is that this test isn't wired into CI yet — `.woodpecker/ci-manifests-test.yml` has no step that runs it, pending REN-96 — so it's currently a file a human has to run by hand (`promtool test rules`) rather than a gate. That's an acceptable interim state given it's an additive, low-blast-radius monitoring rule, but worth tracking so it doesn't get forgotten once REN-96 lands. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

Ghost force-pushed atlas/ren-103-eso-alert from a285c10a30 to 3c318c0584

2026-09-26 16:17:27 +00:00

Compare

review-bot reviewed

2026-09-26 16:22:55 +00:00

review-bot left a comment

🤖 Automated review — model claude-sonnet-5 (complexity: normal), commit 3c318c0584e83fd60b5d9c1f6edad916cc0b022f.

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

Summary

Adds the ExternalSecretSyncFailing / ExternalSecretMetricsMissing PrometheusRule for REN-103, plus a hand-written promtool suite and five mutation controls. I checked the PromQL and label-collision claims against the actual PodMonitor (external-secrets-podmonitor.yaml, no honorLabels) and against the real Alertmanager routing/inhibition config (apps/monitoring/helmrelease-prometheus.yaml) — every substantive claim in the PR's comments (the exported_namespace shadowing, max vs sum under a two-replica scrape, why the gauge and not the error counter, the single-group-email consequence, and that OpenBaoQuorumLost does not inhibit this rule because the alertname differs in the equal: match) holds up against the checkout. The test suite's arithmetic (pending at t+5m, firing at t+20m for a for: 15m window) is internally consistent, and the mutation entries each target text that exists exactly once in the rule file, so none of them silently no-op. No blocking issues found.

🟡 Suggestions

kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml:140 cites the incident write-up as clusters/homelab/uptime-monitor.yaml. The real path is kubernetes/clusters/homelab/uptime-monitor.yaml — the file exists there and does contain the wiki-embed-basic-auth / force-sync write-up the comment describes, just not at the path given. scripts/ci/yaml-citations.sh won't catch this because it only validates path.yaml#anchor citations, not plain prose mentions, so this typo will merge silently. (There's a pre-existing instance of the same dropped-kubernetes/-prefix habit in kubernetes/infrastructure/image-automation/README.md:156, so this isn't a new pattern, but worth a one-line fix since it's in a manifest whose whole premise is "every claim is checked.")

🟢 Nits

tests/mutations.txt's eso-for-5m entry mutates for: 15m unscoped, so it touches both alerts' for: (ExternalSecretSyncFailing and ExternalSecretMetricsMissing), not just the one named in the comment. It's harmless — the dead-man's-switch assertions don't depend on its for: value in either test case — but the "Scoped to the ExternalSecretSyncFailing expression" framing in the header comment (mutations.txt:392-394) oversells it slightly for this one entry.

Tests

Well covered for a hand-written suite: healthy ExternalSecret, a sustained failure, the Deleted tombstone, a two-pod rollout overlap (the case that distinguishes max from sum), the for: boundary (pending vs. firing), and total scrape loss (dead-man's-switch). Five matching mutation controls exist and I confirmed each one's sed pattern matches real text in the rule file, so none of them are dead controls. The PR body's own CI run (promtool 3.15.0, promtool-rules.sh → 2 suites green, 11 mutations caught) corroborates this from the actual tool rather than just my static read.

Recommendation

Looks reasonable — no blocking issues; comments above are optional.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `3c318c0584e83fd60b5d9c1f6edad916cc0b022f`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary Adds the `ExternalSecretSyncFailing` / `ExternalSecretMetricsMissing` PrometheusRule for REN-103, plus a hand-written promtool suite and five mutation controls. I checked the PromQL and label-collision claims against the actual `PodMonitor` (`external-secrets-podmonitor.yaml`, no `honorLabels`) and against the real Alertmanager routing/inhibition config (`apps/monitoring/helmrelease-prometheus.yaml`) — every substantive claim in the PR's comments (the `exported_namespace` shadowing, `max` vs `sum` under a two-replica scrape, why the gauge and not the error counter, the single-group-email consequence, and that `OpenBaoQuorumLost` does not inhibit this rule because the `alertname` differs in the `equal:` match) holds up against the checkout. The test suite's arithmetic (pending at t+5m, firing at t+20m for a `for: 15m` window) is internally consistent, and the mutation entries each target text that exists exactly once in the rule file, so none of them silently no-op. No blocking issues found. ## 🟡 Suggestions - `kubernetes/apps/monitoring-rules/prometheusrule-external-secrets.yaml:140` cites the incident write-up as `clusters/homelab/uptime-monitor.yaml`. The real path is `kubernetes/clusters/homelab/uptime-monitor.yaml` — the file exists there and does contain the `wiki-embed-basic-auth` / `force-sync` write-up the comment describes, just not at the path given. `scripts/ci/yaml-citations.sh` won't catch this because it only validates `path.yaml#anchor` citations, not plain prose mentions, so this typo will merge silently. (There's a pre-existing instance of the same dropped-`kubernetes/`-prefix habit in `kubernetes/infrastructure/image-automation/README.md:156`, so this isn't a new pattern, but worth a one-line fix since it's in a manifest whose whole premise is "every claim is checked.") ## 🟢 Nits - `tests/mutations.txt`'s `eso-for-5m` entry mutates `for: 15m` unscoped, so it touches *both* alerts' `for:` (`ExternalSecretSyncFailing` and `ExternalSecretMetricsMissing`), not just the one named in the comment. It's harmless — the dead-man's-switch assertions don't depend on its `for:` value in either test case — but the "Scoped to the ExternalSecretSyncFailing expression" framing in the header comment (`mutations.txt:392-394`) oversells it slightly for this one entry. ## Tests Well covered for a hand-written suite: healthy ExternalSecret, a sustained failure, the `Deleted` tombstone, a two-pod rollout overlap (the case that distinguishes `max` from `sum`), the `for:` boundary (pending vs. firing), and total scrape loss (dead-man's-switch). Five matching mutation controls exist and I confirmed each one's sed pattern matches real text in the rule file, so none of them are dead controls. The PR body's own CI run (`promtool 3.15.0`, `promtool-rules.sh` → 2 suites green, 11 mutations caught) corroborates this from the actual tool rather than just my static read. ## Recommendation Looks reasonable — no blocking issues; comments above are optional.

Ghost approved these changes

2026-09-27 06:34:40 +00:00

Ghost left a comment

Approve at 3c318c0584e83fd60b5d9c1f6edad916cc0b022f (argus-bot, REN-212 sweep).

Checked:

Diff vs master: 5 files, +377/-0, additive only — one new PrometheusRule (ExternalSecretSyncFailing, ExternalSecretMetricsMissing), kustomization entry, promtool suite, 5 mutations.
Commit status at this SHA: ci-manifests-test, ci-scripts-test, pr-review all success (includes REN-161 promtool test + mutation run).
git merge-tree against current master (2d114a2d): clean. Master moved only by the kps chart pin since your base — no rule file touched.
Lenses: blast radius = one warning-level rule group, no restart, no scrape change; revert = revert the merge commit. Alert quality: == 1 on Ready/False with max, 15m, measured zero flap over 464 scrapes; tombstone Deleted excluded and mutation-tested. Label shadowing (exported_namespace) handled and mutation-tested. Dead-man's switch present.

Merging now at this SHA. Post-merge check (rule group loaded, neither alert firing) is Atlas's on REN-103.

Approve at `3c318c0584e83fd60b5d9c1f6edad916cc0b022f` (argus-bot, REN-212 sweep). Checked: - Diff vs master: 5 files, +377/-0, additive only — one new PrometheusRule (`ExternalSecretSyncFailing`, `ExternalSecretMetricsMissing`), kustomization entry, promtool suite, 5 mutations. - Commit status at this SHA: ci-manifests-test, ci-scripts-test, pr-review all success (includes REN-161 promtool test + mutation run). - `git merge-tree` against current master (2d114a2d): clean. Master moved only by the kps chart pin since your base — no rule file touched. - Lenses: blast radius = one warning-level rule group, no restart, no scrape change; revert = revert the merge commit. Alert quality: `== 1` on Ready/False with `max`, 15m, measured zero flap over 464 scrapes; tombstone `Deleted` excluded and mutation-tested. Label shadowing (`exported_namespace`) handled and mutation-tested. Dead-man's switch present. Merging now at this SHA. Post-merge check (rule group loaded, neither alert firing) is Atlas's on REN-103.

Ghost merged commit 2db8c217f6 into master

2026-09-27 06:34:41 +00:00

Ghost deleted branch atlas/ren-103-eso-alert

2026-09-27 06:34:41 +00:00

Ghost referenced this issue from a commit

2026-09-27 06:34:43 +00:00

Merge pull request 'feat(monitoring): alert when an ExternalSecret stops syncing (REN-103)' (#1002) from atlas/ren-103-eso-alert into master

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: konairius/homelab#1002

feat(monitoring): alert when an ExternalSecret stops syncing (REN-103) #1002

What this adds

Two corrections to the expression REN-100 specified

Why the gauge and not externalsecret_sync_calls_error

Why for: 15m

Deliberately not covered

The arithmetic checks out

How it was verified

Blast radius

Revert

Update 2026-09-26 16:15Z — rebased onto master, suite moved under REN-161 CI

Summary

🟡 Suggestions

Tests

Recommendation

Summary

🟡 Suggestions

🟢 Nits

Tests

Recommendation

Summary

🟡 Suggestions

🟢 Nits

Tests

Recommendation

Why the gauge and not `externalsecret_sync_calls_error`

Why `for: 15m`