Repository navigation
sandboxd: tenant managed volumes that outlive claims - #370
minatoaquaMK2 wants to merge 3 commits into
Conversation
How we built it and what we decidedThis replaces #368 and #369. We re-checked the branch against every edge case #339 settled, then reworked it. Process. We listed 32 cases from the #339 review (busy and stacked mounts, a release racing a change, restarts mid-change, error codes, Retry-After, journal rules) and checked each one against every managed path. Five parallel reviews found 12 must-fix bugs, each proven by a failing test. We compared two designs before any rework. Every fix landed as a failing test, then the fix. Two independent reviews followed, and the second approved. Decisions.
Deferred. Pool-side error statuses (question 2), online growth, idle and attach-only sleep, re-arming the egress proxy on wake, an e2e hibernate test, and archive export. |
Tenants create node-local data disks with `PUT /v1/volumes/{name}`,
attach one to a claim (warm VMs included), and keep it across release,
restart, template changes and hibernate. One claim at a time holds a
writable volume, and the hold is derived from `claims.json` and the
cocoonstack#349 stale-VM inventory. A claim for a volume on another mesh node
redirects there. This implements cocoonstack#300. Online growth of an attached
volume, idle hibernation and archive export stay open.
The test asserted the retried PUT left a different inode, but ext4 on the CI runner reuses the freed inode number, so os.SameFile reported the fresh file as the old one. Seed the leftover file with bytes instead and assert the new 1 GiB file starts with zeros, which is what a reader of the volume observes.
The test removed the backing and recreated it at the same size. ext4 on the CI runner reused the inode number, so the replacement matched the recorded identity and the claim succeeded. Build the replacement beside the original and rename it over, which is a real replacement the identity check must catch.
a2858eb to
2ebd52d
Compare
|
thanks a lot for this, and for checking every case #339 settled. it's careful work. i want to talk through the scope before we go further. my view is that a volume's lifecycle should not be controlled by the sandbox control plane. sandboxd controls the sandbox lifecycle and nothing else. to sandboxd, a disk is only an interface: it attaches it, detaches it, holds it for one writer, and checks the tenant may use it. creating, growing and deleting disks, ownership, placement, and the steps around a hibernate belong to whatever control plane needs them. that can live anywhere, it's up to whoever needs it. so i updated the scope of #300 to say this. it also reverses my earlier comment there that sandboxd owns the detach/capture/reattach sequence, sorry for the back-and-forth. with that split, i think one small PR on the sandboxd side closes the gap. two things are missing today:
with those, a control plane can do the whole flow from outside: stop writers and unmount in the guest, detach, hibernate, and later wake, attach, mount. idle hibernate already skips claims with volumes, so sandboxd never captures one on its own. what do you think? if your use case needs something this split can't do, tell me and we'll look at it. |
|
Thanks for clarifying the boundary. This fits our original use case of changing datasets on a running sandbox without replacing the VM. The earlier version followed the previous #300 hibernate contract, and I can rework the contribution around the new scope. I propose narrowing the sandboxd work to two pieces:
Disk creation, growth, ownership, placement, and the hibernate sequence would stay in the external controller. Once detach and its recovery bookkeeping complete, sandboxd's attachment hold ends. A controller that needs a reservation across sleep must keep it separately and gate workload access until wake, reattach, and mount complete, including requests that can wake the claim directly. I would document these as controller requirements. For runtime catalog entries, I recommend persisting only the registration metadata locally and loading it before claim recovery. That keeps registration identity stable through a daemon restart without bringing disk lifecycle management back into sandboxd. Would local persistence fit your intended scope, or do you want an explicitly ephemeral catalog that the controller must replay? The restart tests would cover physical detach succeeding while the claim write fails. That must retain the old registration binding and block unregister or name reuse until the reference retires. I also plan to validate the composed flow on a real VM before claiming end-to-end coverage. |
|
sounds good, both pieces are the right size. on your question: persist the registration locally. keep only name → backing file (plus whatever you need to check it on load), and load it before claim recovery. no lifecycle state goes in it. the reason is restart. a claim that holds a runtime-registered volume comes back during recovery, before any controller has reconnected. with an ephemeral catalog that claim points at a name sandboxd doesn't know, and the "detach done, claim write failed" case you describe can't be kept safe. a small local record closes that window without bringing disk lifecycle back into sandboxd. a few things i'd like the PR to keep:
looking forward to the smaller PR. thanks again. |
|
Thanks, the scope and persistence requirements are clear now. I've finished the implementation plan and will start on one focused PR, organized into separate commits for runtime catalog registration, live writable/attach-only support, and integration coverage and docs. I'll share it for review once it's ready. |
Tenants create node-local data disks with
PUT /v1/volumes/{name}, attach one to a claim (warm VMs included), and keep it across release, restart, template changes and hibernate. One claim at a time holds a writable volume, and the hold is derived fromclaims.jsonand the #349 stale-VM inventory. A claim for a volume on another mesh node redirects there. This implements #300. Online growth of an attached volume, idle hibernation and archive export stay open.What changed
PUT,GETandDELETE /v1/volumes/{name}, andPOST /v1/volumes/{name}/grow, manage raw sparse files undermanaged_volume_dir.registry.jsonrecords each volume's owner, size and backing file, and nothing else.managed_volume_hibernateset, hibernate of a claim with one mounted writable volume runs a guest hook, unmounts, detaches and captures. Wake reverses the steps. The claim row records the phase asvolume_sleep.Scope
Points we'd like to talk through
We made a call on each of these so the branch works end to end. Any of them can change if you see it differently.
claims.json. It sits on the claim row asvolume_sleep. An older sandboxd would wake such a claim without its disk, so the docs ask for a drain before a rollback.{"error","code"}with statuses picked inpool(VolumeError). We can move them to sentinels inpoolErrHTTPif you prefer that style.flockandtimeoutexec ofhook_argvin the guest, set per node. A silkd-native hook would work too./vm.resize-disk).Blast radius
Nodes without
managed_volume_dirkeep their behavior, with two shared changes. A guest mount conflict or a too-old cocoon on hibernate or wake answers 409 or 503 instead of 500. The Python SDK rejects a bare string or dict as a volume list. Nodes that ran earlier builds of this branch must be drained before upgrade, because the formats have no migration.Verification
go test -racepasses for sandboxd and e2e, except two tests that also fail on main when run as root. golangci-lint passes on linux and darwin, and ruff, pytest and mypy pass.TestManagedVolumeIssue300Acceptancewalks the acceptance line over HTTP.TestManagedSleepRoundTripAndExclusiveHoldcovers hibernate and wake, and two e2e tests cover placement.The first comment explains how we built this and the decisions behind it.