From e53493b99e2a8c7b044bf1495f15421d32d5169e Mon Sep 17 00:00:00 2001 From: Justin Klein Date: Tue, 22 Sep 2026 20:54:54 +0000 Subject: [PATCH] sure: the three failures the install-test found, and what they changed The chart page now describes what was measured rather than only what was read from source: readiness runs through curl because the kubelet's HTTP prober follows the app's own redirect into https, /tmp as an emptyDir is not a temporary directory Ruby accepts, and ephemeral-storage at the scaffold's 10Mi evicts the pod within two minutes. Each of the three also has an entry on the lessons page, since none of them is specific to this chart. --- Lessons-Learned.md | 24 ++++++++++++++++++++++++ sure.md | 38 +++++++++++++++++++++++++++++--------- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/Lessons-Learned.md b/Lessons-Learned.md index bb00041..c5c8cca 100644 --- a/Lessons-Learned.md +++ b/Lessons-Learned.md @@ -140,6 +140,30 @@ The second finding came out of fixing the first, and is the more general one. Th **Now:** a value with no valid default is enforced by `required()` in the template, with a message naming the values path, and **described** as required in the schema — never by a `required` array, which would be a safeguard that cannot fire, dressed as one that can. An optional reference says so in `values.yaml` *and* in the schema description, and its template renders nothing when the value is empty. See [§12](Chart-Style-Guide#12-values-that-must-not-lie). +## The temporary directory Ruby refused + +`sure` mounts `/tmp` as an emptyDir so `readOnlyRootFilesystem: true` can hold. Kubernetes creates an emptyDir **mode 0777 without the sticky bit**, and Ruby's `Dir.tmpdir` skips exactly that shape (`world_writable? && !sticky?`). With the image's root filesystem read-only there was no candidate left at all, so both containers died during boot on `ArgumentError: could not find a temporary directory` — raised from MiniMagick while Rails was still loading, long before anything asked for a temp file. + +The emptyDir root is owned by root, so its mode cannot be changed from a non-root container. What *can* be done needs no privilege at all: create a subdirectory in it (allowed, it is 0777) and `chmod 700` what you just created. + +**Now:** a tiny init container — the app's own image, same non-root uid, no added capability — creates `ruby` and `home` directories inside the tmp emptyDirs, and the containers run with `TMPDIR` and `HOME` pointing there. `HOME` is part of the same lesson: `/home/rails` is read-only, and Bundler answers that by building a throwaway home **per invocation**, worker probe included. + +## The probe that followed a redirect into the wrong scheme + +Readiness for `sure`'s web container was an `httpGet` on `/` — the path that provably reaches the database. The pod never became Ready, with `http: server gave HTTP response to HTTPS client`, while the application was answering correctly. + +The kubelet's HTTP prober **follows redirects to the same host**. Rails, told by `RAILS_ASSUME_SSL=true` to treat the request as secure, answered `302` with `https://:3000/registration/new` — and Puma does not speak TLS. The redirect that *proved* the probe was doing its job (that target is the branch `User.count` decides) was also what broke it. + +**Now:** the probe runs `curl` without `-L` and compares the status code itself, accepting 2xx–3xx exactly as the kubelet would. Generally: **an `httpGet` probe against an app that redirects to itself is a probe on the redirect target**, and a scheme or port the app does not serve turns a correct answer into a failure. + +## The pod evicted for a cache nobody sized + +`ephemeral-storage: 10Mi` is the scaffold's placeholder, and it was copied into a Rails chart unchanged. Every start ended in `Pod ephemeral local storage usage exceeds the total limit of containers 60Mi` after about two minutes — an **eviction**, not a crash, so the container logs showed a healthy boot and nothing else. + +Two things count against that limit and are easy to forget: **emptyDirs** and the container's **stdout log**. This image also rebuilds its Bootsnap cache under `/rails/tmp` on every pod start, because the emptyDir mounted there hides the copy baked into the image. + +**Now:** the chart requests `256Mi` per app container, and says in `values.yaml` that this is headroom rather than a measurement. The general form: **`ephemeral-storage` is not boilerplate** — a chart that mounts emptyDirs has to budget for what is written into them. + ## The recurring shape - **Green is a claim about the assertions that ran, not about the system.** diff --git a/sure.md b/sure.md index 588a048..7730e5f 100644 --- a/sure.md +++ b/sure.md @@ -6,7 +6,7 @@ Deploys [Sure](https://github.com/we-promise/sure), the maintained community for |---|---| | Chart version | `1.0.0+up0.7.4` | | App version | `0.7.4` (image `ghcr.io/we-promise/sure`) | -| Workloads | StatefulSet (1 replica, 2 containers: `web` + `worker`) | +| Workloads | StatefulSet (1 replica, 2 containers: `web` + `worker`, plus one init container) | | Storage | one RWO PVC at `/rails/storage`, mounted into **both** containers | | Ingress | yes, `ingress.domain` + `ingress.clusterIssuer` required | | External dependencies | Postgres, Redis | @@ -87,10 +87,10 @@ The chart renders `OnePasswordItem` CRs; the operator turns each into a Kubernet |---|---|---|---| | `sure.web.securityContext.container` | object | `readOnlyRootFilesystem: true`, all caps dropped, no privilege escalation, non-root | Passed verbatim into the container spec. The writable paths are emptyDirs — see [Which paths have to be writable](#which-paths-have-to-be-writable). | | `sure.web.livenessProbe` | object | `GET /up:http`, period 10s, timeout 5s, 3 failures | Boot check only; it touches no database. See [What each probe actually proves](#what-each-probe-actually-proves). | -| `sure.web.readinessProbe` | object | `GET /:http`, period 10s, timeout 5s, 3 failures | The path that reaches Postgres. | +| `sure.web.readinessProbe` | object | `exec`: `curl` (no `-L`) against `http://127.0.0.1:3000/`, accepted for 2xx–3xx, period 10s, timeout 10s, 3 failures | The path that reaches Postgres. `exec` rather than `httpGet` for a measured reason — see [What each probe actually proves](#what-each-probe-actually-proves). | | `sure.web.startupProbe` | object | `GET /up:http`, period 10s, 60 failures (= 600s) | Gates liveness and readiness. The budget covers `db:prepare` on the first start. | | `sure.web.resources.limitFactor` | number | `3` | Memory and ephemeral-storage limits are computed as factor × request. | -| `sure.web.resources.requests` | object | `cpu: 0.1`, `memory: 512Mi`, `ephemeral-storage: 10Mi` | Upstream reports ~352 MB steady state and asks for at least 1 GB with AI enabled; 512Mi × 3 covers both. | +| `sure.web.resources.requests` | object | `cpu: 0.1`, `memory: 512Mi`, `ephemeral-storage: 256Mi` | Upstream reports ~352 MB steady state and asks for at least 1 GB with AI enabled; 512Mi × 3 covers both. The ephemeral-storage figure is **not** the scaffold's 10Mi — see [Ephemeral storage is not boilerplate here](#ephemeral-storage-is-not-boilerplate-here). | | `sure.web.resources.limits` | object | unset | An explicit limit always wins. A CPU limit is only rendered when set here. | | `sure.web.env` | list | `[]` | Extra environment variables appended to the chart's own. Rendered through `tpl`; `value`, `secretKeyRef` and `configMapKeyRef` forms. **Additive, not overriding** — see [Environment variables](Chart-Style-Guide#10-environment-variables). This is where [mail and AI](#the-chart-configures-no-mail-and-no-ai) go. | @@ -103,7 +103,7 @@ The chart renders `OnePasswordItem` CRs; the operator turns each into a Kubernet | `sure.worker.readinessProbe` | object | same `exec` check, period 30s, timeout 15s, 3 failures | | | `sure.worker.startupProbe` | object | same `exec` check, period 20s, 30 failures (= 600s) | The worker registers only after the schema the web container creates exists. | | `sure.worker.resources.limitFactor` | number | `3` | | -| `sure.worker.resources.requests` | object | `cpu: 0.1`, `memory: 512Mi`, `ephemeral-storage: 10Mi` | The sample-data generator runs **here** and is the one operation upstream measured above 512 MB (~680 MB); the computed 1536Mi limit covers it. | +| `sure.worker.resources.requests` | object | `cpu: 0.1`, `memory: 512Mi`, `ephemeral-storage: 256Mi` | The sample-data generator runs **here** and is the one operation upstream measured above 512 MB (~680 MB); the computed 1536Mi limit covers it. | | `sure.worker.resources.limits` | object | unset | | | `sure.worker.env` | list | `[]` | Same rules as `sure.web.env`. Anything a background job needs — the AI assistant included — belongs here as well as on the web container. | @@ -132,13 +132,15 @@ TLS is stored in `tls--sure-ingress`. Rails' `/up` route (`get "up" => "rails/health#show"`) returns 200 as soon as the app has booted without raising. It touches **no** database and **no** Redis. The widespread assumption that it tests the database is exactly what [style guide §12.3](Chart-Style-Guide#12-values-that-must-not-lie) exists to catch — upstream's own Helm chart, which ships inside the application repository, uses `/up` for readiness, and this chart deliberately does not copy that. -- **web liveness and startup: `/up`.** A Postgres outage therefore does not restart the pod. -- **web readiness: `/`.** Every request runs `authenticate_user!`; without a session cookie — which a kubelet probe never has — it evaluates `self_hosted_first_login?`, i.e. `User.count.zero?`, which is an SQL query. With the database up the request ends in a `302` to the login or registration page, which the kubelet counts as success; with the database down it answers `500` and the pod leaves the Service until Postgres is back. +- **web liveness and startup: `GET /up`.** A Postgres outage therefore does not restart the pod. +- **web readiness: `/`, asked through `curl`.** Every request runs `authenticate_user!`; without a session cookie — which a probe never has — it evaluates `self_hosted_first_login?`, i.e. `User.count.zero?`, which is an SQL query. With the database up the request ends in a `302` to the login or registration page, which counts as success; with the database down it answers `500` and the pod leaves the Service until Postgres is back. + + Why `exec` + `curl` instead of `httpGet`: the kubelet's HTTP prober **follows redirects to the same host**, and with `RAILS_ASSUME_SSL=true` Rails builds that redirect as `https://:3000/...`, which Puma does not speak. The install-test failed with `http: server gave HTTP response to HTTPS client` on a pod that was answering perfectly. `curl` without `-L` asks once and reports the status code, and the probe accepts 2xx–3xx exactly as the kubelet would. `curl` is part of the image. - **worker: all three are `exec` probes** running `bundle exec sidekiqmon processes | grep -q "^${HOSTNAME}:"`. Sidekiq registers each process under the identity `::` and keeps that entry alive with a heartbeat whose Redis key expires after about 60s — so a hung worker disappears from the list while its PID still exists. A plain process check could not tell that, and would in fact never fail at all: Sidekiq is PID 1 in its container, so if it dies the container dies with it. Two things this depends on, both set by the chart: `SELF_HOSTED=true` (in managed mode `/` redirects without the `User.count`), and `RAILS_ASSUME_SSL` matching `RAILS_FORCE_SSL`. Sure's `production.rb` defaults **both** to `true`; with `assume_ssl` off and `force_ssl` on, a plain-HTTP probe is answered with a `301` before the controller runs — a "success" that proves nothing. Overriding `RAILS_ASSUME_SSL` to `"false"` while leaving `RAILS_FORCE_SSL` on turns the readiness probe back into a lie. -> Everything in this section was read off the source of `0.7.4` (the Rails health controller, `app/controllers/concerns/authentication.rb`, `config/environments/production.rb`, Sidekiq 8.0.5's `lib/sidekiq/component.rb`) rather than measured in a running container. The PR install-test is where it first runs for real. +> The design above was read off the source of `0.7.4` (the Rails health controller, `app/controllers/concerns/authentication.rb`, `config/environments/production.rb`, Sidekiq 8.0.5's `lib/sidekiq/component.rb`). Two parts of it are now **measured** in the install-test: readiness on `/` really is decided by `User.count` (against an empty database the answer was a redirect to `/registration/new`, which is that query's branch), and the worker's `sidekiqmon` check really does turn Ready — the whole pod reached Ready about 80 seconds after `helm install`, first start and schema creation included. What is still unmeasured is behaviour **under failure**: nobody has yet taken Postgres or Redis away from a running instance and watched the probes react. ### One pod, one volume, two containers @@ -161,7 +163,25 @@ The image entrypoint (`/rails/bin/docker-entrypoint`) runs `./bin/rails db:prepa | `/rails/tmp` | Rails' own scratch directory: the Bootsnap cache, Puma's pidfile when `PIDFILE` is set, and the combined CA bundle the `00_ssl` initializer writes when `SSL_CA_FILE` is given | | `/rails/log` | production logs go to STDOUT, so nothing is expected here; it stays writable because Rails' default logger path is `log/.log` and a rake or console path falling back to it would otherwise hit a read-only filesystem | -One trade-off worth knowing: the image ships a Bootsnap cache precompiled at build time under `/rails/tmp`, and the emptyDir hides it, so every pod start recompiles it. That is boot time, not correctness. +One trade-off worth knowing: the image ships a Bootsnap cache precompiled at build time under `/rails/tmp`, and the emptyDir hides it, so every pod start recompiles it. That is boot time, not correctness — but it is also what makes the ephemeral-storage budget below matter. + +### An emptyDir is not a temporary directory Ruby accepts + +Mounting `/tmp` as an emptyDir is not enough, and the failure is total rather than subtle: Kubernetes creates an emptyDir **mode 0777 without the sticky bit**, and Ruby's `Dir.tmpdir` skips exactly that shape. With the root filesystem read-only, no candidate is left and the app aborts during boot with `ArgumentError: could not find a temporary directory` — raised from MiniMagick while Rails is still loading. + +The chart therefore runs a small init container (its own image, the same non-root uid, no added capability) that creates two directories inside each tmp emptyDir and `chmod 700`s them; the containers then run with `TMPDIR=/tmp/ruby` and `HOME=/tmp/home`. Creating an entry in a 0777 directory needs no privilege, and neither does `chmod` on a directory one has just created — which is why this needs none of the root-plus-`CHOWN` shape other charts use. + +`HOME` is part of the same fix: `/home/rails` belongs to the read-only image, and Bundler answers that by building a throwaway home directory **on every invocation** — every `bundle exec`, including the worker's probe every 30 seconds. + +### Ephemeral storage is not boilerplate here + +The scaffold's `ephemeral-storage: 10Mi` (limit 30Mi) made the kubelet **evict** the pod about two minutes into every start: `Pod ephemeral local storage usage exceeds the total limit of containers 60Mi`. An eviction, not a crash — the container log shows a healthy boot and says nothing about it. + +Both emptyDirs and the container's stdout log count against that limit, and the Bootsnap cache mentioned above is rebuilt into one of them on every start. The chart requests `256Mi` per app container (limit `768Mi`); that is headroom, not a measured steady state. If a release turns out to need more — large imports write through `/tmp` — raise `sure.web.resources.requests.ephemeral-storage` rather than waiting for the next eviction. + +### The startup warning about unencrypted data + +Every start logs `[SECURITY] ActiveRecord Encryption is NOT configured … stored UNENCRYPTED at rest`. The warning counts only keys set **explicitly**, through `ACTIVE_RECORD_ENCRYPTION_PRIMARY_KEY` and its two companions or through Rails credentials; the self-hosted fallback that derives them from `SECRET_KEY_BASE` does not satisfy it. Setting the three variables through `sure.web.env` / `sure.worker.env` is what silences it — and it is also what decouples the encryption keys from `SECRET_KEY_BASE`, which is worth doing before the first start for the reason in the next section. ### `ingress.domain` is read four times @@ -204,7 +224,7 @@ The registry carries releases (`0.7.4`), prereleases (`-alpha.N`, `-rc.N`, `-hot ## CI -`ci/sure/` holds `test-values.yaml` (op:// refs against the stub CRD, `sure.ci.local`), `fixtures.yaml` (throwaway Postgres 16 — the version upstream's compose file uses — plus Redis and the dummy secrets) and `settings.env` with `INSTALL_TIMEOUT=900`, because the rollout wait has to outlast a first start that creates the whole schema. The install-test runs on every PR that touches the chart — see [CI/CD](CI-CD). +`ci/sure/` holds `test-values.yaml` (op:// refs against the stub CRD, `sure.ci.local`), `fixtures.yaml` (throwaway Postgres 16 — the version upstream's compose file uses — plus Redis and the dummy secrets) and `settings.env` with `INSTALL_TIMEOUT=900`, because the rollout wait has to outlast a first start that creates the whole schema. Measured on a green run: the pod was Ready about 80 seconds after `helm install`, so the budget is deliberately generous rather than tight. The install-test runs on every PR that touches the chart — see [CI/CD](CI-CD). ## See also