fix(monitoring): stop double-scraping two authentik outposts #690

Merged
k.renner merged 2 commits from fix-monitor-selector-comments into master 2026-08-28 07:04:18 +00:00
Member

Ich wollte zwei falsche Kommentare korrigieren und bin dabei auf ein laufendes Problem gestoßen.

Der eigentliche Fund: doppeltes Scraping

Seit serviceMonitorSelector auf {} gelockert wurde, findet Prometheus die ServiceMonitors, die der goauthentik.io-Operator selbst anlegt — genau deshalb wurde gelockert. Die handgeschriebenen Kopien für ldap und paperless-gate, damals nötig, weil der strenge Selector die Operator-eigenen verwarf, scrapen dieselben Endpunkte jetzt ein zweites Mal.

Gemessen, nicht vermutet:

Target Samples/10min
10.244.4.113:9300 (paperless-gate) 40
10.244.4.76:9300 (ldap) 40
alle übrigen Outposts 20

ldap hatte sogar ein drittes Target, weil der handgeschriebene Selector auch den normalen Service traf, nicht nur den -metrics-Service.

Keine duplicate timestamp-Fehler — die zwei Pools scrapen zeitversetzt, die Samples kollidieren also nicht. Der Schaden ist doppelte Ingestion und eine Target-Liste, die über die Zahl der Outposts lügt.

Radius bleibt — und das ist nicht dasselbe

Der Operator-ServiceMonitor für radius selektiert auf app.kubernetes.io/managed-by: goauthentik.io und goauthentik.io/service-type: metrics. Kein Service trägt beides: der des Operators hat das erste, der hier definierte das zweite. Der handgeschriebene SM ist damit das Einzige, was radius überhaupt scraped — ihn mitzulöschen wäre ein stiller Verlust gewesen. Steht jetzt so im Kommentar.

Die Kommentare, wegen derer ich hier angefangen habe

Sechs Dateien behaupteten, das release-Label sei für die Discovery nötig. Ist es nicht — aber die Unterscheidung ist schärfer, als ich zuletzt gesagt habe. Live-Stand:

serviceMonitorSelector   {}                              → Label egal
podMonitorSelector       {}                              → Label egal
probeSelector            release=kube-prometheus-stack   → Label NÖTIG
ruleSelector             release=kube-prometheus-stack   → Label NÖTIG
scrapeConfigSelector     release=kube-prometheus-stack   → Label NÖTIG

probes-blackbox.yaml sagt das jetzt ausdrücklich, sonst lesen sich die korrigierten Kommentare anderswo als Freibrief, das Label auch dort zu entfernen — und dann verschwinden die Probes lautlos.

Zweiter Commit: der CNPG-Backup-Kommentar war widerlegt

Er machte #522 verantwortlich — Backup-CRs würden binnen einer Minute nach Abschluss gelöscht. Passiert nicht. Die CRs liegen da: 31 completed für ak-pg, im-pg und pprl-pg, 19 für tr-pg, 11 für wp-pg, 10 für ha-pg, die jüngsten von heute früh.

Die Metrik liest trotzdem überall 0. Die echte Ursache liegt eine Ebene höher: der Collector leitet sie aus Cluster.status.lastSuccessfulBackup ab, und dieses Feld fehlt auf jedem Cluster, weil alle mit method: plugin sichern und der barman-cloud-Plugin-Pfad es nie zurückschreibt. lastFailedBackup ist auf dreien gesetzt — deshalb sieht die Lücke nach „keine Backups" aus statt nach „kein Reporting".

Die Schlussfolgerung ändert sich nicht (der Alarm lässt sich auf dieser Metrik nicht bauen), aber der genannte Blocker schon. Wer das aufgegriffen hätte, hätte die CR-Retention untersucht und nichts gefunden.

Geprüft

kubectl apply --dry-run=server -k auf beide Kustomizations sauber. prune: true auf der apps-Kustomization ist bestätigt, die zwei entfernten ServiceMonitors verschwinden also beim Reconcile von selbst.


Separater Fund, nicht in diesem PR: monitoring/gf-pg (Grafanas Postgres) hat keinen ScheduledBackup. Sechs von sieben CNPG-Clustern werden gesichert, dieser nicht. Kann bewusst sein — die Dashboards liegen als ConfigMaps im Git — betrifft aber Nutzer, Präferenzen, Annotationen und Alert-State. Soll ich dazu eine Issue aufmachen?

