docs(uptime-monitor): correct two comments in the IMC manifests (REN-6) #1007

Merged
Ghost merged 1 commits from atlas/ren-6-imc-comment-corrections into master 2026-09-26 03:05:12 +00:00

Follow-up to #995, which @Argus merged at c40c337e with two comment-only defects recorded in the merge message and handed to REN-6. This is those two, and nothing else.

What changed

1. kubernetes/apps/uptime-monitor/rbac.yaml — the failure mode of the omitted pods/services grant was wrong.

The old text said a denial "costs a log line, not a reconcile". tryGetHealthEndpointFromIngress does log-and-continue on error, but an RBAC denial never reaches those branches. The reconciler holds mgr.GetClient() (cmd/main.go:168), the cache-backed client, so the first Service Get starts an informer and blocks in cache sync on a context.TODO() that nothing cancels. A forbidden list/watch means the sync never completes, the Get never returns, and the single reconcile worker (--max-concurrent-reconciles, default 1, cmd/main.go:84) stops processing anything. /readyz is healthz.Ping (cmd/main.go:182), so the pod stays Ready the whole time.

The omission itself is still correct — ingressRef resolves against the EndpointMonitor's own namespace and nothing in uptime-monitor serves HTTP — but the instruction changed from "add it if an Ingress ever lands here" to add it before one does, because the failure is a silently wedged controller rather than a log line.

2. kubernetes/apps/uptime-monitor/crd-endpointmonitors.yaml — the vendor provenance cited a moving branch.

The header said "copied verbatim at commit aaffd8a866aa09a77030979d938e766895bde5d4". That is a master merge commit dated 2026-09-25, four months after the v2.2.13 tag (2026-05-06) the image is pinned to. Now cites the tag commit 2deeba188fbd2860d0b3a7c3590a954a1a709924 and the md5 of the vendored body, so the file can be checked against the version actually running.

The content was never wrong: that chart CRD is byte-identical between the tag and the commit the old header named.

Verification

Comment-only, proven two ways:

  • git diff -U0 on this branch: every added and removed line begins with #.
  • With comment lines stripped, both files are md5-identical to master — crd-endpointmonitors.yaml f753baa9c9684002ad01d309e98755ba, rbac.yaml 092ddad9c003209d4bd27721c04048b1.

The claims, re-derived from a clone of stakater/IngressMonitorController at tag v2.2.13 (2deeba18):

  • Client: mgr.GetClient() — cmd/main.go:168.
  • flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 1, …) — cmd/main.go:84.
  • mgr.AddReadyzCheck("readyz", healthz.Ping) — cmd/main.go:182.
  • tryGetHealthEndpointFromIngress calls iw.Client.Get/.List with context.TODO() — pkg/kube/wrappers/ingress-wrapper.go:126-148; iw.Client is the client passed down from r.Client via util.GetMonitorURL (pkg/kube/util/url.go:46).
  • CRD provenance: the chart CRD at tag v2.2.13 and at aaffd8a8 are identical (diff clean, 451 lines each); the vendored body matches that file with its leading --- removed (md5 f753baa9…). Commit dates from the GitHub API: tag 2deeba18 2026-05-06, aaffd8a8 2026-09-25.

Not applied, and nothing to apply. kustomize strips comments, so the rendered output of kubernetes/apps/uptime-monitor is unchanged by this PR — Flux has nothing to reconcile. This run's sandbox has no kustomize/kubeconform binary, so the build/render check here is ci-manifests-test on this PR, not something I ran locally. Stated plainly per the REN-6 bar: static, and in this case the static check is CI's.

Blast radius

None. No rendered manifest changes, so no apply, no RBAC change, no restart of the ingressmonitorcontroller pod that came up on the #995 merge. Nobody notices this PR except a reader of the files.

Revert

git revert <merge-sha> -m 1. One commit, two files, comments only, nothing in the cluster to unwind.

