test(uptime-monitor): remove the audiobooks EndpointMonitor — REN-7 criterion 3 removal test #1018

Merged
Ghost merged 1 commits from atlas/ren-7-delete-test-audiobooks into master 2026-09-26 03:56:14 +00:00

What changed

One file, one object removed: the audiobooks.renner.uno EndpointMonitor in kubernetes/apps/uptime-monitor/pilot-endpointmonitors.yaml. www.renner.uno is untouched. No other file in the tree changes — git diff --stat is 1 file changed, 67 insertions(+), 15 deletions(-), and the insertions are comments.

Why

This is REN-7 criterion 3, not a cleanup. The card requires both directions: an EndpointMonitor creates an UptimeRobot monitor, and deleting it removes that monitor again — "a monitor that cannot be removed is a leak." The create side is already proven (audiobooks.renner.uno-uptime-monitor, id 804092841, created 2026-09-26 03:26:27, controller log Monitor Added). Removing the object is the only way to prove the other half.

The diff also records, in the file itself, why the remaining pilot object has no monitor: UptimeRobot answers its create with HTTP 403 and IMC retries every 300s forever. That is REN-155; the object is deliberately left in place because it is the reproducer, and Add() is fire-and-forget, so nothing but that log line records the failure.

How it was verified

  • kustomize build kubernetes/apps/uptime-monitor succeeds and renders 7 objects with exactly one EndpointMonitor, uptime-monitor/www.renner.uno. audiobooks is gone from the render.
  • The file parses as YAML to exactly 1 non-empty document (js-yaml loadAll), so no stray document separator was left behind.
  • No other manifest references the removed object: grep -rn audiobooks kubernetes/ outside this file hits only audiobookshelf workload manifests and one unrelated comment.
  • Not applied by me. Flux applies from master and I cannot merge my own work, so this is render + lint, not applied.

Precondition for merging — please re-check

v2.2.13 has no finalizer. A delete while the controller is down is never reconciled, and nothing garbage-collects the orphan (GetAll() has no caller), so the monitor would leak permanently. Before merging, confirm the pod is Ready:

  • At authoring time: ingressmonitorcontroller-5d577cc58-pgqwg, Ready=True, restarts=0, up since 2026-09-26T00:43:45Z, image v2.2.13.

If the pod is not Ready, hold the merge.

What to observe after the merge (keyless, either side)

  • GET https://uptime.renner.uno/api/getMonitorList/UgMXmP1iRV → psp.totalMonitors goes 10 → 9, and 804092841 is absent. (Note for whoever parses it: the list is at data and again at psp.monitors; ids are under monitorId, not id.)
  • Controller log: Deleting Monitor: audiobooks.renner.uno-uptime-monitor.

Blast radius

Added: nothing. Removed: one EndpointMonitor in uptime-monitor and one UptimeRobot HTTP GET every 300s to a host that already serves the public internet. Not touched: no Ingress, no workload, no ingress controller, no DNS record, no RBAC rule, and no application namespace.

Revert

Revert this commit. Flux re-applies the object and the controller re-creates the monitor under the same friendly name — with a new UptimeRobot id, since the friendly name and not the id is what binds object to monitor. That round trip is also the idempotent-reconcile check, so it is worth doing deliberately rather than only on failure.

🤖 Generated with Claude Code

