From 2930cdb9a0c24d1ce42a7adb4ed8db0503fa3917 Mon Sep 17 00:00:00 2001 From: Peter Pathirana Date: Tue, 18 Aug 2026 01:57:36 +0000 Subject: [PATCH] fix: wipe /tmp before the Coder agent starts, not after The released template wipes /tmp from script-agent-startup.sh, which deletes the Coder agent's own CLI. Coder's generated bootstrap downloads the agent binary into a per-boot "mktemp -d -t coder.XXXXXX" directory under /tmp, chdirs into it, appends it to the PATH of every session and script the agent runs, and only then runs the startup script - so the wipe removes the binary the agent installed seconds earlier. Every "coder stat" metadata panel reports "coder: command not found"; the agent process survives on an unlinked binary, so nothing else fails visibly. Excluding the agent's paths from the wipe would trade this breakage for a later one. The set is version-dependent and not uniformly named: v2.35.3 owns coder.XXXXXX/, coder-agent.sock (a hardcoded absolute path, not TMPDIR-relative), rotated coder-agent*.log files, coder-script-data/, coder-screen/, and boundary-audit.sock, which carries no "coder" prefix at all. The wipe moves to the workspace container's entrypoint instead. That is the only hook that runs on a container-only restart within a live Pod - the case the wipe exists for, since init containers run once per Pod - and it runs before the agent exists, so there is nothing to exclude. script-container-entrypoint.sh wipes /tmp and execs Coder's /workspace-init.sh, keeping the agent as PID 1 for orphan reaping and the liveness probe. The wipe there is best-effort rather than fatal: that entrypoint is the only path to a running agent, so aborting would turn stale scratch space into a CrashLoopBackOff with no way to shell in. It records its outcome instead, and script-agent-startup.sh asserts both that outcome and that the agent CLI resolves and runs - the check this change lacked the first time, which makes a recurrence a failed startup script in the workspace UI rather than eight quietly broken metadata panels. --- CLAUDE.md | 4 +- DESIGN.md | 4 +- .../kubernetes/homelab-workspace/configmap.tf | 9 +-- .../homelab-workspace/deployment.tf | 23 +++++-- .../homelab-workspace/script-agent-startup.sh | 64 +++++++++++++++---- .../script-container-entrypoint.sh | 63 ++++++++++++++++++ 6 files changed, 144 insertions(+), 23 deletions(-) create mode 100644 templates/kubernetes/homelab-workspace/script-container-entrypoint.sh diff --git a/CLAUDE.md b/CLAUDE.md index 5df8cadd..7337f7d1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -74,6 +74,7 @@ Quick orientation map — for what each piece is *for* and the decisions behind | `scripts.tf` | `coder_script` resources — the memory watchdog daemon and the weekly `vscode-server` GC schedule | | `variables.tf` | `workspace_image`, `test_mode` — both supplied by the release workflow | | `script-agent-startup.sh` / `script-prepare-workspace.sh` | Scripts run on agent/workspace startup | +| `script-container-entrypoint.sh` | The workspace container's `command`. Wipes `/tmp` and `exec`s Coder's generated `/workspace-init.sh` — the wipe must precede the agent, see the gotcha below | | `script-memory-watchdog.sh` | Userspace memory watchdog — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions). It bounds the **standing population of restartable helpers** (per-role PSS budgets, ten-minute dwell, per-role circuit breaker) and records every per-process sweep. It does **not** try to prevent an acute OOM. `memory_watchdog_mode` selects `observe` / `enforce` (helpers — the default) / `enforce-all` (helpers + editor) | | `script-memory-watchdog-test.sh` | Fixture tests for the watchdog's arithmetic, process selection, budgets, dwell and circuit breaker. Run by hand (`./script-memory-watchdog-test.sh`) and by the `watchdog` job in `.github/workflows/test.yaml`. `kill` is shadowed by a function throughout — the fixture pids are real pids in whatever container runs the suite | @@ -102,7 +103,8 @@ Things that look arbitrary in the code but are load-bearing (full reasoning in [ - Watchdog state keys — the dwell clock and the SIGTERM/SIGKILL escalation — are keyed on `pid:starttime`, never on pid alone. A recycled pid must not inherit another process's history and be killed for it. - `parameters.tf`'s `memory_watchdog_mode` also goes through the `local.validated_*` treatment: Coder constrains the value server-side, but it is the single switch deciding whether the watchdog may signal processes, so an unrecognised value falls back to the inert `observe` rather than being passed through. - Adding a package/tool has three possible homes, and picking the wrong one is a real mistake, not a style choice — route by the rule in [DESIGN.md](DESIGN.md#where-the-workspace-environment-comes-from): universal + stable → image (`Dockerfile`); occasionally-needed + apt-only + too heavy to bake in → the template's `system_packages` parameter; personal, fast-moving, or not an apt package → the operator's dotfiles (a *different* repo — see below), never this one. -- `deployment.tf` mounts `/tmp` on its own ephemeral Longhorn volume, not the node's root filesystem and not the NFS-backed home PVC - see [DESIGN.md](DESIGN.md#design-tensions-and-decisions) for why both of those are wrong for it. Its lifecycle is per-Pod, the same as the `system` volume, so it is *not* wiped by a container-only restart within a live Pod - `script-agent-startup.sh` wipes it explicitly on every agent start instead. Anything relying on `/tmp` persisting across an agent restart was already wrong before this (the same was true for free when it was the container's writable overlay). +- `deployment.tf` mounts `/tmp` on its own ephemeral Longhorn volume, not the node's root filesystem and not the NFS-backed home PVC - see [DESIGN.md](DESIGN.md#design-tensions-and-decisions) for why both of those are wrong for it. Its lifecycle is per-Pod, the same as the `system` volume, so it is *not* wiped by a container-only restart within a live Pod - `script-container-entrypoint.sh` wipes it explicitly on every container start instead. Anything relying on `/tmp` persisting across a container restart was already wrong before this (the same was true for free when it was the container's writable overlay). +- **The `/tmp` wipe belongs in the container entrypoint and nowhere later - this is a fixed regression, not a style preference.** The workspace container's `command` is `script-container-entrypoint.sh`, which wipes `/tmp` and then `exec`s Coder's generated `/workspace-init.sh`. That bootstrap unpacks the agent CLI into a per-boot `mktemp` directory under `/tmp`, chdirs into it, appends it to the PATH of every session and script the agent runs, and only then runs the agent startup script - so wiping `/tmp` from `script-agent-startup.sh` deletes the CLI the agent installed seconds earlier and every `coder stat` metadata panel reports `coder: command not found`. Do not "fix" a future collision here with an exclusion list: the agent's `/tmp` paths are version-dependent and not consistently prefixed (v2.35.3 owns `coder.XXXXXX/`, `coder-agent.sock`, `coder-agent*.log`, `coder-script-data/`, `coder-screen/` *and* `boundary-audit.sock`), so an allowlist rots silently. `script-agent-startup.sh` asserts the CLI resolves and runs, which is what makes a recurrence loud. - `deployment.tf`'s Deployment `metadata.name` (`local.workload_name` in `main.tf`) is not cosmetic: the cluster's Prometheus resolves pod → ReplicaSet → Deployment via an existing `kube_pod_owner` recording rule and exposes the result as a `workload` label with no other join needed, so whatever this Deployment is named *is* the identity CPU/memory/PSI/OOM metrics get attributed to. Don't revert it to an opaque identifier (e.g. the workspace UUID) without re-breaking that attribution — see [DESIGN.md](DESIGN.md#design-tensions-and-decisions). ## Neighbouring repos diff --git a/DESIGN.md b/DESIGN.md index ff866cf3..fb133a32 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -61,7 +61,9 @@ The rule that ties the layers together: a package or tool belongs in the *lowest **No staging environment, so the release pipeline carries its own rehearsal path.** There's exactly one live template and one live cluster — no separate staging Coder deployment to try changes against first. Rather than accept "every merge to main is a live-fire test," the release pipeline itself can run in a mode that exercises a real build and a real (but disposable, clearly-named) template push without touching the production template or its persistent state. That path is what makes it safe to iterate on template/image changes at the same pace as everything else in the repo. See [TESTING.md](TESTING.md) for how to use it. -**`/tmp` is node-local scratch space, deliberately not the shared home volume.** Every workspace's `/tmp` used to be whatever the container's writable overlay layer gave it for free - fast, but unbounded, and on the node's root filesystem. On the operator's own long-lived workspace that grew to several GiB (dominated by Claude Code's own scratch directory, `$TMPDIR/claude-/...`, which agent sessions use for downloads and experiments) and pushed the node toward the kubelet's disk-pressure eviction threshold - a risk to every other pod on that node, not just the workspace that caused it. The home PVC has ample free space, but is NFS-backed, which is a bad fit for what actually lives in `/tmp`: build caches and compiler intermediates are exactly the write-heavy, latency-sensitive workload NFS handles worst. A plain `empty_dir` would keep `/tmp` fast but doesn't fix anything, because `empty_dir` lives on the same constrained node root filesystem the container overlay already did. The fix is a Kubernetes "generic ephemeral volume" on a Longhorn storage class that is both node-local (so still fast) and backed by a separate, much larger partition on the same node than the root filesystem is - see the comment on the `tmp` volume in `deployment.tf` for the specific class and why it's the non-replicated one (scratch data costs nothing to lose) rather than the default replicated class other PVCs in this cluster use. Its size is a fixed ceiling rather than left unbounded, so a runaway consumer now fails predictably inside its own volume instead of eventually pressuring the node. Because this volume's lifecycle is tied to the Pod rather than the container, `script-agent-startup.sh` also wipes it on every agent start, so a container restart within a live Pod doesn't just inherit whatever the previous container left behind. +**`/tmp` is node-local scratch space, deliberately not the shared home volume.** Every workspace's `/tmp` used to be whatever the container's writable overlay layer gave it for free - fast, but unbounded, and on the node's root filesystem. On the operator's own long-lived workspace that grew to several GiB (dominated by Claude Code's own scratch directory, `$TMPDIR/claude-/...`, which agent sessions use for downloads and experiments) and pushed the node toward the kubelet's disk-pressure eviction threshold - a risk to every other pod on that node, not just the workspace that caused it. The home PVC has ample free space, but is NFS-backed, which is a bad fit for what actually lives in `/tmp`: build caches and compiler intermediates are exactly the write-heavy, latency-sensitive workload NFS handles worst. A plain `empty_dir` would keep `/tmp` fast but doesn't fix anything, because `empty_dir` lives on the same constrained node root filesystem the container overlay already did. The fix is a Kubernetes "generic ephemeral volume" on a Longhorn storage class that is both node-local (so still fast) and backed by a separate, much larger partition on the same node than the root filesystem is - see the comment on the `tmp` volume in `deployment.tf` for the specific class and why it's the non-replicated one (scratch data costs nothing to lose) rather than the default replicated class other PVCs in this cluster use. Its size is a fixed ceiling rather than left unbounded, so a runaway consumer now fails predictably inside its own volume instead of eventually pressuring the node. Because this volume's lifecycle is tied to the Pod rather than the container, something has to wipe it on every container start, so a container restart within a live Pod doesn't just inherit whatever the previous container left behind. + +**That wipe runs in the container entrypoint because `/tmp` is not only scratch space - the Coder agent lives there too.** The first attempt put the wipe in `script-agent-startup.sh`, which is the wrong side of the ordering and shipped a broken template: Coder's generated bootstrap downloads the agent CLI into a per-boot `mktemp -d -t coder.XXXXXX` directory under `/tmp`, makes it the agent's working directory, appends it to the PATH of every session and script the agent runs, and *then* runs the startup script - which deleted it, leaving every `coder stat` metadata panel reporting `coder: command not found` while the agent process itself carried on from an unlinked binary. The tempting repair is to exclude the agent's own paths from the wipe, and it is the wrong one: that set is an implementation detail of whatever agent version the control plane happens to serve, and it is not even uniformly named - v2.35.3 owns a random-suffixed `coder.XXXXXX/`, `coder-agent.sock` (a hardcoded absolute path, not a `TMPDIR`-relative one), rotated `coder-agent*.log` files, `coder-script-data/`, `coder-screen/`, and `boundary-audit.sock`, which carries no `coder` prefix at all. An allowlist would stop matching on some future upgrade and fail exactly as invisibly as the original bug. The workspace container's `command` is therefore a small entrypoint script that wipes `/tmp` and `exec`s Coder's bootstrap: it is the only hook that runs on a container-only restart within a live Pod (init containers run once per Pod), it runs before the agent exists so there is nothing to exclude, and `exec` keeps the agent as PID 1 for orphan reaping and the liveness probe. The wipe there is best-effort rather than fatal, because that entrypoint is the only path to a running agent and a hard failure would be a `CrashLoopBackOff` nobody can shell into; it records its outcome instead, and `script-agent-startup.sh` asserts both that outcome and that the agent CLI still resolves and runs - the check the original change lacked, which turns a recurrence into a failed startup script in the workspace UI instead of eight quietly broken metadata panels. **Unprivileged by default.** The workspace itself runs as an unprivileged, non-root, fixed-identity container. Anything that genuinely needs elevated privilege (installing packages, preparing shared volume state) is scoped to a narrow, short-lived setup step that runs before the workspace shell exists, not to something the workspace user can reach into. diff --git a/templates/kubernetes/homelab-workspace/configmap.tf b/templates/kubernetes/homelab-workspace/configmap.tf index 8a51470a..17f77635 100644 --- a/templates/kubernetes/homelab-workspace/configmap.tf +++ b/templates/kubernetes/homelab-workspace/configmap.tf @@ -8,9 +8,10 @@ resource "kubernetes_config_map_v1" "workspace_scripts" { } data = { - agent_startup_script = file("${path.cwd}/script-agent-startup.sh") - memory_watchdog_script = file("${path.cwd}/script-memory-watchdog.sh") - prepare_workspace_script = file("${path.cwd}/script-prepare-workspace.sh") - workspace_init_script = coder_agent.main.init_script + agent_startup_script = file("${path.cwd}/script-agent-startup.sh") + container_entrypoint_script = file("${path.cwd}/script-container-entrypoint.sh") + memory_watchdog_script = file("${path.cwd}/script-memory-watchdog.sh") + prepare_workspace_script = file("${path.cwd}/script-prepare-workspace.sh") + workspace_init_script = coder_agent.main.init_script } } diff --git a/templates/kubernetes/homelab-workspace/deployment.tf b/templates/kubernetes/homelab-workspace/deployment.tf index e83c6bda..4debf653 100644 --- a/templates/kubernetes/homelab-workspace/deployment.tf +++ b/templates/kubernetes/homelab-workspace/deployment.tf @@ -79,8 +79,14 @@ resource "kubernetes_deployment_v1" "deployment" { } } container { - name = "workspace" - command = ["/bin/bash", "/workspace-init.sh"] + name = "workspace" + # Not Coder's generated /workspace-init.sh directly: the entrypoint + # wipes /tmp and then execs it. The wipe has to happen before the + # agent unpacks its CLI into /tmp, and this is the only hook that runs + # on a container-only restart within a live Pod (init containers do + # not) - see script-container-entrypoint.sh and the "tmp" volume + # below. + command = ["/bin/bash", "/container-entrypoint.sh"] image = var.workspace_image env { name = "CODER_AGENT_TOKEN" @@ -131,6 +137,11 @@ resource "kubernetes_deployment_v1" "deployment" { name = "coder-scripts" sub_path = "agent_startup_script" } + volume_mount { + mount_path = "/container-entrypoint.sh" + name = "coder-scripts" + sub_path = "container_entrypoint_script" + } volume_mount { mount_path = "/memory-watchdog.sh" name = "coder-scripts" @@ -222,8 +233,12 @@ resource "kubernetes_deployment_v1" "deployment" { # node notices. Its lifecycle matches the Pod's (created fresh, deleted # with it) - like the "system" volume above, that means a Pod restart # gets a clean volume but a container-only restart within a live Pod - # does not, which is why script-agent-startup.sh also wipes /tmp's - # contents explicitly on every agent start instead of relying on this. + # does not, which is why the container's entrypoint + # (script-container-entrypoint.sh) wipes /tmp's contents explicitly on + # every container start instead of relying on this. That wipe belongs in + # the entrypoint and nowhere later: the Coder agent unpacks its own CLI + # into /tmp before it runs anything else, so a wipe from the agent + # startup script deletes it. volume { name = "tmp" ephemeral { diff --git a/templates/kubernetes/homelab-workspace/script-agent-startup.sh b/templates/kubernetes/homelab-workspace/script-agent-startup.sh index c7ef1421..684bf50b 100644 --- a/templates/kubernetes/homelab-workspace/script-agent-startup.sh +++ b/templates/kubernetes/homelab-workspace/script-agent-startup.sh @@ -2,22 +2,60 @@ set -eo pipefail -wipe_tmp() { - # /tmp is a per-Pod volume (see deployment.tf), not per-container, so it - # survives a container restart within a live Pod even though it never used - # to: the previous /tmp was part of the container's writable overlay, which - # a fresh container instance always got a clean copy of for free. This - # restores that property explicitly. It runs before anything else so - # nothing has written into /tmp yet this boot, and pipefail/errexit above - # mean a failure here aborts this blocking startup script rather than - # leaving stale scratch space to accumulate silently - a failed wipe shows - # up as a failed agent startup script in the Coder UI, not as a slow leak. - echo "Wiping /tmp..." - find /tmp -mindepth 1 -delete +# Written by script-container-entrypoint.sh, which wipes /tmp before the agent +# starts. +TMP_WIPE_STATUS_FILE="/tmp/.tmp-wipe-status" + + +# The check that would have caught the regression where wiping /tmp deleted the +# agent's own CLI: every "coder stat" metadata panel reported "coder: command +# not found" for a released template version, and nothing else noticed. +# +# Coder's bootstrap unpacks the agent CLI into a per-boot directory under /tmp +# and the agent appends that directory to the PATH of everything it runs, so +# resolving "coder" here goes through exactly the same lookup a metadata script +# does. errexit above plus startup_script_behavior = "blocking" turn a failure +# into a visibly failed startup script in the workspace UI, while the agent +# process itself keeps running so the workspace stays reachable to debug. +assert_agent_cli() { + local cli + if ! cli="$(command -v coder)"; then + echo "ERROR: the Coder agent CLI is not on PATH." >&2 + echo " Nothing may remove the agent's own files from /tmp - see" >&2 + echo " script-container-entrypoint.sh for why the wipe runs there." >&2 + echo " PATH=${PATH}" >&2 + return 1 + fi + if ! "${cli}" version > /dev/null; then + echo "ERROR: the Coder agent CLI at ${cli} is present but not usable." >&2 + return 1 + fi + echo "Coder agent CLI: ${cli}" +} + + +# The /tmp wipe cannot abort the container entrypoint without risking a +# CrashLoopBackOff, so it reports here instead. This keeps a partial wipe a +# loud failure rather than a silent leak on a fixed-size volume. +assert_tmp_wiped() { + if [[ ! -f "${TMP_WIPE_STATUS_FILE}" ]]; then + echo "ERROR: ${TMP_WIPE_STATUS_FILE} is missing - the container entrypoint" >&2 + echo " did not run. Check the workspace container's command in" >&2 + echo " deployment.tf; /tmp is no longer being cleared on restart." >&2 + return 1 + fi + if [[ "$(head -n 1 "${TMP_WIPE_STATUS_FILE}")" != "ok" ]]; then + echo "ERROR: /tmp was not fully wiped on container start:" >&2 + cat "${TMP_WIPE_STATUS_FILE}" >&2 + return 1 + fi + echo "/tmp wiped on container start" } + main() { - wipe_tmp + assert_tmp_wiped + assert_agent_cli if [[ ! -s ~/.bashrc ]]; then echo "Setting up starter bash rc scripts from /etc/skel..." cp /etc/skel/.bashrc ~/.bashrc diff --git a/templates/kubernetes/homelab-workspace/script-container-entrypoint.sh b/templates/kubernetes/homelab-workspace/script-container-entrypoint.sh new file mode 100644 index 00000000..4c3bc463 --- /dev/null +++ b/templates/kubernetes/homelab-workspace/script-container-entrypoint.sh @@ -0,0 +1,63 @@ +#!/bin/bash +set -eo pipefail + + +# Where wipe_tmp records its outcome for script-agent-startup.sh to assert on. +# Kept in /tmp on purpose: it is written after the wipe, so its presence is also +# evidence that this script ran at all this boot. +TMP_WIPE_STATUS_FILE="/tmp/.tmp-wipe-status" + + +# Restore the "a fresh container gets a fresh /tmp" property that /tmp lost when +# it moved off the container's writable overlay layer onto a per-Pod volume (see +# the "tmp" volume in deployment.tf). A Pod recreate still gets an empty volume +# for free; a container-only restart within a live Pod - an OOM kill, a liveness +# probe failure - does not, and would otherwise inherit whatever the previous +# container left behind. +# +# This has to run in the container entrypoint, before the Coder agent exists, +# and not in the agent startup script. Coder's own bootstrap (the generated +# /workspace-init.sh) unpacks the agent CLI into a per-boot mktemp directory +# under /tmp, chdirs into it, appends it to the PATH of every session and script +# the agent runs, and only then runs the startup script - so a wipe from the +# startup script deletes the CLI the agent installed moments earlier, which is +# exactly how every "coder stat" metadata panel came to report "coder: command +# not found". +# +# Excluding the agent's paths from the wipe instead is not a fix. The set is +# version-dependent and not even consistently named: on the deployed agent it is +# a random-suffixed coder.XXXXXX directory, coder-agent.sock, coder-agent*.log, +# coder-script-data/, coder-screen/ - and also boundary-audit.sock, which does +# not carry the "coder" prefix at all. An allowlist that silently stops matching +# after a Coder upgrade reintroduces this same failure just as invisibly. There +# is nothing to exclude here, because nothing of the agent's exists yet. +# +# The wipe is deliberately best-effort rather than fatal: this script is the +# only path to a running agent, so aborting here turns a stale-scratch-space +# problem into a CrashLoopBackOff with no way to shell in and look. Visibility +# is preserved instead by recording the outcome for the startup script, which +# can fail loudly in the workspace UI without taking the workspace down. +wipe_tmp() { + echo "Wiping /tmp..." + local errors + errors="$(find /tmp -mindepth 1 -maxdepth 1 -exec rm -rf -- {} + 2>&1)" || true + if [[ -n "${errors}" ]]; then + echo "ERROR: failed to fully wipe /tmp:" >&2 + echo "${errors}" >&2 + printf 'failed\n%s\n' "${errors}" > "${TMP_WIPE_STATUS_FILE}" + return 0 + fi + printf 'ok\n' > "${TMP_WIPE_STATUS_FILE}" +} + + +main() { + wipe_tmp + # Hand off to Coder's generated agent bootstrap, replacing this process rather + # than spawning it: the agent has to stay PID 1, both because it reaps orphans + # in this container and because the Deployment's liveness probe pgreps for it. + echo "Starting Coder agent..." + exec /bin/bash /workspace-init.sh +} + +main