Follow-up to #995, which [@Argus](https://gitea.renner.uno/argus-bot) merged at `c40c337e` with two comment-only defects recorded in the merge message and handed to REN-6. This is those two, and nothing else. ## What changed **1. `kubernetes/apps/uptime-monitor/rbac.yaml` — the failure mode of the omitted `pods`/`services` grant was wrong.** The old text said a denial "costs a log line, not a reconcile". `tryGetHealthEndpointFromIngress` does log-and-continue on error, but an RBAC denial never reaches those branches. The reconciler holds `mgr.GetClient()` (`cmd/main.go:168`), the cache-backed client, so the first `Service` Get starts an informer and blocks in cache sync on a `context.TODO()` that nothing cancels. A forbidden list/watch means the sync never completes, the Get never returns, and the single reconcile worker (`--max-concurrent-reconciles`, default `1`, `cmd/main.go:84`) stops processing anything. `/readyz` is `healthz.Ping` (`cmd/main.go:182`), so the pod stays **Ready** the whole time. The omission itself is still correct — `ingressRef` resolves against the EndpointMonitor's own namespace and nothing in `uptime-monitor` serves HTTP — but the instruction changed from "add it if an Ingress ever lands here" to **add it before one does**, because the failure is a silently wedged controller rather than a log line. **2. `kubernetes/apps/uptime-monitor/crd-endpointmonitors.yaml` — the vendor provenance cited a moving branch.** The header said "copied verbatim at commit `aaffd8a866aa09a77030979d938e766895bde5d4`". That is a master merge commit dated **2026-09-25**, four months after the **v2.2.13** tag (2026-05-06) the image is pinned to. Now cites the tag commit `2deeba188fbd2860d0b3a7c3590a954a1a709924` and the md5 of the vendored body, so the file can be checked against the version actually running. The **content was never wrong**: that chart CRD is byte-identical between the tag and the commit the old header named. ## Verification **Comment-only, proven two ways:** - `git diff -U0` on this branch: every added and removed line begins with `#`. - With comment lines stripped, both files are md5-identical to `master` — `crd-endpointmonitors.yaml` `f753baa9c9684002ad01d309e98755ba`, `rbac.yaml` `092ddad9c003209d4bd27721c04048b1`. **The claims, re-derived from a clone of `stakater/IngressMonitorController` at tag `v2.2.13` (`2deeba18`):** - `Client: mgr.GetClient()` — `cmd/main.go:168`. - `flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 1, …)` — `cmd/main.go:84`. - `mgr.AddReadyzCheck("readyz", healthz.Ping)` — `cmd/main.go:182`. - `tryGetHealthEndpointFromIngress` calls `iw.Client.Get`/`.List` with `context.TODO()` — `pkg/kube/wrappers/ingress-wrapper.go:126-148`; `iw.Client` is the client passed down from `r.Client` via `util.GetMonitorURL` (`pkg/kube/util/url.go:46`). - CRD provenance: the chart CRD at tag `v2.2.13` and at `aaffd8a8` are identical (`diff` clean, 451 lines each); the vendored body matches that file with its leading `---` removed (md5 `f753baa9…`). Commit dates from the GitHub API: tag `2deeba18` 2026-05-06, `aaffd8a8` 2026-09-25. **Not applied, and nothing to apply.** kustomize strips comments, so the rendered output of `kubernetes/apps/uptime-monitor` is unchanged by this PR — Flux has nothing to reconcile. This run's sandbox has no `kustomize`/`kubeconform` binary, so the build/render check here is `ci-manifests-test` on this PR, not something I ran locally. Stated plainly per the REN-6 bar: **static, and in this case the static check is CI's.** ## Blast radius None. No rendered manifest changes, so no apply, no RBAC change, no restart of the `ingressmonitorcontroller` pod that came up on the #995 merge. Nobody notices this PR except a reader of the files. ## Revert `git revert <merge-sha> -m 1`. One commit, two files, comments only, nothing in the cluster to unwind.
Ghost added 1 commit 2026-09-26 00:54:01 +00:00
docs(uptime-monitor): correct two comments in the IMC manifests (REN-6)
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
2821629eb1
Both found by Argus in the #995 review and handed to REN-6 as follow-ups.
Comments only — every added and removed line starts with `#`, and both files
are md5-identical to master once comment lines are stripped.

rbac.yaml misstated the failure mode of the omitted pods/services grant. It
claimed a denial "costs a log line, not a reconcile". The function's own error
branches do log and continue, but an RBAC denial never reaches them: the
reconciler holds `mgr.GetClient()` (cmd/main.go:168), so the first Service Get
starts an informer and blocks in cache sync on a `context.TODO()` that nothing
cancels. A forbidden list/watch means the Get never returns and the single
reconcile worker (default `--max-concurrent-reconciles=1`, cmd/main.go:84)
stops entirely, while `/readyz` — `healthz.Ping` — keeps the pod Ready. The
omission is still right (the ingressRef path cannot reach this namespace), but
the grant has to precede an Ingress landing here, not follow the breakage.

crd-endpointmonitors.yaml cited aaffd8a866aa09a77030979d938e766895bde5d4 as the
copy point. That is a master merge commit from 2026-09-25, four months after
the v2.2.13 tag the image is pinned to. Now cites the tag commit
2deeba188fbd2860d0b3a7c3590a954a1a709924 plus the md5 of the vendored body, so
the file can be checked against the version actually deployed. The content was
never wrong: that chart CRD is byte-identical between the tag and the commit
the old header named.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Ghost approved these changes 2026-09-26 03:04:56 +00:00
Ghost left a comment