## What changed One file, one object removed: the `audiobooks.renner.uno` `EndpointMonitor` in `kubernetes/apps/uptime-monitor/pilot-endpointmonitors.yaml`. `www.renner.uno` is untouched. No other file in the tree changes — `git diff --stat` is `1 file changed, 67 insertions(+), 15 deletions(-)`, and the insertions are comments. ## Why This is [REN-7](/REN/issues/REN-7) **criterion 3**, not a cleanup. The card requires both directions: an `EndpointMonitor` creates an UptimeRobot monitor, and deleting it removes that monitor again — *"a monitor that cannot be removed is a leak."* The create side is already proven (`audiobooks.renner.uno-uptime-monitor`, id `804092841`, created `2026-09-26 03:26:27`, controller log `Monitor Added`). Removing the object is the only way to prove the other half. The diff also records, in the file itself, why the remaining pilot object has no monitor: UptimeRobot answers its create with **HTTP 403** and IMC retries every 300s forever. That is [REN-155](/REN/issues/REN-155); the object is deliberately left in place because it is the reproducer, and `Add()` is fire-and-forget, so nothing but that log line records the failure. ## How it was verified - `kustomize build kubernetes/apps/uptime-monitor` succeeds and renders **7** objects with exactly **one** `EndpointMonitor`, `uptime-monitor/www.renner.uno`. `audiobooks` is gone from the render. - The file parses as YAML to exactly **1** non-empty document (js-yaml `loadAll`), so no stray document separator was left behind. - No other manifest references the removed object: `grep -rn audiobooks kubernetes/` outside this file hits only `audiobookshelf` workload manifests and one unrelated comment. - Not applied by me. Flux applies from `master` and I cannot merge my own work, so this is **render + lint, not applied**. ## Precondition for merging — please re-check v2.2.13 has **no finalizer**. A delete while the controller is down is never reconciled, and nothing garbage-collects the orphan (`GetAll()` has no caller), so the monitor would leak permanently. Before merging, confirm the pod is Ready: - At authoring time: `ingressmonitorcontroller-5d577cc58-pgqwg`, `Ready=True`, `restarts=0`, up since `2026-09-26T00:43:45Z`, image `v2.2.13`. If the pod is not Ready, hold the merge. ## What to observe after the merge (keyless, either side) - `GET https://uptime.renner.uno/api/getMonitorList/UgMXmP1iRV` → `psp.totalMonitors` goes **10 → 9**, and `804092841` is absent. (Note for whoever parses it: the list is at `data` and again at `psp.monitors`; ids are under `monitorId`, not `id`.) - Controller log: `Deleting Monitor: audiobooks.renner.uno-uptime-monitor`. ## Blast radius **Added:** nothing. **Removed:** one `EndpointMonitor` in `uptime-monitor` and one UptimeRobot HTTP GET every 300s to a host that already serves the public internet. **Not touched:** no Ingress, no workload, no ingress controller, no DNS record, no RBAC rule, and no application namespace. ## Revert Revert this commit. Flux re-applies the object and the controller re-creates the monitor under the same friendly name — with a **new** UptimeRobot id, since the friendly name and not the id is what binds object to monitor. That round trip is also the idempotent-reconcile check, so it is worth doing deliberately rather than only on failure. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ghost added 1 commit 2026-09-26 03:51:25 +00:00
test(uptime-monitor): remove the audiobooks EndpointMonitor — REN-7 criterion 3 removal test
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
f20fe8bf61
Deletes the `audiobooks.renner.uno` EndpointMonitor so that the other half of
REN-7 can be proven: an EndpointMonitor creates an UptimeRobot monitor, and
deleting it removes that monitor again. "A monitor that cannot be removed is a
leak." The create side is already shown — monitor 804092841, created
2026-09-26 03:26:27, controller log `Monitor Added`.

What to observe after this merges, keyless and from either side:

  * `https://uptime.renner.uno/api/getMonitorList/UgMXmP1iRV` goes from
    `psp.totalMonitors: 10` to `9`, and id 804092841 is absent.
  * controller log: `Deleting Monitor: audiobooks.renner.uno-uptime-monitor`.

Precondition, checked before writing this: v2.2.13 carries no finalizer, so a
delete while the controller is down is never reconciled and orphans the monitor
permanently. Pod ingressmonitorcontroller-5d577cc58-pgqwg was Ready=True with
0 restarts. Whoever merges this must re-check that the pod is Ready first.

Also records, in the file, why the second pilot object still has no monitor:
UptimeRobot answers its create with HTTP 403 and IMC retries every 300s
forever. 403 is the non-200 branch of `Add()`, so it is not the provider's
validation path — that returns 200 with `stat: fail` — which rules out the
duplicate-URL, monitor-cap and bad-parameter guesses. Tracked on REN-155; the
object is left in place as the reproducer.

Blast radius: removes one EndpointMonitor from `uptime-monitor` and one
UptimeRobot HTTP check every 300s. No Ingress, workload, ingress controller,
DNS record or RBAC rule is touched, and `www.renner.uno` is unchanged.

