keycloak: the readiness check that is gated behind the metrics option

Found while verifying the CI result rather than from the plan, and it changes
a default: Keycloak's DATABASE health check is the one row of upstream's check
table whose "Requires Metrics" column says yes. With metrics-enabled=false -
the chart's default in its first draft - /health/ready answers 200 with the
database gone, and the pod keeps taking logins it cannot serve.

That is the urlaubsverwaltung shape a second time, and the generalisation is
worth more than the fix: the dependency was not missing from a health GROUP
this time, it was conditional on an option named for observability, which is
a place nobody looks when reviewing a probe. So the rule grows past health
groups - find the list of checks the endpoint performs, and find out what each
one is conditional on. "This path includes the database" is a claim about
configuration, not about the path.

keycloak.md: metrics default is now true with that reasoning, a new trap
section for it, and the measured install-test figure (71.8s from helm install
to both pods Ready, 0 restarts, Gitea PR #87).

Also corrected an overclaim of my own in the same page: two replicas both
reaching Ready does NOT prove that jdbc-ping found the peer. Readiness reports
one node's own health, so two nodes that never met are both Ready and both
alone. The install-test rules out the louder half (a replica that cannot start
or cannot share the database); the Infinispan view still has to be read out of
the log, which is what step 2 of the first-install checklist is for.

Lessons-Learned.md

@@ -18,6 +18,8 @@ Spring Boot's `readiness` group contains `readinessState` **alone**. The paths w
 
 **Now:** whatever aggregates behind a probe path — Spring health groups, a framework-assembled `/healthz` — is read off the **running** endpoint, not off the path name. The chart names the members explicitly, so the composition lives in the template rather than in an upstream default. See [§12.3](Chart-Style-Guide#12-values-that-must-not-lie).
 
+**It recurred, and the second time it was caught before shipping — by reading the check list rather than the path.** [keycloak](keycloak) has three separate native endpoints, which looks like the problem being solved for you: `/health/live` without a database check, `/health/ready` with one. Except that upstream's table of available checks has a **"Requires Metrics"** column, and the *database* check is the single row that says yes — so with `metrics-enabled=false`, the chart's own default in its first draft, `/health/ready` would have answered 200 with the database gone. The dependency was not missing from a health *group* this time; it was gated behind an option named for **observability**, which is a place nobody looks when reviewing a probe. The chart now defaults metrics on and argues for it as a readiness setting. **So the rule generalises past health groups: find the list of checks the endpoint actually performs, and find out what each one is conditional on.** "This path includes the database" is a claim about configuration, not about the path.
+
 ## The value that meant its own opposite
 
 `satisfactory` shipped `game.seasonalEvents`, passed straight into the image's `DISABLESEASONALEVENTS`. The default `true` switched the events **off** — in every install, for the chart's entire life — while the schema described the key as "true means events are enabled".

keycloak.md

@@ -15,7 +15,7 @@ What it costs is the thing to know before the first upgrade: **Keycloak supports
 | Ingress | **two**: the public one and the admin hostname (switchable off). The admin one is **not** an access control — see [the first trap](#the-admin-hostname-is-not-an-access-control) |
 | External dependencies | an external PostgreSQL. Nothing else |
 | Secrets | 2 × `OnePasswordItem`, four fields in total |
-| Network | ClusterIP Service on 8080 (`http`) and a **separate** ClusterIP Service on 9000 (`management`). The management port serves `/health/*` and optionally `/metrics` **unauthenticated** and is in no Ingress. JGroups uses 7800 pod-to-pod; no NetworkPolicy yet |
+| Network | ClusterIP Service on 8080 (`http`) and a **separate** ClusterIP Service on 9000 (`management`). The management port serves `/health/*` and `/metrics` **unauthenticated** and is in no Ingress. JGroups uses 7800 pod-to-pod; no NetworkPolicy yet |
 | Disruption | `PodDisruptionBudget`, `minAvailable: 1` — the **first** in this repo, and now a convention ([style guide §11](Chart-Style-Guide#11-scaling)) |
 | Install-test | `ci/keycloak/` — fixtures + test values, **with two replicas** and a throwaway PostgreSQL |
 
@@ -86,14 +86,14 @@ Host, port and database name are deliberately **not** in the vault — they are
 
    Ready means `/health/ready` answered 200, which means the database is reachable and the schema is in place. The first start is the slow one: it creates the whole schema, and with two replicas the second waits on the first's migration lock. If a pod sits in `Init`, read the init containers first — see [Why two init containers](#why-this-chart-has-two-init-containers).
 
-2. **Check that the cluster really formed.** This is what the second replica is for, and a silent failure here leaves two independent islands that each believe they are the cluster:
+2. **Check that the cluster really formed. No probe replaces this step and neither does the install-test.** A node that never found its peer is Ready, healthy and *alone* — readiness reports this node's own health — and two such nodes are two islands that each believe they are the cluster.
 
    ```bash
    kubectl -n keycloak logs deployment/keycloak-keycloak-deployment \
      | grep -i "ISPN000094\|view accepted\|members"
    ```
 
-   Look for a view with **two** members. One member per pod means `jdbc-ping` did not find the peer.
+   Look for a view with **two** members. One member per pod means `jdbc-ping` did not find the peer: check that both pods can reach each other on port 7800 and that they really share one database.
 
 3. **Log in** at `https://<ingress.admin.domain>/admin` with the bootstrap credentials from the vault item.
 
@@ -199,7 +199,7 @@ Three practical consequences:
 | `keycloak.db.pool.initialSize` / `.minSize` | int | `2` / `2` | No upstream default. A few warm connections remove the first-request latency after an idle period. |
 | `keycloak.db.pool.maxSize` | int | `20` | **Deliberately not upstream's `100`**, and the sizes are **per replica**: two replicas at 100 would ask a default PostgreSQL (`max_connections = 100`) for twice what it has, and the failure arrives under load as *"too many clients already"* rather than at install time. Raise it together with the server's `max_connections`, and multiply by `keycloak.replicas` when you do. |
 | `keycloak.bootstrapAdmin.enabled` | bool | `true` | The temporary bootstrap administrator. **Switch it off once a permanent administrator exists** — see step 4 above. |
-| `keycloak.metrics.enabled` | bool | `false` | **A build option.** Off because of the *port*, not the data: the management interface is unauthenticated here, and metrics add realm and client names to a surface nothing authenticates. Switch it on together with whatever scrapes it. |
+| `keycloak.metrics.enabled` | bool | `true` | **A build option, and on for the *readiness probe* rather than for the metrics** — see [the readiness trap](#readiness-only-checks-the-database-while-metrics-are-enabled). The price: `/metrics` then answers on the management port, unauthenticated, carrying realm names, client ids and login counters. That port is ClusterIP-only and in no Ingress. Switching this off is a decision about whether readiness can fail. |
 | `keycloak.features` | list | `[]` | Extra feature flags (`--features`). **A build option.** A list and not a comma-separated string, so two entries cannot arrive as one unknown feature name — which Keycloak rejects at *build* time, inside an init container. |
 | `keycloak.cache.clusterName` | string | `null` | `--cache-embedded-cluster-name`; `null` means Keycloak's default `ISPN`. **Read [the trap](#a-shared-database-merges-two-clusters) before setting it.** |
 | `keycloak.ports.http` | int | `8080` | |
@@ -263,6 +263,23 @@ Port 9000 carries `/health`, `/health/live`, `/health/ready`, `/health/started`
 
 Upstream's own alternative, `legacy-observability-interface=true`, moves health back onto the serving port where the Ingress already is. It is explicitly *"not recommended for security reasons"* and this chart does not offer it.
 
+### Readiness only checks the database while metrics are enabled
+
+`keycloak.metrics.enabled` reads like an observability switch and is not only one. Upstream's table of available health checks carries a **"Requires Metrics"** column, and the row that says **yes** is the one that matters here:
+
+| Check | Description | Requires metrics |
+|---|---|---|
+| Database | "Returns the status of the database connection pool" | **yes** |
+| Cluster | "Returns the status of the cluster (network partitions)" | no |
+| Graceful shutdown | returns DOWN once pre-shutdown has started | no |
+| Keycloak Initialized | status of server initialization | no |
+
+So with `metrics-enabled=false`, `/health/ready` answers **200 with the database gone**, and the pod keeps taking logins it cannot serve.
+
+That is exactly the shape [the readiness probe that could not fail](Lessons-Learned#the-readiness-probe-that-could-not-fail) cost this repo on `urlaubsverwaltung`: the probe *path* was right and the aggregate behind it contained nothing external. The difference is only where it hides — there in a Spring health group, here behind an option named for metrics. Which is why **this chart defaults `keycloak.metrics.enabled` to `true`, and argues for it as a readiness setting rather than a scraping one.**
+
+The price is real and is why the value still exists: `/metrics` then answers on the management port, unauthenticated, carrying realm names, client ids and login counters. That port is ClusterIP-only and in no Ingress, so the exposure is in-cluster — see [the trap below](#the-management-port-is-unauthenticated). **Switching it off is a decision about whether the readiness probe can fail at all**, and nothing else in this chart replaces it.
+
 ### The three probes use three different paths on purpose
 
 | Probe | Path | Contains a database check? |
@@ -315,9 +332,11 @@ This chart renders **no** NetworkPolicy. If one is ever added, it has to allow 7
 
 `ci/keycloak/` carries `test-values.yaml`, `fixtures.yaml` (throwaway PostgreSQL plus the two Secrets the operator would materialize) and `settings.env` (`INSTALL_TIMEOUT=900`).
 
-**The install-test runs with two replicas**, which is the single most valuable line in that fixture: the script waits for the Deployment's rollout, and a rollout is complete only when **both** pods are Ready — so cluster formation through `jdbc-ping` is inside the assertion rather than beside it. A one-replica test would be green without ever showing whether the second node is found, which is the mistake [§CI](CI-CD) was taught once already.
+**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.
 
-It also closes three things a render cannot: that the two init containers work at all, that `readOnlyRootFilesystem: true` holds for the serving container with emptyDirs only at the three paths it needs, and that `start --optimized` accepts the pre-built installation instead of re-augmenting onto a read-only path. `metrics.enabled: true` in the fixture so the build container must produce an installation differing from the published image in **two** options rather than one — with metrics off, a build that silently did nothing would look identical.
+**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.
+
+It also closes three things a render cannot: that the two init containers work at all, that `readOnlyRootFilesystem: true` holds for the serving container with emptyDirs only at the three paths it needs, and that `start --optimized` accepts the pre-built installation instead of re-augmenting onto a read-only path. And because `health-enabled` and `metrics-enabled` are build options, the fact that all three probe paths answer on port 9000 **is** the proof that the build container really did its work — a build that silently did nothing would leave them 404.
 
 **What CI cannot prove**: that `hostname-admin` serves the console and its resources cleanly on the second hostname. No ingress controller answers in the test cluster and nothing drives a browser. What it does check is that both Ingress objects are accepted, that the Service port they name exists, and that Keycloak accepts both hostname values as full URLs at start.