You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Closes#421. Unblocks #475 (converged container needs a durable read-write /mnt/data volume that survives delete+recreate).
Summary
Replaces the hardcoded quick_and_dirtymp0 bind mount with a real volumes feature: named, per-container volumes with per-attachment ro/rw permissions, host directories provisioned automatically by the site agent, and data that is durable across container replacement on the same hostname.
The stopgap is fully removed: buildSharedVolumeMp0() is gone and no volume is auto-attached to a new container — a container gets only the volumes its creator requests. Pre-#421 containers keep their existing quick_and_dirty mount, which the backfill migration records in the DB as a legacy builtin row purely for reference; it is never re-applied and new containers never receive it. Host paths are derived from each node storage's actual configured path, and the async gap between the site agent (which creates directories) and the synchronous create job (which sets the bind mount) is closed with a Volume.status barrier.
Design highlights
Host-path bind mounts, created out of band. Proxmox validates (does not create) the mpN host path, so the directory must exist before it is set. The one per-site agent — an unprivileged LXC guest with the shared volumes root bind-mounted in — mkdir/chmods each directory. Its root maps to host UID/GID 100000 (the same mapped root as consuming containers), so RW volumes are writable without any chown (which would be EPERM inside the guest).
Volume.status barrier. Rows are persisted pending; the snapshot advertises them at the site level; the agent reports per-volume results at check-in; the manager writes Volume.status (ready/failed). The create job waits on ready before the container is provisioned, so a provisioning failure never orphans a half-created CT; only then is mpN set.
Shared storage required for cross-node durability; the manager warns (not fails) at node-save time when the volume storage is not shared=1 or not active on every node.
Docker nodes: volumes map to Docker binds; Docker auto-creates the bind source, so no agent/shared-storage precondition on a single host.
Retain-on-delete: the agent never removes directories, so a later container with the same hostname reattaches the data. Volume data is not in vzdump — documented as not backed up by the platform.
Changes
Data model & migrations (create-a-container)
models/volume.js + migrations/…-create-volumes.js — Volume model (name/hostPath/mountPath/mode/scope/builtin/status/statusMessage/appliedAt), name validation (no traversal), buildMountConfig() (renders mpN, skips builtin/host-path-less rows).
migrations/…-backfill-quick-and-dirty-volumes.js — records the already-live quick_and_dirty mount on pre-existing containers as a legacy builtin row (never re-applied) and enqueues the one-time reconciliation.
Host-path derivation
utils/proxmox-api.js:storageConfig() reads the storage's actual path + shared; utils/volumes.js:resolveVolumesRoot() derives <path>/volumes (never assumes /mnt/pve/<storage>); shared deriveVolumeHostPaths used by the create/reconfigure/reconcile jobs.
Jobs
bin/create-container.js — buildSharedVolumeMp0() retired, no auto-seed; prepareVolumes() blocks on Volume.status='ready'before CT creation; mpN applied after. Barrier timeout configurable via VOLUME_READY_TIMEOUT_MS.
bin/reconfigure-container.js — reconciles volume mounts on restart.
bin/reconcile-volumes.js — new idempotent CLI (reports applied/skipped/failed).
Agent (agent)
src/volumes.ts — provisions the site's volume directories (site.volumes[]): mkdir+chmod only (no chown); reports results keyed by Volume.id on the next check-in.
API / OpenAPI / client
routers/api/v1/containers.js — volumes on create (attach) / update (attach + { id, detach: true }); normalizeVolumeAttach validates and rejects the reserved quick_and_dirty name/mount.
routers/api/v1/agents.js — check-in accepts a volumes results map and writes Volume.status/statusMessage/appliedAt.
client/ — Volumes editor in the container form (ro/rw, status, detach); node-save warning toasts.
Dev stack
images/proxmox-ve/create-manager.sh — pre-creates the volumes root (100000:100000) and bind-mounts it into the Manager agent (mp1) so the compose stack is turnkey for volume testing.
Docs (mie-opensource-landing)
New core-concepts/volumes.md; deploying-agents.md corrected to one agent per site with the volumes bind-mount + pre-create steps and the id-map (100000) constraint; containers.md, nodes.md, developers/agent.md updated.
Verification
Lint/build/unit tests + a live Postgres migration check; full cluster integration is the remaining last mile.
Migrations round-trip on both sqlite and Postgres 16 (up/down); the backfill's identifier-quoting fix was verified against a live Postgres (reproduced the crash, confirmed the fix, checked the backfilled row + reconcile job).
Agent builds (tsc) and its suite passes (13); client type-checks and builds.
Dummy-node end-to-end: no volumes → no mounts (stopgap gone); a user rw volume waiting on the barrier before creation then attaching; barrier-failure aborting before CT creation with containerId null (no orphan); backfill recording the legacy row without re-mounting.
Review history
The design was reviewed over three rounds (topology fix for the one-per-site unprivileged-guest agent; full removal of the quick_and_dirty stopgap; and a Postgres-only backfill-migration crash found via live testing, now fixed and guarded). All blocking findings are resolved.
Note: full agent-driven provisioning on a real Proxmox cluster (barrier round-trip, mpN attach, in-container writability, retain-on-delete) is the last mile that unit tests structurally can't cover; the dev stack is now wired to exercise it.
Replace the hardcoded quick_and_dirty mp0 bind mount with a real volumes
feature: named per-container volumes with ro/rw permissions, host directories
provisioned by the site agent, durable across container delete+recreate.
- Volume model + migration; backfill built-in quick_and_dirty for existing
containers and enqueue a one-time reconciliation job.
- Derive hostPath from the node storage's actual configured path (new
ProxmoxApi.storageConfig); volume-name validation (no traversal).
- Retire buildSharedVolumeMp0(); render all Volume rows to mpN in create and
reconfigure. New bin/reconcile-volumes.js CLI.
- Agent creates host dirs (mkdir + chown to the unprivileged CT id-mapped
UID/GID) and reports per-volume results via an extended agent_check_in; the
manager writes Volume.status/statusMessage/appliedAt.
- Create-job sync barrier: block on Volume.status='ready' before setting mpN.
- Node-save shared-storage warning (shared!=1 or not on every node) surfaced on
create/update_node and in the admin UI.
- volumes array on create/update_container (attach/detach, ro/rw) + OpenAPI +
client form; retain-on-delete; Docker nodes map volumes to binds.
- Docs: volumes admin/dev pages, shared-storage requirement, no-backup caveat.
Reviewer found the volume-provisioning mechanism could not work on a real
cluster: the agent is one-per-site running in an unprivileged LXC guest, so it
could not create Proxmox-host directories, matched at most one node, and would
EPERM on chown 100000. Realign the mechanism with the actual topology (keep the
agent; mount the shared volumes root into it) and fix the related findings.
Blocking (#1):
- Advertise volumes at the site level (site.volumes[]); drop per-node
os.hostname matching so the single site agent provisions all site volumes.
- Agent no longer chowns (EPERM in guest); mkdir+chmod only. Ownership comes
from the id-map (agent guest root = host 100000 = consuming containers'
mapped root), so RW volumes are writable without chown.
- Docs corrected to one-agent-per-site; add installer steps to pre-create the
volumes root (100000:100000) and bind-mount it into the agent guest.
Medium/low:
- #2: run the Volume.status barrier BEFORE creating the CT, so a provisioning
failure never orphans a half-created container (containerId stays null).
- #3: reject the reserved quick_and_dirty name/mount in normalizeVolumeAttach.
- #4: reconcile-volumes reports applied/skipped/failed separately.
- #6: barrier timeout configurable via VOLUME_READY_TIMEOUT_MS.
- #7: consolidate host-path derivation into utils/volumes deriveVolumeHostPaths,
shared by the create/reconfigure/reconcile jobs.
Tests updated for the site-level snapshot shape and chmod-only agent; new
normalize-volume-attach suite. All volume suites, agent build/tests, and client
type-check pass.
New containers are no longer auto-attached a quick_and_dirty volume; a
container gets only the volumes its creator requests, and one with none gets no
bind mounts. This replaces the stopgap rather than perpetuating it as a
permanent built-in.
- create-container.js: drop the quick_and_dirty auto-seed in prepareVolumes;
a container with no volumes short-circuits with no mounts.
- utils/volumes.js: remove quickAndDirtyVolumeSpec; deriveVolumeHostPaths now
leaves legacy builtin rows untouched (no host-path derivation).
- models/volume.js: buildMountConfig skips builtin rows and any row without a
host path, so a backfilled legacy row never renders a broken mpN.
- Retained for backward compatibility: the backfill migration (records the
already-live mount on pre-existing containers as a builtin row, never
re-applied) and the reserved-name guard in normalizeVolumeAttach.
- Docs/OpenAPI/comments: nothing is mounted by default; the builtin flag is a
legacy backfill artifact only.
Tests updated (removed the spec test; added a builtin/undived skip assertion to
buildMountConfig). Verified end-to-end on a dummy node: no volumes -> no mounts
and no quick_and_dirty row; user volume still attaches; backfill still records
the legacy row without re-mounting it.
)
The #421 backfill migration used an unquoted 'SELECT id FROM Containers',
which Postgres case-folds to 'containers' and fails with
relation "containers" does not exist — crashing the Manager on startup
against the production database. SQLite is case-insensitive, so the sqlite
migration round-trip could not catch it.
- Quote the identifier via queryInterface.quoteIdentifier('Containers'),
matching the repo pattern (20251104193601-create-node.js). bulkInsert/
bulkDelete were already safe.
- Verified on a live Postgres 16: reproduced the crash with the unquoted
version, confirmed the fix migrates cleanly, backfills the legacy row, and
round-trips up/down.
- Add a dialect-independent regression guard asserting the raw SELECT quotes
the Containers identifier.
- Document the unprivileged id-map base (100000) assumption in
deploying-agents (round-3 review low #2).
…agent (#421)
Make the docker-compose nested-Proxmox stack turnkey for volume testing:
pre-create the derived volumes root (/var/lib/vz/volumes for the dev node's
`local` dir storage) owned by the unprivileged-CT id-mapped root (100000:100000)
and bind-mount it into the Manager agent at the same path (mp1), so the host
path the manager derives resolves identically inside the agent. Mirrors the
one-time setup documented in Deploying Agents.
Adds named, persistent per-container volumes with agent provisioning, readiness barriers, Proxmox/Docker support, API and client changes, migrations, tests, and documentation.
Changes:
Adds volume persistence, path derivation, legacy backfill, and reconciliation.
Extends agent check-ins, container APIs, OpenAPI schemas, and client editing.
Updates deployment guidance and development-stack volume setup.
Documents agent setup and volume mounts. nit (1 vote): The custom-id-map warning references an undefined ownership setting instead of the chown command.
Prepares and mounts the development volume root. critical (1 vote): The Manager guest and root agent do not establish the assumed UID/GID 100000 ownership for writable volumes.
create-a-container/utils/volumes.js
Resolves storage roots and derives volume paths. critical (3 votes): Paths keyed only by hostname and volume name can collide across sites sharing storage. moderate (2 votes): Dummy-node volumes remain pending and can block creation until timeout.
create-a-container/utils/proxmox-api.js
Reads storage path and shared configuration.
create-a-container/utils/dummy-api.js
Adds simulated storage configuration.
create-a-container/utils/docker-api.js
Maps mounts to Docker binds.
create-a-container/utils/agent-config.js
Advertises site volumes to agents. moderate (2 votes): Docker-node volumes should be excluded from the site-agent snapshot.
Adds volume attach/detach persistence and validation. critical (1 vote): Equivalent normalized forms of the reserved legacy mount can bypass the check. critical (1 vote): Delimiters and control characters in mountPath can corrupt provider configuration. moderate (1 vote): Update duplicate names and paths are not prevalidated. critical (1 vote): Volume mutations without restart do not enqueue reconfiguration.
create-a-container/routers/api/v1/agents.js
Applies agent volume results. critical (2 votes): Volume status updates are not scoped to the checking-in site.
Defines volume and warning schemas. moderate (3 votes): The detach shape conflicts with unconditionally required attach fields.
create-a-container/models/volume.js
Defines the Volume model and mount rendering. critical (2 votes): Renumbering after filtering legacy rows can replace a live quick_and_dirty mount. nit (2 votes): The schema reference omits the Volume entity, association, and fields.
Adds the volume editor. critical (2 votes): Volume changes are omitted from restart detection and may not be applied to the running container.
create-a-container/client/src/lib/types.ts
Adds volume and warning types.
create-a-container/bin/reconfigure-container.js
Reconciles mounts during reconfiguration. critical (2 votes): Built-in legacy rows can cause default local-lvm reconfiguration to fail before filtering. moderate (1 vote): Docker mounts appear changed because lxcConfig() does not expose current mpN keys.
create-a-container/bin/reconcile-volumes.js
Adds volume reconciliation CLI. moderate (1 vote): Built-in rows are resolved before filtering, causing failures on block-backed storage.
create-a-container/bin/create-container.js
Adds readiness barriers and mount attachment.
agent/test/volumes.test.js
Tests agent volume provisioning.
agent/src/volumes.ts
Creates and chmods volume directories.
agent/src/types.ts
Defines volume check-in types.
agent/src/index.ts
Integrates volume reconciliation and reporting. critical (3 votes): Failed reconciliation results can be cached by the current ETag, preventing retries after a 304.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 877e8a9. docs/developers/database-schema.md now includes the Volumes entity block, the Containers ||--o{ Volumes edge in the ER diagram, and a "Volume" entry under Core Models describing the fields, the site-scoped host path, the readiness barrier, and the legacy builtin backfill.
High severity:
- models/volume.js: buildMountConfig reserves the legacy mp0 slot (renders user
volumes at mp<builtinCount>) so the first user volume on a pre-#421 container
no longer overwrites its still-live quick_and_dirty mount.
- utils/volumes.js: derived host path is scoped by site
(<root>/site-<siteId>/<hostname>/<name>) so two sites sharing storage can't
collide (hostnames are only unique per (siteId, hostname)).
- agents.js: applyVolumeResults verifies each volume's container belongs to the
checking-in site before updating status — a check-in can't flip another
site's volume.
- containers.js: mountPath now rejects provider delimiters (comma/colon),
backslashes, whitespace, control chars, and ./.. segments, and is compared
canonically so /mnt//quick_and_dirty and trailing-slash variants can't bypass
the reserved-path/uniqueness checks.
- containers.js: a volume attach/detach on a provisioned container now enqueues
a reconfigure job (the migration reconciler is one-time), and update
duplicate names/paths are prevalidated.
- agent/src/index.ts: a failed volume reconcile no longer persists the ETag, so
the next check-in re-fetches config (not 304) and retries instead of staying
failed until the config changes.
- client: restart detection includes volume attach/detach.
- create-manager.sh + agent: the Manager CT is privileged, so the agent runs as
host root; the agent now chowns each dir to the id-mapped owner (100000)
best-effort (no-op/EPERM-tolerated in an unprivileged guest, the real fix in a
privileged one). uid/gid restored to the snapshot.
Medium:
- OpenAPI: model attach vs detach as a VolumeChange oneOf so a valid detach is
schema-valid.
- agent-config.js: exclude Docker-node volumes from the site-agent snapshot.
- utils/volumes.js: mark dummy-node volumes ready so dev/test doesn't block for
the full barrier timeout.
- reconfigure/reconcile: resolve volumesRoot only when a non-builtin volume
needs a host path (avoids failing on block-backed local-lvm for builtin-only
containers); skip the mpN diff on Docker nodes (lxcConfig exposes no mpN).
Low/docs: database-schema.md gains the Volume entity + edge; RBD removed from
shared-storage guidance (block storage has no path); id-map/chown docs corrected;
'underived' test typo fixed.
New tests: containers.volumes.test.js (attach/detach job enqueue, dup/reserved
rejection); expanded normalize-volume-attach, volumes, agent-config-volumes,
agents.volumes. All volume suites, agent build/tests, client type-check pass.
Cross-site isolation: derived host paths are now …/site-<siteId>/<hostname>/<name>, and the check-in handler verifies each volume's container belongs to the checking-in site before touching its status.
Mount-path safety: provider delimiters (,/:), backslashes, whitespace, control chars, and ./.. are rejected; paths are canonicalized so /mnt//quick_and_dirty / trailing-slash variants can't bypass the reserved/uniqueness checks.
Volume mutations apply: attach/detach on a provisioned container now enqueues a reconfigure job (with restart detection on the client); the agent no longer caches a failed volume reconcile behind the ETag, so failures retry.
Ownership: the agent chowns each dir to the id-mapped owner (100000) best-effort — no-op/EPERM-tolerated in an unprivileged guest, the real fix in the privileged Manager CT.
Medium
OpenAPI attach/detach modeled as a VolumeChangeoneOf; Docker-node volumes excluded from the site-agent snapshot; dummy-node volumes marked ready (no barrier deadlock); resolveVolumesRoot skipped for builtin-only containers (no block-storage failure); Docker mpN diff skipped.
Low/docs
database-schema.md gains the Volume entity + edge; RBD removed from shared-storage guidance; id-map/chown docs corrected; underived typo fixed.
New/expanded tests: containers.volumes.test.js (attach/detach job enqueue, duplicate/reserved rejection, cross-provisioned behavior), plus normalize-volume-attach, volumes, agent-config-volumes, and agents.volumes. All volume suites, the agent build/tests (13), and the client type-check pass; migrations round-trip on sqlite and Postgres.
The one item still deferred is the full live-cluster run of agent-driven provisioning (barrier → mpN attach → in-container writability → retain-on-delete), which unit tests structurally can't cover; the dev stack is wired to exercise it.
useFieldArray reserves id for its generated row key, so f.id is truthy even for a newly appended row. The Add volume row is therefore treated as an existing read-only volume and its name/mount fields cannot be edited. Use the watched form value for existing/builtin (as the Services section does) and keep f.id only for the React key.
Remove the reconciliation job during migration rollback
up inserts a pending Jobs row for node bin/reconcile-volumes.js, but down only deletes the volume rows. Rolling back this migration can therefore leave a stale reconciliation job that runs after the Volumes table has been removed and fails. The rollback should also remove or cancel the job created by this migration.
Document only path-backed shared storage destinations
create-a-container/routers/api/v1/nodes.js:85
This warning recommends Ceph RBD as a valid shared-storage destination, but resolveVolumesRoot() explicitly rejects block storage without a host path; RBD cannot provide the bind-mount directory required by this feature. Name only path-backed shared options such as CephFS/NFS (or say “path-backed shared storage”).
overrides this with a custom `lxc.idmap`, pre-create the volumes root with
the matching `chown` (see the `chown 100000:100000` step above) using that
map's base instead of `100000` — otherwise volume directories will be owned
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
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.
Closes #421. Unblocks #475 (converged container needs a durable read-write
/mnt/datavolume that survives delete+recreate).Summary
Replaces the hardcoded
quick_and_dirtymp0bind mount with a real volumes feature: named, per-container volumes with per-attachmentro/rwpermissions, host directories provisioned automatically by the site agent, and data that is durable across container replacement on the same hostname.The stopgap is fully removed:
buildSharedVolumeMp0()is gone and no volume is auto-attached to a new container — a container gets only the volumes its creator requests. Pre-#421 containers keep their existingquick_and_dirtymount, which the backfill migration records in the DB as a legacybuiltinrow purely for reference; it is never re-applied and new containers never receive it. Host paths are derived from each node storage's actual configured path, and the async gap between the site agent (which creates directories) and the synchronous create job (which sets the bind mount) is closed with aVolume.statusbarrier.Design highlights
mpNhost path, so the directory must exist before it is set. The one per-site agent — an unprivileged LXC guest with the shared volumes root bind-mounted in —mkdir/chmods each directory. Its root maps to host UID/GID100000(the same mapped root as consuming containers), so RW volumes are writable without anychown(which would be EPERM inside the guest).Volume.statusbarrier. Rows are persistedpending; the snapshot advertises them at the site level; the agent reports per-volume results at check-in; the manager writesVolume.status(ready/failed). The create job waits onreadybefore the container is provisioned, so a provisioning failure never orphans a half-created CT; only then ismpNset.shared=1or not active on every node.vzdump— documented as not backed up by the platform.Changes
Data model & migrations (
create-a-container)models/volume.js+migrations/…-create-volumes.js—Volumemodel (name/hostPath/mountPath/mode/scope/builtin/status/statusMessage/appliedAt), name validation (no traversal),buildMountConfig()(rendersmpN, skipsbuiltin/host-path-less rows).migrations/…-backfill-quick-and-dirty-volumes.js— records the already-livequick_and_dirtymount on pre-existing containers as a legacybuiltinrow (never re-applied) and enqueues the one-time reconciliation.Host-path derivation
utils/proxmox-api.js:storageConfig()reads the storage's actualpath+shared;utils/volumes.js:resolveVolumesRoot()derives<path>/volumes(never assumes/mnt/pve/<storage>); sharedderiveVolumeHostPathsused by the create/reconfigure/reconcile jobs.Jobs
bin/create-container.js—buildSharedVolumeMp0()retired, no auto-seed;prepareVolumes()blocks onVolume.status='ready'before CT creation;mpNapplied after. Barrier timeout configurable viaVOLUME_READY_TIMEOUT_MS.bin/reconfigure-container.js— reconciles volume mounts on restart.bin/reconcile-volumes.js— new idempotent CLI (reportsapplied/skipped/failed).Agent (
agent)src/volumes.ts— provisions the site's volume directories (site.volumes[]):mkdir+chmodonly (nochown); reports results keyed byVolume.idon the next check-in.API / OpenAPI / client
routers/api/v1/containers.js—volumeson create (attach) / update (attach +{ id, detach: true });normalizeVolumeAttachvalidates and rejects the reservedquick_and_dirtyname/mount.routers/api/v1/agents.js— check-in accepts avolumesresults map and writesVolume.status/statusMessage/appliedAt.routers/api/v1/nodes.js—create_node/update_nodereturn advisory shared-storagewarnings.openapi.v1.yaml—Volume/VolumeAttachschemas, container/create/update fields, nodewarnings, check-involumesmap.client/— Volumes editor in the container form (ro/rw, status, detach); node-save warning toasts.Dev stack
images/proxmox-ve/create-manager.sh— pre-creates the volumes root (100000:100000) and bind-mounts it into the Manager agent (mp1) so the compose stack is turnkey for volume testing.Docs (
mie-opensource-landing)core-concepts/volumes.md;deploying-agents.mdcorrected to one agent per site with the volumes bind-mount + pre-create steps and the id-map (100000) constraint;containers.md,nodes.md,developers/agent.mdupdated.Verification
Lint/build/unit tests + a live Postgres migration check; full cluster integration is the remaining last mile.
create-a-container: volumes (21), docker-volumes (3), agent-config-volumes (2), agents.volumes (1), nodes.volume-warnings (5), normalize-volume-attach (6), backfill-migration guard (1);agent: volumes (4).tsc) and its suite passes (13); client type-checks and builds.rwvolume waiting on the barrier before creation then attaching; barrier-failure aborting before CT creation withcontainerIdnull (no orphan); backfill recording the legacy row without re-mounting.Review history
The design was reviewed over three rounds (topology fix for the one-per-site unprivileged-guest agent; full removal of the
quick_and_dirtystopgap; and a Postgres-only backfill-migration crash found via live testing, now fixed and guarded). All blocking findings are resolved.