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