Ich wollte zwei falsche Kommentare korrigieren und bin dabei auf ein laufendes Problem gestoßen. ## Der eigentliche Fund: doppeltes Scraping Seit `serviceMonitorSelector` auf `{}` gelockert wurde, findet Prometheus die ServiceMonitors, die der goauthentik.io-Operator selbst anlegt — genau deshalb wurde gelockert. Die handgeschriebenen Kopien für **ldap** und **paperless-gate**, damals nötig, weil der strenge Selector die Operator-eigenen verwarf, scrapen dieselben Endpunkte jetzt ein zweites Mal. Gemessen, nicht vermutet: | Target | Samples/10min | |---|---| | `10.244.4.113:9300` (paperless-gate) | **40** | | `10.244.4.76:9300` (ldap) | **40** | | alle übrigen Outposts | 20 | ldap hatte sogar ein *drittes* Target, weil der handgeschriebene Selector auch den normalen Service traf, nicht nur den `-metrics`-Service. Keine `duplicate timestamp`-Fehler — die zwei Pools scrapen zeitversetzt, die Samples kollidieren also nicht. Der Schaden ist doppelte Ingestion und eine Target-Liste, die über die Zahl der Outposts lügt. ## Radius bleibt — und das ist nicht dasselbe Der Operator-ServiceMonitor für radius selektiert auf `app.kubernetes.io/managed-by: goauthentik.io` **und** `goauthentik.io/service-type: metrics`. **Kein Service trägt beides**: der des Operators hat das erste, der hier definierte das zweite. Der handgeschriebene SM ist damit das Einzige, was radius überhaupt scraped — ihn mitzulöschen wäre ein stiller Verlust gewesen. Steht jetzt so im Kommentar. ## Die Kommentare, wegen derer ich hier angefangen habe Sechs Dateien behaupteten, das `release`-Label sei für die Discovery nötig. Ist es nicht — aber die Unterscheidung ist schärfer, als ich zuletzt gesagt habe. Live-Stand: ``` serviceMonitorSelector {} → Label egal podMonitorSelector {} → Label egal probeSelector release=kube-prometheus-stack → Label NÖTIG ruleSelector release=kube-prometheus-stack → Label NÖTIG scrapeConfigSelector release=kube-prometheus-stack → Label NÖTIG ``` `probes-blackbox.yaml` sagt das jetzt ausdrücklich, sonst lesen sich die korrigierten Kommentare anderswo als Freibrief, das Label auch dort zu entfernen — und dann verschwinden die Probes lautlos. ## Zweiter Commit: der CNPG-Backup-Kommentar war widerlegt Er machte #522 verantwortlich — Backup-CRs würden binnen einer Minute nach Abschluss gelöscht. **Passiert nicht.** Die CRs liegen da: 31 `completed` für ak-pg, im-pg und pprl-pg, 19 für tr-pg, 11 für wp-pg, 10 für ha-pg, die jüngsten von heute früh. Die Metrik liest trotzdem überall 0. Die echte Ursache liegt eine Ebene höher: der Collector leitet sie aus `Cluster.status.lastSuccessfulBackup` ab, und dieses Feld fehlt auf **jedem** Cluster, weil alle mit `method: plugin` sichern und der barman-cloud-Plugin-Pfad es nie zurückschreibt. `lastFailedBackup` ist auf dreien gesetzt — deshalb sieht die Lücke nach „keine Backups" aus statt nach „kein Reporting". Die Schlussfolgerung ändert sich nicht (der Alarm lässt sich auf dieser Metrik nicht bauen), aber der genannte Blocker schon. Wer das aufgegriffen hätte, hätte die CR-Retention untersucht und nichts gefunden. ## Geprüft `kubectl apply --dry-run=server -k` auf beide Kustomizations sauber. `prune: true` auf der apps-Kustomization ist bestätigt, die zwei entfernten ServiceMonitors verschwinden also beim Reconcile von selbst. --- **Separater Fund, nicht in diesem PR:** `monitoring/gf-pg` (Grafanas Postgres) hat **keinen ScheduledBackup**. Sechs von sieben CNPG-Clustern werden gesichert, dieser nicht. Kann bewusst sein — die Dashboards liegen als ConfigMaps im Git — betrifft aber Nutzer, Präferenzen, Annotationen und Alert-State. Soll ich dazu eine Issue aufmachen?
claude-bot added 2 commits 2026-08-28 07:01:31 +00:00
Chasing a stale comment turned up a live duplicate. Since serviceMonitorSelector
was loosened to {}, Prometheus discovers the goauthentik.io operator's own
ServiceMonitors — which is why it was loosened. The hand-written copies for ldap
and paperless-gate, added back when the strict selector was dropping the
operator's, now scrape the same endpoints a second time.

