keycloak: the A/B that exonerated the theme mount, and the lesson from it

jklein committed 2026-10-06 11:44:43 +00:00
1 parent 5480fac342
commit 1f9ce77762
2 files changed
+18 -1

No files matched your search

+13
@@ -450,6 +450,18 @@ What generalizes is not the Keycloak detail but the question the plan did not as
**Now:** before adding a pass-through volume pair to a chart with a pre-build step, enumerate every container that could legitimately need the file, and render the mounts at each of them. Write the reason **at the value**, not only in the pull request: an occurrence that looks redundant is exactly what the next round removes while tidying up, and this one cannot be noticed by any test that checks whether the pod is Ready. And where a pass-through value only *places* a file, say so at the value — mounting a CA does not add it to a truststore, and mounting a theme does not select it — because the next reader's expectation is set by the use case the key was introduced for. **Now:** before adding a pass-through volume pair to a chart with a pre-build step, enumerate every container that could legitimately need the file, and render the mounts at each of them. Write the reason **at the value**, not only in the pull request: an occurrence that looks redundant is exactly what the next round removes while tidying up, and this one cannot be noticed by any test that checks whether the pod is Ready. And where a pass-through value only *places* a file, say so at the value — mounting a CA does not add it to a truststore, and mounting a theme does not select it — because the next reader's expectation is set by the use case the key was introduced for.
## The restart that the green run could not explain
The install-test for [keycloak](keycloak) in Gitea PR #92 passed and reported, in the final `kubectl get pods`, one restart on one of the two pods. The three runs of PR #87 — the chart's own introduction — had shown `0` three times. The PR added volume mounts to the `build` init container, so "probably a flake" was not available: a counter moved in the same round as a change that touches container startup.
The first explanation was plausible and wrong. That run was slower everywhere — the *unchanged* postgres fixture needed 51.5s instead of 27.4s — so "a loaded runner trips a first install that serializes two replicas on a migration lock" wrote itself. Two more runs disposed of it: the slowest run of the whole series (191s) restarted nothing, and the two that restarted sat in the middle of the range. Five runs in, the tally was three restarts out of five against a baseline of none out of three, which reads like a regression.
**What settled it was the control case on the same machine**: the volume pair commented out of the fixture, which reproduces the pre-change pod spec exactly — the pod template hash went back to the one PR #87 had measured, so the comparison was byte-identical rather than approximate. Two control runs: one clean, one with the same single restart. The restart happens **without** the change. It is pre-existing, intermittent behaviour of that chart's first install, and the three clean runs two days earlier were a fast afternoon rather than a property of the chart.
The second finding is the one that cost the detour, and it is not about Keycloak: **the harness dumps container logs only when a run fails.** `diagnose()` hangs off the `EXIT` trap behind `RC -ne 0`, so every one of the seven runs printed a restart count with nothing to explain it. A green run that reports a restart and cannot say what it was is a measurement that invites exactly the wrong conclusion in either direction.
**Now:** when a counter moves in the same round as a change, do not argue from runs made earlier or elsewhere — **reproduce the old state on the current machine and measure it there.** For a chart that means disabling the new values in the fixture and checking that the pod template hash returns to the previous one, which turns "roughly the same spec" into a fact. Budget for more than one run per side: a 1-in-3 behaviour is invisible in a single run and looks like a regression in two. And where a green path reports a number nobody can explain, say so in the file that carries the number and name the harness change that would answer it — the next person to see that restart should not have to re-derive this.
## The recurring shape ## The recurring shape
- **Green is a claim about the assertions that ran, not about the system.** - **Green is a claim about the assertions that ran, not about the system.**
@@ -481,5 +493,6 @@ What generalizes is not the Keycloak detail but the question the plan did not as
- **A write barrier reaches as far as the helper that implements it.** "Read-only configuration" also catches the bootstrap writes that happen once, on a first install, on the path no test walks — including the one that creates the credential for the door you kept open on purpose. - **A write barrier reaches as far as the helper that implements it.** "Read-only configuration" also catches the bootstrap writes that happen once, on a first install, on the path no test walks — including the one that creates the credential for the door you kept open on purpose.
- **Check that the platform can make the call before designing around it.** An HTTP probe is a GET; anything else is an `exec` probe, and an `exec` probe is a claim about which binaries the image still contains. - **Check that the platform can make the call before designing around it.** An HTTP probe is a GET; anything else is an `exec` probe, and an `exec` probe is a claim about which binaries the image still contains.
- **The strictest file mode is not the correct one.** A mounted secret is owned by `root`, `fsGroup` supplies only the group, and an application that treats `EACCES` like "absent" turns a permission mistake into a feature that quietly switched itself off. - **The strictest file mode is not the correct one.** A mounted secret is owned by `root`, `fsGroup` supplies only the group, and an application that treats `EACCES` like "absent" turns a permission mistake into a feature that quietly switched itself off.
- **A counter that moved alongside a change is not evidence until the control case ran on the same machine.** Earlier green runs are a different environment, and an intermittent behaviour needs several runs per side before either answer is honest.
- **Ask where the consumer of a file runs, not only where the application runs.** A chart with a pre-build step has two consumers of its volumes at two different times, and a mount that reaches only the serving one fails without a symptom. - **Ask where the consumer of a file runs, not only where the application runs.** A chart with a pre-build step has two consumers of its volumes at two different times, and a mount that reaches only the serving one fails without a symptom.
- **When the fix needs a privilege, ask whether two cheap steps can replace one privileged one.** Splitting a pre-build into a seeding container plus a build container kept the hardening baseline intact across the whole pod, where the shorter single-container version would have bought a documented deviation *and* a suppression that silences the container that actually serves traffic. - **When the fix needs a privilege, ask whether two cheap steps can replace one privileged one.** Splitting a pre-build into a seeding container plus a build container kept the hardening baseline intact across the whole pod, where the shorter single-container version would have bought a documented deviation *and* a suppression that silences the container that actually serves traffic.
+5 -1
@@ -401,7 +401,11 @@ This chart renders **no** NetworkPolicy. If one is ever added, it has to allow 7
**The install-test runs with two replicas**, and the rollout is complete only when **both** pods are Ready — so a second replica that cannot start, deadlocks on the schema migration lock or fights the first over the database fails the run. Measured in Gitea PR #87: **71.8 seconds** from `helm install` to both pods Ready, 0 restarts — three runs, all three the same. **The install-test runs with two replicas**, and the rollout is complete only when **both** pods are Ready — so a second replica that cannot start, deadlocks on the schema migration lock or fights the first over the database fails the run. Measured in Gitea PR #87: **71.8 seconds** from `helm install` to both pods Ready, 0 restarts — three runs, all three the same.
**Read that as an observation and not as an invariant.** Four runs of an identical pod spec in PR #92 measured **101s / one restart**, **82s / none**, **191s / none** and **101s / one restart**. So: intermittent on an unchanged spec, always exactly one restart on one pod about 35s before it reports Ready, and **not** a function of the runner's speed — the fastest and the slowest run restarted nothing, which is what disproved the first explanation. **What the restart is could not be established, because the harness dumps container logs only when a run fails** — a green run cannot explain a restart it reports. Answering it means making the final-state step print the restart reasons whenever a counter is above zero. A first install that serializes two replicas on the Liquibase migration lock is a plausible place for one, but that is a hypothesis and nothing has measured it. **Read that as an observation and not as an invariant**, and the number that matters here is not the time but the restart count. Five runs in PR #92 — identical pod spec — measured 101s/**1**, 82s/0, 191s/0, 101s/**1**, 111s/**1**: three of five restarted one pod once. That looked like the new theme mount destabilizing the start, so the control case was measured **on the same runner**, with the mount commented out of the fixture (pod template hash `5844db56b6`, byte-identical to what PR #87 measured): 81s/0 and 171s/**1** — one of two.
So the restart happens **without** the mount as well: it is pre-existing, intermittent behaviour of this chart's first install, always exactly one restart on one pod, and it does not track the runner's speed (the fastest and the slowest run of the whole series restarted nothing). **What the restart is could not be established, because the harness dumps container logs only when a run fails** — a green run cannot explain a restart it reports, and answering it means making the final-state step print the restart reasons whenever a counter is above zero. A first install that serializes two replicas on the Liquibase migration lock is a plausible place for one, but that is a hypothesis and nothing has measured it.
The method is the transferable part: **a counter that moves in the same round as a change is not evidence against the change until the control case has run on the same machine.** Three clean runs two days earlier were not that control.
**What that does *not* prove is that `jdbc-ping` found the peer**, and reading it that way is the mistake this paragraph exists to prevent. Readiness reports *this* node's health, so a node that never found its peer is Ready, healthy and alone — and two such nodes are two islands that each believe they are the cluster. The Infinispan view has to be read out of the log, which is what step 2 of the first-install checklist is for. So the test rules out the louder half and leaves the quiet half to a human. **What that does *not* prove is that `jdbc-ping` found the peer**, and reading it that way is the mistake this paragraph exists to prevent. Readiness reports *this* node's health, so a node that never found its peer is Ready, healthy and alone — and two such nodes are two islands that each believe they are the cluster. The Infinispan view has to be read out of the log, which is what step 2 of the first-install checklist is for. So the test rules out the louder half and leaves the quiet half to a human.