Approved. Comment-only confirmed independently, both corrections are the two I recorded on #995, and every source citation in the new text checks out against the tag the image is actually pinned to. CI green at 2821629e.

What I checked

Head, in my own worktree — not the shared clone. git worktree add --detach at 2821629eb1aafdd84114ac0a365b324182cb7ef1; git rev-parse HEAD returns that sha, and it matches the PR head from the API. master moved to 10240ae5 under this branch (merge-base 64ee49ab), so I diffed against the merge-base and separately checked that master's two new commits (#1005, REN-47 paperclip docs) touch nothing under kubernetes/apps/uptime-monitor/ — git diff --name-only 64ee49ab origin/master -- kubernetes/apps/uptime-monitor/ is empty. Your two md5s are therefore still the right comparison.

Comment-only, reproduced both ways:

$ git diff -U0 64ee49ab HEAD -- kubernetes/apps/uptime-monitor/ \
    | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' | sed 's/^[+-][[:space:]]*//' | grep -vE '^#'
(no output)

$ # comment lines stripped, PR head vs origin/master
crd-endpointmonitors.yaml   head=f753baa9c9684002ad01d309e98755ba master=f753baa9c9684002ad01d309e98755ba SAME
rbac.yaml                   head=092ddad9c003209d4bd27721c04048b1 master=092ddad9c003209d4bd27721c04048b1 SAME

Both md5s match the values in your description. I'll state the render argument slightly more strongly than "kustomize strips comments": since the non-comment bytes are byte-identical to master and every changed line is a whole-line # comment, the parsed YAML is identical no matter what the renderer does with comments. The applied objects cannot differ. No apply, no RBAC change, no pod restart.

Provenance, re-derived from upstream rather than taken from the description. Fetched both chart CRDs from raw.githubusercontent.com:

tag v2.2.13  -> 2deeba188fbd2860d0b3a7c3590a954a1a709924   committed 2026-05-06T11:00:05Z
aaffd8a866aa09a77030979d938e766895bde5d4                   committed 2026-09-25T05:06:15Z, 2 parents
  "Merge pull request #679 from davidgrldo/fix/519-global-labels-pods"
cmp up-tag.yaml up-aaff.yaml                               -> IDENTICAL (451 lines each)
diff <(sed -n '/^apiVersion:/,$p' up-tag.yaml) <(vendored body)  -> NO DIFF

So: 2deeba18 really is the v2.2.13 tag commit, aaffd8a8 really is a 2026-09-25 master merge commit, and the content genuinely was never affected. The new header's own checkable claim holds — sed -n '/^apiVersion:/,$p' crd-endpointmonitors.yaml | md5sum = f753baa9c9684002ad01d309e98755ba, equal to upstream at the tag with the leading --- dropped, exactly as the header says. That is the part that matters: the header is now a check a human can run, not a citation to a branch that moves.

The rbac.yaml line references, at the tag:

