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 by the same owner 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, new containers never receive it, and its live mp0 slot is reserved so user volumes on those containers start at mp1. 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.
How to review
The branch is five commits, one per layer; each builds and passes tests on its own, so reading them in order is easiest:
volumes: add the Volume model and host-path helpers — model, Volumes migration, validation, buildMountConfig, host-path derivation, Proxmox/Docker/dummy API helpers.
volumes: provision and attach volumes in the container jobs — removes the quick_and_dirty stopgap; the readiness barrier in create/reconfigure; agent snapshot + check-in handling; backfill migration and reconcile CLI for existing containers.
agent: create volume directories and report per-volume results — the site-agent side (mount check, mkdir/chown/chmod, result persistence and retry).
volumes: expose volumes in the API, OpenAPI spec, and client — container/node endpoints, schema, and the form UI.
volumes: document volumes and wire them into the dev stack — docs and create-manager.sh.
Earlier review rounds were folded into these commits; their threads show as outdated but remain readable in the timeline.
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 — with the shared volumes root bind-mounted in — creates each directory:
It first checks (/proc/self/mountinfo) that the path is on a mounted filesystem other than its own root, and reports the volume failed if the volumes root isn't mounted, rather than creating it inside the agent where Proxmox can't see it.
It then mkdirs, chowns to the consuming containers' id-mapped root (100000) best-effort, and chmods per mode. In an unprivileged agent (the pct create default), mkdir already yields that owner and the chown is a tolerated no-op (EINVAL/EPERM); in a privileged agent the chown is what makes RW volumes writable.
Volume.status barrier. Rows are persisted pending; the snapshot advertises them at the site level; the agent reports per-volume results at check-in (persisted across agent runs until delivered, and never cached behind the ETag on failure, so failures retry); the manager writes Volume.status (ready/failed), scoped to the checking-in site. 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.
Isolation of retained data. Directories are …/volumes/site-<siteId>/<owner>/<hostname>/<name>: scoped by site (hostnames are only unique per site) and by owner, so only the same owner reattaches retained data — a different user reusing a deleted hostname gets fresh directories.
Shared storage required for cross-node durability (a path-backed shared filesystem: CephFS or NFS); the manager warns (not fails) at node-save time when the volume storage is not shared=1 or not active on every node.
Agentless backends: Docker volumes map to Docker binds (Docker auto-creates the bind source); dummy-node volumes are simulated. Both are marked ready locally and are never advertised to the site agent.
Retain-on-delete: the agent never removes directories. 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); mount-path validation (absolute, canonicalized, no provider delimiters/whitespace/control chars/./..); buildMountConfig() renders mpN, skipping builtin/host-path-less rows and reserving the legacy slot.
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; down() removes both. Raw SQL uses quoted identifiers (Postgres-safe).
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 / containerVolumeHostPath (site + owner scoping, owner must be a safe path segment) / isAgentlessNodeType used by the jobs and the agent snapshot.
Jobs
bin/create-container.js — 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; volume attach/detach on a provisioned container always enqueues it.
bin/reconcile-volumes.js — new idempotent CLI (reports applied/skipped/failed).
Agent (agent)
src/volumes.ts — provisions the site's volume directories (site.volumes[]): mount check, mkdir, best-effort chown (tolerating EINVAL/EPERM/ENOSYS), chmod; reports results keyed by Volume.id.
src/state.ts / src/index.ts — pending volume results persisted in agent state until a check-in carrying them completes; ETag not saved when a volume failed, so it retries.
API / OpenAPI / client
routers/api/v1/containers.js — volumes on create (attach) / update (attach + { id, detach: true }, strict numeric ids, duplicate/collision checks, reserved quick_and_dirty name/mount rejected); env/entrypoint only changed when the request includes those keys, so volume-only updates don't clobber them.
routers/api/v1/agents.js — check-in accepts a volumes results map and writes Volume.status/statusMessage/appliedAt for volumes in the checking-in site only.
client/ — Volumes editor in the container form (ro/rw, status, detach; restart detection includes volume changes); 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 so the compose stack is turnkey for volume testing. (dev stack: keep node_modules out of the shared checkout #483 generalizes how the Manager LXC's mount points are added; when both land, this mount should move into its MANAGER_BIND_MOUNTS list.)
Docs (mie-opensource-landing)
New core-concepts/volumes.md (owner/site-scoped retention, shared-storage requirement, not backed up, Docker behavior); deploying-agents.md corrected to one agent per site with the volumes bind-mount + pre-create steps, the agent's mount check, and the default id-map requirement (custom lxc.idmap unsupported for volumes); containers.md, nodes.md, developers/agent.md, developers/database-schema.md updated.
Verification
Unit/integration tests — create-a-container: volumes (43), normalize-volume-attach (15), containers.volumes (8), nodes.volume-warnings (5), agent-config-volumes (4), docker-volumes (3), agents.volumes (2), backfill-migration guard (1); agent: 19 (volume provisioning incl. mount-guard refusal and chown tolerance, state persistence). All other existing suites pass individually.
Migrations round-trip on sqlite and Postgres 16; the backfill's identifier quoting was verified against a live Postgres.
Agent builds (tsc); client type-checks and builds.
Dummy-node end-to-end: no volumes → no mounts; a user rw volume attaches at its owner-scoped path; barrier failure aborts before CT creation (no orphan).
Dev stack: the agent's mount check resolves volume paths to the bind-mounted /var/lib/vz/volumes inside the running Manager LXC.
Review history
Reviewed over several rounds (manual and Copilot): agent topology (one per-site agent, bind-mounted volumes root), full removal of the quick_and_dirty stopgap, a Postgres-only backfill crash, then Copilot findings on mount safety, cross-site/cross-owner isolation, retries, env/entrypoint preservation, the volume editor, and agent mount verification. All raised findings are resolved in code.
Note: full agent-driven provisioning on a real Proxmox cluster (barrier round-trip, mpN attach, in-container writability, retain-on-delete) remains the last mile that unit tests structurally can't cover.
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.
The reason will be displayed to describe this comment to others. Learn more.
Good question. Current behavior: the router .trim()s the name, then isValidVolumeName requires ^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$ — so a leading/trailing space is trimmed away, and any internal space (or other symbol outside letters/digits/./-/_) is rejected with a validation error rather than silently rewritten. I kept it strict-and-reject rather than translating spaces to _ on purpose: the name is used verbatim as a host path segment and is uniquely indexed per container, so silent normalization (my volume → my_volume) could surprise the user, mask typos, and create ambiguity if it collided with an explicit my_volume. If you'd prefer a friendlier UX, I think the right place is the client form (show the sanitized name as the user types) rather than server-side coercion — happy to add that if you want it. Behavior verified: " data", "da ta", "data ", "my volume" all rejected; "data", "ok_name" accepted.
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”).
Rebased onto latest origin/main and addressed the round-2 review (2b45893):
Agent chown EINVAL (high): the agent now tolerates EINVAL (as well as EPERM/ENOSYS) — in a normal unprivileged agent namespace chown(100000) targets an unmapped id and returns EINVAL; ignoring it (ownership is already correct from the id-map) stops a normal agent from reporting every volume failed.
ETag durability: pending per-volume results are persisted in State and cleared only after a check-in that carried them completes, so a process exit between reconcile and the next check-in no longer loses them behind a cached ETag.
Docs:volumes.md now shows the real …/site-<siteId>/<hostname>/<name> path; deploying-agents.md documents custom lxc.idmap as unsupported for volumes (the owner id is always 100000) instead of an unworkable workaround.
Name-validation question: answered inline — spaces are trimmed then rejected (not silently rewritten); happy to add client-side sanitization if preferred.
New agent tests (16 total): chown-to-unmapped-id tolerance and State pending-results round-trip. All agent + manager volume suites and the client type-check pass; branch is now up to date with main.
Rebased onto latest main and addressed the round-3 review (fd78696):
High
env/entrypoint no longer clobbered: replaced the isRestartOnly heuristic with presence-based handling — env/entrypoint are only recomputed when the request carries that key. { restart: true, volumes: [...] } preserves saved env/entrypoint; an explicit []/''/null still clears them.
Volume editor fixed: it keyed "existing vs new" off f.id (react-hook-form's field key, always truthy), so new rows rendered read-only and couldn't be filled in. It now derives existing/builtin/values from the watched row's real server id, using f.id only for the React key.
Strict detach id: parsed with Number() instead of parseInt(), so '12garbage' is rejected (400) rather than detaching volume 12.
Medium/Low
Backfill migration down() now removes the pending reconcile-volumes job it enqueued (no orphan on rollback; verified count 1→0).
New tests cover env/entrypoint preservation, explicit-clear, and malformed-detach-id. All volume suites, agent (16), and client type-check pass; migration up/down round-trips. Branch is up to date with main.
Reconcile volumes on unchanged-config 304 responses
agent/src/index.ts:72
Volume reconciliation only runs after a 200 response; the existing 304 branch immediately returns after this check-in. If a volume directory is deleted or permissions change while the config ETag is unchanged, subsequent timer runs never call reconcileVolumes, so the manager continues to see ready and a later container create can fail on a missing host path. Run the volume self-heal on 304s as well, while retaining the pending-result delivery logic.
Restrict builtin volume backfill to eligible containers
This backfills a builtin volume for every container, but the retired quick_and_dirty mount was only added in the Proxmox-template branch of create-container.js; Docker-image containers did not receive that mp0. Docker containers will therefore show a nonexistent built-in volume and reserve a fake legacy slot in buildMountConfig. Restrict the backfill to containers that could actually have the legacy mount (or verify the live provider config) instead of all containers.
The reason will be displayed to describe this comment to others. Learn more.
Updated the PR description. It now describes the actual behavior: mkdir, then a best-effort chown to the id-mapped owner (100000), which is a tolerated no-op (EINVAL/EPERM) in an unprivileged agent — the pct create default — and the real fix in a privileged one, then chmod. I also refreshed the other stale parts: owner/site-scoped retention, the mount check, agentless backends, test counts, and review history.
A detach request for a legacy builtin volume is accepted, but this where clause deletes zero rows and the handler still returns success (and may enqueue a no-op reconfigure job). That contradicts the comment that built-ins cannot be detached and leaves clients believing the mount was removed; reject builtin/nonexistent IDs explicitly or report a conflict instead of silently ignoring them.
Introduce the data layer for user-defined container volumes, replacing the
hardcoded quick_and_dirty mp0 stopgap (removed in the next commit).
- Volume model + Volumes table: per-container name, hostPath, mountPath,
ro/rw mode, scope, a `builtin` flag (legacy quick_and_dirty rows only), and
a pending/ready/failed status with statusMessage/appliedAt that the create
job will block on. Unique per container by name and by mount path.
- Validation: names are single safe path segments; mount paths are absolute,
canonicalized (duplicate/trailing slashes), and may not contain provider
delimiters (',' ':'), backslashes, whitespace, control characters, or '.'/
'..' segments, since they are interpolated into Proxmox mpN and Docker bind
syntax.
- Volume.buildMountConfig renders rows to mpN. Legacy builtin rows are never
rendered, but their low indices are reserved so user volumes added to a
pre-existing container never overwrite its still-live mp0.
- utils/volumes.js:
- resolveVolumesRoot derives <storage path>/volumes from the node storage's
ACTUAL configured path (new ProxmoxApi.storageConfig), never assuming the
/mnt/pve/<storage> layout, and rejects block storage without a path.
- Host paths are <root>/site-<siteId>/<owner>/<hostname>/<name>: site-scoped
because hostnames are only unique per site, owner-scoped so retained data
is only ever reattached for the same owner. Unsafe owner segments are
refused, not rewritten.
- isAgentlessNodeType: docker and dummy volumes don't use the site agent
and are marked ready locally.
- DockerApi maps mpN values to HostConfig.Binds on recreate; DummyApi gains a
simulated storageConfig.
API descriptions incorrectly say omitted fields are cleared
create-a-container/openapi.v1.yaml:939
The update handler now distinguishes omitted fields from explicit clears, so omitted environmentVars and entrypoint are preserved even on non-restart, volume-only updates. The existing descriptions immediately above still say omission clears them, which gives API consumers the wrong contract for the behavior changed in this PR; update those descriptions to say omission preserves and blank/null clears.
Retire the hardcoded quick_and_dirty mp0 (buildSharedVolumeMp0) and have the
container jobs attach the container's Volume rows instead. Nothing is mounted
unless the container has volumes.
Proxmox validates, but does not create, a bind mount's host directory, and has
no mkdir/exec API, so directories are created out of band by the site agent
(next commit). The manager side:
- create-container.js: prepareVolumes derives each volume's host path and
BLOCKS until every Volume.status is `ready` (bounded, VOLUME_READY_TIMEOUT_MS,
default 5 min), failing on `failed` with the agent's message. This runs
BEFORE the CT is created, so a provisioning failure never leaves an orphaned
half-created container. mpN is applied once the CT exists.
- reconfigure-container.js: the same readiness wait, then re-renders mpN on
restart; a mount change forces a restart. Skipped on Docker nodes, whose
binds are applied at recreate and whose lxcConfig exposes no mpN.
- Agent snapshot (agent-config.js): advertises the site's derived, non-builtin
volumes (id, hostPath, mode, owner uid/gid) at the site level — there is one
agent per site — independently of container IPs, so volumes are advertised
during creation. Agentless (docker/dummy) volumes are excluded.
- Agent check-in (POST /agents): accepts a per-volume results map and writes
Volume.status/statusMessage/appliedAt, only for volumes whose container is
in the checking-in site. Built-in rows are never changed by an agent.
- Existing containers: the backfill migration records their still-live
quick_and_dirty mount as a builtin row (informational, never re-applied) and
enqueues bin/reconcile-volumes.js, an idempotent CLI that brings live mpN in
line with Volume rows (no-op for pre-existing containers). down() removes
both. Its raw SELECT quotes the table name (Postgres folds unquoted names).
The site agent provisions the host directories the manager's volume barrier
waits on. There is one agent per site; the site's shared volumes root is
bind-mounted into it (see Deploying Agents), so this one agent provisions
volumes for every node in the site.
For each volume in the snapshot's site.volumes[]:
- Refuse unless hostPath is on a mounted filesystem other than the agent's
root (via /proc/self/mountinfo). Otherwise mkdir -p would silently create the
path inside the agent container and report success while the Proxmox host
path is still missing. Fails closed if the mount table can't be read.
- mkdir -p, then chown to the consumer's id-mapped root (100000) best-effort,
then chmod (rw 0770, ro 0550). In an unprivileged agent (the pct create
default) mkdir already yields that owner and chown returns EINVAL/EPERM,
which is tolerated; in a privileged agent the chown is what makes RW volumes
writable. Directories are never removed (retain-on-delete).
Results are reported on the next check-in as a map keyed by Volume id:
- They are persisted in state.json until a check-in carrying them completes,
so an exit between reconcile and check-in doesn't lose them.
- When any volume failed, the config ETag is not saved, so the next run
re-fetches (200, not 304) and retries instead of staying failed until the
config changes.
Container API:
- POST /containers accepts volumes: [{ name, mountPath, mode }], persisted
`pending` for the create job to provision.
- PUT /containers/:id accepts attaches and { id, detach: true }. On a
provisioned container, any volume change enqueues a reconfigure job (it
derives paths, waits for the agent, and applies mpN), even without
restart: true; on an unprovisioned one the rows apply at create.
- Validation: strict numeric detach ids; duplicate names/mounts within the
request or against existing volumes (409); the reserved quick_and_dirty
name and mount (compared canonically) are rejected. hostPath is never
accepted from clients.
- env/entrypoint are only changed when the request includes those keys, so a
volume-only update (with or without restart) no longer clears them.
- Container payloads include volumes (name, mountPath, mode, status, ...).
Node API: create/update return advisory `warnings` when the volume storage
isn't path-backed, shared, or active on every cluster node (warning, not an
error — single-node sites are fine).
OpenAPI: Volume, VolumeAttach, VolumeDetach (VolumeChange oneOf), the new
container/node fields, and the check-in volumes results map.
Client: a Volumes editor on the container form (attach, ro/rw, status, detach;
volume changes count toward restart detection) and node-save warning toasts.
Docs:
- New admins/core-concepts/volumes.md: how volumes are provisioned and
attached, the site/owner-scoped host path and same-owner reattach
semantics, the path-backed shared-storage requirement (CephFS/NFS; block
storage can't host volumes), that volume data is NOT backed up by the
platform (bind mounts aren't in vzdump), retain-on-delete, and Docker
behavior. Linked from the core-concepts index and nav.
- deploying-agents.md: one agent per site (not per node); the one-time
volumes setup (pre-create the root 100000:100000 on shared storage and
bind-mount it into the agent at the same path); the agent's mount check;
custom lxc.idmap unsupported for volumes.
- containers.md, nodes.md, developers/agent.md, developers/database-schema.md
updated for volumes and the check-in contract.
Dev stack: create-manager.sh pre-creates /var/lib/vz/volumes (the derived root
for the dev node's `local` storage) owned 100000:100000 and bind-mounts it into
the Manager CT, whose embedded agent provisions volumes.
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.
cmyers-mieweb
left a comment
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 by the same owner 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, new containers never receive it, and its livemp0slot is reserved so user volumes on those containers start atmp1. 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.How to review
The branch is five commits, one per layer; each builds and passes tests on its own, so reading them in order is easiest:
volumes: add the Volume model and host-path helpers— model,Volumesmigration, validation,buildMountConfig, host-path derivation, Proxmox/Docker/dummy API helpers.volumes: provision and attach volumes in the container jobs— removes thequick_and_dirtystopgap; the readiness barrier in create/reconfigure; agent snapshot + check-in handling; backfill migration and reconcile CLI for existing containers.agent: create volume directories and report per-volume results— the site-agent side (mount check, mkdir/chown/chmod, result persistence and retry).volumes: expose volumes in the API, OpenAPI spec, and client— container/node endpoints, schema, and the form UI.volumes: document volumes and wire them into the dev stack— docs andcreate-manager.sh.Earlier review rounds were folded into these commits; their threads show as outdated but remain readable in the timeline.
Design highlights
mpNhost path, so the directory must exist before it is set. The one per-site agent — with the shared volumes root bind-mounted in — creates each directory:/proc/self/mountinfo) that the path is on a mounted filesystem other than its own root, and reports the volumefailedif the volumes root isn't mounted, rather than creating it inside the agent where Proxmox can't see it.mkdirs,chowns to the consuming containers' id-mapped root (100000) best-effort, andchmods per mode. In an unprivileged agent (thepct createdefault),mkdiralready yields that owner and the chown is a tolerated no-op (EINVAL/EPERM); in a privileged agent the chown is what makes RW volumes writable.Volume.statusbarrier. Rows are persistedpending; the snapshot advertises them at the site level; the agent reports per-volume results at check-in (persisted across agent runs until delivered, and never cached behind the ETag on failure, so failures retry); the manager writesVolume.status(ready/failed), scoped to the checking-in site. The create job waits onreadybefore the container is provisioned, so a provisioning failure never orphans a half-created CT; only then ismpNset.…/volumes/site-<siteId>/<owner>/<hostname>/<name>: scoped by site (hostnames are only unique per site) and by owner, so only the same owner reattaches retained data — a different user reusing a deleted hostname gets fresh directories.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); mount-path validation (absolute, canonicalized, no provider delimiters/whitespace/control chars/./..);buildMountConfig()rendersmpN, skippingbuiltin/host-path-less rows and reserving the legacy slot.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;down()removes both. Raw SQL uses quoted identifiers (Postgres-safe).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>); sharedderiveVolumeHostPaths/containerVolumeHostPath(site + owner scoping, owner must be a safe path segment) /isAgentlessNodeTypeused by the jobs and the agent snapshot.Jobs
bin/create-container.js— 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; volume attach/detach on a provisioned container always enqueues it.bin/reconcile-volumes.js— new idempotent CLI (reportsapplied/skipped/failed).Agent (
agent)src/volumes.ts— provisions the site's volume directories (site.volumes[]): mount check,mkdir, best-effortchown(toleratingEINVAL/EPERM/ENOSYS),chmod; reports results keyed byVolume.id.src/state.ts/src/index.ts— pending volume results persisted in agent state until a check-in carrying them completes; ETag not saved when a volume failed, so it retries.API / OpenAPI / client
routers/api/v1/containers.js—volumeson create (attach) / update (attach +{ id, detach: true }, strict numeric ids, duplicate/collision checks, reservedquick_and_dirtyname/mount rejected); env/entrypoint only changed when the request includes those keys, so volume-only updates don't clobber them.routers/api/v1/agents.js— check-in accepts avolumesresults map and writesVolume.status/statusMessage/appliedAtfor volumes in the checking-in site only.routers/api/v1/nodes.js—create_node/update_nodereturn advisory shared-storagewarnings.openapi.v1.yaml—Volume,VolumeAttach/VolumeDetach(VolumeChangeoneOf), container/create/update fields, nodewarnings, check-involumesmap.client/— Volumes editor in the container form (ro/rw, status, detach; restart detection includes volume changes); 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 so the compose stack is turnkey for volume testing. (dev stack: keep node_modules out of the shared checkout #483 generalizes how the Manager LXC's mount points are added; when both land, this mount should move into itsMANAGER_BIND_MOUNTSlist.)Docs (
mie-opensource-landing)core-concepts/volumes.md(owner/site-scoped retention, shared-storage requirement, not backed up, Docker behavior);deploying-agents.mdcorrected to one agent per site with the volumes bind-mount + pre-create steps, the agent's mount check, and the default id-map requirement (customlxc.idmapunsupported for volumes);containers.md,nodes.md,developers/agent.md,developers/database-schema.mdupdated.Verification
create-a-container: volumes (43), normalize-volume-attach (15), containers.volumes (8), nodes.volume-warnings (5), agent-config-volumes (4), docker-volumes (3), agents.volumes (2), backfill-migration guard (1);agent: 19 (volume provisioning incl. mount-guard refusal and chown tolerance, state persistence). All other existing suites pass individually.tsc); client type-checks and builds.rwvolume attaches at its owner-scoped path; barrier failure aborts before CT creation (no orphan)./var/lib/vz/volumesinside the running Manager LXC.Review history
Reviewed over several rounds (manual and Copilot): agent topology (one per-site agent, bind-mounted volumes root), full removal of the
quick_and_dirtystopgap, a Postgres-only backfill crash, then Copilot findings on mount safety, cross-site/cross-owner isolation, retries, env/entrypoint preservation, the volume editor, and agent mount verification. All raised findings are resolved in code.