chore(localmode): sync compose template with volcano-hosting - #156
Conversation
There was a problem hiding this comment.
Pull request overview
Syncs the generated local-mode Compose asset with volcano-hosting to enable v2 agent worker containers.
Changes:
- Mounts the host Docker socket into the server.
- Retains the existing local-storage volume.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # WARNING: This grants the server container permission to run any container | ||
| # with the server's privileges. Only safe in local development on your machine. | ||
| volumes: | ||
| - /var/run/docker.sock:/var/run/docker.sock |
| # Mount docker socket to allow agent worker launch (local mode only). | ||
| # WARNING: This grants the server container permission to run any container | ||
| # with the server's privileges. Only safe in local development on your machine. |
tkkhq
left a comment
There was a problem hiding this comment.
See inline feedback.
Verdict: Approve — The diff is a minimal, correctly-scoped sync of a generated Compose asset; no P0/P1 defects were found, and the one P2 finding is a documentation-accuracy nit in a generated file, not a functional or blocking issue.
| # Mount docker socket to allow agent worker launch (local mode only). | ||
| # WARNING: This grants the server container permission to run any container | ||
| # with the server's privileges. Only safe in local development on your machine. |
There was a problem hiding this comment.
[P2] Docker-socket warning understates actual blast radius
The new comment at lines 76-78 says the socket mount "grants the server container permission to run any container with the server's privileges." That understates the real risk: /var/run/docker.sock talks to the host Docker daemon, which normally runs as root. Any process with access to the socket can ask the daemon to start a new container with --privileged and a host-root bind mount (e.g. -v /:/host), giving effective root on the host, regardless of what privilege level the server process itself runs at inside its own container. The warning's phrasing could lead a developer to believe the blast radius is capped at "whatever the server can already do," when it is actually host-root-equivalent.
Since this file is generated ("Do not edit by hand") from volcano-hosting's infrastructure/local-mode/docker-compose.template.yml, the wording fix belongs in that upstream template, not this copy.
There was a problem hiding this comment.
Not changing here — this file is generated verbatim from volcano-hosting's infrastructure/local-mode/docker-compose.template.yml ("Do not edit by hand"), so a wording fix in this synced copy would be overwritten on the next sync. The blast-radius point is valid; it belongs in the upstream volcano-hosting template, not this PR.
Tracking
Summary
Regenerates the embedded local-mode Compose asset from volcano-hosting, so
volcano startproduces a server that can launch a v2 agent worker./var/run/docker.sockmount on theserverservice, which the local-mode server needs to start the agent worker as a sibling containermake volcano-cli-localmode-assets-generatein volcano-hosting; the file is not hand-editedvolcano-hosting owns
infrastructure/local-mode/docker-compose.template.yml. This repository embeds a generated copy so the CLI can start the stack without a hosting checkout. Hosting PR #807 changed the template, which left this copy stale: a developer runningvolcano startgot a server that silently could not launch a worker.The mount is local-mode only. It grants the server container the ability to start containers, which is the local equivalent of the ECS
RunTaskpermission the Fargate launcher holds in cloud.Merge order
Land volcano-hosting PR #807 first, so the source template is final.
Verification
make volcano-cli-localmode-assets-checkin volcano-hosting, pointed at this branch ->volcano-cli local-mode assets check passedgo build ./...-> passgo vet ./...-> passgo test ./... -count=1-> all passgit diff --check-> cleanserverservicevolumesblock; no other service, port, or volume changed