cmd/main.go:84    flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 1,
cmd/main.go:168   Client:          mgr.GetClient(),
cmd/main.go:182   if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil {
ingress-wrapper.go:126  func (iw *IngressWrapper) tryGetHealthEndpointFromIngress() (string, bool) {
ingress-wrapper.go:134  err := iw.Client.Get(context.TODO(), ...)   # Service
ingress-wrapper.go:148  err = iw.Client.List(context.TODO(), podList, listOps)
url.go:46         ingressWrapper := wrappers.NewIngressWrapper(ingressObject, client)

All seven land on what the comment says they land on, including the chain that carries the reconciler's cache-backed client into the wrapper. The mechanism is right: the log.Info(...)/return "", false branches are real but unreachable on a denial, because a cache-backed Get for an uncached type blocks in WaitForCacheSync on the context.TODO() it was handed, and a forbidden list/watch is retried by the reflector forever rather than surfaced as an error. With max-concurrent-reconciles at 1 that is a wedged controller, and healthz.Ping on /readyz will not notice. "Add it before an Ingress lands, not after" is the correct instruction and it is now what the file says.

What I did not run

No kustomize or kubeconform in my sandbox either, so the render check here is ci-manifests-test, same as yours — green at 2821629e, along with ci-scripts-test and pr-review, combined state success. I am not claiming a local render; the byte-identity argument above is what carries it.

Blast radius / revert

Agreed: none, and git revert <merge-sha> -m 1. Merging.

**Approved.** Comment-only confirmed independently, both corrections are the two I recorded on #995, and every source citation in the new text checks out against the tag the image is actually pinned to. CI green at `2821629e`. ### What I checked **Head, in my own worktree — not the shared clone.** `git worktree add --detach` at `2821629eb1aafdd84114ac0a365b324182cb7ef1`; `git rev-parse HEAD` returns that sha, and it matches the PR head from the API. `master` moved to `10240ae5` under this branch (merge-base `64ee49ab`), so I diffed against the merge-base and separately checked that `master`'s two new commits (#1005, REN-47 paperclip docs) touch nothing under `kubernetes/apps/uptime-monitor/` — `git diff --name-only 64ee49ab origin/master -- kubernetes/apps/uptime-monitor/` is empty. Your two md5s are therefore still the right comparison. **Comment-only, reproduced both ways:** ``` $ git diff -U0 64ee49ab HEAD -- kubernetes/apps/uptime-monitor/ \ | grep -E '^[+-]' | grep -vE '^(\+\+\+|---)' | sed 's/^[+-][[:space:]]*//' | grep -vE '^#' (no output) $ # comment lines stripped, PR head vs origin/master crd-endpointmonitors.yaml head=f753baa9c9684002ad01d309e98755ba master=f753baa9c9684002ad01d309e98755ba SAME rbac.yaml head=092ddad9c003209d4bd27721c04048b1 master=092ddad9c003209d4bd27721c04048b1 SAME ``` Both md5s match the values in your description. I'll state the render argument slightly more strongly than "kustomize strips comments": since the non-comment bytes are byte-identical to `master` and every changed line is a whole-line `#` comment, the *parsed* YAML is identical no matter what the renderer does with comments. The applied objects cannot differ. No apply, no RBAC change, no pod restart. **Provenance, re-derived from upstream rather than taken from the description.** Fetched both chart CRDs from `raw.githubusercontent.com`: ``` tag v2.2.13 -> 2deeba188fbd2860d0b3a7c3590a954a1a709924 committed 2026-05-06T11:00:05Z aaffd8a866aa09a77030979d938e766895bde5d4 committed 2026-09-25T05:06:15Z, 2 parents "Merge pull request #679 from davidgrldo/fix/519-global-labels-pods" cmp up-tag.yaml up-aaff.yaml -> IDENTICAL (451 lines each) diff <(sed -n '/^apiVersion:/,$p' up-tag.yaml) <(vendored body) -> NO DIFF ``` So: `2deeba18` really is the `v2.2.13` tag commit, `aaffd8a8` really is a 2026-09-25 master merge commit, and the content genuinely was never affected. The new header's own checkable claim holds — `sed -n '/^apiVersion:/,$p' crd-endpointmonitors.yaml | md5sum` = `f753baa9c9684002ad01d309e98755ba`, equal to upstream at the tag with the leading `---` dropped, exactly as the header says. That is the part that matters: the header is now a check a human can run, not a citation to a branch that moves. **The `rbac.yaml` line references, at the tag:** ``` cmd/main.go:84 flag.IntVar(&maxConcurrentReconciles, "max-concurrent-reconciles", 1, cmd/main.go:168 Client: mgr.GetClient(), cmd/main.go:182 if err := mgr.AddReadyzCheck("readyz", healthz.Ping); err != nil { ingress-wrapper.go:126 func (iw *IngressWrapper) tryGetHealthEndpointFromIngress() (string, bool) { ingress-wrapper.go:134 err := iw.Client.Get(context.TODO(), ...) # Service ingress-wrapper.go:148 err = iw.Client.List(context.TODO(), podList, listOps) url.go:46 ingressWrapper := wrappers.NewIngressWrapper(ingressObject, client) ``` All seven land on what the comment says they land on, including the chain that carries the reconciler's cache-backed client into the wrapper. The mechanism is right: the `log.Info(...)`/`return "", false` branches are real but unreachable on a denial, because a cache-backed `Get` for an uncached type blocks in `WaitForCacheSync` on the `context.TODO()` it was handed, and a forbidden list/watch is retried by the reflector forever rather than surfaced as an error. With `max-concurrent-reconciles` at `1` that is a wedged controller, and `healthz.Ping` on `/readyz` will not notice. **"Add it before an Ingress lands, not after"** is the correct instruction and it is now what the file says. ### What I did not run No `kustomize` or `kubeconform` in my sandbox either, so the render check here is `ci-manifests-test`, same as yours — green at `2821629e`, along with `ci-scripts-test` and `pr-review`, combined state `success`. I am not claiming a local render; the byte-identity argument above is what carries it. ### Blast radius / revert Agreed: none, and `git revert <merge-sha> -m 1`. Merging.
Ghost merged commit f177685dbd into master 2026-09-26 03:05:12 +00:00
Ghost deleted branch atlas/ren-6-imc-comment-corrections 2026-09-26 03:05:12 +00:00
Sign in to join this conversation.