Skip to content

fix(monitoring): stop GrafanaSAProvisionerFailing permanent false alarm - #1453

Merged
Aviator-Coding merged 2 commits into
mainfrom
fm/homeops-grafana-sa-provisioning-alert-fix
Aug 27, 2026
Merged

fix(monitoring): stop GrafanaSAProvisionerFailing permanent false alarm#1453
Aviator-Coding merged 2 commits into
mainfrom
fm/homeops-grafana-sa-provisioning-alert-fix

Conversation

@Aviator-Coding

Copy link
Copy Markdown
Owner

Intent

Post-merge live verification of PR #1450 (grafana-sa-provisioner self-healing Grafana service-account/token reconciler) uncovered a real bug in the merged PrometheusRule alert, which this follow-up fixes.

What I verified live against the cluster after PR #1450 merged and the captain restarted the Grafana pod (the documented one-time activation restart): the grafana-sa-provisioner CronJob's very first post-restart cycle succeeded and recreated the Viewer-scoped SA + token; the token propagated through the reconciler's own Secret -> PushSecret -> the existing 1Password item grafana-mcp/GRAFANA_SERVICE_ACCOUNT_TOKEN -> the toolhive-grafana ExternalSecret (confirmed byte-identical token value on both ends); the Reloader annotation correctly triggered a rollout of the grafana-mcp StatefulSet (confirmed via a live "Reloaded ... Changes detected in toolhive-grafana of type SECRET" Kubernetes event, and confirmed the new pod's live GRAFANA_SERVICE_ACCOUNT_TOKEN env var matches exactly what the reconciler minted); and the previously-401ing federated query now works end-to-end (direct curl proof: GET /api/org 200, GET /api/datasources 200, and a live PromQL query through the Prometheus datasource proxy returning real cluster metrics - all via the rotated token). A6 Viewer-only scope was also verified behaviorally: a write attempt (POST /api/dashboards/db) with this token correctly returns 403.

While confirming the GrafanaSAProvisionerFailing alert's behavior over an extended observation window (CronJob had been succeeding every 5 minutes for 2.5+ hours), I found the alert was firing continuously and never clearing. Root-caused by querying the live Prometheus instance directly: the shipped expression "time() - (kube_cronjob_status_last_successful_time{...} or vector(0)) > 3600" does not behave as a fallback-only-when-absent. PromQL's "or" unions series by label set rather than substituting one operand for the other; vector(0) produces a single series with an empty label set, which never matches the real metric's (non-empty) label set, so "metric or vector(0)" always returns BOTH series when the metric exists, not just the metric's. "time() - 0 > 3600" is essentially always true, so that phantom zero-label series stays in the alert's result set permanently regardless of the real metric's freshness - I verified this directly by querying "metric or vector(0)" against live Prometheus and seeing two result series (the real one plus the phantom empty-label one).

This PR fixes the expression to "absent(metric) or (time() - metric > 3600)", which I verified live: absent() on this metric with the real matchers only produces a result when the series is genuinely missing (confirmed empty result while the metric exists and is fresh; confirmed a populated result when querying a nonexistent cronjob name as a sanity check), so the alert now correctly stays silent while the CronJob is healthy and will only fire when it has genuinely never succeeded or has gone stale for over an hour - matching the original design intent ("silence is the failure mode", but not the reverse: constant false alarm is not a design goal either). No other files changed; this is a single-expression fix to the PrometheusRule already shipped in kubernetes/apps/base/monitoring/grafana-sa-provisioner/app/prometheusrule.yaml.

What Changed

  • Rewrote the GrafanaSAProvisionerFailing PromQL expression from time() - (metric or vector(0)) > 3600 to absent(metric) or (time() - metric > 3600) so a healthy CronJob no longer leaves a permanent phantom empty-label series in the alert result set.
  • Updated the provisioner CI contract to reject vector(0) fallbacks and require absent() for the never-succeeded case.
  • Added promtool-backed unit tests that evaluate healthy, stale, absent, and old-expression regression scenarios against Prometheus' real rule engine.

Risk Assessment