Measured, not assumed: 10.244.4.113:9300 and 10.244.4.76:9300 were each
producing 40 samples per 10 minutes against 20 for every other outpost, and
ldap picked up a third target because the hand-written selector matched the
plain Service as well as the -metrics one. No duplicate-timestamp errors,
because the two pools scrape at different offsets — the cost is doubled
ingestion and a target list that lies about how many outposts there are.

Radius stays, and the comment now says why it is not the same case: the
operator's ServiceMonitor for it selects on managed-by=goauthentik.io together
with service-type=metrics, and no Service carries both — the operator's has the
first, the one defined here has the second. It is the only thing scraping
radius, so deleting it would have been a silent loss.

Also corrects the comments that claimed the release label is required for
discovery. It is not, for ServiceMonitors and PodMonitors: those two selectors
are empty. It very much is for Probes, PrometheusRules and ScrapeConfigs, whose
selectors still match on it, so probes-blackbox.yaml now says so explicitly —
otherwise the corrected comments elsewhere read as license to drop the label
there too.

Live selector state, for the record:
  serviceMonitorSelector {}   podMonitorSelector {}
  probeSelector / ruleSelector / scrapeConfigSelector  release=kube-prometheus-stack
docs(monitoring): correct why the CNPG backup-age alert is absent
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
7f71e6655f
The comment blamed #522 — Backup CRs deleted within a minute of completing, so
CNPG has no record to derive the metric from. That is not what is happening.
The CRs are retained: 31 completed backups for ak-pg, im-pg and pprl-pg, 19 for
tr-pg, 11 for wp-pg, 10 for ha-pg, the newest of each from this morning.

The metric still reads 0 everywhere. The actual cause is one level up: the
collector derives it from Cluster.status.lastSuccessfulBackup, and that field
is absent on every Cluster because they all back up with method: plugin and the
barman-cloud plugin path never writes it back. lastFailedBackup is populated on
three of them, which is what makes the absence look like "no backups" rather
than "no reporting".

The conclusion does not change — the alert still cannot be built on that metric
— but the stated blocker does, and #522 is not it. Whoever picks this up next
would have gone looking at CR retention and found nothing wrong.

Also drops the incident retellings and the dated "verified on" stamps from this
file's comments, which is the cleanup already applied elsewhere in the repo.
review-bot left a comment
Member

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

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


Summary

