dev stack: keep node_modules out of the shared checkout - #483
Open
runleveldev wants to merge 1 commit into
Open
runleveldev wants to merge 1 commit into
runleveldev wants to merge 1 commit into
Conversation
The compose stack installed node_modules into the bind-mounted checkout, so the host and the stack shared one directory. Native modules (e.g. @vscode/sqlite3) are compiled against the installing environment's glibc: the stack's node:24-trixie (glibc 2.41) produces binaries a typical host (e.g. Rocky 9, glibc 2.34) cannot load, so every `docker compose up` broke host-side tests and tooling, and host reinstalls clobbered the stack. The agent's `make build` (npm ci + npm prune --omit=dev) also stripped TypeScript from the host's agent/node_modules. - compose.yml: create-a-container, client, and agent node_modules are named volumes layered over the checkout in the node/client services, and mounted read-only into the proxmox service. The host keeps its own node_modules. - create-manager.sh: Proxmox bind mount points are non-recursive (`mount -o bind`), so the Manager LXC's repo mount (mp0) would still show the host's node_modules underneath. Pass the two node_modules volumes to the LXC as their own mount points. Mount points are defined in one list and added by ensure_bind_mounts, which takes the index after the highest in use (Proxmox mounts in index order, so nested mounts land on top of the repo and never reuse another mount's index). Already-provisioned CTs get missing mounts added (stopped/started around the change) instead of an early exit, so existing dev stacks migrate without being recreated. - development-workflow.md: document the host/stack split and the caveat that host `npm ci` while the stack runs detaches the volume until restart.
Contributor
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is coherent and live-verified; only a minor documentation clarification remains.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Separates host and development-stack dependencies to prevent native module ABI conflicts.
Changes:
- Adds dedicated
node_modulesvolumes. - Migrates Manager LXC bind mounts idempotently.
- Documents dependency isolation and reset behavior.
| File | Description |
|---|---|
compose.yml |
Defines and mounts isolated dependency volumes. |
images/proxmox-ve/create-manager.sh |
Adds and migrates nested LXC bind mounts. |
mie-opensource-landing/docs/developers/development-workflow.md |
Documents dependency-volume behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+98
to
+101
| `node_modules` with your host. `create-a-container/node_modules`, | ||
| `create-a-container/client/node_modules`, and `agent/node_modules` are named | ||
| volumes layered over the checkout, and the Manager CT receives them as their own | ||
| mount points. The stack installs into those volumes and builds native modules |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The compose dev stack installs
node_modulesinto the bind-mounted checkout, so the host and the stack share one directory. Native modules are compiled against the installing environment's glibc:node:24-trixie(glibc 2.41) builds@vscode/sqlite3needingGLIBC_2.38, which a typical host can't load (e.g. Rocky 9, glibc 2.34):So every
docker compose upbreaks host-side tests and tooling, and a host reinstall has to be redone after the nextup. The agent'smake build(npm ci+npm prune --omit=dev) also strips TypeScript out of the host'sagent/node_modules.Change
compose.yml—create-a-container/node_modules,create-a-container/client/node_modules, andagent/node_modulesare named volumes layered over the checkout in thenode/clientservices and mounted read-only intoproxmox. The host keeps its ownnode_modules; the stack keeps its own.images/proxmox-ve/create-manager.sh— Proxmox bind mount points are non-recursive (PVE::LXC::__bindmount_dorunsmount -o bind), so the Manager LXC's repo mount (mp0) would still show the host'snode_modulesunderneath the volume. The two volumes are now passed to the LXC as their own mount points.MANAGER_BIND_MOUNTS) and added byensure_bind_mounts, which uses the index after the highest one in use. Proxmox mountsmp0, mp1, …in index order (mount_all→foreach_volume), so nested mounts land on top of the repo and never reuse an index another mount depends on.exit 0immediately. It now gets missing mounts added (the CT is stopped and started around the change), and a second run is a no-op.development-workflow.md— documents the host/stack split, the new volumes, and one caveat: runningnpm cion the host while the stack is running deletes the directory the volume is mounted on. That detaches the volume from running containers until they restart.npm installis unaffected.Verification
End-to-end on a fresh stack from this branch (Podman 5.8 in a rootful
podman machine). The hostnode_modulesdirectories held only marker files, so the Manager could only start if it loaded the volumes:node/clientinstalled into the volumes (repeatednpm ciinto a volume-backednode_modulesworks on npm 11; noEBUSY);client/diststill written to the checkout.mp0repo,mp1create-a-container/node_modules,mp2agent/node_modules. Inside the CT, both paths are the volumes (396 / 18 entries), not the host markers.container-creatorrunsnodemonfrom the volume;@vscode/sqlite3loads in the CT; Manager HTTPS 200; agent checking in;bootstrap-managercreated the site, node, and domain.node_modulesstill contain only their markers after the whole run; checkout clean.mp0only (the state of every current dev stack) and re-rancreate-manager.sh. It addedmp1/mp2and restarted the CT; the Manager came back healthy on the volume. A second run was a no-op.ensure_bind_mountsindexing was also exercised against a stubpctfor: a fresh CT, an existingmainstack, a Support user-defined shared volumes for containers (#421) #476 stack (which already hasmp1= volumes root → getsmp2/mp3), and an already-migrated CT.Notes
mp1directly increate-manager.sh. When it's rebased onto this, its mount should be added toMANAGER_BIND_MOUNTSrather than hard-coding an index. Existing Support user-defined shared volumes for containers (#421) #476 stacks are handled either way (they getmp2/mp3, as tested).ip_unprivileged_port_start=1024withpodman machine), current Podman refuses to startproxmoxbecause of the127.0.0.1:80/443mappings. I remapped those locally with an uncommitted override to run the verification.