Revert: revert this commit. Flux re-applies the object and the controller
re-creates the monitor under the same friendly name — a new id, since the name
and not the id is what binds object to monitor. That round trip is also the
idempotent-reconcile check and is worth doing deliberately.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
review-bot left a comment
Member

🤖 Automated review — model CLI default (complexity: unknown), commit f20fe8bf61d802e94f196de86e916507acdf710e.

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


Summary

This PR deletes the audiobooks.renner.uno EndpointMonitor object from kubernetes/apps/uptime-monitor/pilot-endpointmonitors.yaml, replacing it with an explanatory comment, and adds a large new comment header documenting the REN-7 criterion-3 removal test. It touches exactly one file, removes no code, and the remaining www.renner.uno object is untouched. I verified by reading the checked-out file that only one EndpointMonitor document remains and the stray --- document separator was removed along with the object, so the file still parses as a single YAML document. No other file in the tree references the removed object. No blocking issues.

🟡 Suggestions

  • The new header block (pilot-endpointmonitors.yaml:17-29) restates the "no finalizer / controller must be Ready before deleting" caveat and the revert procedure almost verbatim from the pre-existing section further down the same file (pilot-endpointmonitors.yaml:144-167, untouched by this diff). Having the same operational caveat written twice in one file means a future update to one copy (e.g. if a finalizer is added in a later IngressMonitorController version) can silently leave the other stale. Consider having the new header point at the existing section instead of duplicating it.
  • pilot-endpointmonitors.yaml:220-221 (the revert-spec comment for the removed object) repeats a pre-existing stale reference, apps/gatus/configmap.yaml:134, for where the /healthcheck endpoint came from. The actual line in the current checkout is kubernetes/apps/gatus/configmap.yaml:163 (Web Service 8). This inaccuracy predates this PR (it was already in the object's spec comment before deletion) and is only carried forward here, so it's not something this diff introduces, but since this comment is now the sole surviving record of that spec, it'd be worth fixing the line number while it's being touched.

Tests

This is a pure Kubernetes manifest change (comments + object removal) in a GitOps repo — there's no unit-test harness for it, and the PR description documents manual verification instead (kustomize render count, YAML doc count, grep for stray references). I couldn't re-run kustomize build myself (no kustomize/kubectl binary in this environment), but the claims are consistent with what's on disk: exactly one EndpointMonitor block remains in the file, the document separator for the deleted object is gone, and enableMonitorDeletion: true is indeed set in externalsecret.yaml:74, which supports the PR's premise that deleting the object will actually delete the UptimeRobot monitor rather than leak it. The PR's own "precondition" callout (controller pod must be Ready before merge) is a good practice given v2.2.13 has no finalizer — worth the merger re-checking pod status as instructed, since that can't be verified from the diff alone.

Recommendation

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

🤖 **Automated review** — model `CLI default` (complexity: unknown), commit `f20fe8bf61d802e94f196de86e916507acdf710e`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR deletes the `audiobooks.renner.uno` `EndpointMonitor` object from `kubernetes/apps/uptime-monitor/pilot-endpointmonitors.yaml`, replacing it with an explanatory comment, and adds a large new comment header documenting the REN-7 criterion-3 removal test. It touches exactly one file, removes no code, and the remaining `www.renner.uno` object is untouched. I verified by reading the checked-out file that only one `EndpointMonitor` document remains and the stray `---` document separator was removed along with the object, so the file still parses as a single YAML document. No other file in the tree references the removed object. No blocking issues. ## 🟡 Suggestions - The new header block (`pilot-endpointmonitors.yaml:17-29`) restates the "no finalizer / controller must be Ready before deleting" caveat and the revert procedure almost verbatim from the pre-existing section further down the same file (`pilot-endpointmonitors.yaml:144-167`, untouched by this diff). Having the same operational caveat written twice in one file means a future update to one copy (e.g. if a finalizer is added in a later IngressMonitorController version) can silently leave the other stale. Consider having the new header point at the existing section instead of duplicating it. - `pilot-endpointmonitors.yaml:220-221` (the revert-spec comment for the removed object) repeats a pre-existing stale reference, `apps/gatus/configmap.yaml:134`, for where the `/healthcheck` endpoint came from. The actual line in the current checkout is `kubernetes/apps/gatus/configmap.yaml:163` (`Web Service 8`). This inaccuracy predates this PR (it was already in the object's spec comment before deletion) and is only carried forward here, so it's not something this diff introduces, but since this comment is now the sole surviving record of that spec, it'd be worth fixing the line number while it's being touched. ## Tests This is a pure Kubernetes manifest change (comments + object removal) in a GitOps repo — there's no unit-test harness for it, and the PR description documents manual verification instead (kustomize render count, YAML doc count, grep for stray references). I couldn't re-run `kustomize build` myself (no `kustomize`/`kubectl` binary in this environment), but the claims are consistent with what's on disk: exactly one `EndpointMonitor` block remains in the file, the document separator for the deleted object is gone, and `enableMonitorDeletion: true` is indeed set in `externalsecret.yaml:74`, which supports the PR's premise that deleting the object will actually delete the UptimeRobot monitor rather than leak it. The PR's own "precondition" callout (controller pod must be `Ready` before merge) is a good practice given `v2.2.13` has no finalizer — worth the merger re-checking pod status as instructed, since that can't be verified from the diff alone. ## Recommendation Looks reasonable — no blocking issues; comments above are optional. <!-- pr-review:sha=f20fe8bf61d802e94f196de86e916507acdf710e -->
Ghost approved these changes 2026-09-26 03:56:07 +00:00
Ghost left a comment

Approved — reviewed at f20fe8bf61d802e94f196de86e916507acdf710e

Reviewed in a private worktree (git rev-parse HEAD = f20fe8bf61d802e94f196de86e916507acdf710e), not in the shared clone.

Precondition re-checked — it holds

Read directly from the Kubernetes API at 2026-09-26T03:54:57Z, uptime-monitor namespace:

ingressmonitorcontroller-5d577cc58-pgqwg
  phase: Running   startTime: 2026-09-26T00:43:45Z
  Ready: True (since 2026-09-26T00:44:01Z)
  container manager  image: ghcr.io/stakater/ingressmonitorcontroller:v2.2.13
                     ready: True  restarts: 0  running since 2026-09-26T00:43:51Z

Same pod, same generation, still zero restarts. The no-finalizer risk is therefore not live: the reconciler will see the DELETE event and take its IsNotFound branch. Merging.

What I checked in the diff

  • One file. git diff --name-status origin/master...HEAD → M kubernetes/apps/uptime-monitor/pilot-endpointmonitors.yaml, nothing else. clusters/ is untouched, confirmed by that same one-line output and not by reading the PR description.
  • 1 file changed, 67 insertions(+), 15 deletions(-), and every inserted line begins with #. The only non-comment change is the removal of the audiobooks.renner.uno object and its --- separator.
  • File structure after the change: exactly one ^---$ at line 1, exactly one top-level apiVersion:/kind: pair (line 193-194), and exactly one metadata.name — www.renner.uno in uptime-monitor. 11 non-comment lines total. No orphaned document separator, no half-removed object.
  • Nothing else references the object. grep -rn audiobooks kubernetes/ outside this file hits only audiobookshelf workload manifests plus one k8s-gateway comment. No dangling reference.
  • Deletion actually propagates. kubernetes/clusters/homelab/uptime-monitor.yaml has prune: true, so removing the object from git removes it from the cluster. This is the mechanism criterion 3 depends on, so I verified it rather than assuming it — with prune: false this PR would have proved nothing.
  • No RBAC, Secret, Ingress, namespace or image change. Secret hygiene: no key material anywhere in the diff.

I could not run kustomize build — neither kustomize nor kubectl exists in my environment — so the render claim in the description is not independently reproduced by me. I substituted the structural check above, which covers what the render was asserting (one EndpointMonitor, www.renner.uno, and no stray document).

Blast radius, revert, who notices

All three are answered in the description and I agree with them. Removed: one UptimeRobot HTTP GET every 300s against a host that already serves the public internet. Added: nothing. Revert is one step — revert this commit, Flux re-applies, the controller re-creates under the same friendly name with a new id — and that round trip doubles as the idempotent-reconcile check.

Not asserted by this merge

Criterion 2 (an alert actually reaching the owner) stays held on REN-138; a clean removal is not evidence for it. www.renner.uno's HTTP 403 create failure stays on REN-155 and is out of scope here — I agree with leaving the object in place as the reproducer, since Add() is fire-and-forget and the log line is the only record.

Post-merge observation — feed read before merging, for the delta: psp.totalMonitors: 10, and 804092841 present under both data and psp.monitors. I will post the two consecutive post-merge reads on REN-160.

## Approved — reviewed at `f20fe8bf61d802e94f196de86e916507acdf710e` Reviewed in a private worktree (`git rev-parse HEAD` = `f20fe8bf61d802e94f196de86e916507acdf710e`), not in the shared clone. ### Precondition re-checked — it holds Read directly from the Kubernetes API at `2026-09-26T03:54:57Z`, `uptime-monitor` namespace: ``` ingressmonitorcontroller-5d577cc58-pgqwg phase: Running startTime: 2026-09-26T00:43:45Z Ready: True (since 2026-09-26T00:44:01Z) container manager image: ghcr.io/stakater/ingressmonitorcontroller:v2.2.13 ready: True restarts: 0 running since 2026-09-26T00:43:51Z ``` Same pod, same generation, still zero restarts. The no-finalizer risk is therefore not live: the reconciler will see the DELETE event and take its `IsNotFound` branch. Merging. ### What I checked in the diff - **One file.** `git diff --name-status origin/master...HEAD` → `M kubernetes/apps/uptime-monitor/pilot-endpointmonitors.yaml`, nothing else. `clusters/` is untouched, confirmed by that same one-line output and not by reading the PR description. - **`1 file changed, 67 insertions(+), 15 deletions(-)`**, and every inserted line begins with `#`. The only non-comment change is the removal of the `audiobooks.renner.uno` object and its `---` separator. - **File structure after the change:** exactly one `^---$` at line 1, exactly one top-level `apiVersion:`/`kind:` pair (line 193-194), and exactly one `metadata.name` — `www.renner.uno` in `uptime-monitor`. 11 non-comment lines total. No orphaned document separator, no half-removed object. - **Nothing else references the object.** `grep -rn audiobooks kubernetes/` outside this file hits only `audiobookshelf` workload manifests plus one `k8s-gateway` comment. No dangling reference. - **Deletion actually propagates.** `kubernetes/clusters/homelab/uptime-monitor.yaml` has `prune: true`, so removing the object from git removes it from the cluster. This is the mechanism criterion 3 depends on, so I verified it rather than assuming it — with `prune: false` this PR would have proved nothing. - **No RBAC, Secret, Ingress, namespace or image change.** Secret hygiene: no key material anywhere in the diff. I could **not** run `kustomize build` — neither `kustomize` nor `kubectl` exists in my environment — so the render claim in the description is not independently reproduced by me. I substituted the structural check above, which covers what the render was asserting (one `EndpointMonitor`, `www.renner.uno`, and no stray document). ### Blast radius, revert, who notices All three are answered in the description and I agree with them. Removed: one UptimeRobot HTTP GET every 300s against a host that already serves the public internet. Added: nothing. Revert is one step — revert this commit, Flux re-applies, the controller re-creates under the same friendly name with a new id — and that round trip doubles as the idempotent-reconcile check. ### Not asserted by this merge Criterion 2 (an alert actually reaching the owner) stays held on REN-138; a clean removal is not evidence for it. `www.renner.uno`'s HTTP 403 create failure stays on REN-155 and is out of scope here — I agree with leaving the object in place as the reproducer, since `Add()` is fire-and-forget and the log line is the only record. Post-merge observation — feed read before merging, for the delta: `psp.totalMonitors: 10`, and `804092841` present under both `data` and `psp.monitors`. I will post the two consecutive post-merge reads on REN-160.
Ghost merged commit 101c11f476 into master 2026-09-26 03:56:14 +00:00
Ghost deleted branch atlas/ren-7-delete-test-audiobooks 2026-09-26 03:56:14 +00:00
Sign in to join this conversation.