This PR removes two hand-written ServiceMonitors (ldap, paperless-gate) in kubernetes/apps/authentik/outpost-servicemonitors.yaml that duplicated ServiceMonitors auto-created by the goauthentik.io operator now that serviceMonitorSelector is {}, and rewrites a batch of stale/misleading comments across 8 other monitoring manifests to reflect that reality (release: kube-prometheus-stack is not required for ServiceMonitor/PodMonitor discovery, but is still required for Probes/PrometheusRules). It also corrects a previously-wrong root-cause comment for the missing CNPG backup-age metric (blamed CR deletion via #522; actually Cluster.status.lastSuccessfulBackup is never populated because these clusters back up via the method: plugin / barman-cloud path). The radius outpost's hand-written Service+ServiceMonitor are correctly kept, with a new comment explaining why the operator's own ServiceMonitor can't cover it. No blocking issues found — everything outside the removed ldap/paperless-gate manifests is comment-only, verified against git diff --stat against the merge-base.

🟡 Suggestions

  • kubernetes/apps/authentik/outpost-servicemonitors.yaml:17-20: the new comment claims the operator's radius ServiceMonitor selects on app.kubernetes.io/managed-by: goauthentik.io and goauthentik.io/service-type: metrics, and that "the one below has the second" (i.e., the ak-outpost-local-radius-metrics Service defined in this same file carries goauthentik.io/service-type: metrics). Looking at that Service's actual labels: block (lines 26-29), it does not have a goauthentik.io/service-type: metrics label — only app.kubernetes.io/managed-by: homelab, goauthentik.io/outpost-name, goauthentik.io/outpost-type. This doesn't break anything functionally (the ServiceMonitor's selector.matchLabels only references labels that are present, and non-overlap with the operator's Service is still guaranteed via managed-by), but the comment's specific label claim isn't corroborated by the manifest it's pointing at, and the operator's runtime-created objects aren't visible in this repo to check directly. Worth a second look against the live cluster (kubectl -n authentik get svc -l goauthentik.io/service-type=metrics -o yaml or equivalent) since the PR's stated methodology is "measured, not guessed" elsewhere.

Tests

No tests exist or are expected for this repo (GitOps/Kustomize manifests); the PR body states kubectl apply --dry-run=server -k was run against both affected Kustomizations, which is the appropriate validation for this kind of change. The functional claim (operator-created ServiceMonitors already cover ldap/paperless-gate, eliminating double-scrape) is asserted from live Prometheus target/sample counts in the PR description rather than something checkable from this checkout — reasonable given it depends on runtime operator behavior, but worth confirming post-merge that the 10.244.4.113:9300 / 10.244.4.76:9300 targets drop back to one series each and the third (non-metrics) ldap target is gone.

Recommendation

Looks reasonable — no blocking issues; comment above is optional.

🤖 **Automated review** — model `claude-sonnet-5` (complexity: normal), commit `7f71e6655f3a8f72eabf1ebc486023f8dce6a1dd`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR removes two hand-written `ServiceMonitor`s (ldap, paperless-gate) in `kubernetes/apps/authentik/outpost-servicemonitors.yaml` that duplicated ServiceMonitors auto-created by the goauthentik.io operator now that `serviceMonitorSelector` is `{}`, and rewrites a batch of stale/misleading comments across 8 other monitoring manifests to reflect that reality (`release: kube-prometheus-stack` is not required for ServiceMonitor/PodMonitor discovery, but is still required for Probes/PrometheusRules). It also corrects a previously-wrong root-cause comment for the missing CNPG backup-age metric (blamed CR deletion via #522; actually `Cluster.status.lastSuccessfulBackup` is never populated because these clusters back up via the `method: plugin` / barman-cloud path). The `radius` outpost's hand-written Service+ServiceMonitor are correctly kept, with a new comment explaining why the operator's own ServiceMonitor can't cover it. No blocking issues found — everything outside the removed ldap/paperless-gate manifests is comment-only, verified against `git diff --stat` against the merge-base. ## 🟡 Suggestions - `kubernetes/apps/authentik/outpost-servicemonitors.yaml:17-20`: the new comment claims the operator's radius ServiceMonitor selects on `app.kubernetes.io/managed-by: goauthentik.io` **and** `goauthentik.io/service-type: metrics`, and that "the one below has the second" (i.e., the `ak-outpost-local-radius-metrics` Service defined in this same file carries `goauthentik.io/service-type: metrics`). Looking at that Service's actual `labels:` block (lines 26-29), it does **not** have a `goauthentik.io/service-type: metrics` label — only `app.kubernetes.io/managed-by: homelab`, `goauthentik.io/outpost-name`, `goauthentik.io/outpost-type`. This doesn't break anything functionally (the ServiceMonitor's `selector.matchLabels` only references labels that *are* present, and non-overlap with the operator's Service is still guaranteed via `managed-by`), but the comment's specific label claim isn't corroborated by the manifest it's pointing at, and the operator's runtime-created objects aren't visible in this repo to check directly. Worth a second look against the live cluster (`kubectl -n authentik get svc -l goauthentik.io/service-type=metrics -o yaml` or equivalent) since the PR's stated methodology is "measured, not guessed" elsewhere. ## Tests No tests exist or are expected for this repo (GitOps/Kustomize manifests); the PR body states `kubectl apply --dry-run=server -k` was run against both affected Kustomizations, which is the appropriate validation for this kind of change. The functional claim (operator-created ServiceMonitors already cover ldap/paperless-gate, eliminating double-scrape) is asserted from live Prometheus target/sample counts in the PR description rather than something checkable from this checkout — reasonable given it depends on runtime operator behavior, but worth confirming post-merge that the `10.244.4.113:9300` / `10.244.4.76:9300` targets drop back to one series each and the third (non-metrics) ldap target is gone. ## Recommendation Looks reasonable — no blocking issues; comment above is optional. <!-- pr-review:sha=7f71e6655f3a8f72eabf1ebc486023f8dce6a1dd -->
k.renner merged commit 8598f1ad65 into master 2026-08-28 07:04:18 +00:00
Sign in to join this conversation.