chore(docker): slim default image — remove helm & Claude Code CLI with init-script guidance (WIN-2144)#10000
Conversation
helm is no longer bundled in the default Windmill image. The default now flips WITH_HELM to false; opt back in with --build-arg WITH_HELM=true. When helm is not bundled, a /usr/local/bin/helm stub is installed that explains the removal, prints an example worker init script to reinstall it, and links the init-scripts docs before exiting 127. This gives jobs that call `helm` an actionable message instead of "command not found". Fixes WIN-2144 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Claude finished @rubenfiszel's task in 1m 14s —— View job PR ReviewGood to merge. Reviewed the Dockerfile-only change ( What I checked
Nits (non-blocking)
Test coverageConfig/Dockerfile-only change — no automated tests expected. Manual verification the author already did (building the stub and confirming it prints the notice to stderr and exits |
/usr/local/bin is first in the image PATH, so a stub there would shadow a helm installed later via an init script into ~/.local/bin, /tmp/.local/bin, etc. Put the stub in /usr/bin (last in PATH) instead: any real helm on an earlier PATH entry wins automatically, and the stub only fires when nothing else provides helm. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Deploying windmill with
|
| Latest commit: |
bf563c3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dd4bdbaa.windmill.pages.dev |
| Branch Preview URL: | https://ruben-win-2144-remove-helm-f.windmill.pages.dev |
|
Claude finished @rubenfiszel's task in 1m 22s —— View job PR Review (updated for commit
|
Pi ReviewMergeable, but should ideally address nits: hardcoded helm version in stub example, hardcoded architecture in stub example. The PR is clean and low-risk. The second commit ( Two nits from the prior Claude review remain unaddressed: P2 —
|
Codex Reviewcc @rubenfiszel Mergeable, but should ideally address nits: Helm init-script example is stale and amd64-only Found 1 issue:
Test coverageDockerfile-only change, so no automated test coverage is expected. Manual verification should include building/running the default image path and invoking |
…fault The lockfile-less python fallback returns PyVAlias::default(), which moved from 3.11 to 3.12 in #7405. Update the comments and the dedicated-worker warn log to reference "the default version" instead of a hardcoded 3.11 so they stay correct across future latest-stable bumps. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pi ReviewMergeable, but should ideally address nits: hardcoded helm version and architecture in stub example command. Reviewed the full diff (commits Standing nits (still unaddressed from prior reviews)P2 — The stub's example command hardcodes P2 — The example command hardcodes New commit
|
Codex Reviewcc @rubenfiszel Mergeable, but should ideally address nits: stale/amd64-only Helm init-script example, history-style comments in Dockerfile Found 2 issues:
Test coverageDockerfile/config plus Rust comment/log-message changes, so no automated tests are expected. Manual verification should build the default image and run |
…ls fast The Claude Code CLI (~245MB, the single largest optional component) was bundled unconditionally and is used only by the claude sandbox template. Gate it behind WITH_CLAUDE_CODE (default false); opt back in with --build-arg WITH_CLAUDE_CODE=true or install it at worker startup via an init script. To keep the failure actionable, the claude sandbox template now checks for the CLI at /usr/bin/claude before invoking the agent SDK and, when absent, throws a clear error naming the next step: the exact init-script install command plus a docs link. The hardcoded executable path is lifted into a CLAUDE_CODE_EXECUTABLE constant referenced by both the check and the SDK call so they can't drift. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pi ReviewMergeable, but should ideally address nits: hardcoded helm version and architecture in stub example command. cc @rubenfiszel Reviewed the full diff (commits through Two standing nits from prior reviews remain unaddressed: P2 —
|
Codex Reviewcc @rubenfiszel Mergeable, but should ideally address nits: stale/amd64-only Helm init-script example, history-style comments Found 2 issues:
Test coverageDockerfile/config plus Rust log/comment changes and a frontend template change. No automated tests are expected for these layers. Manual verification should build the default image and run |
…inary error The runtime existsSync guard + multi-line throw was bloat in a user-facing template. The claude-agent-sdk already preflights pathToClaudeCodeExecutable and throws a clear "Claude Code executable not found at /usr/bin/claude. Please ensure Claude Code is installed..." when it's absent, so the guard was redundant. A stub binary would be worse: it makes the path exist, so the SDK spawns it, and since the SDK sets child stderr to "ignore" by default the stub's message is discarded, leaving only an opaque "exited with code 127". Keep only a two-line header comment documenting the worker-init-script install step (the SDK error says "native installer", not Windmill's init scripts). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pi Reviewcc @rubenfiszel Mergeable, but should ideally address nits: history-style comments, hardcoded helm version/arch in stub example, stale Dockerfile comment about Claude template preflight. The core changes — removing helm and Claude Code CLI from the default image with clear opt-back-in paths — are sound. The helm stub's Three P2 nits follow. Two were flagged in prior reviews and remain unaddressed; one is new from the latest commit. P2 —
|
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pi Reviewcc @rubenfiszel Mergeable, but should ideally address nits: history-style comments (violates AGENTS.md), hardcoded helm version/arch in stub example, stale Dockerfile comment about Claude template preflight. The core changes — removing helm and the Claude Code CLI from the default image with clear opt-back-in paths — are sound. The helm stub's Three P2 nits from the prior review remain unaddressed: P2 —
|
Codex Reviewcc @rubenfiszel Mergeable, but should ideally address nits: history-style Dockerfile comments, stale/amd64-only Helm init-script example, stale Claude template preflight comment Found 3 issues:
Test coverageDockerfile/config plus Rust log/comment wording and a frontend template comment change, so no automated tests are expected. Manual verification should build the default image and run |
… stub
Address review feedback:
- Reword helm/claude ARG and stub comments as current-state constraints
("Default images omit …") instead of drafting-history phrasing.
- The helm stub's suggested install command now mirrors the WITH_HELM=true
branch: ${HELM_VERSION} is baked in at build time and $arch is resolved at
run time via dpkg, so the hint is correct on every arch and can't drift from
the real version.
- Fix the stale Claude comment that claimed the template "fails fast with
instructions"; the runtime error comes from the agent SDK's missing-binary
preflight, with the template header comment naming the init-script install.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pi ReviewGood to merge. The latest commit (
All other aspects — the helm stub's Test coverageDockerfile, template comment, and Rust doc-comment changes only — no automated tests expected. The PR body documents thorough manual verification (both branches of each |
Codex Reviewcc @rubenfiszel Mergeable, but should ideally address nits: non-ASCII template comment Found 1 issue:
Test coverageDockerfile/config, Rust comment/log wording, and a frontend template comment only, so no automated tests are expected. Manual verification should build the default image and run |
Replace Unicode em-dashes with ASCII in files that were ASCII-only, per AGENTS.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
All findings from prior reviews have been addressed in the most recent commits. The Codex P2 about a "Unicode dash" in Pi ReviewGood to merge. All four P2 nits from prior reviews are resolved in the latest commits:
The Codex P2 (Unicode dash in No new issues found. The helm stub's Test coverageDockerfile, template-comment, and Rust doc-comment changes only — no automated tests expected. The PR body documents thorough manual verification (both |
Codex ReviewGood to merge. No issues found. The prior Helm version/arch, history-style wording, stale Claude preflight comment, and non-ASCII template punctuation findings are addressed at this head. Test coverageDockerfile/config plus Rust comment/log wording and a frontend template header comment; no automated tests are expected for these changes. Manual verification should cover building the default image and invoking |
What & why
Trims two large, optional binaries from the default Windmill runtime image, each with a clear "how to add it back" path so nothing silently breaks.
v3.14.3helm/usr/bin/helmstub prints removal notice + init-script example, exits 127Neither is needed by Windmill itself — in particular, autoscaling talks to the Kubernetes API via the
kubeRust crate, notkubectl/helm, and the# sandbox <image>runtime usescrane+ nsjail, not the docker client.Changes
helm (
Dockerfile)WITH_HELMdefaulttrue→false.--build-arg WITH_HELM=truerestores the real binary (that branch is unchanged).helmstub at/usr/bin/helm— deliberately last in the image PATH, so any real helm an init script installs (to/usr/local/bin,~/.local/bin,/tmp/.local/bin, …) shadows it automatically. The stub prints the removal notice, an example init script, and the docs link, then exits 127.Claude Code CLI (
Dockerfile+ template)Gate the install behind a new
ARG WITH_CLAUDE_CODE=false.--build-arg WITH_CLAUDE_CODE=trueinstalls it as before. When off, nothing is installed at/usr/bin/claude(no stub — see below).frontend/src/lib/templates/claude_sandbox.ts.template: no runtime logic added — just a two-line header comment documenting the worker-init-script install. When the binary is absent the@anthropic-ai/claude-agent-sdkquery()already preflightspathToClaudeCodeExecutableand throws:A stub was deliberately avoided: it would make the path exist so the SDK spawns it, and the SDK sets the child's stderr to
"ignore"by default, discarding the stub's message and leaving only an opaqueexited with code 127. Absence yields the clearer error, and the header comment supplies the Windmill-specific next step (the SDK message only says "native installer"):Drive-by: stale python-fallback comments
While auditing the image I noticed the lockfile-less Python fallback comments still said "3.11", but the fallback is
PyVAlias::default()which moved to 3.12 in #7405. Reworded the comments and the dedicated-worker warn log to reference "the default version" so they won't re-stale on the next bump. No behavior change.Validation
docker build --check+ real builds of both the helm and claudeRUNblocks, each branch — confirmed the conditionals and that standalone#comment lines inside aRUNare stripped (don't swallow the command). Verified the helm stub shadowing with a PATH-resolution test.claude-agent-sdksource — confirmed itexistsSync-checkspathToClaudeCodeExecutableand throws the clear "executable not found" error, and that it defaults child stderr to"ignore"(why a stub would be worse).node:*404s).cargo watchrebuilt clean and healthy after the comment edits.Screenshots
Claude Sandbox template after simplification (two header comment lines, no runtime guard):
Fixes WIN-2144
🤖 Generated with Claude Code