✅ Low: Single, live-verified PromQL correction replaces the broken metric-or-vector(0) union with absent() plus a staleness check, restoring the intended fire/clear behavior with no other behavioral surface.

Testing

Exercised the shipped alert with Prometheus promtool end-to-end: the absent()/age expression is silent while the CronJob is healthy and fires for stale or never-succeeded cases, while the pre-fix vector(0) form keeps a permanent false alarm; the provisioner CI script was updated to lock that behavior in and the full focused run passed.

Evidence: PromQL regression proof (old false-positive vs fixed silence)
GrafanaSAProvisionerFailing PromQL fix — regression proof (promtool)
========================================================================

SHIPPED EXPR:
absent(kube_cronjob_status_last_successful_time{namespace="monitoring", cronjob="grafana-sa-provisioner"})
or
(time() - kube_cronjob_status_last_successful_time{namespace="monitoring", cronjob="grafana-sa-provisioner"} > 3600)

1) NEW fixed alert on healthy CronJob (last success tracks time)
   exit=0  (0 = silent as required)
   SUCCESS

2) OLD buggy expr on same healthy series (expect false positive)
   exit=1  (non-zero = still fires = bug confirmed)
FAILED:
    name: OLD buggy: healthy should be silent (expect FAIL = false positive),
    alertname: GrafanaSAProvisionerFailing, time: 1h10m, 
        exp:[], 
        got:[
            0:
              Labels:{alertname="GrafanaSAProvisionerFailing", severity="warning"}
              Annotations:{summary="Grafana Viewer service account reconciler has been failing for over an hour."}
            ]

3) Series-level: metric OR vector(0) returns TWO series (real + phantom empty labels);
   shipped absent(...) OR (time()-metric > 3600) returns ZERO samples when healthy.
   exit=0
   SUCCESS

RESULT: PASS — fixed expr silent when healthy; old expr permanently false-alarms;
series semantics match the live root cause (PromQL or unions by label set).
Evidence: Structured promtool proof JSON
{
  "shipped_expr": "absent(kube_cronjob_status_last_successful_time{namespace=\"monitoring\", cronjob=\"grafana-sa-provisioner\"})\nor\n(time() - kube_cronjob_status_last_successful_time{namespace=\"monitoring\", cronjob=\"grafana-sa-provisioner\"} > 3600)",
  "proof": {
    "new_fixed_alert_silent_when_healthy": true,
    "old_vector0_alert_false_positive_when_healthy": true,
    "expr_series_semantics": true
  },
  "details": {
    "new_healthy_silent": {
      "exit": 0,
      "output": "SUCCESS"
    },
    "old_healthy_false_positive": {
      "exit": 1,
      "output": "FAILED:\n    name: OLD buggy: healthy should be silent (expect FAIL = false positive),\n    alertname: GrafanaSAProvisionerFailing, time: 1h10m, \n        exp:[], \n        got:[\n            0:\n              Labels:{alertname=\"GrafanaSAProvisionerFailing\", severity=\"warning\"}\n              Annotations:{summary=\"Grafana Viewer service account reconciler has been failing for over an hour.\"}\n            ]"
    },
    "expr_series_old_vs_new": {
      "exit": 0,
      "output": "SUCCESS"
    }
  }
}
Evidence: Full grafana-sa-provisioner-test.py run
==> start mock Grafana + Kubernetes APIs
    grafana=http://127.0.0.1:55123 k8s=http://127.0.0.1:55124
==> scenario: valid stored token is a no-op
    OK
==> scenario: missing token creates Viewer SA + Secret
    OK
==> scenario: invalid token rotates + patches Secret
    OK
==> scenario: drifted SA role re-asserted to Viewer (A6)
    OK
==> scenario: admin auth failure fails job, writes nothing
    OK
==> kustomize/semantic: grafana-sa-provisioner
    OK docs=4 schedule=*/5 * * * * push=grafana-mcp/GRAFANA_SERVICE_ACCOUNT_TOKEN promtool={'alert_rules': 'PASS', 'expr_semantics': 'PASS', 'alert_out_tail': 'SUCCESS', 'expr_out_tail': 'SUCCESS'}
==> kustomize/semantic: grafana-mcp consumer
    OK refresh=5m reloader=toolhive-grafana
PASS: grafana-sa-provisioner self-heal contracts hold
covered:
  - valid stored token is a no-op
  - missing token creates Viewer SA + Secret
  - invalid token rotates + patches Secret
  - drifted SA role re-asserted to Viewer (A6)
  - admin auth failure fails job, writes nothing
  - provisioner manifests
  - promtool alert + expr semantics
  - consumer manifests
Evidence: Old buggy alert false-positive transcript


  FAILED:
    alertname: GrafanaSAProvisionerFailing, time: 1h10m, 
        exp:[], 
        got:[
            0:
              Labels:{alertname="GrafanaSAProvisionerFailing", severity="warning"}
              Annotations:{summary="Grafana Viewer service account reconciler has been failing for over an hour."}
            ]

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

✅ **Review** - passed

✅ No issues found.

✅ **Test** - passed

✅ No issues found.

  • python3 scripts/ci/grafana-sa-provisioner-test.py (reconciler scenarios + kustomize semantics + promtool alert/expr checks)
  • podman … promtool test rules on shipped PrometheusRule: healthy silent, stale>1h fires, metric-absent fires
  • podman … promtool test rules regression: old metric or vector(0) false-positive on healthy series (empty-label phantom)
  • podman … promtool promql_expr_test: metric or vector(0) returns two series; shipped expr returns zero samples when healthy
✅ **Document** - passed

✅ No issues found.

✅ **Lint** - passed

✅ No issues found.

✅ **Push** - passed

✅ No issues found.

…ntly

Verified live post-merge: the CronJob succeeded every 5 minutes for
2.5+ hours, but the alert never cleared. `time() - (metric or vector(0))
> 3600` doesn't fall back only when metric is absent - PromQL `or` unions
by label set, and vector(0)'s empty label set never matches the metric's
real labels, so both series survive and the always-true zero-label branch
keeps the alert firing forever regardless of actual CronJob health.

Switched to `absent(metric) or (time() - metric > 3600)`, which only
produces a result from the absent() branch when the metric is truly
missing. Confirmed empty-vs-populated behavior directly against the live
Prometheus instance before and after the fix.
@mortyops

mortyops Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor
--- kubernetes/apps/base/monitoring/grafana-sa-provisioner/app Kustomization: monitoring/grafana-sa-provisioner PrometheusRule: monitoring/grafana-sa-provisioner-rules

+++ kubernetes/apps/base/monitoring/grafana-sa-provisioner/app Kustomization: monitoring/grafana-sa-provisioner PrometheusRule: monitoring/grafana-sa-provisioner-rules

@@ -22,12 +22,14 @@

           here is the failure mode by design. Check `kubectl -n monitoring logs -l
           app.kubernetes.io/name=grafana-sa-provisioner --tail=50`; a 401 from admin
           auth in those logs means grafana-admin-secret has drifted from live Grafana
           state and needs a Grafana pod restart to clear (see kubernetes/apps/base/monitoring/grafana-sa-provisioner/README.md).
         summary: Grafana Viewer service account reconciler has been failing for over
           an hour.
-      expr: time() - (kube_cronjob_status_last_successful_time{namespace="monitoring",
-        cronjob="grafana-sa-provisioner"} or vector(0)) > 3600
+      expr: |-
+        absent(kube_cronjob_status_last_successful_time{namespace="monitoring", cronjob="grafana-sa-provisioner"})
+        or
+        (time() - kube_cronjob_status_last_successful_time{namespace="monitoring", cronjob="grafana-sa-provisioner"} > 3600)
       for: 5m
       labels:
         severity: warning
 

@Aviator-Coding
Aviator-Coding merged commit c7dda20 into main Aug 27, 2026
17 checks passed
@Aviator-Coding
Aviator-Coding deleted the fm/homeops-grafana-sa-provisioning-alert-fix branch August 27, 2026 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant