diff --git a/README.md b/README.md index 7843d205..6d155e34 100644 --- a/README.md +++ b/README.md @@ -121,7 +121,8 @@ TEMPLATE=rt:24.04 scripts/sandboxd-e2e.sh # without a Go toolchain. # VOLUME_IMAGE=/absolute/dataset.img enables the volumes proof (ro sharing; # the image contains volume-e2e.txt); VOLUME_RW_IMAGE=/absolute/scratch.img -# adds the rw claim/release leg. Prebuilt runs also set VOLUME_SMOKE_BIN. +# adds the rw claim/release leg; VOLUME_LIVE=1 adds the live volume leg. +# Prebuilt runs also set VOLUME_SMOKE_BIN. ``` ## CI diff --git a/docs/deploy.md b/docs/deploy.md index 00b1155d..4cf64f84 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -13,7 +13,8 @@ the cocoon CLI and needs a template image with silkd baked in. [performance](performance.md) numbers assume; v0.6.9 prints `[]` for an empty `snapshot list --format json`, which sandboxd's snapshot reconcile reads. sandboxd logs a warning at startup when the detected cocoon is below - v0.6.9 (a dev/`master-` build is assumed current) + v0.6.9 (a dev/`master-` build is assumed current); changing volumes on + a running claim needs v0.7.2 - The sandbox boot artifact installed where cocoon finds it (`/boot/vmlinuz-sandbox`, `/boot/initrd.img-sandbox` — from `ghcr.io/cocoonstack/sandbox/boot:`) @@ -156,8 +157,36 @@ fragment the warm pools): ### Dataset volumes +Running claims can change mounted read-only catalog entries through the +[GET, PUT, and PATCH volume API](sandboxd-api.md#get-v1sandboxesidvolumes) +and both SDKs. The image must already exist on the owner node. Live changes +require Cocoon v0.7.2 or newer for idempotent disk attach and detach. PUT and +PATCH reject an older release, or a runtime whose version cannot be queried, +with 503 before any intent, hold, or device change; a dev build is assumed +current. sandboxd reads the version once, so restart it after upgrading cocoon. +GET and release remain available. + +The claim journal stores confirmed applied `Volumes`, plus desired +`VolumesWant` while it differs. +Admission holds cover their union across daemon restart and teardown. Retry +an interrupted operation with the same PUT target. Release cancels convergence +and transfers holds to teardown. Capture is refused while either set is +nonempty. + +A sandbox whose VMM exits while holding volumes is release-only. Release it and +create a new sandbox. Daemon restart recovery assumes the original VMM remains +alive and does not recreate guest processes. Unfinished changes report an +interrupted diagnostic after restart and require a caller retry. + +`e2e/cmd/volumesmoke -live` (`VOLUME_LIVE=1` for `scripts/sandboxd-e2e.sh`) +exercises GET, repeated full-set PUT, PATCH add and remove, and clearing a new +claim. + Each catalog path must name a disk image containing a mountable whole-device -filesystem. A read-only entry's image must stay immutable: do not replace, +filesystem. Keep catalog names bound to the same image while any claim holds +them. Use versioned names such as `dataset-v1` and +`dataset-v2`; sandboxd cannot detect a repointed catalog entry. A read-only +entry's image must stay immutable: do not replace, truncate, or delete it while attached; publish a new catalog name or path instead. A missing path produces a startup warning and fails only claims that request it, allowing images to be distributed after sandboxd starts. @@ -681,7 +710,8 @@ using prebuilt binaries, also set `VOLUME_SMOKE_BIN` beside `SANDBOXD_BIN`, Add `VOLUME_RW_IMAGE=/srv/datasets/scratch.img` (a second, writable image) to also run the writable leg: a durable write across release, second-writer -exclusion, and a clean read-only claim afterward. +exclusion, and a clean read-only claim afterward. `VOLUME_LIVE=1` adds the live +volume leg on cocoon v0.7.2 or newer. ## TLS for SDK clients diff --git a/docs/live-volumes.md b/docs/live-volumes.md new file mode 100644 index 00000000..790e0d21 --- /dev/null +++ b/docs/live-volumes.md @@ -0,0 +1,93 @@ +# Replace a dataset in a running sandbox + +This example replaces `dataset-v1` with `dataset-v2` at `/datasets/current` +while preserving the sandbox, session, guest boot, and independent process. + +Register both read-only catalog entries on the owning node and grant the +sandbox's tenant access to them. Put `v1` and `v2` in each image's `/version` +file. Keep each name bound to the same immutable image while held. Use Cocoon +v0.7.2 or newer and set `SANDBOX_ADDR` and `SANDBOX_API_TOKEN` for the client. +A sandbox token alone cannot manage volumes. + +Pause application reads before replacement. Close files, mmap mappings, cwd, +and bind mount references on the old disk. Replacement unmounts the old disk +before mounting the new one, so the path has a short unavailable interval. +The independent process below uses `/tmp` and can continue running. + +```python +import os + +from cocoonsandbox import Client + + +def process_identity(sb, session, pid): + stat = sb.exec("cat", f"/proc/{pid}/stat") + starttime = stat.rsplit(") ", 1)[1].split()[19] + boot = sb.exec("cat", "/proc/sys/kernel/random/boot_id").strip() + return sb.id, session.id, boot, pid, starttime + + +client = Client(os.environ["SANDBOX_ADDR"], api_token=os.environ["SANDBOX_API_TOKEN"]) +v1 = [{"name": "dataset-v1", "mount": "/datasets/current"}] +v2 = [{"name": "dataset-v2", "mount": "/datasets/current"}] + +with client.new("rt:24.04", ttl_seconds=900) as sb: + with sb.session(cwd="/tmp", env={"DATASET_SESSION": "same"}) as session: + pid = sb.spawn("sleep", "3600", cwd="/tmp") + try: + identity = process_identity(sb, session, pid) + sb.set_volumes(v1) + assert sb.exec("cat", "/datasets/current/version").strip() == "v1" + + sb.patch_volumes(remove=["dataset-v1"], add=v2) + sb.set_volumes(v2) + + assert sb.exec("cat", "/datasets/current/version").strip() == "v2" + assert process_identity(sb, session, pid) == identity + assert session.id in sb.sessions() + assert session.exec("printenv", "DATASET_SESSION").strip() == "same" + view = sb.get_volumes() + assert view["volumes_want"] == view["volumes"] == v2 + assert view["converging"] is False + + sb.set_volumes([]) + assert process_identity(sb, session, pid) == identity + finally: + sb.kill(pid) +``` + +A successful run reads `v1`, then `v2`, and preserves every identity assertion. +The repeated full target verifies that a settled mount can be retained. +This example starts without other volumes. If your sandbox has writable or +attach-only volumes, retain them in a full PUT or use PATCH to replace only +the dataset names. `None`, a nil Go slice, and a missing `volumes` field do not +mean an empty target. + +Go uses the same owning handle and client API token. + +```go +view, err := sb.PatchVolumes(ctx, sandbox.VolumePatch{ + Remove: []string{"dataset-v1"}, + Add: []sandbox.Volume{{Name: "dataset-v2", Mount: "/datasets/current"}}, +}) +if err != nil { + return err +} +if view.Converging { + return errors.New("volume request still running") +} +``` + +For a 409 mount conflict, use the guest path in `states[].error` to close +references or remove unexpected mounts, then retry. For a transport failure or +timeout, GET the current target and confirmed set before retrying the target. +A failed call can already have changed devices. `VolumeError` retains a +complete failure view in Go's `Response` or Python's `.response`. + +`pending` or `detaching` with `converging: false` needs a caller retry. GET does +not run device repair. After sandboxd restarts with the original VM still +alive, unfinished entries report an interrupted operation and keep their +admission holds. Detailed errors survive only within one daemon lifetime. +If the VMM exits, a sandbox holding volumes is release-only. Release it and +create a new sandbox; neither PUT with an empty target nor daemon restart +restores the original processes. diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index 15918865..add40aed 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -255,6 +255,132 @@ list; the field is emitted (as `true`) only for a writable entry this node declares and omitted otherwise (peer-only rows never carry it), so a read-only entry's response is byte-identical to v1. +## GET /v1/sandboxes/{id}/volumes + +Returns the persisted desired set (`volumes_want`), confirmed applied set +(`volumes`), per-volume `states`, and `converging`. Auth requires a client API +token. Root can access every sandbox; a tenant can access only its own. +Sandbox tokens are rejected when authentication is enabled. The owning node +handles the request without redirecting. + +```json +{ + "volumes_want": [{"name": "imagenet", "mount": "/datasets/training"}], + "volumes": [{"name": "imagenet", "mount": "/datasets/training"}], + "states": [{"name": "imagenet", "state": "attached"}], + "converging": false +} +``` + +All three arrays are present, including when empty. A state is `attached` +when the volume is in both sets, `pending` when desired but not yet confirmed, +or `detaching` when applied but no longer desired. An interrupted device call +can leave the actual guest state different from the confirmed set. A failed +PUT or PATCH adds a top-level `error` naming the refusal, and the affected +state may carry its own `error`. `converging` is true only while a request worker is running. +`pending` with `converging: false` requires an explicit retry; there is no +background convergence. GET retains the last safe per-volume error for the +current daemon lifetime. After daemon restart, unfinished states report an +interrupted operation that needs retry. Successful retries clear resolved errors. +The sandbox index exposes both sets for operator inspection. + +## PUT /v1/sandboxes/{id}/volumes + +Replaces the full desired set and converges synchronously. Auth matches GET. +The owner tenant's catalog access list applies to newly admitted names, including +for root. Retaining, repairing, or removing held names does not reauthorize +them. A new image must exist on the owning node. The request must include a non-null `volumes` +array. An explicit empty array removes all mutable entries. + +```json +{"volumes": [{"name": "imagenet", "mount": "/datasets/training"}]} +``` + +Only mounted read-only catalog entries can change live (`mode` omitted or +`ro`). A PUT must retain every existing writable or attach-only entry unchanged. +The claim-time name, mount, and eight-volume limits apply to the desired set. +An omitted mount defaults to `/volumes/`, except that a held attach-only +entry is retained by sending it as the claim returned it, without `mount`. To +change an existing name's +mount or mode, first remove that name and wait for successful convergence. +An identical target can be retried. Claims with `on_expire: archive` must change +that policy before adding a volume. + +The sandbox must be running. Paused, archived, and failed sandboxes return 409 +without waking. A failed sandbox holding volumes is release-only after its VMM +exits; PUT with an empty target cannot restore its processes. Missing, invalid, +or sandbox bearer tokens return 401. Another tenant's sandbox and an unknown id +return 404. Invalid, inaccessible, or non-local entries return 400. Removed +tenant tokens are rejected at authentication. Root can still repair or remove +held entries, but admitting a new name for a removed tenant returns 403. A conflicting +writer or dirty image returns 409. + +## PATCH /v1/sandboxes/{id}/volumes + +Changes the desired set by name, then converges synchronously. Auth, validation, +and responses match PUT. Omitted `add` or `remove` arrays leave that part +unchanged. An empty patch retries the current desired set. + +```json +{"add": [{"name": "imagenet", "mount": "/datasets/training"}], "remove": ["old-data"]} +``` + +Adding an identical entry or removing an absent valid name is harmless. +Duplicate names, a name in both arrays, or changes to writable or attach-only +entries return 400. Removal refuses busy or foreign mounts and uses neither +lazy nor forced unmount. A foreign mount, another bind mount, or a completed +ordinary unmount with nonzero exit returns 409 with the guest mount path and +a safe per-volume reason. Guest transport failures remain server errors; +timeouts return 504 and cancellation returns 408. + +### Partial failure and recovery + +PUT and PATCH return the GET response shape on success. A convergence error +returns a non-2xx status with the same view, so callers can inspect desired, +applied, and per-volume state; semantic validation 400s carry it too. A +malformed body, a missing `volumes` array, authentication and an unknown sandbox +carry only `error`. A failed request may already have persisted its target and +completed some device changes. GET reads the last confirmed set, rather than +probing the devices. Resolve guest mount references or runtime failures before +retrying. Retry the same PUT to converge, or send a new +target to abandon unfinished additions. Concurrent requests serialize per +sandbox without blocking renewals or new exec, file, and port connections. + +Before applying a target, sandboxd journals `VolumesWant` beside `Volumes`, +storing it only while it differs from `Volumes`. A +superseding request may first clean up an older desired-only attachment while +the old target stays recorded. If that cleanup fails, the response retains the +old view; retry the same PUT to finish accepting the new target. Admission +holds cover the deduplicated union of both sets, including during restart and +teardown; a teardown keeps them until the VM is confirmed gone. Failed or +uncertain operations retain their holds until successful cleanup. Startup +reserves those holds without replaying attachments. Release cancels convergence and transfers the held union to teardown without waiting +for a blocked device call. Capture operations (hibernate, fork, checkpoint, and +promote) are refused while either set is nonempty or a convergence request +still owns admitted volume holds. + +Device calls honor cancellation and have a 30-second ceiling per volume; a +caller that disconnects cancels the run, leaving interrupted states to retry. Journal writes +are atomic. Host-crash durability follows `sync_claims`. An older daemon +ignores `VolumesWant`; finish pending volume changes before downgrading. + +Live convergence requires Cocoon v0.7.2 or newer for idempotent matching +attach and absent detach. PUT and PATCH reject an older release, or a runtime +whose version cannot be queried, with 503 before changing intent, holds, or +devices; a dev build is assumed current. sandboxd caches the first successful +version read, so restart it after upgrading cocoon. GET and release remain +available. + +Catalog names must keep the same backing image while held, including pending +holds. Publish changes under versioned names such as `dataset-v1` and +`dataset-v2`. The journal cannot detect a catalog name repointed to a new +image. + +A replacement first unmounts the old disk, then mounts the new disk. Pause reads +and close files, mmap, cwd, and bind mount references during this gap. Existing +independent guest processes and sessions continue. See the +[same-sandbox replacement tutorial](live-volumes.md). + ## POST /v1/sandboxes/{id}/release Auth: the sandbox's own token, or the root token for operator cleanup by id. @@ -669,7 +795,7 @@ an SDK caller should set. Auth: node API token. Root sees every live claim; a tenant sees only its own. The index is `{"sandboxes": [{id, key, tenant?, deadline, claimed_at?, hibernated, archived?, from_checkpoint?, claim_ref?, metadata?, on_expire?, cpu_count, mem_total_bytes, -volumes?: [{name, mount, mode?}], restarts?, restarted_at?, failed?}]}` — `cpu_count` and `mem_total_bytes` are the +volumes?: [{name, mount?, mode?}], volumes_want?: [{name, mount?, mode?}], restarts?, restarted_at?, failed?}]}` — `cpu_count` and `mem_total_bytes` are the size tier's allocation; `restarts` counts the cold boots that replaced an exited VMM and `restarted_at` stamps the last one (each loses guest memory and the instance-metadata document, so a caller re-establishes guest state when diff --git a/docs/sdk-python.md b/docs/sdk-python.md index 212ed9a6..d726d366 100644 --- a/docs/sdk-python.md +++ b/docs/sdk-python.md @@ -161,6 +161,26 @@ peer advertising both the template and every volume; when that intersection is empty, one volume holder may self-verify access to a shared template store before provisioning. +## Changing read-only datasets + +Use `sb.get_volumes()`, `sb.set_volumes([...])`, and +`sb.patch_volumes(add=[...], remove=[...])` on the same running sandbox. These +methods use the client's API token on the owning node; a sandbox token cannot +manage datasets. `set_volumes([])` clears mutable entries; `None` is rejected. +PUT must retain writable and attach-only entries. PATCH retains omitted names. +Each method takes `timeout=` (default: the client timeout, 120 s); a large +replacement can need longer, and a call that times out cancels the server run, +so retry the same target. + +Responses include `volumes_want`, `volumes`, `states`, and `converging`. +`VolumePhase` and `VOLUME_ATTACHED`, `VOLUME_PENDING`, and `VOLUME_DETACHING` +name the three states. A pending state with `converging=False` needs an explicit +retry. `VolumeError`, exported from `cocoonsandbox` and `cocoonsandbox.errors`, +retains a complete failure view in `.response`. Complete replies update both +public sets; incomplete errors preserve the last view. Serialize calls on one +handle. See the [replacement tutorial](live-volumes.md) and +[API recovery contract](sandboxd-api.md#partial-failure-and-recovery). + ## Renewing ```python diff --git a/docs/sdk.md b/docs/sdk.md index 6f4fee36..e2ffd912 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -234,6 +234,58 @@ checkpoint branches support neither volumes nor a claim reference in this versio `Checkpoint.New`: the labels follow the [claim bounds](sandboxd-api.md#post-v1claim), and a branch never inherits its source's labels or expiry action. +For datasets chosen after the sandbox starts, set the desired read-only mounts +on the same claim: + +```go +view, err := sb.SetVolumes(ctx, []sandbox.Volume{ + {Name: "imagenet", Mount: "/datasets/training"}, +}) +view, err = sb.GetVolumes(ctx) +view, err = sb.PatchVolumes(ctx, sandbox.VolumePatch{Remove: []string{"imagenet"}}) +// An explicit empty slice clears mutable mounts. A nil slice is rejected. +view, err = sb.SetVolumes(ctx, []sandbox.Volume{}) +``` + +Python exposes the same operations and response fields: + +```python +view = sb.set_volumes([{"name": "imagenet", "mount": "/datasets/training"}]) +view = sb.get_volumes() +view = sb.patch_volumes(remove=["imagenet"]) +view = sb.set_volumes([]) # None is rejected. +``` + +Each response contains `volumes_want`, `volumes`, `states`, and `converging`. +Go exposes them as `SandboxVolumesResponse.VolumesWant`, `.Volumes`, `.States`, +and `.Converging`. `VolumePhase` has `VolumeAttached`, `VolumePending`, and +`VolumeDetaching` constants. Python exposes `VolumePhase` and the corresponding +`VOLUME_ATTACHED`, `VOLUME_PENDING`, and `VOLUME_DETACHING` constants. Each state +can include an `error`. Methods use the client API token on the owning node +and update both public sets from each complete response, including a partial failure. Serialize changes to one handle as with +`Renew`. Returned views do not share the handle's slices or dictionaries. + +Go returns `*sandbox.VolumeError` with a `Response` alongside the returned view. +It wraps `*sandbox.APIError`, so `errors.As` can inspect the status. Python raises +`VolumeError`, a subclass of `APIError`, with `.response`. A response is absent +when the reply carries no complete view (a malformed request, authentication, +an unknown sandbox, or a non-JSON proxy reply). In that case +the handle retains its last known sets. Retry an interrupted operation with the +same full PUT target, or set a new target to recover. `Client.Sandboxes` exposes +`VolumesWant` and `Volumes` for inspection with an API token. + +A PUT must retain existing writable and attach-only entries unchanged. PATCH +cannot remove or change them. Paused sandboxes are not woken, and images must +exist on the owner node. Admission holds cover the union of desired and applied +sets. A settled retained entry receives no new attach or mount calls. `converging` +is true only during an active request. Pending or detaching entries with false +require an explicit retry. Daemon restart replaces unfinished diagnostics with +an interrupted message. Capture is refused while either set is nonempty. +Release cancels convergence and hands held volumes to teardown. See the +[API recovery contract](sandboxd-api.md#partial-failure-and-recovery) and +[same-sandbox replacement tutorial](live-volumes.md). A failed sandbox with +volume holds is release-only after VMM exit; an empty target cannot recover it. + `WithVolumesAttachOnly()` claims the same volumes without mounting them: the entries in `Sandbox.Volumes` carry an empty `Mount`, and the workload finds each device by polling `/sys/block/*/serial` for the catalog name — not diff --git a/e2e/cmd/volumesmoke/main.go b/e2e/cmd/volumesmoke/main.go index 0696b4d7..7ee5c534 100644 --- a/e2e/cmd/volumesmoke/main.go +++ b/e2e/cmd/volumesmoke/main.go @@ -1,5 +1,5 @@ // volumesmoke validates shared read-only dataset mounts, and optionally one -// writable dataset mount, on a live Cloud Hypervisor node. +// writable dataset mount and live volume changes, on a live Cloud Hypervisor node. package main import ( @@ -24,16 +24,17 @@ func main() { template := flag.String("template", "rt:24.04", "template ref") volume := flag.String("volume", "", "catalog volume name") rwVolume := flag.String("rw-volume", "", "writable catalog volume name (adds the writable leg)") + live := flag.Bool("live", false, "add the live volume leg (needs cocoon v0.7.2)") probe := flag.String("probe", "volume-e2e.txt", "non-empty file inside the volume") flag.Parse() - if err := run(*addr, *token, *template, *volume, *rwVolume, *probe); err != nil { + if err := run(*addr, *token, *template, *volume, *rwVolume, *probe, *live); err != nil { fmt.Fprintln(os.Stderr, "volumesmoke:", err) os.Exit(1) } } -func run(addr, token, template, volume, rwVolume, probe string) error { +func run(addr, token, template, volume, rwVolume, probe string, live bool) error { if volume == "" { return errors.New("volume is required") } @@ -104,13 +105,8 @@ func run(addr, token, template, volume, rwVolume, probe string) error { return fmt.Errorf("concurrent reads differ: first_bytes=%d second_bytes=%d", len(outputs[0]), len(outputs[1])) } - _, err = sbA.Exec(ctx, "touch", path.Join(mountA, ".sandboxd-write-probe")) - if err == nil { - return errors.New("write to read-only volume succeeded") - } - exitErr, ok := errors.AsType[*sandbox.ExitError](err) - if !ok || !strings.Contains(strings.ToLower(exitErr.Stderr), "read-only file system") { - return fmt.Errorf("write failed without EROFS: %w", err) + if err := requireReadOnly(ctx, sbA, mountA); err != nil { + return err } if err := errors.Join(sbA.Close(), sbB.Close()); err != nil { @@ -118,12 +114,82 @@ func run(addr, token, template, volume, rwVolume, probe string) error { } released = true fmt.Printf("VOLUME PASS concurrent_read_bytes=%d read_only=true released=true\n", len(outputs[0])) + if live { + if err := runLive(ctx, client, template, volume, probe, outputs[0]); err != nil { + return err + } + } if rwVolume == "" { return nil } return runWritable(ctx, client, template, rwVolume) } +func runLive(ctx context.Context, client *sandbox.Client, template, volume, probe, want string) error { + const mount = "/datasets/live" + sb, err := client.New(ctx, template, sandbox.WithNetwork(sandbox.NetNone)) + if err != nil { + return err + } + defer func() { _ = sb.Close() }() + for range 2 { + if _, err = sb.SetVolumes(ctx, []sandbox.Volume{{Name: volume, Mount: mount}}); err != nil { + return fmt.Errorf("live set volumes: %w", err) + } + } + view, err := sb.GetVolumes(ctx) + if err != nil { + return fmt.Errorf("live get volumes: %w", err) + } + wantVolumes := []sandbox.Volume{{Name: volume, Mount: mount}} + if !slices.Equal(view.VolumesWant, wantVolumes) || !slices.Equal(view.Volumes, wantVolumes) || len(view.States) != 1 || view.States[0].State != sandbox.VolumeAttached { + return fmt.Errorf("live volume view not converged: %+v", view) + } + got, err := sb.Exec(ctx, "cat", path.Join(mount, probe)) + if err != nil { + return fmt.Errorf("live volume read: %w", err) + } + if got != want { + return fmt.Errorf("live volume read: bytes=%d want=%d", len(got), len(want)) + } + if err = requireReadOnly(ctx, sb, mount); err != nil { + return fmt.Errorf("live volume: %w", err) + } + for range 2 { + if _, err = sb.PatchVolumes(ctx, sandbox.VolumePatch{Remove: []string{volume}}); err != nil { + return fmt.Errorf("live patch remove: %w", err) + } + } + if _, err = sb.Exec(ctx, "test", "!", "-e", path.Join(mount, probe)); err != nil { + return fmt.Errorf("detached volume probe should be absent: %w", err) + } + for range 2 { + if _, err = sb.PatchVolumes(ctx, sandbox.VolumePatch{Add: wantVolumes}); err != nil { + return fmt.Errorf("live patch add: %w", err) + } + } + if got, err = sb.Exec(ctx, "cat", path.Join(mount, probe)); err != nil || got != want { + return fmt.Errorf("live patch add read: bytes=%d want=%d error=%v", len(got), len(want), err) + } + for range 2 { + if _, err = sb.SetVolumes(ctx, []sandbox.Volume{}); err != nil { + return fmt.Errorf("live set empty volumes: %w", err) + } + } + view, err = sb.GetVolumes(ctx) + if err != nil { + return fmt.Errorf("live get empty volumes: %w", err) + } + if len(view.VolumesWant) != 0 || len(view.Volumes) != 0 || len(view.States) != 0 { + return fmt.Errorf("live clear did not converge: %+v", view) + } + if _, err = sb.Exec(ctx, "test", "!", "-e", path.Join(mount, probe)); err != nil { + return fmt.Errorf("cleared volume probe should be absent: %w", err) + } + fmt.Printf("LIVE VOLUME PASS read_bytes=%d repeated_put=true repeated_patch=true cleared=true\n", len(got)) + return nil +} + // runWritable proves the publish workflow: a writer's bytes survive its own // release, it excludes concurrent claims, and the next reader sees them. func runWritable(ctx context.Context, client *sandbox.Client, template, volume string) error { @@ -187,3 +253,14 @@ func runWritable(ctx context.Context, client *sandbox.Client, template, volume s volume, float64(claimRW.Microseconds())/1000) return nil } + +func requireReadOnly(ctx context.Context, sb *sandbox.Sandbox, mount string) error { + _, err := sb.Exec(ctx, "touch", path.Join(mount, ".sandboxd-write-probe")) + if err == nil { + return errors.New("write to read-only volume succeeded") + } + if exitErr, ok := errors.AsType[*sandbox.ExitError](err); !ok || !strings.Contains(strings.ToLower(exitErr.Stderr), "read-only file system") { + return fmt.Errorf("write failed without EROFS: %w", err) + } + return nil +} diff --git a/e2e/fakeengine_test.go b/e2e/fakeengine_test.go index 561cf243..b4f571fe 100644 --- a/e2e/fakeengine_test.go +++ b/e2e/fakeengine_test.go @@ -2,6 +2,7 @@ package e2e import ( "context" + "errors" "fmt" "io" "net" @@ -40,6 +41,8 @@ func newFakeEngine(dir string) *fakeEngine { } } +func (f *fakeEngine) CheckLiveVolumeSupport(context.Context) error { return nil } + func (f *fakeEngine) Clone(_ context.Context, _, name string, _ types.PoolKey) (types.VMRecord, error) { return f.createRecord(name) } @@ -137,7 +140,7 @@ func (f *fakeEngine) Probe(ctx context.Context, vsockSocket string, timeout time } func (f *fakeEngine) DialGuestPort(context.Context, string, uint16) (net.Conn, error) { - return nil, fmt.Errorf("fakeEngine has no guest port") + return nil, errors.New("fakeEngine has no guest port") } func (f *fakeEngine) InstallCACert(context.Context, string, []byte) error { return nil } @@ -163,6 +166,10 @@ func (f *fakeEngine) MountVolume(_ context.Context, _, name, mount string, rw bo return nil } +func (f *fakeEngine) MountLiveVolume(ctx context.Context, sock, name, mount string, rw bool) error { + return f.MountVolume(ctx, sock, name, mount, rw) +} + func (f *fakeEngine) UnmountVolume(_ context.Context, _, mount string) error { f.mu.Lock() defer f.mu.Unlock() @@ -177,6 +184,16 @@ func (f *fakeEngine) SyncGuest(_ context.Context, _ string) error { return nil } +func (f *fakeEngine) DetachVolume(ctx context.Context, _, sock, name, mount string) error { + if err := f.UnmountVolume(ctx, sock, mount); err != nil { + return err + } + f.mu.Lock() + defer f.mu.Unlock() + f.volumeOps = append(f.volumeOps, "detach:"+name) + return nil +} + func (f *fakeEngine) volumeOpsLog() []string { f.mu.Lock() defer f.mu.Unlock() diff --git a/e2e/volume_mutation_test.go b/e2e/volume_mutation_test.go new file mode 100644 index 00000000..9fd17e57 --- /dev/null +++ b/e2e/volume_mutation_test.go @@ -0,0 +1,62 @@ +package e2e + +import ( + "errors" + "net/http" + "slices" + "strings" + "testing" + + "github.com/cocoonstack/sandbox/sandboxd/config" + sandbox "github.com/cocoonstack/sandbox/sdk/go" +) + +func TestLiveVolumesEndToEnd(t *testing.T) { + image := writeVolumeImage(t, "dataset.img", "dataset-bytes") + stack := startTenantStack(t, "node-token", nil, []config.VolumeSpec{{Name: "dataset", Path: image}}) + sb, err := stack.client.New(t.Context(), "rt:24.04") + if err != nil { + t.Fatal(err) + } + defer sb.Close() + for range 2 { + got, err := sb.SetVolumes(t.Context(), []sandbox.Volume{{Name: "dataset"}}) + if err != nil || !slices.Equal(got.Volumes, []sandbox.Volume{{Name: "dataset", Mount: "/volumes/dataset"}}) { + t.Fatalf("attach: %v %v", got, err) + } + } + if err := sb.Hibernate(t.Context()); err == nil { + t.Fatal("captured an attached volume") + } + for range 2 { + got, err := sb.SetVolumes(t.Context(), []sandbox.Volume{}) + if err != nil || len(got.Volumes) != 0 { + t.Fatalf("detach: %v %v", got, err) + } + } + want := []string{"attach:dataset:ro", "mount:dataset:/volumes/dataset:ro", "umount:/volumes/dataset", "detach:dataset"} + if ops := stack.eng.volumeOpsLog(); !slices.Equal(ops, want) { + t.Fatalf("device operations=%v want=%v", ops, want) + } +} + +func TestRejectedVolumeTargetExplainsRefusalWithoutChangingState(t *testing.T) { + image := writeVolumeImage(t, "dataset.img", "dataset-bytes") + stack := startTenantStack(t, "node-token", nil, []config.VolumeSpec{{Name: "dataset", Path: image}}) + sb, err := stack.client.New(t.Context(), "rt:24.04") + if err != nil { + t.Fatal(err) + } + defer sb.Close() + if _, checkErr := sb.SetVolumes(t.Context(), []sandbox.Volume{{Name: "dataset"}}); checkErr != nil { + t.Fatal(checkErr) + } + _, err = sb.SetVolumes(t.Context(), []sandbox.Volume{{Name: "dataset"}, {Name: "dataset"}}) + if refusal, ok := errors.AsType[*sandbox.VolumeError](err); !ok || refusal.Status != http.StatusBadRequest || !strings.Contains(refusal.Message, "duplicate volume") { + t.Errorf("invalid target explanation: %v", err) + } + view, err := sb.GetVolumes(t.Context()) + if err != nil || view.Error != "" || view.Converging || len(view.States) != 1 || view.States[0].Error != "" || view.States[0].State != sandbox.VolumeAttached { + t.Fatalf("rejected request changed settled state: %+v err=%v", view, err) + } +} diff --git a/sandboxd/engine/engine.go b/sandboxd/engine/engine.go index 7a5f5d68..11dd07ed 100644 --- a/sandboxd/engine/engine.go +++ b/sandboxd/engine/engine.go @@ -15,6 +15,7 @@ import ( "os/exec" "strconv" "strings" + "sync/atomic" "time" "github.com/projecteru2/core/log" @@ -28,6 +29,8 @@ import ( const ( // RequiredCocoon carries the snapshot/store baseline and the JSON form of an empty snapshot list. RequiredCocoon = "v0.6.9" + // LiveVolumeCocoon adds idempotent disk attach and detach for live volume changes. + LiveVolumeCocoon = "v0.7.2" StaleCreateCollected StaleCreateOutcome = "collected" StaleCreateBusy StaleCreateOutcome = "busy" @@ -50,7 +53,7 @@ const ( outputTail = 400 portForwardMax = 4096 // port_forward handshake reply line cap vmNotFound = "vm not found" // cocoon's hypervisor.ErrNotFound text - // at 16 concurrent memory dumps cocoon's sqlite writers already time out on its 5 s lock wait; 8 measured clean + // At 16 concurrent memory dumps cocoon's sqlite writers already time out on its 5 s lock wait; 8 measured clean memoryDumps = 8 ) @@ -72,6 +75,8 @@ type Engine struct { noBalloon bool restoreMode types.RestoreMode dumps chan struct{} + // version caches the first successful query; replacing cocoon takes a restart. + version atomic.Pointer[string] } // New returns a cocoon engine with node-wide network and disk policy. @@ -81,6 +86,9 @@ func New(bin string, bridges, networks []string, noDirectIO, noBalloon bool, res // Version reports cocoon's version string: a "vX.Y.Z" release or a "master-" dev build. func (e *Engine) Version(ctx context.Context) (string, error) { + if v := e.version.Load(); v != nil { + return *v, nil + } ctx, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() out, err := e.run(ctx, "version") @@ -89,7 +97,9 @@ func (e *Engine) Version(ctx context.Context) (string, error) { } for line := range strings.SplitSeq(string(out), "\n") { if v, ok := strings.CutPrefix(strings.TrimSpace(line), "Version:"); ok { - return strings.TrimSpace(v), nil + v = strings.TrimSpace(v) + e.version.Store(&v) + return v, nil } } return "", fmt.Errorf("cocoon version: no Version line in output") @@ -101,7 +111,7 @@ func (e *Engine) VersionWarning(ctx context.Context) (version, warning string) { if err != nil { return "", fmt.Sprintf("cannot determine cocoon version (need >= %s): %v", RequiredCocoon, err) } - if below, known := belowFloor(v); known && below { + if below, known := belowFloor(v, RequiredCocoon); known && below { return v, fmt.Sprintf("cocoon %s is below the required %s — upgrade cocoon", v, RequiredCocoon) } return v, "" @@ -134,6 +144,7 @@ func (e *Engine) Start(ctx context.Context, name string) error { return err } +// Stop force-stops a VM and keeps its disk for a later Start. func (e *Engine) Stop(ctx context.Context, name string) error { _, err := e.run(ctx, "vm", "stop", "--force", name) return err @@ -493,13 +504,13 @@ func respFail(resp wire.Response) string { return "unexpected frame " + resp.RespType() } -// belowFloor reports whether v is below RequiredCocoon; known is false for a dev build. -func belowFloor(v string) (below, known bool) { +// belowFloor reports whether v is below floor; known is false for a dev build. +func belowFloor(v, floor string) (below, known bool) { v = "v" + strings.TrimPrefix(v, "v") if semver.Canonical(v) != v { return false, false } - return semver.Compare(v, RequiredCocoon) < 0, true + return semver.Compare(v, floor) < 0, true } func readBufLine(r *bufio.Reader, limit int) (string, error) { diff --git a/sandboxd/engine/installca.go b/sandboxd/engine/installca.go index 1face152..da0d4279 100644 --- a/sandboxd/engine/installca.go +++ b/sandboxd/engine/installca.go @@ -18,6 +18,15 @@ const ( guestExecPATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ) +type guestExitError struct { + code int32 + output string +} + +func (e *guestExitError) Error() string { + return fmt.Sprintf("exit code %d: %s", e.code, e.output) +} + // InstallCACert makes the guest trust the cluster root without update-ca-certificates. func (e *Engine) InstallCACert(ctx context.Context, vsockSocket string, certPEM []byte) error { ctx, cancel := context.WithTimeout(ctx, cmdTimeout) @@ -64,9 +73,14 @@ func (e *Engine) silkdWriteFile(ctx context.Context, vsockSocket, path string, m } func (e *Engine) silkdExec(ctx context.Context, vsockSocket string, argv ...string) error { + _, err := e.silkdOutput(ctx, vsockSocket, argv...) + return err +} + +func (e *Engine) silkdOutput(ctx context.Context, vsockSocket string, argv ...string) ([]byte, error) { s, err := e.dialSilkdSession(ctx, vsockSocket) if err != nil { - return err + return nil, err } defer s.close() sendErr := s.send(wire.Exec{Argv: argv, Env: map[string]string{"PATH": guestExecPATH}}) @@ -74,27 +88,27 @@ func (e *Engine) silkdExec(ctx context.Context, vsockSocket string, argv ...stri // a child exiting without reading stdin races this close; prefer the buffered exit frame. sendErr = s.send(wire.StdinClose{}) } - var out []byte + var out, stdout []byte for { frame, err := s.recv() if err != nil { - return errors.Join(sendErr, err) + return nil, errors.Join(sendErr, err) } switch resp := frame.(type) { case *wire.Started: case *wire.Stdout: - out = appendCapped(out, resp.Data) + out, stdout = appendCapped(out, resp.Data), appendCapped(stdout, resp.Data) case *wire.Stderr: out = appendCapped(out, resp.Data) case *wire.Exit: if resp.Code != 0 { - return fmt.Errorf("exit code %d: %s", resp.Code, bytes.TrimSpace(out)) + return nil, &guestExitError{code: resp.Code, output: string(bytes.TrimSpace(out))} } - return nil + return stdout, nil case *wire.ErrorResp: - return fmt.Errorf("silkd %w", resp) + return nil, fmt.Errorf("silkd %w", resp) default: - return fmt.Errorf("unexpected silkd frame %q", resp.RespType()) + return nil, fmt.Errorf("unexpected silkd frame %q", resp.RespType()) } } } diff --git a/sandboxd/engine/installca_test.go b/sandboxd/engine/installca_test.go index 46362aca..09deafe1 100644 --- a/sandboxd/engine/installca_test.go +++ b/sandboxd/engine/installca_test.go @@ -78,9 +78,13 @@ type fakeSilkd struct { execEnv map[string]string execCode int32 execFailAt int + execHook func([]string) + realpath map[string]string + noRealpath bool writeErr string writeKind string execErr string + execDrop bool block []wire.DirEntry serial map[string][]byte readMisses map[string]int @@ -193,16 +197,32 @@ func (f *fakeSilkd) handleExec(conn net.Conn, argv []string, env map[string]stri f.execArgv = argv f.execCalls = append(f.execCalls, argv) f.execEnv = env - code, reply := f.execCode, f.execErr + code, reply, disconnect := f.execCode, f.execErr, f.execDrop + if argv[0] == "realpath" { + code, reply, disconnect = 0, "", false + if f.noRealpath { + code = 1 + } + } if f.execFailAt > 0 && len(f.execCalls) != f.execFailAt { code, reply = 0, "" } + if code == 0 && reply == "" && f.execHook != nil { + f.execHook(argv) + } + resolved := f.realpath[argv[len(argv)-1]] f.mu.Unlock() + if disconnect { + return + } if reply != "" { _, _ = io.WriteString(conn, `{"type":"error","kind":"internal","message":"`+reply+`"}`+"\n") return } _, _ = io.WriteString(conn, `{"type":"started","pid":1}`+"\n") + if argv[0] == "realpath" && code == 0 { + writeFakeSilkdResponse(conn, &wire.Stdout{Data: []byte(cmp.Or(resolved, argv[len(argv)-1]) + "\n")}) + } _, _ = fmt.Fprintf(conn, `{"type":"exit","code":%d}`+"\n", code) } diff --git a/sandboxd/engine/silkd.go b/sandboxd/engine/silkd.go index 16fa2134..d7bfab3d 100644 --- a/sandboxd/engine/silkd.go +++ b/sandboxd/engine/silkd.go @@ -13,6 +13,7 @@ import ( const ( silkdListMax = 4096 silkdReadMax = 64 << 10 + mountInfoMax = 1 << 20 ) // silkdSession is a dialed silkd conn bound to a ctx, with wire-typed request/reply helpers. @@ -107,7 +108,7 @@ func (e *Engine) silkdList(ctx context.Context, vsockSocket, path string) ([]wir return entries, nil } -func (e *Engine) silkdReadFile(ctx context.Context, vsockSocket, path string) ([]byte, error) { +func (e *Engine) silkdReadFile(ctx context.Context, vsockSocket, path string, limit int) ([]byte, error) { var data []byte err := e.silkdStream(ctx, vsockSocket, wire.FsRead{Path: path}, func(frame wire.Response) error { resp, ok := frame.(*wire.DataResp) @@ -115,8 +116,8 @@ func (e *Engine) silkdReadFile(ctx context.Context, vsockSocket, path string) ([ return fmt.Errorf("unexpected silkd frame %q", frame.RespType()) } data = append(data, resp.Data...) - if len(data) > silkdReadMax { - return fmt.Errorf("silkd read of %s exceeds %d bytes", path, silkdReadMax) + if len(data) > limit { + return fmt.Errorf("silkd read of %s exceeds %d bytes", path, limit) } return nil }) diff --git a/sandboxd/engine/version_test.go b/sandboxd/engine/version_test.go index b71b03bd..0969b28d 100644 --- a/sandboxd/engine/version_test.go +++ b/sandboxd/engine/version_test.go @@ -1,12 +1,17 @@ package engine -import "testing" +import ( + "errors" + "fmt" + "strings" + "testing" +) func TestBelowFloor(t *testing.T) { tests := []struct { - v string - below bool - comparable bool + v string + below bool + known bool }{ {"v0.6.9", false, true}, {"v0.6.10", false, true}, @@ -22,10 +27,63 @@ func TestBelowFloor(t *testing.T) { } for _, tt := range tests { t.Run(tt.v, func(t *testing.T) { - below, comparable := belowFloor(tt.v) - if below != tt.below || comparable != tt.comparable { - t.Errorf("belowFloor(%q) = (%v, %v), want (%v, %v)", tt.v, below, comparable, tt.below, tt.comparable) + below, known := belowFloor(tt.v, RequiredCocoon) + if below != tt.below || known != tt.known { + t.Errorf("belowFloor(%q) = (%v, %v), want (%v, %v)", tt.v, below, known, tt.below, tt.known) } }) } } + +func TestCheckLiveVolumeSupport(t *testing.T) { + for _, tt := range []struct { + version string + fail, allowed bool + }{ + {"v0.7.1", false, false}, + {"v0.7.2-rc.1", false, false}, + {"v0.7.2", false, true}, + {"v0.8.0", false, true}, + {"master-abcdef", false, true}, + {"v0.7.2", true, false}, + } { + t.Run(fmt.Sprintf("%s/fail=%v", tt.version, tt.fail), func(t *testing.T) { + t.Setenv("COCOON_TEST_VERSION", tt.version) + t.Setenv("COCOON_TEST_VERSION_FAIL", "") + if tt.fail { + t.Setenv("COCOON_TEST_VERSION_FAIL", "1") + } + fakeCocoon(t, `#!/bin/sh +if [ "$#" != 1 ] || [ "$1" != version ]; then exit 2; fi +if [ -n "$COCOON_TEST_VERSION_FAIL" ]; then echo /private/cocoon >&2; exit 1; fi +printf 'Version: %s\n' "$COCOON_TEST_VERSION" +`) + err := New("cocoon", nil, nil, false, false, "").CheckLiveVolumeSupport(t.Context()) + if tt.allowed { + if err != nil { + t.Fatal(err) + } + return + } + if _, ok := errors.AsType[*LiveVolumeUnsupported](err); !ok || !strings.Contains(err.Error(), LiveVolumeCocoon) || strings.Contains(err.Error(), "/private") { + t.Fatalf("error %v", err) + } + }) + } +} + +func TestVersionQueriesCocoonOnce(t *testing.T) { + t.Setenv("COCOON_TEST_VERSION_FAIL", "") + fakeCocoon(t, `#!/bin/sh +if [ -n "$COCOON_TEST_VERSION_FAIL" ]; then exit 1; fi +printf 'Version: v0.7.2\n' +`) + eng := New("cocoon", nil, nil, false, false, "") + if _, err := eng.Version(t.Context()); err != nil { + t.Fatal(err) + } + t.Setenv("COCOON_TEST_VERSION_FAIL", "1") + if v, err := eng.Version(t.Context()); err != nil || v != "v0.7.2" { + t.Fatalf("cached version %q, %v", v, err) + } +} diff --git a/sandboxd/engine/volume.go b/sandboxd/engine/volume.go index 62daa94f..2cc405c2 100644 --- a/sandboxd/engine/volume.go +++ b/sandboxd/engine/volume.go @@ -15,6 +15,8 @@ import ( const ( // VolumeCallTimeout bounds one guest teardown exec. VolumeCallTimeout = 2 * time.Second + // VolumeMutationTimeout bounds one live attach or detach, guest steps included. + VolumeMutationTimeout = 30 * time.Second volumePollInterval = 10 * time.Millisecond volumeProbeTimeout = 2 * time.Second @@ -55,17 +57,7 @@ func (e *Engine) MountVolume(ctx context.Context, vsockSocket, name, mount strin if err != nil { return fmt.Errorf("wait for volume device %s: %w", name, err) } - if err := e.silkdExec(ctx, vsockSocket, "mkdir", "-p", "--", mount); err != nil { - return fmt.Errorf("create volume mount point %s: %w", mount, err) - } - mode := types.VolumeModeRO - if rw { - mode = types.VolumeModeRW - } - if err := e.silkdExec(ctx, vsockSocket, "mount", "-o", mode, "--", device, mount); err != nil { - return fmt.Errorf("mount volume %s: %w", name, err) - } - return nil + return e.mountDevice(ctx, vsockSocket, device, name, mount, rw) } // UnmountVolume flushes and detaches a guest mount before the VM is removed. @@ -115,7 +107,7 @@ func (e *Engine) findVolumeDevice(ctx context.Context, vsockSocket, name string) return "", false, err } for _, entry := range entries { - serial, err := e.silkdReadFile(ctx, vsockSocket, "/sys/block/"+entry.Name+"/serial") + serial, err := e.silkdReadFile(ctx, vsockSocket, "/sys/block/"+entry.Name+"/serial", silkdReadMax) if isNotFound(err) { continue } @@ -138,6 +130,16 @@ func (e *Engine) findVolumeDevice(ctx context.Context, vsockSocket, name string) return "", false, nil } +func (e *Engine) mountDevice(ctx context.Context, vsockSocket, device, name, mount string, rw bool) error { + if err := e.silkdExec(ctx, vsockSocket, "mkdir", "-p", "--", mount); err != nil { + return fmt.Errorf("create volume mount point %s: %w", mount, err) + } + if err := e.silkdExec(ctx, vsockSocket, "mount", "-o", mountMode(rw), "--", device, mount); err != nil { + return fmt.Errorf("mount volume %s: %w", name, err) + } + return nil +} + func (e *Engine) diskAttachArgs(vmName string, spec VolumeSpec) ([]string, error) { directIO, err := spec.directIO() if err != nil { @@ -158,3 +160,10 @@ func isNotFound(err error) bool { respErr, ok := errors.AsType[*wire.ErrorResp](err) return ok && respErr.Kind == wire.KindNotFound } + +func mountMode(rw bool) string { + if rw { + return types.VolumeModeRW + } + return types.VolumeModeRO +} diff --git a/sandboxd/engine/volume_live.go b/sandboxd/engine/volume_live.go new file mode 100644 index 00000000..e1033a84 --- /dev/null +++ b/sandboxd/engine/volume_live.go @@ -0,0 +1,169 @@ +package engine + +import ( + "cmp" + "context" + "errors" + "fmt" + "path/filepath" + "slices" + "strings" + + "github.com/projecteru2/core/log" +) + +var mountUnescaper = strings.NewReplacer(`\040`, " ", `\011`, "\t", `\012`, "\n", `\134`, `\`) + +// VolumeMountConflict reports a guest mount that must be resolved before retrying. +type VolumeMountConflict struct { + Message string +} + +func (e *VolumeMountConflict) Error() string { return e.Message } + +// LiveVolumeUnsupported reports a runtime that cannot safely change live volumes. +type LiveVolumeUnsupported struct { + Version string +} + +func (e *LiveVolumeUnsupported) Error() string { + return fmt.Sprintf("live volume changes require Cocoon >= %s; installed version is %s", LiveVolumeCocoon, cmp.Or(e.Version, "unavailable")) +} + +type volumeMount struct { + devNum, root, path, options string +} + +// CheckLiveVolumeSupport rejects runtimes without idempotent live disk attach and detach. +func (e *Engine) CheckLiveVolumeSupport(ctx context.Context) error { + version, err := e.Version(ctx) + if err != nil { + log.WithFunc("engine.CheckLiveVolumeSupport").Warnf(ctx, "query cocoon version: %v", err) + return &LiveVolumeUnsupported{} + } + if below, known := belowFloor(version, LiveVolumeCocoon); known && below { + return &LiveVolumeUnsupported{Version: version} + } + return nil +} + +// MountLiveVolume mounts an attached disk unless the same mount already exists. +func (e *Engine) MountLiveVolume(ctx context.Context, vsockSocket, name, mount string, rw bool) error { + ctx, cancel := context.WithTimeout(ctx, volumeSetupTimeout) + defer cancel() + device, err := e.waitForVolumeDevice(ctx, vsockSocket, name) + if err != nil { + return fmt.Errorf("wait for volume device %s: %w", name, err) + } + mounted, err := e.volumeMounted(ctx, vsockSocket, device, mount, rw) + if err != nil || mounted { + return err + } + return e.mountDevice(ctx, vsockSocket, device, name, mount, rw) +} + +// DetachVolume unmounts a guest volume before confirming host removal. +func (e *Engine) DetachVolume(ctx context.Context, vmName, vsockSocket, name, mount string) error { + ctx, cancel := context.WithTimeout(ctx, VolumeMutationTimeout) + defer cancel() + device, found, err := e.findVolumeDevice(ctx, vsockSocket, name) + if err != nil { + return err + } + if found { + if err = e.unmountLiveVolume(ctx, vsockSocket, device, mount); err != nil { + return err + } + } + _, err = e.run(ctx, "vm", "disk", "detach", vmName, argName, name) + return err +} + +func (e *Engine) unmountLiveVolume(ctx context.Context, vsockSocket, device, mount string) error { + devNum, mounts, err := e.volumeMounts(ctx, vsockSocket, device) + if err != nil { + return err + } + target := e.resolveMount(ctx, vsockSocket, mount, mounts) + atTarget := slices.DeleteFunc(mounts, func(m volumeMount) bool { return m.path != target }) + if len(atTarget) == 1 && atTarget[0].devNum == devNum && atTarget[0].root == "/" { + if err = e.UnmountVolume(ctx, vsockSocket, mount); err != nil { + if _, ok := errors.AsType[*guestExitError](err); ok { + return &VolumeMountConflict{Message: fmt.Sprintf("cannot unmount %q; close guest references and retry", mount)} + } + return err + } + } + return e.refuseOtherMounts(ctx, vsockSocket, device) +} + +func (e *Engine) volumeMounts(ctx context.Context, vsockSocket, device string) (string, []volumeMount, error) { + devNum, err := e.silkdReadFile(ctx, vsockSocket, "/sys/block/"+filepath.Base(device)+"/dev", silkdReadMax) + if err != nil { + return "", nil, err + } + raw, err := e.silkdReadFile(ctx, vsockSocket, "/proc/self/mountinfo", mountInfoMax) + if err != nil { + return "", nil, err + } + var mounts []volumeMount + for line := range strings.SplitSeq(strings.TrimSpace(string(raw)), "\n") { + fields := strings.Fields(line) + if len(fields) < 10 || !slices.Contains(fields[6:], "-") { + return "", nil, errors.New("invalid guest mountinfo") + } + mounts = append(mounts, volumeMount{fields[2], mountUnescaper.Replace(fields[3]), mountUnescaper.Replace(fields[4]), fields[5]}) + } + return strings.TrimSpace(string(devNum)), mounts, nil +} + +func (e *Engine) volumeMounted(ctx context.Context, vsockSocket, device, mount string, rw bool) (bool, error) { + devNum, mounts, err := e.volumeMounts(ctx, vsockSocket, device) + if err != nil { + return false, err + } + target := e.resolveMount(ctx, vsockSocket, mount, mounts) + matched := false + for _, entry := range mounts { + if entry.path != target { + if entry.devNum == devNum { + return false, bindConflict(entry.path) + } + continue + } + if mode, _, _ := strings.Cut(entry.options, ","); matched || entry.devNum != devNum || entry.root != "/" || mode != mountMode(rw) { + return false, &VolumeMountConflict{Message: fmt.Sprintf("volume mount %q has a different device, root, or mode; restore the expected mount before retrying", target)} + } + matched = true + } + return matched, nil +} + +func (e *Engine) resolveMount(ctx context.Context, vsockSocket, mount string, mounts []volumeMount) string { + if slices.ContainsFunc(mounts, func(m volumeMount) bool { return m.path == mount }) { + return mount + } + resolved, err := e.silkdOutput(ctx, vsockSocket, "realpath", mount) + if err != nil { + log.WithFunc("engine.resolveMount").Warnf(ctx, "resolve volume mount %s, checking it as given: %v", mount, err) + return mount + } + return strings.TrimSuffix(string(resolved), "\n") +} + +func (e *Engine) refuseOtherMounts(ctx context.Context, vsockSocket, device string) error { + devNum, mounts, err := e.volumeMounts(ctx, vsockSocket, device) + if err != nil { + return err + } + for _, mount := range mounts { + if mount.devNum == devNum { + return bindConflict(mount.path) + } + } + return nil +} + +func bindConflict(path string) *VolumeMountConflict { + return &VolumeMountConflict{Message: fmt.Sprintf("volume device has another mount at %q; remove guest mount references before retrying", path)} +} diff --git a/sandboxd/engine/volume_live_test.go b/sandboxd/engine/volume_live_test.go new file mode 100644 index 00000000..a731f2e1 --- /dev/null +++ b/sandboxd/engine/volume_live_test.go @@ -0,0 +1,284 @@ +package engine + +import ( + "errors" + "os" + "path/filepath" + "slices" + "strconv" + "strings" + "testing" +) + +const rootMountInfo = "1 0 8:0 / / rw - ext4 /dev/vda rw\n" + +func TestLiveVolumeConvergesMountAndDetach(t *testing.T) { + for _, tt := range []struct { + name, mount, kernelPath string + realpath map[string]string + noRealpath bool + lookups int + }{ + {name: "canonical path", mount: "/data", kernelPath: "/data", lookups: 1}, + {name: "symlinked parent", mount: "/workspace/data", kernelPath: "/home/u/ws/data", realpath: map[string]string{"/workspace/data": "/home/u/ws/data"}, lookups: 3}, + {name: "trailing space behind a symlink", mount: "/ws/data ", kernelPath: `/home/u/ws/data\040`, realpath: map[string]string{"/ws/data ": "/home/u/ws/data "}, lookups: 3}, + {name: "unresolvable path", mount: "/missing/data", kernelPath: "/missing/data", noRealpath: true, lookups: 1}, + } { + t.Run(tt.name, func(t *testing.T) { + sock := sockPath(t) + fake := serveFakeSilkd(t, sock) + configureVolumeDevices(fake) + spec := liveDiskCLI(t, "") + if err := os.Remove(os.Getenv("COCOON_TEST_ATTACHED")); err != nil { + t.Fatal(err) + } + fake.mu.Lock() + fake.realpath, fake.noRealpath = tt.realpath, tt.noRealpath + fake.serial["/sys/block/vdc/dev"] = []byte("8:32\n") + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo) + fake.execHook = func(argv []string) { + switch argv[0] { + case "mount": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / " + tt.kernelPath + " ro - ext4 /dev/vdc ro\n") + case "umount": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo) + } + } + fake.mu.Unlock() + e := New("cocoon", nil, nil, false, false, "") + for range 2 { + if err := e.DiskAttach(t.Context(), "sbx-1", spec); err != nil { + t.Fatal(err) + } + if err := e.MountLiveVolume(t.Context(), sock, spec.Name, tt.mount, false); err != nil { + t.Fatal(err) + } + } + if err := e.DetachVolume(t.Context(), "sbx-1", sock, spec.Name, tt.mount); err != nil { + t.Fatal(err) + } + fake.mu.Lock() + defer fake.mu.Unlock() + changes := guestChanges(fake.execCalls) + if len(changes) != 3 || changes[2][0] != "umount" { + t.Fatalf("expected one mkdir/mount/umount, got %v", fake.execCalls) + } + if lookups := len(fake.execCalls) - len(changes); lookups != tt.lookups { + t.Fatalf("realpath lookups = %d, want %d: %v", lookups, tt.lookups, fake.execCalls) + } + args, err := os.ReadFile(os.Getenv("COCOON_TEST_ATTACH_ARGS")) + if err != nil || !strings.Contains(string(args), "--readonly --directio off") { + t.Fatalf("attach args=%s error=%v", args, err) + } + }) + } +} + +func TestLiveMountRefusesAnOccupiedPathBehindASymlink(t *testing.T) { + sock := sockPath(t) + fake := serveFakeSilkd(t, sock) + configureVolumeDevices(fake) + spec := liveDiskCLI(t, "") + fake.mu.Lock() + fake.serial["/sys/block/vdc/dev"] = []byte("8:32\n") + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:48 / /home/u/ws/data ro - ext4 /dev/vdd ro\n") + fake.realpath = map[string]string{"/workspace/data": "/home/u/ws/data"} + fake.mu.Unlock() + e := New("cocoon", nil, nil, false, false, "") + if err := e.DiskAttach(t.Context(), "sbx-1", spec); err != nil { + t.Fatal(err) + } + err := e.MountLiveVolume(t.Context(), sock, spec.Name, "/workspace/data", false) + if _, ok := errors.AsType[*VolumeMountConflict](err); !ok || !strings.Contains(err.Error(), `"/home/u/ws/data" has a different device`) { + t.Fatalf("mounted over another device behind a symlink or hid its real path: %v", err) + } + fake.mu.Lock() + defer fake.mu.Unlock() + if slices.ContainsFunc(fake.execCalls, func(argv []string) bool { return argv[0] == "mount" }) { + t.Fatalf("mount ran over an occupied path: %v", fake.execCalls) + } +} + +func TestLiveMountReadsMountinfoBeyondTheSysfsCap(t *testing.T) { + sock := sockPath(t) + fake := serveFakeSilkd(t, sock) + configureVolumeDevices(fake) + spec := liveDiskCLI(t, "") + overlays := strings.Repeat("9 1 0:99 / /var/lib/docker/overlay2/layer/merged rw - overlay overlay rw,lowerdir=/var/lib/docker/overlay2/l/ABCDEFGHIJKLMNOPQRSTUVWXYZ\n", 2000) + fake.mu.Lock() + fake.serial["/sys/block/vdc/dev"] = []byte("8:32\n") + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + overlays) + fake.mu.Unlock() + if len(rootMountInfo+overlays) <= silkdReadMax { + t.Fatalf("fixture of %d bytes does not exceed the sysfs cap", len(rootMountInfo+overlays)) + } + e := New("cocoon", nil, nil, false, false, "") + if err := e.DiskAttach(t.Context(), "sbx-1", spec); err != nil { + t.Fatal(err) + } + if err := e.MountLiveVolume(t.Context(), sock, spec.Name, "/data", false); err != nil { + t.Fatal(err) + } +} + +func TestLiveAttachRejectsForeignMounts(t *testing.T) { + for _, row := range []string{ + "2 1 8:99 / /data ro - ext4 /dev/vdz ro\n", + "2 1 8:32 / /data rw - ext4 /dev/vdc rw\n", + "2 1 8:32 /subdir /data ro - ext4 /dev/vdc ro\n", + "malformed\n", + } { + t.Run(strings.TrimSpace(row), func(t *testing.T) { + sock := sockPath(t) + fake := serveFakeSilkd(t, sock) + configureVolumeDevices(fake) + spec := liveDiskCLI(t, "") + fake.mu.Lock() + fake.serial["/sys/block/vdc/dev"] = []byte("8:32\n") + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + row) + fake.mu.Unlock() + err := New("cocoon", nil, nil, false, false, "").MountLiveVolume(t.Context(), sock, spec.Name, "/data", false) + if err == nil { + t.Fatal("accepted a foreign mount") + } + conflict, isConflict := errors.AsType[*VolumeMountConflict](err) + if isConflict != (row != "malformed\n") { + t.Fatalf("conflict type: %v", err) + } + if isConflict && !strings.Contains(conflict.Error(), `"/data" has a different device`) { + t.Fatalf("conflict: %v", conflict) + } + fake.mu.Lock() + defer fake.mu.Unlock() + if changes := guestChanges(fake.execCalls); len(changes) != 0 { + t.Fatalf("modified foreign mount: %v", changes) + } + }) + } +} + +func TestLiveDetachKeepsBusyAndUnobservableDevices(t *testing.T) { + for _, mode := range []string{"busy", "second mount", "probe error", "guest error", "transport disconnect", "host error", "stacked mount", "stacked bind"} { + t.Run(mode, func(t *testing.T) { + sock := sockPath(t) + fake := serveFakeSilkd(t, sock) + configureVolumeDevices(fake) + hostErr := "" + if mode == "host error" { + hostErr = "device removal initiated but guest has not ejected it" + } + spec := liveDiskCLI(t, hostErr) + fake.mu.Lock() + fake.serial["/sys/block/vdc/dev"] = []byte("8:32\n") + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo) + switch mode { + case "busy": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / /data ro - ext4 /dev/vdc ro\n") + fake.execCode = 32 + case "second mount": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / /other ro - ext4 /dev/vdc ro\n") + case "transport disconnect": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / /data ro - ext4 /dev/vdc ro\n") + fake.execDrop = true + case "guest error": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / /data ro - ext4 /dev/vdc ro\n") + fake.execErr = "guest connection failed /private/runtime" + case "probe error": + fake.readErr["/proc/self/mountinfo"] = "read failed" + case "stacked mount": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / /data ro - ext4 /dev/vdc ro\n3 2 0:39 / /data rw - tmpfs guesttmp rw\n") + case "stacked bind": + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / /data ro - ext4 /dev/vdc ro\n3 2 8:32 /sub /data ro - ext4 /dev/vdc ro\n") + } + fake.mu.Unlock() + err := New("cocoon", nil, nil, false, false, "").DetachVolume(t.Context(), "sbx-1", sock, spec.Name, "/data") + if err == nil { + t.Fatal("detach must fail closed") + } + conflict, isConflict := errors.AsType[*VolumeMountConflict](err) + wantPath := map[string]string{"busy": "/data", "second mount": "/other", "stacked mount": "/data", "stacked bind": "/data"}[mode] + conflictExpected := wantPath != "" + if isConflict != conflictExpected { + t.Fatalf("conflict=%v error=%v", conflictExpected, err) + } + if conflictExpected && (!strings.Contains(conflict.Error(), strconv.Quote(wantPath)) || strings.Contains(conflict.Error(), "/private")) { + t.Fatalf("unsafe or missing guest path %s: %v", wantPath, conflict) + } + if mode == "busy" && !strings.HasPrefix(conflict.Error(), "cannot unmount") { + t.Fatalf("busy conflict: %v", conflict) + } + fake.mu.Lock() + defer fake.mu.Unlock() + if !slices.Contains([]string{"busy", "guest error", "transport disconnect"}, mode) && len(guestChanges(fake.execCalls)) != 0 { + t.Fatalf("changed guest mounts: %v", fake.execCalls) + } + }) + } +} + +func TestLiveDetachMissingGuestStillChecksHost(t *testing.T) { + for _, hostErr := range []string{"", `disk "imagenet" not attached`, "vm not running", "backend unavailable"} { + t.Run(hostErr, func(t *testing.T) { + sock := sockPath(t) + serveFakeSilkd(t, sock) + spec := liveDiskCLI(t, hostErr) + err := New("cocoon", nil, nil, false, false, "").DetachVolume(t.Context(), "sbx-1", sock, spec.Name, "/data") + wantErr := hostErr != "" + if (err != nil) != wantErr { + t.Fatalf("got %v, want error=%v", err, wantErr) + } + }) + } +} + +func TestLiveDetachIgnoresAnotherDeviceAtThePath(t *testing.T) { + sock := sockPath(t) + fake := serveFakeSilkd(t, sock) + configureVolumeDevices(fake) + spec := liveDiskCLI(t, "") + fake.mu.Lock() + fake.serial["/sys/block/vdc/dev"] = []byte("8:32\n") + fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:48 / /data ro - ext4 /dev/vdd ro\n") + fake.mu.Unlock() + if err := New("cocoon", nil, nil, false, false, "").DetachVolume(t.Context(), "sbx-1", sock, spec.Name, "/data"); err != nil { + t.Fatalf("detach of an unmounted device blocked by another device's mount: %v", err) + } + fake.mu.Lock() + defer fake.mu.Unlock() + if changes := guestChanges(fake.execCalls); len(changes) != 0 { + t.Fatalf("touched another device's mount: %v", changes) + } +} + +func liveDiskCLI(t *testing.T, detachErr string) VolumeSpec { + t.Helper() + dir := t.TempDir() + image := filepath.Join(dir, "volume.img") + if err := os.WriteFile(image, nil, 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("COCOON_TEST_DETACH_ERROR", detachErr) + attached := filepath.Join(dir, "attached") + if err := os.WriteFile(attached, nil, 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("COCOON_TEST_ATTACHED", attached) + t.Setenv("COCOON_TEST_ATTACH_ARGS", filepath.Join(dir, "args")) + fakeCocoon(t, `#!/bin/sh +if [ "$1 $2 $3" = "vm disk attach" ]; then + printf '%s\n' "$*" >> "$COCOON_TEST_ATTACH_ARGS" + touch "$COCOON_TEST_ATTACHED" + exit 0 +fi +if [ "$1 $2 $3" = "vm disk detach" ]; then + if [ -n "$COCOON_TEST_DETACH_ERROR" ]; then echo "$COCOON_TEST_DETACH_ERROR" >&2; exit 1; fi + exit 0 +fi +exit 2 +`) + return VolumeSpec{Name: "imagenet", Path: image} +} + +func guestChanges(calls [][]string) [][]string { + return slices.DeleteFunc(slices.Clone(calls), func(argv []string) bool { return argv[0] == "realpath" }) +} diff --git a/sandboxd/engine/volume_test.go b/sandboxd/engine/volume_test.go index f9825acc..e41c5c82 100644 --- a/sandboxd/engine/volume_test.go +++ b/sandboxd/engine/volume_test.go @@ -265,7 +265,7 @@ func TestSilkdListAndReadAreBounded(t *testing.T) { if _, err := e.silkdList(t.Context(), path, "/sys/block"); err == nil || !strings.Contains(err.Error(), "exceeds") { t.Errorf("list of %d entries: %v, want a bound error", silkdListMax+1, err) } - if _, err := e.silkdReadFile(t.Context(), path, "/sys/block/vda/serial"); err == nil || !strings.Contains(err.Error(), "exceeds") { + if _, err := e.silkdReadFile(t.Context(), path, "/sys/block/vda/serial", silkdReadMax); err == nil || !strings.Contains(err.Error(), "exceeds") { t.Errorf("read of %d bytes: %v, want a bound error", silkdReadMax+1, err) } } diff --git a/sandboxd/pool/checkpoint.go b/sandboxd/pool/checkpoint.go index c894d935..926388d9 100644 --- a/sandboxd/pool/checkpoint.go +++ b/sandboxd/pool/checkpoint.go @@ -40,9 +40,6 @@ func (m *Manager) Checkpoint(ctx context.Context, id string, cred Cred, name, te if name != "" && !types.NameRe.MatchString(name) { return types.Checkpoint{}, fmt.Errorf("%w: %q must match %s", ErrBadName, name, types.NameRe) } - if hasAppliedVolumes(sb) { - return types.Checkpoint{}, ErrVolumeCapture - } if !sb.Key.Capturable() { return types.Checkpoint{}, ErrNoEgressFork } diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index a3c6dd12..3b49b378 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -165,11 +165,12 @@ func (m *Manager) Renew(ctx context.Context, id string, cred Cred, ttl time.Dura return time.Time{}, err } } - if err := archivable(sb.Key, hasAppliedVolumes(sb), onExpire); err != nil { - return time.Time{}, err - } lease := clampTTL(ttl) m.mu.Lock() + if err := archivable(sb.Key, m.hasVolumeHolds(sb), onExpire); err != nil { + m.mu.Unlock() + return time.Time{}, err + } if m.claimed[id] != sb { m.mu.Unlock() return time.Time{}, ErrUnknownSandbox @@ -287,6 +288,7 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand marked = ck m.mu.Lock() } + volumeHolds := m.cancelVolumeRun(sb) snap, ck, vmName := sb.HibernateSnap, sb.ArchiveCk, sb.VMName if ck != "" { m.pendingCks[ck] = struct{}{} @@ -298,6 +300,7 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand if saveErr := m.store.commit(js); saveErr != nil { m.mu.Lock() m.claimed[id] = sb // roll back so memory matches the still-durable claim + m.restoreVolumeHolds(sb, volumeHolds) m.tenantDelta(sb.Tenant, 1) delete(m.pendingCks, ck) rb := m.store.set(sb) @@ -319,6 +322,7 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand m.untrack(m.pendingCks, ck) } td := m.quiesceVolumes(ctx, sb) + td.holds = volumeHolds var err error removed := vmName == "" switch { @@ -473,7 +477,9 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t func (m *Manager) rollbackClaim(ctx context.Context, sbs []*types.Sandbox) { m.mu.Lock() ids := make([]string, len(sbs)) + holds := make([][]types.Volume, len(sbs)) for i, sb := range sbs { + holds[i] = m.cancelVolumeRun(sb) delete(m.claimed, sb.ID) m.tenantDelta(sb.Tenant, -1) ids[i] = sb.ID @@ -481,8 +487,9 @@ func (m *Manager) rollbackClaim(ctx context.Context, sbs []*types.Sandbox) { rb := m.store.del(ids...) m.mu.Unlock() m.recommit(ctx, rb) - for _, sb := range sbs { + for i, sb := range sbs { td := m.quiesceVolumes(ctx, sb) + td.holds = holds[i] m.out.Disarm(sb.ID, m.removeOrRetry(ctx, sb.VMName, sb.ID, "", td)) } } @@ -499,6 +506,7 @@ func (m *Manager) reapOnce(ctx context.Context) { action reapAction id, vmName, snap, ck, tenant string sb *types.Sandbox + holds []types.Volume } m.mu.Lock() var expired []victim @@ -524,7 +532,7 @@ func (m *Manager) reapOnce(ctx context.Context) { } expired = append(expired, victim{action: action, id: id, sb: sb}) default: - expired = append(expired, victim{action: reapDestroy, id: id, vmName: sb.VMName, snap: sb.HibernateSnap, tenant: sb.Tenant, sb: sb}) + expired = append(expired, victim{action: reapDestroy, id: id, vmName: sb.VMName, snap: sb.HibernateSnap, tenant: sb.Tenant, sb: sb, holds: m.cancelVolumeRun(sb)}) delete(m.claimed, id) m.tenantDelta(sb.Tenant, -1) dropped = append(dropped, id) @@ -581,6 +589,7 @@ func (m *Manager) reapOnce(ctx context.Context) { delete(m.archiving, v.id) // never removed from m.claimed } else { m.claimed[v.id] = v.sb // roll back so memory matches the still-durable claim + m.restoreVolumeHolds(v.sb, v.holds) m.tenantDelta(v.tenant, 1) delete(m.pendingCks, v.ck) restored = append(restored, v.sb) @@ -614,6 +623,7 @@ func (m *Manager) reapOnce(ctx context.Context) { logSweepResult(ctx, logger, m.pauseExpired(ctx, v.sb), "archived expired sandbox "+v.id, "archive expired sandbox "+v.id) default: td := m.quiesceVolumes(ctx, v.sb) + td.holds = v.holds m.out.Disarm(v.id, m.removeOrRetry(ctx, v.vmName, v.id, "", td)) m.dropSnap(ctx, v.snap) m.counters.reaps.Add(1) diff --git a/sandboxd/pool/claims.go b/sandboxd/pool/claims.go index 03f1e2c7..6011216c 100644 --- a/sandboxd/pool/claims.go +++ b/sandboxd/pool/claims.go @@ -39,6 +39,7 @@ type claimDTO struct { Metadata types.Metadata `json:"metadata,omitempty"` OnExpire types.ExpireAction `json:"on_expire,omitempty"` Volumes []types.Volume `json:"volumes,omitempty"` + VolumesWant []types.Volume `json:"volumes_want,omitzero"` Env types.Env `json:"env,omitempty"` VsockSocket string `json:"vsock_socket,omitempty"` TAP string `json:"tap,omitempty"` @@ -88,6 +89,11 @@ func (s *claimStore) load() (map[string]*types.Sandbox, error) { if err := json.Unmarshal(raw, &claims); err != nil { return nil, fmt.Errorf("parse claims: %w", err) } + for _, sb := range claims { + if sb.VolumesWant == nil { + sb.VolumesWant = sb.Volumes + } + } return claims, nil } @@ -220,9 +226,17 @@ func dtoOf(sb *types.Sandbox) claimDTO { Deadline: sb.Deadline, ClaimedAt: sb.ClaimedAt, LeaseSeconds: sb.LeaseSeconds, Layer: sb.Layer, PolicySource: sb.PolicySource, NoEgress: sb.NoEgress, Tenant: sb.Tenant, EgressClass: sb.EgressClass, ClaimRef: sb.ClaimRef, Metadata: sb.Metadata, OnExpire: sb.OnExpire, - Volumes: slices.Clone(sb.Volumes), Env: sb.Env, VsockSocket: sb.VsockSocket, + Volumes: slices.Clone(sb.Volumes), VolumesWant: volumesWantDTO(sb), Env: sb.Env, VsockSocket: sb.VsockSocket, TAP: sb.TAP, HibernateSnap: sb.HibernateSnap, PendingSnap: sb.PendingSnap, ArchiveCk: sb.ArchiveCk, FromCheckpoint: sb.FromCheckpoint, Restarts: sb.Restarts, RestartedAt: sb.RestartedAt, Failed: sb.Failed, } } + +// volumesWantDTO persists the desired set only when it differs; absent reads back as equal to Volumes. +func volumesWantDTO(sb *types.Sandbox) []types.Volume { + if slices.Equal(sb.VolumesWant, sb.Volumes) { + return nil + } + return append([]types.Volume{}, sb.VolumesWant...) +} diff --git a/sandboxd/pool/claims_test.go b/sandboxd/pool/claims_test.go index 3281eecf..f9c0009c 100644 --- a/sandboxd/pool/claims_test.go +++ b/sandboxd/pool/claims_test.go @@ -77,24 +77,25 @@ func TestRecommitDrainsAChangeItWasTooBusyToSee(t *testing.T) { } func TestStoreRoundTrip(t *testing.T) { + dataset := []types.Volume{{Name: "dataset-a", Mount: "/datasets/a"}} claims := map[string]*types.Sandbox{ - "sb_a": {ID: "sb_a", VMName: "sbx-1", Key: testKey, Token: "t1", Deadline: time.Now().Add(time.Minute).UTC(), ClaimedAt: time.Now().Add(-time.Minute).UTC(), ClaimRef: "ns/workload", Metadata: types.Metadata{"team": "a"}, OnExpire: types.ExpireArchive, Volumes: []types.Volume{{Name: "dataset-a", Mount: "/datasets/a"}}, VsockSocket: "/v/1"}, + "sb_a": {ID: "sb_a", VMName: "sbx-1", Key: testKey, Token: "t1", Deadline: time.Now().Add(time.Minute).UTC(), ClaimedAt: time.Now().Add(-time.Minute).UTC(), ClaimRef: "ns/workload", Metadata: types.Metadata{"team": "a"}, OnExpire: types.ExpireArchive, Volumes: dataset, VolumesWant: dataset, VsockSocket: "/v/1"}, "sb_b": {ID: "sb_b", VMName: "sbx-2", Key: testKey, Token: "t2"}, } - for _, sync := range []bool{false, true} { - s := newClaimStore(t.TempDir(), sync) + for _, fsync := range []bool{false, true} { + s := newClaimStore(t.TempDir(), fsync) if err := s.save(claims); err != nil { - t.Fatalf("save (sync=%v): %v", sync, err) + t.Fatalf("save (sync=%v): %v", fsync, err) } got, err := s.load() if err != nil { - t.Fatalf("load (sync=%v): %v", sync, err) + t.Fatalf("load (sync=%v): %v", fsync, err) } if !reflect.DeepEqual(got, claims) { - t.Errorf("sync=%v: got %+v, want %+v", sync, got, claims) + t.Errorf("sync=%v: got %+v, want %+v", fsync, got, claims) } if _, err := os.Stat(s.path + ".tmp"); !errors.Is(err, fs.ErrNotExist) { - t.Errorf("sync=%v: temp file left behind: %v", sync, err) + t.Errorf("sync=%v: temp file left behind: %v", fsync, err) } } } diff --git a/sandboxd/pool/egress_test.go b/sandboxd/pool/egress_test.go index bd7910f1..462a8102 100644 --- a/sandboxd/pool/egress_test.go +++ b/sandboxd/pool/egress_test.go @@ -85,7 +85,7 @@ func TestGoldenStampGatesAdoptionOnLane(t *testing.T) { func TestEgressLaneDoesNotHibernate(t *testing.T) { m := newTestManager(t, newFakeEngine()) - sb := &types.Sandbox{ID: "sb_eg_h", Key: types.PoolKey{Template: "rt:24.04", Net: types.NetEgress, Size: types.SizeSmall}} + sb := &types.Sandbox{ID: "sb_eg_h", Key: egKey} sb.Transition.Lock() err := m.hibernateLocked(t.Context(), sb) sb.Transition.Unlock() @@ -359,6 +359,7 @@ func TestSetPoolsPreservesEgressPolicy(t *testing.T) { if !policyLive() { t.Fatal("drain + re-add wiped the pool egress policy") } + waitPoolSettled(t, m, egKey) } func TestEgressLaneCannotForkOrCheckpoint(t *testing.T) { diff --git a/sandboxd/pool/fork.go b/sandboxd/pool/fork.go index 56ba3741..83c2ad8d 100644 --- a/sandboxd/pool/fork.go +++ b/sandboxd/pool/fork.go @@ -24,9 +24,6 @@ func (m *Manager) Fork(ctx context.Context, id string, cred Cred, count int, ttl if count < 1 || count > m.maxFork { return nil, fmt.Errorf("%w: %d not in 1..%d", ErrBadCount, count, m.maxFork) } - if hasAppliedVolumes(sb) { - return nil, ErrVolumeCapture - } if !sb.Key.Capturable() { return nil, ErrNoEgressFork } @@ -78,6 +75,9 @@ func (m *Manager) forkClones(ctx context.Context, sb *types.Sandbox, count int) func (m *Manager) forkSource(ctx context.Context, sb *types.Sandbox) (create vmProvisioner, cleanup func(), inherited bool, err error) { sb.Transition.Lock() defer sb.Transition.Unlock() + if err = m.captureAllowed(sb); err != nil { + return nil, nil, false, err + } inherited = m.heldGuestEnv(sb) if sb.HibernateSnap == "" { snap, drop, snapErr := m.sourceSnap(ctx, sb) diff --git a/sandboxd/pool/hibernate.go b/sandboxd/pool/hibernate.go index ada924ed..e39b7823 100644 --- a/sandboxd/pool/hibernate.go +++ b/sandboxd/pool/hibernate.go @@ -48,7 +48,10 @@ func (m *Manager) hibernateLocked(ctx context.Context, sb *types.Sandbox) error if sb.Failed != "" { return ErrFailed } - if hasAppliedVolumes(sb) { + m.mu.Lock() + volumeBlocked := m.hasVolumeHolds(sb) + m.mu.Unlock() + if volumeBlocked { return ErrVolumeCapture } if sb.Key.Net == types.NetEgress { @@ -331,6 +334,6 @@ func (m *Manager) recordHibernate(ctx context.Context, sb *types.Sandbox) { } func skipIdle(sb *types.Sandbox) bool { - return sb.Key.Net == types.NetEgress || hasAppliedVolumes(sb) || + return sb.Key.Net == types.NetEgress || hasVolumes(sb) || sb.HibernateSnap != "" || sb.ArchiveCk != "" || sb.Failed != "" || sb.Busy() } diff --git a/sandboxd/pool/liveness.go b/sandboxd/pool/liveness.go index da653d1a..c87398ba 100644 --- a/sandboxd/pool/liveness.go +++ b/sandboxd/pool/liveness.go @@ -132,6 +132,7 @@ func (m *Manager) recoverVMM(ctx context.Context, sb *types.Sandbox) { now := time.Now() m.mu.Lock() live := m.claimed[sb.ID] == sb && runningShaped(sb) && !m.leaseLapsed(sb, now) + volumes := m.hasVolumeHolds(sb) m.mu.Unlock() if !live { return @@ -150,7 +151,7 @@ func (m *Manager) recoverVMM(ctx context.Context, sb *types.Sandbox) { sb.ExitRecorded = true m.recordUsage(ctx, usageEvent{Event: "vmm_exit", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant}) } - reason := coldBootBlocker(sb, found) + reason := coldBootBlocker(sb, found, volumes) if reason == "" && m.vmmRestart == types.VMMRestartNone { reason = "vmm exited" } @@ -179,7 +180,7 @@ func (m *Manager) restartFailed(ctx context.Context, sb *types.Sandbox) error { sb.Transition.Lock() defer sb.Transition.Unlock() m.mu.Lock() - live, failed := m.claimed[sb.ID] == sb, sb.Failed != "" + live, failed, volumes := m.claimed[sb.ID] == sb, sb.Failed != "", m.hasVolumeHolds(sb) m.mu.Unlock() switch { case !live: @@ -191,7 +192,7 @@ func (m *Manager) restartFailed(ctx context.Context, sb *types.Sandbox) error { if err != nil { return fmt.Errorf("wake %s: %w", sb.ID, err) } - if reason := coldBootBlocker(sb, found); reason != "" { + if reason := coldBootBlocker(sb, found, volumes); reason != "" { return fmt.Errorf("%w: %s", ErrFailed, reason) } sb.RestartLog = nil @@ -267,14 +268,14 @@ func runningShaped(sb *types.Sandbox) bool { } // coldBootBlocker names why sb's VM cannot be cold-booted, "" when it can. -func coldBootBlocker(sb *types.Sandbox, found bool) string { +func coldBootBlocker(sb *types.Sandbox, found, volumes bool) string { switch { case !found: return "vm record gone" case sb.Key.Net == types.NetEgress: // the netdev nft chain dies with the tap, so the guest would boot with an unlocked NIC return "vmm exited; an egress-lane sandbox cannot cold-boot" - case hasAppliedVolumes(sb): + case volumes: return "vmm exited; volume mounts do not survive a cold boot" default: return "" diff --git a/sandboxd/pool/liveness_test.go b/sandboxd/pool/liveness_test.go index 5a1a4747..60a9e5b3 100644 --- a/sandboxd/pool/liveness_test.go +++ b/sandboxd/pool/liveness_test.go @@ -129,21 +129,7 @@ func TestLivenessRestartFailureRetriesThenFails(t *testing.T) { killVMM(eng, sb.VMName) eng.startErr = errors.New("boot failed") - for range vmmRestartBudget - 1 { - m.recoverVMM(t.Context(), sb) - if got, _ := m.Sandbox(sb.ID); got.Failed != "" { - t.Fatalf("failed %q after a retryable attempt", got.Failed) - } - } - m.recoverVMM(t.Context(), sb) - - got, _ := m.Sandbox(sb.ID) - if got.Failed != reasonColdBootFailed || got.Restarts != 0 { - t.Errorf("summary %+v, want the fixed reason %q after %d attempts", got, reasonColdBootFailed, vmmRestartBudget) - } - if len(eng.stops) != vmmRestartBudget { - t.Errorf("stops %v, want every failed start stopped", eng.stops) - } + assertRestartFailsUntilBudget(t, m, eng, sb) if got, want := usageEventsOf(t, m, sb.ID), "claim,vmm_exit,vmm_failed"; got != want { t.Errorf("events %s, want one exit across the attempts: %s", got, want) } @@ -453,7 +439,7 @@ func TestColdBootBlocker(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := coldBootBlocker(tt.sb, tt.found) + got := coldBootBlocker(tt.sb, tt.found, hasVolumes(tt.sb)) if (tt.want == "") != (got == "") || !strings.Contains(got, tt.want) { t.Errorf("got %q, want %q", got, tt.want) } @@ -524,6 +510,11 @@ func TestReconcileKeepsRunningClaimsTheHostLost(t *testing.T) { } } +type fakeVMEvent struct { + sync []engine.VMStatus + change engine.VMChange +} + func assertRestartFailsUntilBudget(t *testing.T, m *Manager, eng *fakeEngine, sb *types.Sandbox) { t.Helper() for attempt := range vmmRestartBudget { @@ -553,11 +544,6 @@ func runLiveness(t *testing.T, m *Manager) { waitFor(t, func() bool { return !m.livenessSweep.Load() }) } -type fakeVMEvent struct { - sync []engine.VMStatus - change engine.VMChange -} - func killVMM(eng *fakeEngine, name string) { eng.mu.Lock() defer eng.mu.Unlock() diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 8ebbaff3..77f3d649 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -139,8 +139,11 @@ type Engine interface { MarkLane(ctx context.Context, vsockSocket string, lane engine.Lane) error Warmup(ctx context.Context, vsockSocket string, argv []string) error TrimCow(ctx context.Context, vsockSocket string) error + CheckLiveVolumeSupport(ctx context.Context) error DiskAttach(ctx context.Context, vmName string, spec engine.VolumeSpec) error + DetachVolume(ctx context.Context, vmName, sock, name, mount string) error MountVolume(ctx context.Context, vsockSocket, name, mount string, rw bool) error + MountLiveVolume(ctx context.Context, vsockSocket, name, mount string, rw bool) error UnmountVolume(ctx context.Context, vsockSocket, mount string) error SyncGuest(ctx context.Context, vsockSocket string) error WriteInstanceMetadata(ctx context.Context, vsockSocket string, doc []byte) error @@ -161,6 +164,7 @@ type SandboxSummary struct { CPUCount int `json:"cpu_count,omitzero"` MemTotalBytes int64 `json:"mem_total_bytes,omitzero"` Volumes []types.Volume `json:"volumes,omitempty"` + VolumesWant []types.Volume `json:"volumes_want,omitempty"` Hibernated bool `json:"hibernated"` Archived bool `json:"archived,omitzero"` FromCheckpoint string `json:"from_checkpoint,omitempty"` @@ -294,6 +298,8 @@ type Manager struct { volumes map[string]catalogVolume // volumeAdmission counts the live holders of each volume name; guarded by m.mu. volumeAdmission map[string]volumeHolders + volumeRuns map[*types.Sandbox]*volumeRun + volumeErrors map[*types.Sandbox]map[string]string poolStore *poolStore // poolShared is the cell's pool set with a meta store; poolVersion, under m.mu, is its newest version applied. @@ -396,6 +402,8 @@ func NewManager(ctx context.Context, cfg *config.Config, eng Engine, secrets *eg store: newClaimStore(cfg.DataDir, cfg.SyncClaims), volumes: make(map[string]catalogVolume, len(cfg.Volumes)), volumeAdmission: map[string]volumeHolders{}, + volumeRuns: map[*types.Sandbox]*volumeRun{}, + volumeErrors: map[*types.Sandbox]map[string]string{}, poolStore: newPoolStore(cfg.DataDir), pools: make(map[types.PoolKey]*pool, len(cfg.Pools)), claimed: map[string]*types.Sandbox{}, @@ -688,7 +696,7 @@ func summarize(sb *types.Sandbox) SandboxSummary { FromCheckpoint: sb.FromCheckpoint, ClaimRef: sb.ClaimRef, Metadata: sb.Metadata, OnExpire: sb.OnExpire, Restarts: sb.Restarts, RestartedAt: sb.RestartedAt, Failed: sb.Failed, CPUCount: spec.CPU, MemTotalBytes: spec.MemoryBytes, - Volumes: slices.Clone(sb.Volumes), + Volumes: slices.Clone(sb.Volumes), VolumesWant: slices.Clone(sb.VolumesWant), } } @@ -710,8 +718,8 @@ func tenantOwns(tenant, owner string) bool { return tenant == "" || tenant == owner } -func hasAppliedVolumes(sb *types.Sandbox) bool { - return len(sb.Volumes) > 0 +func hasVolumes(sb *types.Sandbox) bool { + return len(sb.Volumes) > 0 || len(sb.VolumesWant) > 0 } // logSweepResult reports one background-sweep outcome; benign races stay silent. diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 6253b546..0d9fc1db 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -153,9 +153,8 @@ func TestClaimCanceledRequestStillDestroys(t *testing.T) { func TestClaimEgressNeedsAttachment(t *testing.T) { m := newTestManager(t, newFakeEngine()) - key := types.PoolKey{Template: "rt:24.04", Net: types.NetEgress, Size: types.SizeSmall} - if _, err := claimAny(t.Context(), m, key, 0); !errors.Is(err, ErrNoEgress) { + if _, err := claimAny(t.Context(), m, egKey, 0); !errors.Is(err, ErrNoEgress) { t.Errorf("got %v, want ErrNoEgress", err) } } @@ -1190,83 +1189,6 @@ func (c *closeWriteConn) CloseWrite() error { return nil } -func newTestManager(t testing.TB, eng *fakeEngine, pools ...config.PoolSpec) *Manager { - t.Helper() - return newTestManagerAt(t, eng, t.TempDir(), pools...) -} - -func claimAny(ctx context.Context, m *Manager, key types.PoolKey, ttl time.Duration) (*types.Sandbox, error) { - sb, err := m.ClaimWarm(ctx, key, ClaimOptions{TTL: ttl}) - if errors.Is(err, ErrNoWarm) { - return m.ClaimProvision(ctx, key, ClaimOptions{TTL: ttl}) - } - return sb, err -} - -func mustClaim(t *testing.T, m *Manager, key types.PoolKey) *types.Sandbox { - t.Helper() - sb, err := claimAny(t.Context(), m, key, 0) - if err != nil { - t.Fatalf("Claim: %v", err) - } - return sb -} - -func newTestManagerAt(t testing.TB, eng *fakeEngine, dataDir string, pools ...config.PoolSpec) *Manager { - t.Helper() - m, err := NewManager(t.Context(), &config.Config{DataDir: dataDir, Pools: pools}, eng, testSecrets(t)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } - return m -} - -func testSecrets(t testing.TB, specs ...egress.SecretSpec) *egress.SecretStore { - t.Helper() - s, err := egress.NewSecretStore(specs) - if err != nil { - t.Fatalf("secrets: %v", err) - } - return s -} - -func lockCount(m *Manager) int { - m.recLocksMu.Lock() - defer m.recLocksMu.Unlock() - return len(m.recLocks) -} - -func hasRecLock(m *Manager, id string) bool { - m.recLocksMu.Lock() - defer m.recLocksMu.Unlock() - _, ok := m.recLocks[id] - return ok -} - -func waitFor(t *testing.T, cond func() bool) { - t.Helper() - deadline := time.Now().Add(3 * time.Second) - for time.Now().Before(deadline) { - if cond() { - return - } - time.Sleep(5 * time.Millisecond) - } - t.Fatal("condition not met within 3s") -} - -func seedGolden(t *testing.T, m *Manager, imageID string) string { - t.Helper() - g := filepath.Join(m.goldensDir(), testKey.Hash()) - if err := os.MkdirAll(g, 0o750); err != nil { - t.Fatalf("seed golden: %v", err) - } - if err := os.WriteFile(g+goldenStampSuffix, []byte(m.goldenStamp(m.view.Load(), testKey, imageID)), 0o644); err != nil { - t.Fatalf("seed golden stamp: %v", err) - } - return g -} - type fakeEngine struct { mu sync.Mutex vms map[string]string @@ -1307,6 +1229,8 @@ type fakeEngine struct { diskAttachCancel context.CancelFunc mountVolumeErr error unmountVolumeErr error + detachVolumeErr error + liveVolumeHook func(string) error stopped map[string]bool creating map[string]bool staleOutcome engine.StaleCreateOutcome @@ -1315,14 +1239,14 @@ type fakeEngine struct { vsockLateN int sockRoot string - cloneErr, runColdErr, probeErr, hibernateErr, restoreErr, snapSaveErr, snapListErr, startErr error - removeErrFor string - cloneFailNth int - hibernateErrCompletes bool - tap string - listCount int - inspects int - vmEvents chan fakeVMEvent + cloneErr, runColdErr, probeErr, hibernateErr, restoreErr, snapListErr, startErr error + removeErrFor string + cloneFailNth int + hibernateErrCompletes bool + tap string + listCount int + inspects int + vmEvents chan fakeVMEvent attachRendezvous *sync.WaitGroup @@ -1443,7 +1367,7 @@ func (f *fakeEngine) SnapshotSave(_ context.Context, _, snapName string) error { f.mu.Lock() defer f.mu.Unlock() f.snapSaves = append(f.snapSaves, snapName) - return f.snapSaveErr + return nil } func (f *fakeEngine) SnapshotExport(_ context.Context, snapName, toDir string) error { @@ -1618,7 +1542,7 @@ func (f *fakeEngine) DialGuestPort(context.Context, string, uint16) (net.Conn, e if f.guestDial != nil { return f.guestDial() } - return nil, fmt.Errorf("fakeEngine has no guest port") + return nil, errors.New("fakeEngine has no guest port") } func (f *fakeEngine) InstallCACert(_ context.Context, vsockSocket string, _ []byte) error { @@ -1654,7 +1578,17 @@ func (f *fakeEngine) TrimCow(_ context.Context, sock string) error { return f.trimErr } -func (f *fakeEngine) DiskAttach(_ context.Context, _ string, spec engine.VolumeSpec) error { +func (f *fakeEngine) CheckLiveVolumeSupport(context.Context) error { return nil } + +func (f *fakeEngine) DiskAttach(ctx context.Context, _ string, spec engine.VolumeSpec) error { + if f.liveVolumeHook != nil { + if err := f.liveVolumeHook("attach"); err != nil { + return err + } + } + if err := ctx.Err(); err != nil { + return err + } f.mu.Lock() f.volumeSpecs = append(f.volumeSpecs, spec) f.volumeOps = append(f.volumeOps, "attach:"+spec.Name) @@ -1686,6 +1620,30 @@ func (f *fakeEngine) MountVolume(_ context.Context, _, name, mount string, rw bo return f.mountVolumeErr } +func (f *fakeEngine) MountLiveVolume(ctx context.Context, sock, name, mount string, rw bool) error { + return f.MountVolume(ctx, sock, name, mount, rw) +} + +func (f *fakeEngine) DetachVolume(ctx context.Context, _, sock, name, mount string) error { + if f.liveVolumeHook != nil { + if err := f.liveVolumeHook("detach"); err != nil { + return err + } + } + if err := ctx.Err(); err != nil { + return err + } + if mount != "" { + if err := f.UnmountVolume(ctx, sock, mount); err != nil { + return err + } + } + f.mu.Lock() + defer f.mu.Unlock() + f.volumeOps = append(f.volumeOps, "detach:"+name) + return f.detachVolumeErr +} + func (f *fakeEngine) UnmountVolume(_ context.Context, _, mount string) error { f.mu.Lock() defer f.mu.Unlock() @@ -1863,3 +1821,80 @@ func (f *fakeEngine) snapRemoved(name string) bool { defer f.mu.Unlock() return slices.Contains(f.snapRemoves, name) } + +func newTestManager(t testing.TB, eng *fakeEngine, pools ...config.PoolSpec) *Manager { + t.Helper() + return newTestManagerAt(t, eng, t.TempDir(), pools...) +} + +func claimAny(ctx context.Context, m *Manager, key types.PoolKey, ttl time.Duration) (*types.Sandbox, error) { + sb, err := m.ClaimWarm(ctx, key, ClaimOptions{TTL: ttl}) + if errors.Is(err, ErrNoWarm) { + return m.ClaimProvision(ctx, key, ClaimOptions{TTL: ttl}) + } + return sb, err +} + +func mustClaim(t *testing.T, m *Manager, key types.PoolKey) *types.Sandbox { + t.Helper() + sb, err := claimAny(t.Context(), m, key, 0) + if err != nil { + t.Fatalf("Claim: %v", err) + } + return sb +} + +func newTestManagerAt(t testing.TB, eng *fakeEngine, dataDir string, pools ...config.PoolSpec) *Manager { + t.Helper() + m, err := NewManager(t.Context(), &config.Config{DataDir: dataDir, Pools: pools}, eng, testSecrets(t)) + if err != nil { + t.Fatalf("setup manager: %v", err) + } + return m +} + +func testSecrets(t testing.TB, specs ...egress.SecretSpec) *egress.SecretStore { + t.Helper() + s, err := egress.NewSecretStore(specs) + if err != nil { + t.Fatalf("secrets: %v", err) + } + return s +} + +func lockCount(m *Manager) int { + m.recLocksMu.Lock() + defer m.recLocksMu.Unlock() + return len(m.recLocks) +} + +func hasRecLock(m *Manager, id string) bool { + m.recLocksMu.Lock() + defer m.recLocksMu.Unlock() + _, ok := m.recLocks[id] + return ok +} + +func waitFor(t *testing.T, cond func() bool) { + t.Helper() + deadline := time.Now().Add(3 * time.Second) + for time.Now().Before(deadline) { + if cond() { + return + } + time.Sleep(5 * time.Millisecond) + } + t.Fatal("condition not met within 3s") +} + +func seedGolden(t *testing.T, m *Manager, imageID string) string { + t.Helper() + g := filepath.Join(m.goldensDir(), testKey.Hash()) + if err := os.MkdirAll(g, 0o750); err != nil { + t.Fatalf("seed golden: %v", err) + } + if err := os.WriteFile(g+goldenStampSuffix, []byte(m.goldenStamp(m.view.Load(), testKey, imageID)), 0o644); err != nil { + t.Fatalf("seed golden stamp: %v", err) + } + return g +} diff --git a/sandboxd/pool/poolstore_test.go b/sandboxd/pool/poolstore_test.go index bd1a8197..f36e9051 100644 --- a/sandboxd/pool/poolstore_test.go +++ b/sandboxd/pool/poolstore_test.go @@ -217,6 +217,7 @@ func TestAStaleCellVersionIsNotApplied(t *testing.T) { if floor != 3 { t.Errorf("warm floor %d after an older and a repeated version, want 3 from version 5", floor) } + waitPoolSettled(t, m, apiKey) } func TestAPutTheCellCannotStoreChangesNothing(t *testing.T) { @@ -269,33 +270,6 @@ func TestBootPrefersTheCellSetOverPoolsJSON(t *testing.T) { } } -func metaStoreNode(t *testing.T, cell string) *Manager { - t.Helper() - cfg := &config.Config{ - DataDir: t.TempDir(), - Pools: []config.PoolSpec{{PoolKey: seedKey, Warm: 1}}, - MetaStore: &config.MetaStoreConfig{Kind: "pg", DSNEnv: "SANDBOX_TEST_DSN", Cell: cell}, - } - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } - t.Cleanup(func() { _ = m.tenants.Close() }) - if m.poolShared != nil { - t.Cleanup(func() { _ = m.poolShared.Close() }) - go m.followCellPools(t.Context()) - } - return m -} - -func hasAPISet(m *Manager) bool { - m.mu.Lock() - defer m.mu.Unlock() - p, api := m.activePool(apiKey) - _, seed := m.activePool(seedKey) - return api && p.floor == 3 && !seed -} - type fakeShared struct { set *poolset.Set version int64 @@ -328,3 +302,30 @@ func (f *fakeShared) Watch(ctx context.Context, changed func()) { func (f *fakeShared) Close() error { return nil } + +func metaStoreNode(t *testing.T, cell string) *Manager { + t.Helper() + cfg := &config.Config{ + DataDir: t.TempDir(), + Pools: []config.PoolSpec{{PoolKey: seedKey, Warm: 1}}, + MetaStore: &config.MetaStoreConfig{Kind: "pg", DSNEnv: "SANDBOX_TEST_DSN", Cell: cell}, + } + m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t)) + if err != nil { + t.Fatalf("setup manager: %v", err) + } + t.Cleanup(func() { _ = m.tenants.Close() }) + if m.poolShared != nil { + t.Cleanup(func() { _ = m.poolShared.Close() }) + go m.followCellPools(t.Context()) + } + return m +} + +func hasAPISet(m *Manager) bool { + m.mu.Lock() + defer m.mu.Unlock() + p, api := m.activePool(apiKey) + _, seed := m.activePool(seedKey) + return api && p.floor == 3 && !seed +} diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index 74449619..1bf80f74 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -66,7 +66,7 @@ func (m *Manager) Reconcile(ctx context.Context) error { } m.claimed[id] = sb m.tenantDelta(sb.Tenant, 1) - m.adoptVolumes(sb.Volumes) + m.adoptVolumes(heldVolumes(sb)) owned[sb.VMName] = true referenced[sb.HibernateSnap] = true } diff --git a/sandboxd/pool/refill.go b/sandboxd/pool/refill.go index 635f47c0..7214d759 100644 --- a/sandboxd/pool/refill.go +++ b/sandboxd/pool/refill.go @@ -374,9 +374,25 @@ func (m *Manager) sourceSnap(ctx context.Context, sb *types.Sandbox) (string, fu return snap, func() { m.dropSnap(ctx, snap) }, nil } +func (m *Manager) captureAllowed(sb *types.Sandbox) error { + m.mu.Lock() + defer m.mu.Unlock() + if m.claimed[sb.ID] != sb { + return ErrUnknownSandbox + } + if m.hasVolumeHolds(sb) { + return ErrVolumeCapture + } + return nil +} + // exportSource captures a claimed sandbox into exportDir and returns its snapshot, VM name and guest-env presence. func (m *Manager) exportSource(ctx context.Context, sb *types.Sandbox, exportDir string) (snap, vmName string, guestEnv bool, err error) { sb.Transition.Lock() + if captureErr := m.captureAllowed(sb); captureErr != nil { + sb.Transition.Unlock() + return "", "", false, captureErr + } m.trimForCapture(ctx, sb) snap, cleanup, err := m.sourceSnap(ctx, sb) if err != nil { diff --git a/sandboxd/pool/reload_test.go b/sandboxd/pool/reload_test.go index 6c09a3fd..e5e38799 100644 --- a/sandboxd/pool/reload_test.go +++ b/sandboxd/pool/reload_test.go @@ -355,6 +355,16 @@ func refilledTo(t *testing.T, m *Manager, key types.PoolKey, n int) bool { return warmLen(m, key) == n } +func waitPoolSettled(t *testing.T, m *Manager, key types.PoolKey) { + t.Helper() + waitFor(t, func() bool { + m.mu.Lock() + defer m.mu.Unlock() + p := m.pools[key] + return p != nil && !p.building && p.refilling == 0 + }) +} + func warmLen(m *Manager, key types.PoolKey) int { m.mu.Lock() defer m.mu.Unlock() diff --git a/sandboxd/pool/template.go b/sandboxd/pool/template.go index 5570bb2e..8f473f2d 100644 --- a/sandboxd/pool/template.go +++ b/sandboxd/pool/template.go @@ -47,9 +47,6 @@ func (m *Manager) Promote(ctx context.Context, id string, cred Cred, template, t if !types.NameRe.MatchString(template) { return types.PoolKey{}, "", fmt.Errorf("%w: template %q must match %s", ErrBadKey, template, types.NameRe) } - if hasAppliedVolumes(sb) { - return types.PoolKey{}, "", ErrVolumeCapture - } if !sb.Key.Capturable() { return types.PoolKey{}, "", ErrNoEgressFork } diff --git a/sandboxd/pool/volume.go b/sandboxd/pool/volume.go index 5054afb1..5a1e8eed 100644 --- a/sandboxd/pool/volume.go +++ b/sandboxd/pool/volume.go @@ -134,7 +134,7 @@ func (m *Manager) resolveVolumes(ctx context.Context, tenant string, requested [ // an attach-only claim has no mounts to default; re-defaulting here would mount them applied, err := types.ValidateVolumes(requested, types.VolumesAttachOnly(requested)) if err != nil { - return nil, fmt.Errorf("%w: %v", ErrBadVolume, err) + return nil, fmt.Errorf("%w: %w", ErrBadVolume, err) } resolved := make([]resolvedVolume, 0, len(applied)) for _, volume := range applied { @@ -142,22 +142,34 @@ func (m *Manager) resolveVolumes(ctx context.Context, tenant string, requested [ if !ok || !entry.allowed(tenant) { return nil, ErrVolumeUnavailable } - if volume.RW() && !entry.writable { - return nil, fmt.Errorf("%w: volume %q is not writable", ErrBadVolume, volume.Name) + resolvedEntry, resolveErr := m.resolveCatalogVolume(ctx, volume) + if resolveErr != nil { + return nil, resolveErr } - if _, statErr := os.Stat(entry.disk.Path); statErr != nil { - // the operator's path stays server-side; the refusal matches the unknown one - log.WithFunc("pool.resolveVolumes").Warnf(ctx, "stat volume %q image %s: %v", volume.Name, entry.disk.Path, statErr) - return nil, ErrVolumeUnavailable - } - disk := entry.disk - disk.RW = volume.RW() - volume.AttachOnly = false // past validation the empty mount carries it - resolved = append(resolved, resolvedVolume{disk: disk, applied: volume}) + resolved = append(resolved, resolvedEntry) } return resolved, nil } +func (m *Manager) resolveCatalogVolume(ctx context.Context, volume types.Volume) (resolvedVolume, error) { + entry, ok := m.volumes[volume.Name] + if !ok { + return resolvedVolume{}, ErrVolumeUnavailable + } + if volume.RW() && !entry.writable { + return resolvedVolume{}, fmt.Errorf("%w: volume %q is not writable", ErrBadVolume, volume.Name) + } + if _, statErr := os.Stat(entry.disk.Path); statErr != nil { + // the operator's path stays server-side; the refusal matches the unknown one + log.WithFunc("pool.resolveCatalogVolume").Warnf(ctx, "stat volume %q image %s: %v", volume.Name, entry.disk.Path, statErr) + return resolvedVolume{}, ErrVolumeUnavailable + } + disk := entry.disk + disk.RW = volume.RW() + volume.AttachOnly = false // past validation the empty mount carries it + return resolvedVolume{disk: disk, applied: volume}, nil +} + // confirmVolumesClean refuses a read-only claim of a dirty image; run after holds are taken. func (m *Manager) confirmVolumesClean(volumes []resolvedVolume) error { for _, volume := range volumes { @@ -175,17 +187,17 @@ func (m *Manager) applyVolumes(ctx context.Context, sb *types.Sandbox, volumes [ } group, groupCtx := errgroup.WithContext(ctx) for _, volume := range volumes { - group.Go(func() error { return m.applyVolume(groupCtx, sb, volume) }) + group.Go(func() error { return m.applyVolume(groupCtx, sb, volume, false) }) } if err := group.Wait(); err != nil { return err } - sb.Volumes = applied + sb.Volumes, sb.VolumesWant = applied, applied return nil } // applyVolume keeps one volume's steps strictly ordered; siblings overlap freely. -func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume resolvedVolume) error { +func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume resolvedVolume, live bool) error { // attach-only mounts nothing, so there is no umount to verify and no marker. attachOnly := volume.applied.Mount == "" // write-ahead: the marker must be durable before any guest write can be. @@ -200,7 +212,14 @@ func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume res if attachOnly { return nil } - if err := m.eng.MountVolume(ctx, sb.VsockSocket, volume.applied.Name, volume.applied.Mount, volume.disk.RW); err != nil { + if err := ctx.Err(); err != nil { + return err + } + mount := m.eng.MountVolume + if live { + mount = m.eng.MountLiveVolume + } + if err := mount(ctx, sb.VsockSocket, volume.applied.Name, volume.applied.Mount, volume.disk.RW); err != nil { return fmt.Errorf("setup volume %q: %w", volume.applied.Name, err) } return nil @@ -208,7 +227,7 @@ func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume res // quiesceVolumes unmounts a claim's writable mounts in reverse; a failed unmount keeps the marker. func (m *Manager) quiesceVolumes(ctx context.Context, sb *types.Sandbox) volumeTeardown { - td := volumeTeardown{holds: sb.Volumes} + td := volumeTeardown{holds: heldVolumes(sb)} mounts := writableMounts(sb.Volumes) if mounts == 0 { return td diff --git a/sandboxd/pool/volume_convergence_test.go b/sandboxd/pool/volume_convergence_test.go new file mode 100644 index 00000000..19927ec8 --- /dev/null +++ b/sandboxd/pool/volume_convergence_test.go @@ -0,0 +1,435 @@ +package pool + +import ( + "context" + "errors" + "os" + "slices" + "strings" + "testing" + "time" + + "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/engine" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +func TestSettledVolumesDoNotCallDevicesOrResolveCatalog(t *testing.T) { + eng := newFakeEngine() + catalog := liveCatalog(t) + m := newVolumeManager(t, eng, catalog) + sb := mustClaim(t, m, testKey) + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err != nil { + t.Fatal(err) + } + delete(m.volumes, "data") + before := len(eng.volumeSpecs) + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}, {Name: "other"}}); err != nil { + t.Fatal(err) + } + if len(eng.volumeSpecs) != before+1 || eng.volumeSpecs[before].Name != "other" { + t.Fatalf("retained volume reapplied: %v", eng.volumeSpecs) + } + before = len(eng.volumeSpecs) + journaled := m.store.mark() + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}, {Name: "other"}}); err != nil { + t.Fatal(err) + } + if len(eng.volumeSpecs) != before || m.store.mark() != journaled { + t.Fatal("identical target reached devices or the claims journal") + } +} + +func TestRemovedTenantRetainsAndRepairsHeldVolumes(t *testing.T) { + eng := newFakeEngine() + m, sb := newTenantVolumeManager(t, eng, liveCatalog(t)) + if _, err := m.SetVolumes(t.Context(), sb.ID, "acme", []types.Volume{{Name: "data"}, {Name: "other"}}); err != nil { + t.Fatal(err) + } + if err := m.DeleteTenant(t.Context(), "acme"); err != nil { + t.Fatal(err) + } + if _, err := m.PatchVolumes(t.Context(), sb.ID, "", types.VolumePatch{Remove: []string{"other"}}); err != nil { + t.Fatalf("remove other while retaining data: %v", err) + } + eng.detachVolumeErr = errors.New("detach result unknown") + if _, err := m.SetVolumes(t.Context(), sb.ID, "", []types.Volume{}); err == nil { + t.Fatal("expected detaching state") + } + eng.detachVolumeErr = nil + entry := m.volumes["data"] + entry.tenants = []string{"other-tenant"} + m.volumes["data"] = entry + if _, err := m.SetVolumes(t.Context(), sb.ID, "", []types.Volume{{Name: "data"}}); err != nil { + t.Fatalf("held repair checked new authorization: %v", err) + } + if _, err := m.PatchVolumes(t.Context(), sb.ID, "", types.VolumePatch{Add: []types.Volume{{Name: "other"}}}); !errors.Is(err, ErrTenantRemoved) { + t.Fatalf("new admission after tenant removal: %v", err) + } +} + +func TestReaddedDetachIsPendingAtTheDurableIntentBoundary(t *testing.T) { + eng := newFakeEngine() + catalog := liveCatalog(t) + m := newVolumeManager(t, eng, catalog) + sb := mustClaim(t, m, testKey) + target := []types.Volume{{Name: "data"}} + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, target); err != nil { + t.Fatal(err) + } + eng.detachVolumeErr = errors.New("guest unmounted; eject outcome unknown") + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{}); err == nil { + t.Fatal("expected detach failure") + } + ctx, run, err := m.beginVolumeRun(t.Context(), sb) + if err != nil { + t.Fatal(err) + } + target = slices.Clone(sb.Volumes) + if checkErr := m.prepareVolumeTarget(ctx, sb, run, target, nil, nil); checkErr != nil { + t.Fatal(checkErr) + } + m.finishVolumeRun(sb, run) + m2 := newVolumeManagerAt(t, eng, m.dataDir, catalog) + if checkErr := m2.Reconcile(t.Context()); checkErr != nil { + t.Fatal(checkErr) + } + view, err := m2.GetVolumes(sb.ID, sb.Tenant) + if err != nil || len(view.Volumes) != 0 || len(view.VolumesWant) != 1 || view.States[0].State != types.VolumePending || view.Converging { + t.Fatalf("reloaded reversal: %+v %v", view, err) + } + if !strings.Contains(view.States[0].Error, "interrupted") { + t.Fatalf("reloaded diagnostics: %+v", view) + } + eng.detachVolumeErr = nil + before := len(eng.volumeSpecs) + if _, err := m2.SetVolumes(t.Context(), sb.ID, sb.Tenant, target); err != nil { + t.Fatal(err) + } + if len(eng.volumeSpecs) != before+1 { + t.Fatal("readded volume was not repaired") + } +} + +func TestReleaseBeforeReservationDoesNotLeakAdmission(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + ctx, run, err := m.beginVolumeRun(t.Context(), sb) + if err != nil { + t.Fatal(err) + } + if err := m.checkVolumeRun(ctx, sb, run); err != nil { + t.Fatal(err) + } + if err := m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Fatal(err) + } + target := []types.Volume{{Name: "data", Mount: "/volumes/data"}} + if err := m.prepareVolumeTarget(ctx, sb, run, target, target, nil); err == nil { + t.Fatal("released claim admitted volume") + } + m.finishVolumeRun(sb, run) + if _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: []types.Volume{{Name: "data", Mode: "rw"}}}); err != nil { + t.Fatalf("canceled request leaked hold: %v", err) + } +} + +func TestVolumeDiagnosticsTrackActualWorkerAndRetry(t *testing.T) { + base := newFakeEngine() + m := newVolumeManager(t, base, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + eng := newGatedVolumeEngine(base) + m.eng = eng + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan error, 1) + go func() { _, err := m.SetVolumes(ctx, sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); done <- err }() + <-eng.entered + cancel() + view, err := m.GetVolumes(sb.ID, sb.Tenant) + if err != nil || !view.Converging { + t.Fatalf("canceled worker still owns hardware: %+v %v", view, err) + } + close(eng.resume) + if checkErr := <-done; !errors.Is(checkErr, context.Canceled) { + t.Fatal(checkErr) + } + view, err = m.GetVolumes(sb.ID, sb.Tenant) + if err != nil || view.Converging || view.States[0].Error == "" { + t.Fatalf("completed worker diagnostics: %+v %v", view, err) + } + view, err = m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + if err != nil || view.States[0].Error != "" || view.Converging { + t.Fatalf("successful retry retained errors: %+v %v", view, err) + } + conflict := unmountConflict() + base.detachVolumeErr = conflict + view, err = m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{}) + if !errors.Is(err, conflict) || view.States[0].Error != conflict.Error() { + t.Fatalf("conflict diagnostics: %+v %v", view, err) + } + got, err := m.GetVolumes(sb.ID, sb.Tenant) + if err != nil || got.States[0].Error != view.States[0].Error { + t.Fatalf("GET lost last conflict: %+v %v", got, err) + } +} + +func TestReleaseDropsVolumeDiagnostics(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err != nil { + t.Fatal(err) + } + eng.detachVolumeErr = unmountConflict() + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{}); err == nil { + t.Fatal("expected mount conflict") + } + eng.detachVolumeErr = nil + if err := m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Fatal(err) + } + m.mu.Lock() + defer m.mu.Unlock() + if len(m.volumeErrors) != 0 { + t.Fatalf("released claim kept diagnostics: %v", m.volumeErrors) + } +} + +func TestLiveVolumeRunStampsActivityBeforeDeviceCalls(t *testing.T) { + base := newFakeEngine() + m := newVolumeManager(t, base, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + sb.TouchAt(time.Now().Add(-time.Hour)) + eng := newGatedVolumeEngine(base) + m.eng = eng + done := make(chan error, 1) + go func() { + _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + done <- err + }() + <-eng.entered + idle := time.Since(sb.LastSeen()) + close(eng.resume) + if err := <-done; err != nil { + t.Fatal(err) + } + if idle > time.Minute { + t.Fatalf("a running volume change left the sandbox idle for %v", idle) + } +} + +func TestRejectedVolumeRequestLeavesActivityUntouched(t *testing.T) { + for _, tt := range []struct { + name string + request []types.Volume + paused bool + want error + }{ + {"invalid target", []types.Volume{{Name: "data"}, {Name: "data"}}, false, ErrBadVolume}, + {"hibernated claim", []types.Volume{{Name: "data"}}, true, ErrPaused}, + } { + t.Run(tt.name, func(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + if tt.paused { + m.mu.Lock() + sb.HibernateSnap = "snap" + m.mu.Unlock() + } + idleSince := time.Now().Add(-time.Hour) + sb.TouchAt(idleSince) + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, tt.request); !errors.Is(err, tt.want) { + t.Fatalf("got %v, want %v", err, tt.want) + } + if !sb.LastSeen().Equal(idleSince) { + t.Fatalf("a refused request stamped activity: %v", sb.LastSeen()) + } + }) + } +} + +func TestRejectedVolumeTargetPreservesConvergenceDiagnostics(t *testing.T) { + for _, priorConflict := range []bool{false, true} { + name := "settled" + if priorConflict { + name = "conflict" + } + t.Run(name, func(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err != nil { + t.Fatal(err) + } + if priorConflict { + eng.detachVolumeErr = unmountConflict() + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{}); err == nil { + t.Fatal("expected mount conflict") + } + } + before, err := m.GetVolumes(sb.ID, sb.Tenant) + if err != nil { + t.Fatal(err) + } + if _, checkErr := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}, {Name: "data"}}); !errors.Is(checkErr, ErrBadVolume) { + t.Fatalf("invalid target: %v", checkErr) + } + after, err := m.GetVolumes(sb.ID, sb.Tenant) + if err != nil || !slices.Equal(after.States, before.States) || !slices.Equal(after.Volumes, before.Volumes) || !slices.Equal(after.VolumesWant, before.VolumesWant) { + t.Fatalf("rejected request changed convergence view: before=%+v after=%+v err=%v", before, after, err) + } + }) + } +} + +func TestUnsupportedRuntimeDoesNotChangeVolumeIntent(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + m.eng = &unsupportedVolumeEngine{eng} + before, err := os.ReadFile(m.store.path) + if err != nil { + t.Fatal(err) + } + for _, patch := range []bool{false, true} { + if patch { + _, err = m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Add: []types.Volume{{Name: "data"}}}) + } else { + _, err = m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + } + if _, ok := errors.AsType[*engine.LiveVolumeUnsupported](err); !ok { + t.Fatalf("runtime gate: %v", err) + } + } + after, err := os.ReadFile(m.store.path) + if err != nil { + t.Fatal(err) + } + if string(before) != string(after) || len(eng.volumeSpecs) != 0 || volumeHoldersOf(m, "data") != (volumeHolders{}) { + t.Fatal("unsupported runtime changed target, hold, or device") + } +} + +func TestVolumeHoldsRejectCaptureBeforeTrim(t *testing.T) { + for _, state := range []string{"applied", "desired", "admitted"} { + t.Run(state, func(t *testing.T) { + eng := newFakeEngine() + m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1, CaptureTrim: true}) + sb := mustClaim(t, m, testKey) + held := []types.Volume{{Name: "data", Mount: "/volumes/data"}} + switch state { + case "applied": + sb.Volumes = held + case "desired": + sb.VolumesWant = held + case "admitted": + _, run, err := m.beginVolumeRun(t.Context(), sb) + if err != nil { + t.Fatal(err) + } + m.mu.Lock() + err = m.reserveVolumes(held) + run.holds = held + m.mu.Unlock() + if err != nil { + t.Fatal(err) + } + defer m.finishVolumeRun(sb, run) + } + if _, err := m.Checkpoint(t.Context(), sb.ID, Cred{Token: sb.Token}, "blocked", ""); !errors.Is(err, ErrVolumeCapture) { + t.Fatalf("checkpoint: %v", err) + } + if _, _, err := m.Promote(t.Context(), sb.ID, Cred{Token: sb.Token}, "blocked", ""); !errors.Is(err, ErrVolumeCapture) { + t.Fatalf("promote: %v", err) + } + if err := m.Hibernate(t.Context(), sb.ID, Cred{Token: sb.Token}); !errors.Is(err, ErrVolumeCapture) { + t.Fatalf("hibernate: %v", err) + } + if len(eng.trims) != 0 { + t.Fatalf("capture trimmed held volume: %v", eng.trims) + } + }) + } +} + +func TestUncertainTargetCommitReleasesOnlyConfirmedPrecleanedHolds(t *testing.T) { + for _, durable := range []string{"old", "new"} { + t.Run(durable, func(t *testing.T) { + eng := newFakeEngine() + catalog := liveCatalog(t) + m := newVolumeManager(t, eng, catalog) + sb := mustClaim(t, m, testKey) + eng.mountVolumeErr = errors.New("mount result unknown") + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err == nil { + t.Fatal("expected pending volume") + } + eng.mountVolumeErr = nil + m.store.sync = true + eng.liveVolumeHook = func(verb string) error { + if verb == "detach" { + if err := os.Mkdir(m.store.path+".tmp", 0o700); err != nil { + t.Fatal(err) + } + } + return nil + } + target := []types.Volume{{Name: "other", Mount: "/volumes/data"}} + view, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, target) + if err == nil || len(view.VolumesWant) != 1 || view.VolumesWant[0].Name != "other" || len(view.Volumes) != 0 { + t.Fatalf("uncertain target evidence: %+v %v", view, err) + } + if got := volumeHoldersOf(m, "other"); got.readers != 1 { + t.Fatalf("uncertain target lost new hold: %+v", got) + } + if got := volumeHoldersOf(m, "data"); got != (volumeHolders{}) { + t.Fatalf("confirmed preclean retained hold: %+v", got) + } + if !slices.Contains(eng.volumeOpsLog(), "detach:data") || len(eng.volumeSpecs) != 1 { + t.Fatalf("device calls before uncertain target: attaches=%v detaches=%v", eng.volumeSpecs, eng.volumeOpsLog()) + } + if durable == "old" { + m.store.writeMu.Lock() + t.Cleanup(func() { + m.store.writeMu.Unlock() + waitFor(t, func() bool { return !m.recommitting.Load() }) + }) + } + if checkErr := os.Remove(m.store.path + ".tmp"); checkErr != nil { + t.Fatal(checkErr) + } + eng.liveVolumeHook = nil + if durable == "new" { + if checkErr := m.store.commit(m.store.mark()); checkErr != nil { + t.Fatal(checkErr) + } + waitFor(t, func() bool { return !m.recommitting.Load() }) + } + m2 := newVolumeManagerAt(t, eng, m.dataDir, catalog) + if checkErr := m2.Reconcile(t.Context()); checkErr != nil { + t.Fatal(checkErr) + } + view, err = m2.SetVolumes(t.Context(), sb.ID, sb.Tenant, target) + if err != nil || len(view.VolumesWant) != 1 || view.VolumesWant[0].Name != "other" { + t.Fatalf("retry did not accept target: %+v %v", view, err) + } + if got := volumeHoldersOf(m2, "data"); got != (volumeHolders{}) { + t.Fatalf("successful retry retained discarded hold: %+v", got) + } + if _, err := m2.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: []types.Volume{{Name: "data", Mode: "rw"}}}); err != nil { + t.Fatalf("writer after preclean and recovery: %v", err) + } + }) + } +} + +type unsupportedVolumeEngine struct{ *fakeEngine } + +func (e *unsupportedVolumeEngine) CheckLiveVolumeSupport(context.Context) error { + return &engine.LiveVolumeUnsupported{} +} + +func unmountConflict() *engine.VolumeMountConflict { + return &engine.VolumeMountConflict{Message: `cannot unmount "/volumes/data"; close guest references and retry`} +} diff --git a/sandboxd/pool/volume_mutation.go b/sandboxd/pool/volume_mutation.go new file mode 100644 index 00000000..79104100 --- /dev/null +++ b/sandboxd/pool/volume_mutation.go @@ -0,0 +1,484 @@ +package pool + +import ( + "context" + "errors" + "fmt" + "maps" + "slices" + + "github.com/cocoonstack/sandbox/sandboxd/engine" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +const ( + volumeChangeFailed = "volume change failed; retry the desired target" + volumeChangeInterrupted = "volume change interrupted; retry the desired target" +) + +type volumeEdit func([]types.Volume) ([]types.Volume, error) + +type volumeRun struct { + cancel context.CancelFunc + done chan struct{} + holds []types.Volume +} + +// GetVolumes reads a claim's desired and applied volumes; a tenant reaches only its own claims. +func (m *Manager) GetVolumes(id, tenant string) (types.SandboxVolumesResponse, error) { + m.mu.Lock() + defer m.mu.Unlock() + sb := m.claimed[id] + if sb == nil || !tenantOwns(tenant, sb.Tenant) { + return types.SandboxVolumesResponse{}, ErrUnknownSandbox + } + return m.volumeView(sb), nil +} + +// SetVolumes converges a running claim onto the full desired volume set. +func (m *Manager) SetVolumes(ctx context.Context, id, tenant string, want []types.Volume) (types.SandboxVolumesResponse, error) { + return m.changeVolumes(ctx, id, tenant, func([]types.Volume) ([]types.Volume, error) { return slices.Clone(want), nil }) +} + +// PatchVolumes adds and removes a running claim's read-only mounted volumes by name. +func (m *Manager) PatchVolumes(ctx context.Context, id, tenant string, patch types.VolumePatch) (types.SandboxVolumesResponse, error) { + return m.changeVolumes(ctx, id, tenant, func(current []types.Volume) ([]types.Volume, error) { + if err := validateDetachNames(patch.Remove); err != nil { + return nil, err + } + additions, err := normalizeLiveVolumes(patch.Add, current) + if err != nil { + return nil, err + } + for _, v := range additions { + if slices.Contains(patch.Remove, v.Name) { + return nil, fmt.Errorf("%w: volume %q is both added and removed", ErrBadVolume, v.Name) + } + if i := volumeIndex(current, v.Name); i >= 0 { + if current[i] != v { + return nil, fmt.Errorf("%w: volume %q already has a different mount or mode", ErrBadVolume, v.Name) + } + } else { + current = append(current, v) + } + } + return slices.DeleteFunc(current, func(v types.Volume) bool { return slices.Contains(patch.Remove, v.Name) }), nil + }) +} + +func (m *Manager) changeVolumes(ctx context.Context, id, tenant string, change volumeEdit) (view types.SandboxVolumesResponse, err error) { + sb, ok := m.byID(id) + if !ok || !tenantOwns(tenant, sb.Tenant) { + return view, ErrUnknownSandbox + } + var run *volumeRun + if err = m.eng.CheckLiveVolumeSupport(ctx); err == nil { + ctx, run, err = m.beginVolumeRun(ctx, sb) + } + if err != nil { + view, _ = m.GetVolumes(id, tenant) + return view, err + } + failedName := "" + defer func() { + m.recordVolumeError(sb, run, failedName, err) + view = m.finishVolumeRun(sb, run) + }() + m.mu.Lock() + current, held := slices.Clone(sb.VolumesWant), heldVolumes(sb) + m.mu.Unlock() + target, err := change(current) + if err != nil { + return view, err + } + target, err = normalizeLiveVolumes(target, held) + if err != nil { + return view, err + } + if err = validateLiveTarget(held, target); err != nil { + return view, err + } + additions := volumeDifference(target, held) + if len(additions) != 0 { + if err = m.tenantRemoved(ctx, sb.Tenant); err != nil { + return view, err + } + } + resolved, err := m.resolveVolumes(ctx, sb.Tenant, additions) + if err != nil { + return view, err + } + if err = m.checkVolumeRun(ctx, sb, run); err != nil { + return view, err + } + sb.Touch() + m.mu.Lock() + uncertain := volumeDifference(volumeDifference(sb.VolumesWant, sb.Volumes), target) + m.mu.Unlock() + for _, v := range uncertain { + failedName = v.Name + if err = m.detachVolumeRun(ctx, sb, run, v); err != nil { + return view, err + } + } + failedName = "" + if err = m.prepareVolumeTarget(ctx, sb, run, target, additions, resolved); err != nil { + return view, err + } + m.mu.Lock() + removals := volumeDifference(sb.Volumes, target) + m.mu.Unlock() + for _, v := range removals { + failedName = v.Name + if err = m.detachVolumeRun(ctx, sb, run, v); err != nil { + return view, err + } + if err = m.commitVolumeApplied(ctx, sb, run, v, false); err != nil { + return view, err + } + m.clearVolumeError(sb, run, v.Name) + } + m.mu.Lock() + pending := volumeDifference(sb.VolumesWant, sb.Volumes) + m.mu.Unlock() + for _, v := range pending { + failedName = v.Name + if err = m.checkVolumeRun(ctx, sb, run); err != nil { + return view, err + } + repair, resolveErr := m.resolveCatalogVolume(ctx, v) + if resolveErr != nil { + return view, resolveErr + } + applyCtx, cancel := context.WithTimeout(ctx, engine.VolumeMutationTimeout) + err = m.applyVolume(applyCtx, sb, repair, true) + cancel() + if err != nil { + return view, err + } + if err = m.checkVolumeRun(ctx, sb, run); err != nil { + return view, err + } + if err = m.commitVolumeApplied(ctx, sb, run, v, true); err != nil { + return view, err + } + m.clearVolumeError(sb, run, v.Name) + } + return view, nil +} + +func (m *Manager) beginVolumeRun(ctx context.Context, sb *types.Sandbox) (context.Context, *volumeRun, error) { + for { + if err := ctx.Err(); err != nil { + return ctx, nil, err + } + m.mu.Lock() + if m.claimed[sb.ID] != sb { + m.mu.Unlock() + return ctx, nil, ErrUnknownSandbox + } + if active := m.volumeRuns[sb]; active != nil { + done := active.done + m.mu.Unlock() + select { + case <-ctx.Done(): + return ctx, nil, ctx.Err() + case <-done: + } + continue + } + runCtx, cancel := context.WithCancel(ctx) + run := &volumeRun{cancel: cancel, done: make(chan struct{}), holds: heldVolumes(sb)} + m.volumeRuns[sb] = run + m.mu.Unlock() + return runCtx, run, nil + } +} + +func (m *Manager) finishVolumeRun(sb *types.Sandbox, run *volumeRun) types.SandboxVolumesResponse { + m.mu.Lock() + if m.claimed[sb.ID] == sb { + m.releaseVolumes(volumeDifference(run.holds, heldVolumes(sb))) + } + delete(m.volumeRuns, sb) + view := m.volumeView(sb) + close(run.done) + m.mu.Unlock() + run.cancel() + return view +} + +func (m *Manager) restoreVolumeHolds(sb *types.Sandbox, holds []types.Volume) { + if m.volumeRuns[sb] == nil { + m.releaseVolumes(volumeDifference(holds, heldVolumes(sb))) + } +} + +func (m *Manager) checkVolumeRun(ctx context.Context, sb *types.Sandbox, run *volumeRun) error { + m.mu.Lock() + defer m.mu.Unlock() + return m.checkVolumeRunLocked(ctx, sb, run) +} + +func (m *Manager) checkVolumeRunLocked(ctx context.Context, sb *types.Sandbox, run *volumeRun) error { + if err := ctx.Err(); err != nil { + return err + } + if !m.ownsRunLocked(sb, run) { + return ErrUnknownSandbox + } + switch { + case sb.Failed != "": + return ErrFailed + case sb.ArchiveCk != "": + return ErrArchived + case sb.HibernateSnap != "" || sb.PendingSnap != "" || sb.VsockSocket == "": + return ErrPaused + } + return nil +} + +func (m *Manager) cancelVolumeRun(sb *types.Sandbox) []types.Volume { + delete(m.volumeErrors, sb) + if run := m.volumeRuns[sb]; run != nil { + run.cancel() + return slices.Clone(run.holds) + } + return heldVolumes(sb) +} + +func (m *Manager) prepareVolumeTarget(ctx context.Context, sb *types.Sandbox, run *volumeRun, target, additions []types.Volume, resolved []resolvedVolume) error { + js, changed, err := m.stageVolumeTarget(ctx, sb, run, target, additions, resolved) + if err != nil || !changed { + return err + } + return m.commitVolumeRun(ctx, sb, run, js) +} + +func (m *Manager) stageVolumeTarget(ctx context.Context, sb *types.Sandbox, run *volumeRun, target, additions []types.Volume, resolved []resolvedVolume) (claimSnapshot, bool, error) { + sb.Transition.Lock() + defer sb.Transition.Unlock() + m.mu.Lock() + if err := m.checkVolumeRunLocked(ctx, sb, run); err != nil { + m.mu.Unlock() + return claimSnapshot{}, false, err + } + if sb.OnExpire == types.ExpireArchive && len(target) > 0 { + m.mu.Unlock() + return claimSnapshot{}, false, ErrVolumeCapture + } + if err := m.reserveVolumes(additions); err != nil { + m.mu.Unlock() + return claimSnapshot{}, false, err + } + run.holds = append(run.holds, additions...) + m.mu.Unlock() + cleanErr := m.confirmVolumesClean(resolved) + m.mu.Lock() + defer m.mu.Unlock() + if err := m.checkVolumeRunLocked(ctx, sb, run); err != nil { + return claimSnapshot{}, false, err + } + if cleanErr != nil { + m.releaseVolumes(additions) + run.holds = volumeDifference(run.holds, additions) + return claimSnapshot{}, false, cleanErr + } + replaced := func(v types.Volume) bool { + return volumeIndex(sb.VolumesWant, v.Name) < 0 && volumeIndex(target, v.Name) >= 0 + } + if len(additions) == 0 && slices.Equal(target, sb.VolumesWant) && !slices.ContainsFunc(sb.Volumes, replaced) && m.store.synced() { + return claimSnapshot{}, false, nil + } + sb.Volumes = slices.DeleteFunc(slices.Clone(sb.Volumes), replaced) + sb.VolumesWant = slices.Clone(target) + return m.store.set(sb), true, nil +} + +func (m *Manager) detachVolumeRun(ctx context.Context, sb *types.Sandbox, run *volumeRun, v types.Volume) error { + if err := m.checkVolumeRun(ctx, sb, run); err != nil { + return err + } + if err := m.eng.DetachVolume(ctx, sb.VMName, sb.VsockSocket, v.Name, v.Mount); err != nil { + return fmt.Errorf("detach volume %q: %w", v.Name, err) + } + return m.checkVolumeRun(ctx, sb, run) +} + +func (m *Manager) commitVolumeApplied(ctx context.Context, sb *types.Sandbox, run *volumeRun, v types.Volume, attached bool) error { + m.mu.Lock() + if err := m.checkVolumeRunLocked(ctx, sb, run); err != nil { + m.mu.Unlock() + return err + } + sb.Volumes = slices.DeleteFunc(slices.Clone(sb.Volumes), func(a types.Volume) bool { return a.Name == v.Name }) + if attached { + sb.Volumes = append(sb.Volumes, v) + } + js := m.store.set(sb) + m.mu.Unlock() + return m.commitVolumeRun(ctx, sb, run, js) +} + +func (m *Manager) commitVolumeRun(ctx context.Context, sb *types.Sandbox, run *volumeRun, js claimSnapshot) error { + if err := m.store.commit(js); err != nil { + m.recommit(ctx, js) + return fmt.Errorf("persist volumes: %w", err) + } + m.mu.Lock() + defer m.mu.Unlock() + if !m.ownsRunLocked(sb, run) { + return ErrUnknownSandbox + } + if err := ctx.Err(); err != nil { + return err + } + dropped := volumeDifference(run.holds, heldVolumes(sb)) + m.releaseVolumes(dropped) + run.holds = volumeDifference(run.holds, dropped) + return nil +} + +func (m *Manager) ownsRunLocked(sb *types.Sandbox, run *volumeRun) bool { + return m.claimed[sb.ID] == sb && m.volumeRuns[sb] == run +} + +func (m *Manager) hasVolumeHolds(sb *types.Sandbox) bool { + run := m.volumeRuns[sb] + return hasVolumes(sb) || run != nil && len(run.holds) > 0 +} + +func (m *Manager) volumeView(sb *types.Sandbox) types.SandboxVolumesResponse { + view := types.SandboxVolumesResponse{ + Volumes: slices.Clone(sb.Volumes), + VolumesWant: slices.Clone(sb.VolumesWant), + States: []types.VolumeState{}, + Converging: m.volumeRuns[sb] != nil, + } + for _, v := range heldVolumes(sb) { + state := types.VolumeAttached + if volumeIndex(sb.VolumesWant, v.Name) < 0 { + state = types.VolumeDetaching + } else if volumeIndex(sb.Volumes, v.Name) < 0 { + state = types.VolumePending + } + message := m.volumeErrors[sb][v.Name] + if message == "" && state != types.VolumeAttached && !view.Converging { + message = volumeChangeInterrupted + } + view.States = append(view.States, types.VolumeState{Name: v.Name, State: state, Error: message}) + } + return view +} + +func (m *Manager) recordVolumeError(sb *types.Sandbox, run *volumeRun, name string, err error) { + m.mu.Lock() + defer m.mu.Unlock() + if !m.ownsRunLocked(sb, run) { + return + } + if err == nil { + delete(m.volumeErrors, sb) + return + } + errs := m.volumeErrors[sb] + if name != "" { + message := volumeChangeFailed + if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) { + message = volumeChangeInterrupted + } + if conflict, ok := errors.AsType[*engine.VolumeMountConflict](err); ok { + message = conflict.Error() + } + if errs == nil { + errs = map[string]string{} + } + errs[name] = message + } + held := heldVolumes(sb) + maps.DeleteFunc(errs, func(name, _ string) bool { return volumeIndex(held, name) < 0 }) + if len(errs) == 0 { + delete(m.volumeErrors, sb) + return + } + m.volumeErrors[sb] = errs +} + +func (m *Manager) clearVolumeError(sb *types.Sandbox, run *volumeRun, name string) { + m.mu.Lock() + defer m.mu.Unlock() + if m.ownsRunLocked(sb, run) { + delete(m.volumeErrors[sb], name) + } +} + +func normalizeLiveVolumes(volumes, held []types.Volume) ([]types.Volume, error) { + if len(volumes) > types.MaxClaimVolumes { + return nil, fmt.Errorf("%w: too many volumes", ErrBadVolume) + } + out := make([]types.Volume, 0, len(volumes)) + for _, v := range volumes { + attachOnly := v.AttachOnly + if i := volumeIndex(held, v.Name); i >= 0 && held[i].Mount == "" && v.Mount == "" { + attachOnly = true + } + normalized, err := types.ValidateVolumes([]types.Volume{v}, attachOnly) + if err != nil { + return nil, fmt.Errorf("%w: %w", ErrBadVolume, err) + } + if volumeIndex(out, v.Name) >= 0 { + return nil, fmt.Errorf("%w: duplicate volume %q", ErrBadVolume, v.Name) + } + if attachOnly { + normalized[0].AttachOnly = false + } + out = append(out, normalized[0]) + } + mounted := slices.DeleteFunc(slices.Clone(out), func(v types.Volume) bool { return v.Mount == "" }) + if _, err := types.ValidateVolumes(mounted, false); err != nil { + return nil, fmt.Errorf("%w: %w", ErrBadVolume, err) + } + return out, nil +} + +func validateLiveTarget(held, target []types.Volume) error { + for _, v := range held { + i := volumeIndex(target, v.Name) + if i >= 0 && target[i] != v { + return fmt.Errorf("%w: remove volume %q before changing its mount or mode", ErrBadVolume, v.Name) + } + if immutableVolume(v) && i < 0 { + return fmt.Errorf("%w: writable and attach-only volumes cannot change live", ErrBadVolume) + } + } + if slices.ContainsFunc(volumeDifference(target, held), immutableVolume) { + return fmt.Errorf("%w: live additions require read-only mounts", ErrBadVolume) + } + return nil +} + +func immutableVolume(v types.Volume) bool { return v.RW() || v.AttachOnly || v.Mount == "" } + +func volumeIndex(volumes []types.Volume, name string) int { + return slices.IndexFunc(volumes, func(v types.Volume) bool { return v.Name == name }) +} + +func volumeDifference(left, right []types.Volume) []types.Volume { + return slices.DeleteFunc(slices.Clone(left), func(v types.Volume) bool { return volumeIndex(right, v.Name) >= 0 }) +} + +func validateDetachNames(names []string) error { + if len(names) > types.MaxClaimVolumes { + return fmt.Errorf("%w: too many volume names", ErrBadVolume) + } + for i, name := range names { + if !types.ValidVolumeName(name) || slices.Contains(names[:i], name) { + return fmt.Errorf("%w: invalid or duplicate volume name %q", ErrBadVolume, name) + } + } + return nil +} + +func heldVolumes(sb *types.Sandbox) []types.Volume { + return slices.Concat(sb.Volumes, volumeDifference(sb.VolumesWant, sb.Volumes)) +} diff --git a/sandboxd/pool/volume_mutation_test.go b/sandboxd/pool/volume_mutation_test.go new file mode 100644 index 00000000..caddb5a0 --- /dev/null +++ b/sandboxd/pool/volume_mutation_test.go @@ -0,0 +1,880 @@ +package pool + +import ( + "context" + "errors" + "fmt" + "os" + "slices" + "strings" + "sync/atomic" + "testing" + "testing/synctest" + "time" + + "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/engine" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +func TestLiveVolumesCommitAndRetry(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + request := []types.Volume{{Name: "data"}, {Name: "other", Mount: "/models"}} + for range 2 { + got, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request) + if err != nil || len(got.Volumes) != 2 || !slices.Equal(sb.VolumesWant, sb.Volumes) { + t.Fatalf("attach = %v, %v; pending=%v", got, err, sb.VolumesWant) + } + got.Volumes[0].Name = "caller copy" + } + if got := volumeHoldersOf(m, "data"); got.readers != 1 { + t.Fatalf("duplicate reservation: %+v", got) + } + assertLiveJournal(t, m, sb) + for range 2 { + got, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data", "other"}}) + if err != nil || len(got.Volumes) != 0 { + t.Fatalf("detach = %v, %v", got, err) + } + } + assertLiveJournal(t, m, sb) + if got := volumeHoldersOf(m, "data"); got != (volumeHolders{}) { + t.Fatalf("leaked holds: %+v", got) + } + if _, err := m.Checkpoint(t.Context(), sb.ID, Cred{Token: sb.Token}, "after-detach", ""); err != nil { + t.Fatalf("capture after detach: %v", err) + } +} + +func TestLiveAttachPreservesWritableAndAttachOnlyVolumes(t *testing.T) { + for _, attachOnly := range []bool{false, true} { + t.Run(fmt.Sprintf("attach_only=%t", attachOnly), func(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: []types.Volume{{Name: "other", Mode: "rw", AttachOnly: attachOnly}}}) + if err != nil { + t.Fatal(err) + } + original := sb.Volumes[0] + if _, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Add: []types.Volume{{Name: "data"}}}); err != nil { + t.Fatal(err) + } + if sb.Volumes[0] != original { + t.Fatalf("changed existing volume: %v -> %v", original, sb.Volumes[0]) + } + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "other", Mount: original.Mount}}); !errors.Is(err, ErrBadVolume) { + t.Fatalf("accepted mode change: %v", err) + } + if _, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data", "other"}}); !errors.Is(err, ErrBadVolume) || len(sb.Volumes) != 2 { + t.Fatalf("writable detach changed the batch: %v", err) + } + }) + } +} + +func TestLiveVolumeValidationPrecedesDeviceCalls(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + for _, request := range [][]types.Volume{ + {{Name: "data", Mode: "rw"}}, + {{Name: "data", AttachOnly: true}}, + {{Name: "data", Mount: "/etc"}}, + {{Name: "data"}, {Name: "data"}}, + {{Name: "data", Mount: "/a"}, {Name: "other", Mount: "/a/b"}}, + {{Name: "missing"}}, + } { + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request); !errors.Is(err, ErrBadVolume) { + t.Fatalf("request %v: %v", request, err) + } + } + if _, err := m.SetVolumes(t.Context(), sb.ID, "wrong", []types.Volume{{Name: "data"}}); !errors.Is(err, ErrUnknownSandbox) { + t.Fatalf("wrong tenant: %v", err) + } + if _, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data", "data"}}); !errors.Is(err, ErrBadVolume) { + t.Fatalf("duplicate detach: %v", err) + } + if len(eng.volumeSpecs) != 0 || !slices.Equal(sb.VolumesWant, sb.Volumes) { + t.Fatal("invalid request reached engine") + } +} + +func TestLiveAttachFailureSurvivesRestartAndCanBeDetached(t *testing.T) { + eng := newFakeEngine() + catalog := liveCatalog(t) + m := newVolumeManager(t, eng, catalog) + sb := mustClaim(t, m, testKey) + eng.mountVolumeErr = errors.New("guest unavailable after host attach") + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err == nil { + t.Fatal("want mount failure") + } + assertLiveJournal(t, m, sb) + if slices.Equal(sb.VolumesWant, sb.Volumes) || volumeHoldersOf(m, "data").readers != 1 { + t.Fatal("lost pending device or hold") + } + m2 := newVolumeManagerAt(t, eng, m.dataDir, catalog) + if reconcileErr := m2.Reconcile(t.Context()); reconcileErr != nil { + t.Fatal(reconcileErr) + } + if volumeHoldersOf(m2, "data").readers != 1 { + t.Fatal("restart lost pending hold") + } + if _, err := m2.Checkpoint(t.Context(), sb.ID, Cred{Token: sb.Token}, "pending", ""); !errors.Is(err, ErrVolumeCapture) { + t.Fatalf("captured pending device: %v", err) + } + eng.detachVolumeErr = errors.New("host detach failed") + if _, err := m2.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data"}}); err == nil || volumeHoldersOf(m2, "data").readers != 1 { + t.Fatal("failed detach released its hold") + } + eng.detachVolumeErr = nil + if _, err := m2.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data"}}); err != nil { + t.Fatal(err) + } + if volumeHoldersOf(m2, "data") != (volumeHolders{}) { + t.Fatal("successful cleanup leaked hold") + } +} + +func TestLiveVolumePersistenceFailureRetainsRecoverableState(t *testing.T) { + for _, stage := range []string{"intent", "attach result", "detach result"} { + t.Run(stage, func(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + request := []types.Volume{{Name: "data"}} + if stage == "detach result" { + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request); err != nil { + t.Fatal(err) + } + } + var restore func() + if stage == "intent" { + restore = breakClaimStore(t, m) + } else { + eng.liveVolumeHook = func(string) error { restore = breakClaimStore(t, m); return nil } + } + var err error + if stage == "detach result" { + _, err = m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data"}}) + } else { + _, err = m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request) + } + if err == nil { + t.Fatal("expected persistence failure") + } + eng.liveVolumeHook = nil + restore() + assertLiveJournal(t, m, sb) + wantApplied := map[string]int{"intent": 0, "attach result": 1, "detach result": 0}[stage] + wantHolders := map[string]int{"intent": 1, "attach result": 1, "detach result": 0}[stage] + if len(sb.Volumes) != wantApplied || volumeHoldersOf(m, "data").readers != wantHolders { + t.Fatalf("failed write lost the physical result or its hold: volumes=%v holders=%+v", sb.Volumes, volumeHoldersOf(m, "data")) + } + if stage == "intent" && len(eng.volumeSpecs) != 0 { + t.Fatal("failed intent reached the engine") + } + var view types.SandboxVolumesResponse + if stage == "detach result" { + view, err = m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data"}}) + } else { + view, err = m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request) + } + if err != nil { + t.Fatal(err) + } + if slices.ContainsFunc(view.States, func(s types.VolumeState) bool { return s.Error != "" }) { + t.Fatalf("converged view kept a stale error: %+v", view.States) + } + assertLiveJournal(t, m, sb) + }) + } +} + +func TestLiveBatchPartialFailureRetriesWithoutDoubleHolds(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + request := []types.Volume{{Name: "data"}, {Name: "other"}} + eng.diskAttachErrFor = "other" + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request); err == nil { + t.Fatal("want partial failure") + } + if len(sb.Volumes) != 1 || slices.Equal(sb.VolumesWant, sb.Volumes) || len(sb.VolumesWant) != 2 { + t.Fatalf("partial state: %v %v", sb.Volumes, sb.VolumesWant) + } + eng.diskAttachErrFor = "" + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request); err != nil { + t.Fatal(err) + } + for _, name := range []string{"data", "other"} { + if volumeHoldersOf(m, name).readers != 1 { + t.Fatal("retry double-reserved " + name) + } + } +} + +func TestLiveVolumeDoesNotBlockLifecycle(t *testing.T) { + for _, action := range []string{"release", "checkpoint", "fork", "promote", "hibernate", "reap", "renew", "get", "relay"} { + t.Run(action, func(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + entered, resume := make(chan struct{}), make(chan struct{}) + eng.liveVolumeHook = func(string) error { close(entered); <-resume; return nil } + attached := make(chan error, 1) + go func() { + _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + attached <- err + }() + <-entered + assertLiveJournal(t, m, sb) + done := make(chan error, 1) + go func() { + var err error + switch action { + case "release": + err = m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) + case "checkpoint": + _, err = m.Checkpoint(t.Context(), sb.ID, Cred{Token: sb.Token}, "race", "") + case "fork": + _, err = m.Fork(t.Context(), sb.ID, Cred{Token: sb.Token}, 1, 0, "", "") + case "promote": + _, _, err = m.Promote(t.Context(), sb.ID, Cred{Token: sb.Token}, "race", "") + case "hibernate": + err = m.Hibernate(t.Context(), sb.ID, Cred{Token: sb.Token}) + case "reap": + m.mu.Lock() + sb.Deadline = time.Now().Add(-time.Second) + m.mu.Unlock() + m.reapOnce(t.Context()) + case "renew": + _, err = m.Renew(t.Context(), sb.ID, Cred{Token: sb.Token}, time.Minute, "") + case "get": + _, err = m.GetVolumes(sb.ID, sb.Tenant) + case "relay": + var unhold func() + _, unhold, err = m.WakeAgentSocket(t.Context(), sb.ID, sb.Token) + if unhold != nil { + unhold() + } + } + done <- err + }() + var err error + select { + case err = <-done: + case <-time.After(2 * time.Second): + close(resume) + <-attached + t.Fatal("lifecycle blocked behind device call") + } + if slices.Contains([]string{"checkpoint", "fork", "promote", "hibernate"}, action) { + if !errors.Is(err, ErrVolumeCapture) { + t.Errorf("capture=%v", err) + } + } else if err != nil { + t.Errorf("lifecycle=%v", err) + } + close(resume) + attachErr := <-attached + if action == "reap" { + waitFor(t, func() bool { return volumeHoldersOf(m, "data") == (volumeHolders{}) }) + } + if action == "release" || action == "reap" { + if attachErr == nil { + t.Fatal("released worker reported success") + } + claims, err := m.store.load() + if err != nil || claims[sb.ID] != nil { + t.Fatalf("late worker resurrected claim: %v %v", claims, err) + } + if volumeHoldersOf(m, "data") != (volumeHolders{}) { + t.Fatal("teardown leaked holds") + } + } else if attachErr != nil { + t.Fatal(attachErr) + } + }) + } +} + +func TestLiveVolumeCanceledRequestRetainsIntent(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + ctx, cancel := context.WithCancel(t.Context()) + eng.liveVolumeHook = func(string) error { cancel(); return nil } + _, err := m.SetVolumes(ctx, sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + if !errors.Is(err, context.Canceled) || slices.Equal(sb.VolumesWant, sb.Volumes) { + t.Fatalf("canceled attach: %v, pending=%v", err, sb.VolumesWant) + } + eng.liveVolumeHook = nil + if err := m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil || volumeHoldersOf(m, "data") != (volumeHolders{}) { + t.Fatalf("release pending claim: %v", err) + } +} + +func TestLiveVolumeAdmissionAndState(t *testing.T) { + for _, state := range []string{"paused", "failed", "archived", "archive policy", "dirty", "writer"} { + t.Run(state, func(t *testing.T) { + eng := newFakeEngine() + catalog := liveCatalog(t) + m := newVolumeManager(t, eng, catalog) + sb := mustClaim(t, m, testKey) + var want error + switch state { + case "paused": + sb.HibernateSnap, want = "sleep", ErrPaused + case "failed": + sb.Failed, want = "down", ErrFailed + case "archived": + sb.ArchiveCk, want = "ck_old", ErrArchived + case "archive policy": + sb.OnExpire, want = types.ExpireArchive, ErrVolumeCapture + case "dirty": + if err := markVolumeDirty(catalog[0].Path); err != nil { + t.Fatal(err) + } + want = ErrVolumeNeedsRecovery + case "writer": + if _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: []types.Volume{{Name: "data", Mode: "rw"}}}); err != nil { + t.Fatal(err) + } + want = ErrVolumeBusy + } + before := len(eng.volumeSpecs) + _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + if !errors.Is(err, want) || len(eng.volumeSpecs) != before || !slices.Equal(sb.VolumesWant, sb.Volumes) { + t.Fatalf("got %v want %v", err, want) + } + }) + } +} + +func TestLiveVolumesRespectOwningTenantAndRemoval(t *testing.T) { + eng := newFakeEngine() + catalog := liveCatalog(t) + catalog[1].Tenants = []string{"beta"} + m, sb := newTenantVolumeManager(t, eng, catalog) + for _, auth := range []string{sb.Tenant, ""} { + if _, err := m.SetVolumes(t.Context(), sb.ID, auth, []types.Volume{{Name: "other"}}); !errors.Is(err, ErrVolumeUnavailable) { + t.Fatalf("catalog ACL: %v", err) + } + } + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err != nil { + t.Fatal(err) + } + if err := m.DeleteTenant(t.Context(), "acme"); err != nil { + t.Fatal(err) + } + if _, err := m.SetVolumes(t.Context(), sb.ID, "", []types.Volume{{Name: "data"}}); err != nil { + t.Fatalf("held volume after tenant removal: %v", err) + } + if _, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data"}}); err != nil { + t.Fatalf("cleanup after tenant removal: %v", err) + } +} + +func TestDesiredVolumesJournalMigration(t *testing.T) { + for _, tt := range []struct { + name, field string + want []types.Volume + }{ + {"legacy", "", []types.Volume{{Name: "data", Mount: "/data"}}}, + {"explicit empty", `,"volumes_want":[]`, nil}, + {"desired", `,"volumes_want":[{"name":"other","mount":"/other"}]`, []types.Volume{{Name: "other", Mount: "/other"}}}, + } { + t.Run(tt.name, func(t *testing.T) { + store := newClaimStore(t.TempDir(), false) + raw := `{"sb_1":{"id":"sb_1","volumes":[{"name":"data","mount":"/data"}]` + tt.field + `}}` + if err := os.WriteFile(store.path, []byte(raw), 0o600); err != nil { + t.Fatal(err) + } + claims, err := store.load() + if err != nil { + t.Fatal(err) + } + if got := claims["sb_1"].VolumesWant; !slices.Equal(got, tt.want) { + t.Fatalf("loaded target %v, want %v", got, tt.want) + } + if err = store.save(claims); err != nil { + t.Fatal(err) + } + rawBytes, err := os.ReadFile(store.path) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(rawBytes), `"volumes_want":`) != (tt.field != "") { + t.Fatalf("persisted target %s, want field only when it differs", rawBytes) + } + }) + } +} + +func TestFailedDetachReaddRepairsAppliedMountAfterRestart(t *testing.T) { + eng := newFakeEngine() + catalog := liveCatalog(t) + m := newVolumeManager(t, eng, catalog) + sb := mustClaim(t, m, testKey) + target := []types.Volume{{Name: "data"}} + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, target); err != nil { + t.Fatal(err) + } + eng.detachVolumeErr = errors.New("eject did not complete") + view, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, nil) + if err == nil || len(view.Volumes) != 1 || len(view.VolumesWant) != 0 || view.States[0].State != "detaching" || view.States[0].Error == "" { + t.Fatalf("detach state=%+v error=%v", view, err) + } + m2 := newVolumeManagerAt(t, eng, m.dataDir, catalog) + if reconcileErr := m2.Reconcile(t.Context()); reconcileErr != nil { + t.Fatal(reconcileErr) + } + before := len(eng.volumeMounts) + eng.detachVolumeErr = nil + view, err = m2.SetVolumes(t.Context(), sb.ID, sb.Tenant, target) + if err != nil || len(eng.volumeMounts) != before+1 || view.States[0].State != "attached" { + t.Fatalf("readd did not repair: %+v %v", view, err) + } + if volumeHoldersOf(m2, "data").readers != 1 { + t.Fatal("repair changed reservation count") + } +} + +func TestSupersedingTargetPrecleansUncertainAddBeforeJournaling(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + eng.mountVolumeErr = errors.New("mount interrupted") + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err == nil { + t.Fatal("expected attach failure") + } + eng.mountVolumeErr = nil + entered, resume := make(chan struct{}), make(chan struct{}) + eng.liveVolumeHook = func(verb string) error { + if verb == "detach" { + close(entered) + <-resume + } + return nil + } + done := make(chan error, 1) + go func() { + _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "other"}}) + done <- err + }() + <-entered + view, err := m.GetVolumes(sb.ID, sb.Tenant) + if err != nil || len(view.VolumesWant) != 1 || view.VolumesWant[0].Name != "data" || volumeHoldersOf(m, "data").readers != 1 { + t.Errorf("forgot uncertain disk before detach: %+v %v", view, err) + } + assertLiveJournal(t, m, sb) + close(resume) + if err := <-done; err != nil { + t.Fatal(err) + } + if volumeHoldersOf(m, "data") != (volumeHolders{}) || volumeHoldersOf(m, "other").readers != 1 { + t.Fatal("superseding target leaked or lost holds") + } +} + +func TestCanceledVolumeWriterOwnsHardwareUntilReturn(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + base := newFakeEngine() + m := newVolumeManager(t, base, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + eng := newGatedVolumeEngine(base) + m.eng = eng + ctx, cancel := context.WithCancel(t.Context()) + first := make(chan error, 1) + go func() { _, err := m.SetVolumes(ctx, sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); first <- err }() + <-eng.entered + cancel() + waitingCtx, stopWaiting := context.WithTimeout(t.Context(), 25*time.Millisecond) + defer stopWaiting() + _, err := m.PatchVolumes(waitingCtx, sb.ID, sb.Tenant, types.VolumePatch{Add: []types.Volume{{Name: "other"}}}) + if !errors.Is(err, context.DeadlineExceeded) || eng.calls.Load() != 1 { + close(eng.resume) + t.Fatalf("new writer overtook canceled device call: %v calls=%d", err, eng.calls.Load()) + } + close(eng.resume) + if firstErr := <-first; !errors.Is(firstErr, context.Canceled) { + t.Fatalf("canceled result=%v", firstErr) + } + if len(base.volumeMounts) != 0 { + t.Fatal("canceled writer issued another device call") + } + view, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Add: []types.Volume{{Name: "other"}}}) + if err != nil || len(view.Volumes) != 2 { + t.Fatalf("retry lost prior desired state: %+v %v", view, err) + } + }) +} + +func TestReleaseOwnsUncertainHoldsUntilVMRemoval(t *testing.T) { + base := newFakeEngine() + m := newVolumeManager(t, base, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + eng := newGatedVolumeEngine(base) + m.eng = eng + base.removeErrFor = sb.VMName + changed := make(chan error, 1) + go func() { + _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + changed <- err + }() + <-eng.entered + released := make(chan error, 1) + go func() { released <- m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) }() + select { + case err := <-released: + if err == nil { + t.Fatal("expected failed VM removal") + } + case <-time.After(time.Second): + close(eng.resume) + t.Fatal("release waited for volume writer") + } + if volumeHoldersOf(m, "data").readers != 1 { + t.Fatal("unconfirmed removal dropped hold") + } + close(eng.resume) + if err := <-changed; err == nil { + t.Fatal("removed claim accepted late device result") + } + if volumeHoldersOf(m, "data").readers != 1 { + t.Fatal("late writer freed teardown-owned hold") + } + claims, err := m.store.load() + if err != nil || claims[sb.ID] != nil { + t.Fatalf("late writer resurrected claim: %v %v", claims, err) + } + base.removeErrFor = "" + m.retryRemovals(t.Context()).Wait() + if volumeHoldersOf(m, "data") != (volumeHolders{}) { + t.Fatal("confirmed VM removal leaked hold") + } +} + +func TestConcurrentVolumePatchesUseLatestDesiredSet(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + base := newFakeEngine() + m := newVolumeManager(t, base, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + eng := newGatedVolumeEngine(base) + m.eng = eng + first := make(chan error, 1) + go func() { + _, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Add: []types.Volume{{Name: "data"}}}) + first <- err + }() + <-eng.entered + second := make(chan error, 1) + go func() { + _, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Add: []types.Volume{{Name: "other"}}}) + second <- err + }() + synctest.Wait() + if len(second) != 0 || eng.calls.Load() != 1 { + close(eng.resume) + t.Fatalf("second patch was not queued behind the first: calls=%d", eng.calls.Load()) + } + close(eng.resume) + if err := <-first; err != nil { + t.Fatal(err) + } + if err := <-second; err != nil { + t.Fatal(err) + } + view, err := m.GetVolumes(sb.ID, sb.Tenant) + if err != nil || len(view.Volumes) != 2 || len(view.VolumesWant) != 2 { + t.Fatalf("lost concurrent patch: %+v %v", view, err) + } + for _, name := range []string{"data", "other"} { + if volumeHoldersOf(m, name).readers != 1 { + t.Fatal("duplicate hold for " + name) + } + } + }) +} + +func TestSyncIntentFailureRetainsConservativeDesiredHold(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + m.store.sync = true + restore := breakClaimStore(t, m) + view, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}) + if err == nil || len(view.VolumesWant) != 1 || len(view.Volumes) != 0 || len(eng.volumeSpecs) != 0 || volumeHoldersOf(m, "data").readers != 1 { + t.Fatalf("ambiguous write lost recovery state: %+v %v", view, err) + } + restore() + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, nil); err != nil { + t.Fatal(err) + } + if volumeHoldersOf(m, "data") != (volumeHolders{}) { + t.Fatal("cleanup leaked conservative hold") + } +} + +func TestFailedReleaseRestoresVolumeHoldOwnership(t *testing.T) { + for _, action := range []string{"release", "reap"} { + for _, runFinished := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/finished=%t", action, runFinished), func(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + run := beginAdmittedVolumeRun(t, m, sb) + m.mu.Lock() + if action == "reap" { + sb.Deadline = time.Now().Add(-time.Second) + } + m.mu.Unlock() + restore := breakClaimStore(t, m) + m.store.writeMu.Lock() + done := make(chan error, 1) + go func() { + if action == "reap" { + m.reapOnce(t.Context()) + done <- nil + } else { + done <- m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) + } + }() + waitFor(t, func() bool { m.mu.Lock(); defer m.mu.Unlock(); return m.claimed[sb.ID] == nil }) + if runFinished { + m.finishVolumeRun(sb, run) + } + m.store.writeMu.Unlock() + err := <-done + if action == "release" && err == nil { + t.Fatal("release ignored commit failure") + } + m.mu.Lock() + restored := m.claimed[sb.ID] == sb + m.mu.Unlock() + if !restored { + t.Fatal("failed release did not restore claim") + } + if !runFinished { + m.finishVolumeRun(sb, run) + } + if volumeHoldersOf(m, "data") != (volumeHolders{}) { + t.Fatal("rollback orphaned transient admission hold") + } + restore() + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err != nil { + t.Fatal(err) + } + if volumeHoldersOf(m, "data").readers != 1 { + t.Fatal("retry duplicated admission hold") + } + }) + } + } +} + +func TestArchiveRenewRejectsAdmittedUnpublishedVolumes(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + run := beginAdmittedVolumeRun(t, m, sb) + if _, err := m.Renew(t.Context(), sb.ID, Cred{Token: sb.Token}, time.Minute, types.ExpireArchive); !errors.Is(err, ErrVolumeCapture) { + t.Fatalf("archive admitted during volume intent: %v", err) + } + m.finishVolumeRun(sb, run) + if _, err := m.Renew(t.Context(), sb.ID, Cred{Token: sb.Token}, time.Minute, types.ExpireArchive); err != nil { + t.Fatalf("archive still blocked after abandoned intent: %v", err) + } +} + +func TestRetriedTargetPersistsIntentBeforeDeviceCalls(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + restore := breakClaimStore(t, m) + request := []types.Volume{{Name: "data"}} + for range 2 { + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request); err == nil { + t.Fatal("expected the intent write to fail") + } + } + if len(eng.volumeSpecs) != 0 { + t.Fatalf("device calls before the intent was durable: %v", eng.volumeSpecs) + } + restore() + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, request); err != nil { + t.Fatal(err) + } + assertLiveJournal(t, m, sb) +} + +func TestRollbackClaimTakesOverAdmittedRunHolds(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + run := beginAdmittedVolumeRun(t, m, sb) + m.rollbackClaim(t.Context(), []*types.Sandbox{sb}) + m.finishVolumeRun(sb, run) + waitFor(t, m.store.synced) + if got := volumeHoldersOf(m, "data"); got != (volumeHolders{}) { + t.Fatalf("rolled-back claim leaked its admitted hold: %+v", got) + } +} + +func TestLiveAttachBoundsWholeDeviceOperation(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + base := newFakeEngine() + m := newVolumeManager(t, base, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + eng := &deadlineVolumeEngine{fakeEngine: base, deadlines: make(chan time.Time, 1)} + m.eng = eng + start := time.Now() + if _, err := m.SetVolumes(t.Context(), sb.ID, sb.Tenant, []types.Volume{{Name: "data"}}); err != nil { + t.Fatal(err) + } + if deadline := <-eng.deadlines; !deadline.Equal(start.Add(engine.VolumeMutationTimeout)) { + t.Fatalf("live attach deadline=%v, want %v", deadline, start.Add(engine.VolumeMutationTimeout)) + } + }) +} + +func TestLivenessRefusesAdmittedVolumeRun(t *testing.T) { + eng := newFakeEngine() + m := newVolumeManager(t, eng, liveCatalog(t)) + sb := mustClaim(t, m, testKey) + run := beginAdmittedVolumeRun(t, m, sb) + killVMM(eng, sb.VMName) + m.recoverVMM(t.Context(), sb) + if len(eng.starts) != 0 || !strings.Contains(sb.Failed, "volume mounts") { + t.Fatalf("cold boot raced volume intent: starts=%v failed=%q", eng.starts, sb.Failed) + } + m.finishVolumeRun(sb, run) +} + +func TestRemovedTenantCanRemoveReadOnlyVolumeBesideImmutableVolume(t *testing.T) { + eng := newFakeEngine() + m, sb := newTenantVolumeManager(t, eng, liveCatalog(t), types.Volume{Name: "other", Mode: "rw"}) + if _, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Add: []types.Volume{{Name: "data"}}}); err != nil { + t.Fatal(err) + } + if err := m.DeleteTenant(t.Context(), "acme"); err != nil { + t.Fatal(err) + } + view, err := m.PatchVolumes(t.Context(), sb.ID, sb.Tenant, types.VolumePatch{Remove: []string{"data"}}) + if err != nil || len(view.Volumes) != 1 || view.Volumes[0].Name != "other" || !view.Volumes[0].RW() { + t.Fatalf("removed tenant cleanup=%+v %v", view, err) + } + if volumeHoldersOf(m, "data") != (volumeHolders{}) || volumeHoldersOf(m, "other").writers != 1 { + t.Fatal("cleanup altered immutable hold") + } +} + +type gatedVolumeEngine struct { + *fakeEngine + entered chan struct{} + resume chan struct{} + calls atomic.Int32 +} + +func newGatedVolumeEngine(base *fakeEngine) *gatedVolumeEngine { + return &gatedVolumeEngine{fakeEngine: base, entered: make(chan struct{}), resume: make(chan struct{})} +} + +func (e *gatedVolumeEngine) DiskAttach(ctx context.Context, vmName string, spec engine.VolumeSpec) error { + if e.calls.Add(1) == 1 { + close(e.entered) + <-e.resume + ctx = context.WithoutCancel(ctx) + } + return e.fakeEngine.DiskAttach(ctx, vmName, spec) +} + +type deadlineVolumeEngine struct { + *fakeEngine + deadlines chan time.Time +} + +func (e *deadlineVolumeEngine) DiskAttach(ctx context.Context, vmName string, spec engine.VolumeSpec) error { + deadline, _ := ctx.Deadline() + e.deadlines <- deadline + return e.fakeEngine.DiskAttach(ctx, vmName, spec) +} + +func liveCatalog(t *testing.T) []config.VolumeSpec { + t.Helper() + return []config.VolumeSpec{ + {Name: "data", Path: writeVolumeImage(t, "data.img", "data"), Writable: true}, + {Name: "other", Path: writeVolumeImage(t, "other.img", "other"), Writable: true}, + } +} + +func newTenantVolumeManager(t *testing.T, eng *fakeEngine, catalog []config.VolumeSpec, volumes ...types.Volume) (*Manager, *types.Sandbox) { + t.Helper() + m, err := NewManager(t.Context(), &config.Config{DataDir: t.TempDir(), APIToken: "root", Volumes: catalog}, eng, testSecrets(t)) + if err != nil { + t.Fatal(err) + } + putTenants(t, m, config.TenantSpec{Name: "acme", Token: "acme-token"}) + sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Tenant: "acme", Volumes: volumes}) + if err != nil { + t.Fatal(err) + } + return m, sb +} + +func assertLiveJournal(t *testing.T, m *Manager, sb *types.Sandbox) { + t.Helper() + claims, err := newClaimStore(m.dataDir, false).load() + if err != nil { + t.Fatal(err) + } + got := claims[sb.ID] + if got == nil || !slices.Equal(got.Volumes, sb.Volumes) { + t.Fatalf("journal volumes: %+v", got) + } + if !slices.Equal(got.VolumesWant, sb.VolumesWant) { + t.Fatalf("journal target: %+v, want %+v", got.VolumesWant, sb.VolumesWant) + } + if _, err := os.Stat(m.store.path); err != nil { + t.Fatal(err) + } +} + +func beginAdmittedVolumeRun(t *testing.T, m *Manager, sb *types.Sandbox) *volumeRun { + t.Helper() + _, run, err := m.beginVolumeRun(t.Context(), sb) + if err != nil { + t.Fatal(err) + } + m.mu.Lock() + defer m.mu.Unlock() + transient := []types.Volume{{Name: "data", Mount: "/volumes/data"}} + if err = m.reserveVolumes(transient); err != nil { + t.Fatal(err) + } + run.holds = append(run.holds, transient...) + return run +} + +func breakClaimStore(t *testing.T, m *Manager) (restore func()) { + t.Helper() + tmp := m.store.path + ".tmp" + m.store.writeMu.Lock() + err := os.Mkdir(tmp, 0o700) + m.store.writeMu.Unlock() + if err != nil { + t.Fatal(err) + } + return func() { + t.Helper() + m.store.writeMu.Lock() + err := os.Remove(tmp) + m.store.writeMu.Unlock() + if err != nil { + t.Fatal(err) + } + waitFor(t, func() bool { return !m.recommitting.Load() }) + } +} diff --git a/sandboxd/server/server.go b/sandboxd/server/server.go index 6b923dde..28522715 100644 --- a/sandboxd/server/server.go +++ b/sandboxd/server/server.go @@ -53,8 +53,8 @@ var poolErrHTTP = []struct { {pool.ErrBadConfig, http.StatusBadRequest, ""}, {pool.ErrReloadRefused, http.StatusConflict, ""}, {pool.ErrTenantRemoved, http.StatusForbidden, ""}, - {pool.ErrTenantStoreDown, http.StatusServiceUnavailable, ""}, - {pool.ErrPoolStoreDown, http.StatusServiceUnavailable, ""}, + {pool.ErrTenantStoreDown, http.StatusServiceUnavailable, "tenant store down; retry"}, + {pool.ErrPoolStoreDown, http.StatusServiceUnavailable, "pool set store down; retry"}, {pool.ErrNoEgress, http.StatusConflict, ""}, {pool.ErrNoEgressHibernate, http.StatusConflict, ""}, {pool.ErrNoEgressFork, http.StatusConflict, ""}, @@ -94,6 +94,9 @@ type Manager interface { TenantClaims() map[string]int VolumePlacement(key types.PoolKey, tenant string, names []string) (bool, error) Volumes(tenant string, holders map[string]int) []types.VolumeInfo + GetVolumes(id, tenant string) (types.SandboxVolumesResponse, error) + SetVolumes(ctx context.Context, id, tenant string, volumes []types.Volume) (types.SandboxVolumesResponse, error) + PatchVolumes(ctx context.Context, id, tenant string, patch types.VolumePatch) (types.SandboxVolumesResponse, error) Sandboxes(tenant, claimRef string, metadata types.Metadata) []pool.SandboxSummary Sandbox(id string) (pool.SandboxSummary, bool) Stats(ctx context.Context, id string) (pool.SandboxStats, bool) @@ -262,6 +265,9 @@ func (s *Server) Handler() http.Handler { mux.HandleFunc("POST /v1/sandboxes/{id}/release", s.handleSandboxVerb("release", s.mgr.Release)) mux.HandleFunc("POST /v1/sandboxes/{id}/hibernate", s.handleSandboxVerb("hibernate", s.mgr.Hibernate)) mux.HandleFunc("POST /v1/sandboxes/{id}/wake", s.handleSandboxVerb("wake", s.mgr.Wake)) + mux.HandleFunc("GET /v1/sandboxes/{id}/volumes", s.requireToken(s.handleSandboxVolumes)) + mux.HandleFunc("PUT /v1/sandboxes/{id}/volumes", s.requireToken(s.handleSandboxVolumes)) + mux.HandleFunc("PATCH /v1/sandboxes/{id}/volumes", s.requireToken(s.handleSandboxVolumes)) mux.HandleFunc("POST /v1/sandboxes/{id}/renew", s.handleRenew) mux.HandleFunc("GET /v1/sandboxes/{id}", s.requireRoot(s.handleSandbox)) mux.HandleFunc("GET /v1/sandboxes/{id}/stats", s.requireRoot(s.handleSandboxStats)) diff --git a/sandboxd/server/server_http.go b/sandboxd/server/server_http.go index e239ec35..bc98fb95 100644 --- a/sandboxd/server/server_http.go +++ b/sandboxd/server/server_http.go @@ -163,16 +163,23 @@ func metadataFilter(pairs []string) (types.Metadata, error) { } func writePoolErr(w http.ResponseWriter, err error) bool { + code, msg, ok := mapPoolErr(w, err) + if ok { + writeErr(w, code, msg) + } + return ok +} + +func mapPoolErr(w http.ResponseWriter, err error) (code int, msg string, ok bool) { for _, m := range poolErrHTTP { if errors.Is(err, m.err) { if m.code == http.StatusServiceUnavailable { w.Header().Set("Retry-After", retryAfterSeconds) } - writeErr(w, m.code, cmp.Or(m.msg, err.Error())) - return true + return m.code, cmp.Or(m.msg, err.Error()), true } } - return false + return 0, "", false } func writeResult(w http.ResponseWriter, r *http.Request, op, id, failMsg string, err error, ok func()) { diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index e5320de1..499f192a 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -135,15 +135,7 @@ func TestClaimErrorMapping(t *testing.T) { func TestHibernateVolumeCaptureMapsConflict(t *testing.T) { mgr := &fakeManager{hibernate: func(string, string) error { return pool.ErrVolumeCapture }} ts := newTestServer(t, "", mgr, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, ts.URL+"/v1/sandboxes/sb_1/hibernate", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer tok") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("hibernate: %v", err) - } + resp := doReq(t, http.MethodPost, ts.URL+"/v1/sandboxes/sb_1/hibernate", "tok", "") defer resp.Body.Close() if resp.StatusCode != http.StatusConflict { t.Errorf("status %d, want 409", resp.StatusCode) @@ -244,15 +236,7 @@ func TestRenewRejectsWrongToken(t *testing.T) { func TestInfoReportsTheNodeAdvertiseAddr(t *testing.T) { ts := newTestServer(t, "sekret", &fakeManager{}, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, ts.URL+"/v1/info", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer sekret") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("do: %v", err) - } + resp := doReq(t, http.MethodGet, ts.URL+"/v1/info", "sekret", "") defer resp.Body.Close() var info InfoResponse if err := json.UnmarshalRead(resp.Body, &info); err != nil { @@ -494,18 +478,8 @@ func TestVolumeCatalogReturnsScopedFleetProjection(t *testing.T) { }} placer := &fakePlacer{volumeHolders: map[string]int{"imagenet": 3}} mgr.tenants = []config.TenantSpec{{Name: "acme", Token: "acme-tok"}} - srv := New("root", "node:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(ts.Close) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, ts.URL+"/v1/volumes", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer acme-tok") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("volumes: %v", err) - } + ts := newMeshTestServer(t, "root", mgr, placer, nil) + resp := doReq(t, http.MethodGet, ts.URL+"/v1/volumes", "acme-tok", "") defer resp.Body.Close() var got types.VolumeListResponse if err := json.UnmarshalRead(resp.Body, &got); err != nil { @@ -529,15 +503,7 @@ func TestSandboxIndexReturnsScopedAppliedVolumes(t *testing.T) { }} }} ts := newTenantTestServer(t, "root", []config.TenantSpec{{Name: "acme", Token: "acme-tok"}}, mgr, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, ts.URL+"/v1/sandboxes", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer acme-tok") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("sandboxes: %v", err) - } + resp := doReq(t, http.MethodGet, ts.URL+"/v1/sandboxes", "acme-tok", "") defer resp.Body.Close() var got SandboxListResponse if err := json.UnmarshalRead(resp.Body, &got); err != nil { @@ -555,15 +521,7 @@ func TestSandboxesReportClaimedAt(t *testing.T) { return []pool.SandboxSummary{{ID: "sb_1", ClaimedAt: claimed}} }} ts := newTenantTestServer(t, "root", nil, mgr, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, ts.URL+"/v1/sandboxes", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer root") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("sandboxes: %v", err) - } + resp := doReq(t, http.MethodGet, ts.URL+"/v1/sandboxes", "root", "") defer resp.Body.Close() var got SandboxListResponse if err := json.UnmarshalRead(resp.Body, &got); err != nil { @@ -623,15 +581,7 @@ func TestSandboxesNarrowToAClaimRef(t *testing.T) { return nil }} ts := newTenantTestServer(t, "root", nil, mgr, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, ts.URL+tt.path, nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer root") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("sandboxes: %v", err) - } + resp := doReq(t, http.MethodGet, ts.URL+tt.path, "root", "") resp.Body.Close() if resp.StatusCode != http.StatusOK || got != tt.want { t.Errorf("%s: status %d, manager saw claim_ref %q, want %q", tt.path, resp.StatusCode, got, tt.want) @@ -725,16 +675,7 @@ func TestMetricsTenantGauge(t *testing.T) { func TestPutPoolsRejectsUnknownFields(t *testing.T) { ts := newTestServer(t, "sekret", &fakeManager{}, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodPut, ts.URL+"/v1/pools", - strings.NewReader(`{"pools":[{"template":"rt:24.04","net":"none","size":"small","egres":{"allow":[]}}]}`)) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer sekret") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("do: %v", err) - } + resp := doReq(t, http.MethodPut, ts.URL+"/v1/pools", "sekret", `{"pools":[{"template":"rt:24.04","net":"none","size":"small","egres":{"allow":[]}}]}`) defer resp.Body.Close() if resp.StatusCode != http.StatusBadRequest { t.Fatalf("status %d, want 400", resp.StatusCode) @@ -798,15 +739,7 @@ func TestDrainEndpoints(t *testing.T) { {http.MethodPost, true}, {http.MethodDelete, false}, } { - req, err := http.NewRequestWithContext(t.Context(), tt.method, ts.URL+"/v1/drain", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer sekret") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("do: %v", err) - } + resp := doReq(t, tt.method, ts.URL+"/v1/drain", "sekret", "") var info InfoResponse if err := json.UnmarshalRead(resp.Body, &info); err != nil { t.Fatalf("decode: %v", err) @@ -834,16 +767,7 @@ func TestPutPoolsUpdatesTargets(t *testing.T) { } ts := newTestServer(t, "sekret", mgr, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodPut, ts.URL+"/v1/pools", - strings.NewReader(`{"pools":[{"template":"rt:24.04","net":"none","size":"small","warm":2}]}`)) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer sekret") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("do: %v", err) - } + resp := doReq(t, http.MethodPut, ts.URL+"/v1/pools", "sekret", `{"pools":[{"template":"rt:24.04","net":"none","size":"small","warm":2}]}`) defer resp.Body.Close() if resp.StatusCode != http.StatusOK { t.Fatalf("status %d, want 200", resp.StatusCode) @@ -1208,20 +1132,7 @@ func TestCheckpointFlow(t *testing.T) { } ts := newTestServer(t, "api", mgr, &fakeDialer{}) - post := func(path, body string) *http.Response { - req, err := http.NewRequestWithContext(t.Context(), http.MethodPost, ts.URL+path, strings.NewReader(body)) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer api") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("%s: %v", path, err) - } - return resp - } - - resp := post("/v1/sandboxes/sb_1/checkpoint", `{"token":"tok","name":"step-1"}`) + resp := postJSON(t, ts.URL+"/v1/sandboxes/sb_1/checkpoint", "api", `{"token":"tok","name":"step-1"}`) defer resp.Body.Close() var cr types.CheckpointResponse if err := json.UnmarshalRead(resp.Body, &cr); err != nil || cr.Checkpoint.ID != "ck_0011223344556677" { @@ -1231,28 +1142,20 @@ func TestCheckpointFlow(t *testing.T) { t.Errorf("name %q, want step-1", cr.Checkpoint.Name) } - resp2 := post("/v1/checkpoints/"+cr.Checkpoint.ID+"/claim", `{}`) + resp2 := postJSON(t, ts.URL+"/v1/checkpoints/"+cr.Checkpoint.ID+"/claim", "api", `{}`) defer resp2.Body.Close() var claim types.ClaimResponse if err := json.UnmarshalRead(resp2.Body, &claim); err != nil || claim.ID != "sb_branch" { t.Fatalf("claim from checkpoint: status %d, %+v, %v", resp2.StatusCode, claim, err) } - resp3 := post("/v1/checkpoints/ck_ffffffffffffffff/claim", `{}`) + resp3 := postJSON(t, ts.URL+"/v1/checkpoints/ck_ffffffffffffffff/claim", "api", `{}`) defer resp3.Body.Close() if resp3.StatusCode != http.StatusNotFound { t.Errorf("unknown checkpoint claim: status %d, want 404", resp3.StatusCode) } - req, err := http.NewRequestWithContext(t.Context(), http.MethodDelete, ts.URL+"/v1/checkpoints/"+cr.Checkpoint.ID, nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer api") - resp4, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("delete: %v", err) - } + resp4 := doReq(t, http.MethodDelete, ts.URL+"/v1/checkpoints/"+cr.Checkpoint.ID, "api", "") defer resp4.Body.Close() if resp4.StatusCode != http.StatusNoContent { t.Errorf("delete checkpoint: status %d, want 204", resp4.StatusCode) @@ -1264,15 +1167,7 @@ func TestDeleteCheckpointNoForwardQueryParam(t *testing.T) { ts := newTestServer(t, "api", mgr, &fakeDialer{}) del := func(path string) int { - req, err := http.NewRequestWithContext(t.Context(), http.MethodDelete, ts.URL+path, nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer api") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("delete: %v", err) - } + resp := doReq(t, http.MethodDelete, ts.URL+path, "api", "") defer resp.Body.Close() return resp.StatusCode } @@ -1297,16 +1192,8 @@ func TestDeleteCheckpointForgetsProbeCache(t *testing.T) { t.Run("original delete", func(t *testing.T) { prober.forgotten = nil mgr := &fakeManager{deleteCheckpoint: func(string) error { return nil }} - ts := newPlacerTestServer(t, "api", mgr, prober) - req, err := http.NewRequestWithContext(t.Context(), http.MethodDelete, ts.URL+"/v1/checkpoints/ck_0011223344556677", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer api") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("delete: %v", err) - } + ts := newProberTestServer(t, "api", mgr, prober) + resp := doReq(t, http.MethodDelete, ts.URL+"/v1/checkpoints/ck_0011223344556677", "api", "") defer resp.Body.Close() if len(prober.forgotten) != 1 || prober.forgotten[0] != "ck_0011223344556677" { t.Errorf("forgotten = %v, want [ck_0011223344556677]", prober.forgotten) @@ -1316,16 +1203,8 @@ func TestDeleteCheckpointForgetsProbeCache(t *testing.T) { t.Run("forwarded delete finding nothing locally", func(t *testing.T) { prober.forgotten = nil mgr := &fakeManager{deleteCheckpoint: func(string) error { return pool.ErrUnknownCheckpoint }} - ts := newPlacerTestServer(t, "api", mgr, prober) - req, err := http.NewRequestWithContext(t.Context(), http.MethodDelete, ts.URL+"/v1/checkpoints/ck_0011223344556677?no_forward=1", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer api") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("delete: %v", err) - } + ts := newProberTestServer(t, "api", mgr, prober) + resp := doReq(t, http.MethodDelete, ts.URL+"/v1/checkpoints/ck_0011223344556677?no_forward=1", "api", "") defer resp.Body.Close() if len(prober.forgotten) != 1 || prober.forgotten[0] != "ck_0011223344556677" { t.Errorf("forgotten = %v, want [ck_0011223344556677] even though nothing was held locally", prober.forgotten) @@ -1339,9 +1218,7 @@ func TestClaimRedirectsOnWarmMiss(t *testing.T) { provisioned = true return &types.Sandbox{ID: "sb_local"}, nil }} - srv := New("", "node-a:7777", mgr, &fakeDialer{}, &fakePlacer{addrs: []string{"node-b:7777", "node-c:7777"}}, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, &fakePlacer{addrs: []string{"node-b:7777", "node-c:7777"}}, nil) resp, err := http.Post(ts.URL+"/v1/claim", "application/json", strings.NewReader(`{"template":"rt:24.04"}`)) if err != nil { @@ -1381,9 +1258,7 @@ func TestClaimRedirectsToTemplateOwner(t *testing.T) { return &types.Sandbox{ID: "sb_local", Token: "tok"}, nil }, } - srv := New("", "node-a:7777", mgr, &fakeDialer{}, &fakePlacer{owners: []string{"node-b:7777"}}, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, &fakePlacer{owners: []string{"node-b:7777"}}, nil) resp, err := http.Post(ts.URL+"/v1/claim", "application/json", strings.NewReader(`{"template":"tpl"}`)) if err != nil { @@ -1409,24 +1284,14 @@ func TestATemplateDigestGatesDeleteAndLabelWrites(t *testing.T) { mgr := &fakeManager{deleteGolden: func(types.PoolKey) error { return pool.ErrTemplateReplaced }} ts := newTestServer(t, "sekret", mgr, nil) - req, _ := http.NewRequest(http.MethodDelete, ts.URL+"/v1/templates?template=ns%2Fapp&digest=sha256%3Aold", nil) - req.Header.Set("Authorization", "Bearer sekret") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatal(err) - } + resp := doReq(t, http.MethodDelete, ts.URL+"/v1/templates?template=ns%2Fapp&digest=sha256%3Aold", "sekret", "") defer resp.Body.Close() if resp.StatusCode != http.StatusPreconditionFailed || mgr.gotDigest != "sha256:old" { t.Errorf("status=%d digest=%q, want 412 for a replaced generation and the digest passed through", resp.StatusCode, mgr.gotDigest) } mgr.labelErr, mgr.gotDigest = pool.ErrTemplateReplaced, "" - req, _ = http.NewRequest(http.MethodPut, ts.URL+"/v1/templates/labels?template=ns%2Fapp&digest=sha256%3Aold", strings.NewReader(`{"labels":{"v1":"sha256:new"}}`)) - req.Header.Set("Authorization", "Bearer sekret") - resp, err = http.DefaultClient.Do(req) - if err != nil { - t.Fatal(err) - } + resp = doReq(t, http.MethodPut, ts.URL+"/v1/templates/labels?template=ns%2Fapp&digest=sha256%3Aold", "sekret", `{"labels":{"v1":"sha256:new"}}`) defer resp.Body.Close() if resp.StatusCode != http.StatusPreconditionFailed || mgr.gotDigest != "sha256:old" { t.Errorf("labels: status=%d digest=%q, want 412 for a replaced generation and the digest passed through", resp.StatusCode, mgr.gotDigest) @@ -1435,9 +1300,7 @@ func TestATemplateDigestGatesDeleteAndLabelWrites(t *testing.T) { func TestDeleteTemplateRedirectsToOwner(t *testing.T) { mgr := &fakeManager{} - srv := New("", "node-a:7777", mgr, &fakeDialer{}, &fakePlacer{owners: []string{"node-b:7777"}}, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, &fakePlacer{owners: []string{"node-b:7777"}}, nil) req, err := http.NewRequestWithContext(t.Context(), http.MethodDelete, ts.URL+"/v1/templates?template=tpl", nil) if err != nil { @@ -1472,9 +1335,7 @@ func TestDeleteTemplateRedirectsToOwner(t *testing.T) { t.Errorf("status %d, want 404 with no_redirect despite known owners", resp2.StatusCode) } - srvNoOwner := New("", "node-a:7777", mgr, &fakeDialer{}, &fakePlacer{}, nil, nil, nil) - ts2 := httptest.NewServer(srvNoOwner.Handler()) - t.Cleanup(func() { ts2.Close(); srvNoOwner.CloseRelays() }) + ts2 := newMeshTestServer(t, "", mgr, &fakePlacer{}, nil) req3, err := http.NewRequestWithContext(t.Context(), http.MethodDelete, ts2.URL+"/v1/templates?template=tpl", nil) if err != nil { t.Fatalf("request: %v", err) @@ -1493,9 +1354,7 @@ func TestClaimProvisionsWhenNoCandidate(t *testing.T) { mgr := &fakeManager{claim: func(context.Context, types.PoolKey, time.Duration) (*types.Sandbox, error) { return &types.Sandbox{ID: "sb_local", Token: "tok"}, nil }} - srv := New("", "node-a:7777", mgr, &fakeDialer{}, &fakePlacer{addrs: nil}, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, &fakePlacer{addrs: nil}, nil) resp, err := http.Post(ts.URL+"/v1/claim", "application/json", strings.NewReader(`{"template":"rt:24.04","claim_ref":"ns1/w1"}`)) if err != nil { @@ -1547,9 +1406,7 @@ func TestVolumeClaimUsesWarmBeforeRedirectOrProvision(t *testing.T) { placer := &fakePlacer{ addrs: []string{"wrong-general-candidate:7777"}, volumeCandidates: tt.volumeCandidates, } - srv := New("", "node-a:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, placer, nil) resp, err := http.Post(ts.URL+"/v1/claim", "application/json", strings.NewReader(`{"template":"rt:24.04","volumes":[{"name":"imagenet"}]}`)) if err != nil { @@ -1658,9 +1515,7 @@ func TestVolumeClaimUsesVolumeAndTemplateIntersections(t *testing.T) { addrs: []string{"warm-peer:7777"}, owners: tt.templateOwners, volumeOwners: tt.volumeOwners, templateVolumeOwners: tt.templateVolumeOwner, } - srv := New("", "node-a:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, placer, nil) request := types.ClaimRequest{ Template: "tpl", Volumes: []types.Volume{{Name: "imagenet"}}, @@ -1741,9 +1596,7 @@ func TestVolumeClaimKeepsPoolContentAgainstPeerTemplates(t *testing.T) { } } placer := &fakePlacer{owners: []string{"template:7777"}, templateVolumeOwners: []string{"both:7777"}} - srv := New("", "node-a:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, placer, nil) body, err := json.Marshal(types.ClaimRequest{ Template: "tpl", Volumes: []types.Volume{{Name: "imagenet"}}, @@ -1802,17 +1655,10 @@ func TestForeignTemplateGossipNeverEscalates(t *testing.T) { types.TemplateGossipHash(hash, types.TemplateRootScope): {"peer:7777"}, }} mgr.tenants = tenants - srv := New("root-tok", "node-a:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "root-tok", mgr, placer, nil) body, _ := json.Marshal(types.ClaimRequest{Template: "tpl", Volumes: tt.volumes}) - req, _ := http.NewRequest(http.MethodPost, ts.URL+"/v1/claim", bytes.NewReader(body)) - req.Header.Set("Authorization", "Bearer "+tt.token) - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("claim: %v", err) - } + resp := postJSON(t, ts.URL+"/v1/claim", tt.token, string(body)) defer resp.Body.Close() if resp.StatusCode != http.StatusOK { t.Fatalf("status=%d, want 200", resp.StatusCode) @@ -1854,9 +1700,7 @@ func TestARequiredPromotedClaimIsNeverSentToAWarmPeer(t *testing.T) { mgr := &fakeManager{claim: func(context.Context, types.PoolKey, time.Duration) (*types.Sandbox, error) { return nil, tt.provision }} - srv := New("sekret", "node-a:7777", mgr, &fakeDialer{}, &fakePlacer{addrs: []string{"warm:7777"}}, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "sekret", mgr, &fakePlacer{addrs: []string{"warm:7777"}}, nil) body, _ := json.Marshal(types.ClaimRequest{Template: "ns/app", RequirePromoted: tt.require}) resp := postJSON(t, ts.URL+"/v1/claim", "sekret", string(body)) @@ -1920,9 +1764,7 @@ func TestVolumeClaimValidatesKeyBeforePlacement(t *testing.T) { }, } placer := &fakePlacer{volumeOwners: []string{"volume:7777"}} - srv := New("", "node-a:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, placer, nil) resp, err := http.Post(ts.URL+"/v1/claim", "application/json", strings.NewReader( `{"template":"rt:24.04","net":"egress","volumes":[{"name":"imagenet"}]}`)) @@ -1946,9 +1788,7 @@ func TestVolumeClaimQuotaDoesNotRedirect(t *testing.T) { placer := &fakePlacer{ addrs: []string{"wrong-general-candidate:7777"}, volumeCandidates: []string{"wrong-volume-candidate:7777"}, } - srv := New("", "node-a:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, placer, nil) resp, err := http.Post(ts.URL+"/v1/claim", "application/json", strings.NewReader(`{"template":"rt:24.04","volumes":[{"name":"imagenet"}]}`)) if err != nil { @@ -1979,16 +1819,13 @@ func TestVolumeClaimRejectsShapeBeforePlacement(t *testing.T) { } { mgr := &fakeManager{} placer := &fakePlacer{addrs: []string{"warm-peer:7777"}, owners: []string{"owner:7777"}} - srv := New("", "node-a:7777", mgr, &fakeDialer{}, placer, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) + ts := newMeshTestServer(t, "", mgr, placer, nil) resp, err := http.Post(ts.URL+"/v1/claim", "application/json", strings.NewReader(body)) if err != nil { t.Fatalf("claim: %v", err) } resp.Body.Close() - ts.Close() - srv.CloseRelays() if resp.StatusCode != http.StatusBadRequest { t.Errorf("body=%s status=%d, want 400", body, resp.StatusCode) } @@ -2005,16 +1842,9 @@ func TestOwnerEndpoint(t *testing.T) { } return "", pool.ErrUnknownSandbox }} - srv := New("", "node-b:7777", mgr, &fakeDialer{}, nil, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := newMeshTestServer(t, "", mgr, nil, nil) - req, _ := http.NewRequestWithContext(t.Context(), http.MethodGet, ts.URL+"/v1/sandboxes/sb_1/owner", nil) - req.Header.Set("Authorization", "Bearer good") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("owner: %v", err) - } + resp := doReq(t, http.MethodGet, ts.URL+"/v1/sandboxes/sb_1/owner", "good", "") defer resp.Body.Close() if resp.StatusCode != http.StatusOK { t.Fatalf("status %d, want 200", resp.StatusCode) @@ -2023,16 +1853,11 @@ func TestOwnerEndpoint(t *testing.T) { OwnerAddr string `json:"owner_addr"` } _ = json.UnmarshalRead(resp.Body, &body) - if body.OwnerAddr != "node-b:7777" { - t.Errorf("owner %q, want node-b:7777", body.OwnerAddr) + if body.OwnerAddr != "node-a:7777" { + t.Errorf("owner %q, want node-a:7777", body.OwnerAddr) } - req2, _ := http.NewRequestWithContext(t.Context(), http.MethodGet, ts.URL+"/v1/sandboxes/sb_1/owner", nil) - req2.Header.Set("Authorization", "Bearer wrong") - resp2, err := http.DefaultClient.Do(req2) - if err != nil { - t.Fatalf("owner: %v", err) - } + resp2 := doReq(t, http.MethodGet, ts.URL+"/v1/sandboxes/sb_1/owner", "wrong", "") defer resp2.Body.Close() if resp2.StatusCode != http.StatusNotFound { t.Errorf("status %d, want 404", resp2.StatusCode) @@ -2044,9 +1869,7 @@ func TestPreviewHandlerZeroDeadlineMintsLiveToken(t *testing.T) { return time.Time{}, nil }} ps := NewPreviewServer("secret", "node:7777", "node:7777", &fakePreviewMgr{}) - srv := New("", "node:7777", mgr, &fakeDialer{}, nil, nil, nil, ps) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := serveTestServer(t, New("", "node:7777", mgr, &fakeDialer{}, nil, nil, nil, ps)) resp, err := http.Post(ts.URL+"/v1/sandboxes/sb_1/preview", "application/json", strings.NewReader(`{"token":"tok","port":8080,"ttl_seconds":300}`)) @@ -2077,9 +1900,7 @@ func TestPreviewHandlerRejectsPortZero(t *testing.T) { return time.Now().Add(time.Hour), nil }} ps := NewPreviewServer("secret", "node:7777", "node:7777", &fakePreviewMgr{}) - srv := New("", "node:7777", mgr, &fakeDialer{}, nil, nil, nil, ps) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { ts.Close(); srv.CloseRelays() }) + ts := serveTestServer(t, New("", "node:7777", mgr, &fakeDialer{}, nil, nil, nil, ps)) resp, err := http.Post(ts.URL+"/v1/sandboxes/sb_1/preview", "application/json", strings.NewReader(`{"token":"tok","port":0}`)) @@ -2095,7 +1916,7 @@ func TestPreviewHandlerRejectsPortZero(t *testing.T) { func TestCheckpointClaimRedirectsToOwner(t *testing.T) { mgr := &fakeManager{} prober := &fakeProber{owners: []string{"owner-a:7777"}} - ts := newPlacerTestServer(t, "sekret", mgr, prober) + ts := newProberTestServer(t, "sekret", mgr, prober) resp := postJSON(t, ts.URL+"/v1/checkpoints/ck_00000000000000aa/claim", "sekret", `{}`) defer resp.Body.Close() @@ -2117,7 +1938,7 @@ func TestCheckpointClaimRedirectsToOwner(t *testing.T) { func TestCheckpointClaimNoRedirectResolvesLocally(t *testing.T) { prober := &fakeProber{owners: []string{"owner-a:7777"}} - ts := newPlacerTestServer(t, "sekret", &fakeManager{}, prober) + ts := newProberTestServer(t, "sekret", &fakeManager{}, prober) resp := postJSON(t, ts.URL+"/v1/checkpoints/ck_00000000000000aa/claim", "sekret", `{"no_redirect":true}`) defer resp.Body.Close() @@ -2129,7 +1950,7 @@ func TestCheckpointClaimNoRedirectResolvesLocally(t *testing.T) { func TestCheckpointClaimNoOwnersIs404(t *testing.T) { mgr := &fakeManager{} - ts := newPlacerTestServer(t, "sekret", mgr, nil) + ts := newProberTestServer(t, "sekret", mgr, nil) resp := postJSON(t, ts.URL+"/v1/checkpoints/ck_00000000000000aa/claim", "sekret", `{}`) defer resp.Body.Close() @@ -2145,7 +1966,7 @@ func TestCheckpointClaimNoOwnersIs404(t *testing.T) { func TestCheckpointClaimRedirectBeatsHeal(t *testing.T) { mgr := &fakeManager{} prober := &fakeProber{owners: []string{"owner-a:7777"}} - ts := newPlacerTestServer(t, "sekret", mgr, prober) + ts := newProberTestServer(t, "sekret", mgr, prober) resp := postJSON(t, ts.URL+"/v1/checkpoints/ck_00000000000000aa/claim", "sekret", `{}`) defer resp.Body.Close() @@ -2171,7 +1992,7 @@ func TestCheckpointClaimFallsBackToHealWhenNoOwner(t *testing.T) { return &types.Sandbox{ID: "sb_healed", Token: "htok", FromCheckpoint: ckptID}, nil }, } - ts := newPlacerTestServer(t, "sekret", mgr, nil) + ts := newProberTestServer(t, "sekret", mgr, nil) resp := postJSON(t, ts.URL+"/v1/checkpoints/ck_00000000000000aa/claim", "sekret", `{}`) defer resp.Body.Close() @@ -2198,7 +2019,7 @@ func TestCheckpointClaimNoRedirectGoesStraightToHeal(t *testing.T) { }, } prober := &fakeProber{owners: []string{"owner-a:7777"}} - ts := newPlacerTestServer(t, "sekret", mgr, prober) + ts := newProberTestServer(t, "sekret", mgr, prober) resp := postJSON(t, ts.URL+"/v1/checkpoints/ck_00000000000000aa/claim", "sekret", `{"no_redirect":true}`) defer resp.Body.Close() @@ -2227,7 +2048,7 @@ func TestCheckpointClaimHealBusyIs503WithRetryAfter(t *testing.T) { return nil, pool.ErrHealBusy }, } - ts := newPlacerTestServer(t, "sekret", mgr, nil) + ts := newProberTestServer(t, "sekret", mgr, nil) resp := postJSON(t, ts.URL+"/v1/checkpoints/ck_00000000000000aa/claim", "sekret", `{}`) defer resp.Body.Close() @@ -2243,16 +2064,7 @@ func TestCheckpointClaimHealBusyIs503WithRetryAfter(t *testing.T) { func TestCheckpointBlobUnknownIs404(t *testing.T) { ts := newTestServer(t, "sekret", &fakeManager{}, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, - ts.URL+"/v1/checkpoints/ck_00000000000000aa/blob", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer sekret") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("do: %v", err) - } + resp := doReq(t, http.MethodGet, ts.URL+"/v1/checkpoints/ck_00000000000000aa/blob", "sekret", "") defer resp.Body.Close() if resp.StatusCode != http.StatusNotFound { @@ -2268,16 +2080,7 @@ func TestCheckpointBlobStreamsRecord(t *testing.T) { mgr := &fakeManager{ckptDir: src} ts := newTestServer(t, "sekret", mgr, nil) - req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, - ts.URL+"/v1/checkpoints/ck_00000000000000aa/blob", nil) - if err != nil { - t.Fatalf("request: %v", err) - } - req.Header.Set("Authorization", "Bearer sekret") - resp, err := http.DefaultClient.Do(req) - if err != nil { - t.Fatalf("do: %v", err) - } + resp := doReq(t, http.MethodGet, ts.URL+"/v1/checkpoints/ck_00000000000000aa/blob", "sekret", "") defer resp.Body.Close() if resp.StatusCode != http.StatusOK { @@ -2364,15 +2167,11 @@ func TestCheckpointProbeRequiresValidMAC(t *testing.T) { func TestCheckpointClaimProbesRealPeerAndRedirects(t *testing.T) { const ckptID = "ck_00000000000000aa" mgrB := &fakeManager{hasCheckpoint: map[string]bool{ckptID: true}} - srvB := New("sekret", "node-b:7777", mgrB, &fakeDialer{}, nil, nil, nil, nil) - tsB := httptest.NewServer(srvB.Handler()) - t.Cleanup(tsB.Close) + tsB := newTestServer(t, "sekret", mgrB, nil) prober := &peer.HTTPProber{Peers: func() []string { return []string{tsB.URL} }} mgrA := &fakeManager{} - srvA := New("sekret", "node-a:7777", mgrA, &fakeDialer{}, nil, prober, nil, nil) - tsA := httptest.NewServer(srvA.Handler()) - t.Cleanup(tsA.Close) + tsA := newProberTestServer(t, "sekret", mgrA, prober) resp := postJSON(t, tsA.URL+"/v1/checkpoints/"+ckptID+"/claim", "sekret", `{}`) defer resp.Body.Close() @@ -2552,18 +2351,26 @@ func newTenantTestServer(t *testing.T, apiToken string, tenants []config.TenantS if fm, ok := mgr.(*fakeManager); ok { fm.tenants = tenants } - srv := New(apiToken, "node:7777", mgr, dialer, nil, nil, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(func() { - ts.Close() - srv.CloseRelays() - }) - return ts + return serveTestServer(t, New(apiToken, "node:7777", mgr, dialer, nil, nil, nil, nil)) } func newProbeAuthTestServer(t *testing.T, mgr Manager, probeKey []byte) *httptest.Server { t.Helper() - srv := New("", "node:7777", mgr, &fakeDialer{}, nil, nil, probeKey, nil) + return serveTestServer(t, New("", "node:7777", mgr, &fakeDialer{}, nil, nil, probeKey, nil)) +} + +func newMeshTestServer(t *testing.T, apiToken string, mgr Manager, placer Placer, prober CheckpointProber) *httptest.Server { + t.Helper() + return serveTestServer(t, New(apiToken, "node-a:7777", mgr, &fakeDialer{}, placer, prober, nil, nil)) +} + +func newProberTestServer(t *testing.T, apiToken string, mgr Manager, prober CheckpointProber) *httptest.Server { + t.Helper() + return newMeshTestServer(t, apiToken, mgr, nil, prober) +} + +func serveTestServer(t *testing.T, srv *Server) *httptest.Server { + t.Helper() ts := httptest.NewServer(srv.Handler()) t.Cleanup(func() { ts.Close() @@ -2586,6 +2393,11 @@ func doReq(t *testing.T, method, url, token, body string) *http.Response { return resp } +func postJSON(t *testing.T, url, token, body string) *http.Response { + t.Helper() + return doReq(t, http.MethodPost, url, token, body) +} + func credToken(cred pool.Cred) string { if cred.Operator { return "" @@ -2593,6 +2405,21 @@ func credToken(cred pool.Cred) string { return cred.Token } +func fakeClaimProvision(ctx context.Context, f *fakeManager, key types.PoolKey, o pool.ClaimOptions) (*types.Sandbox, error) { + f.provisionCalls++ + f.gotTenant = o.Tenant + f.gotClaimRef = o.ClaimRef + f.gotMetadata = o.Metadata + f.gotEnv = o.Env + f.gotOnExpire = o.OnExpire + f.gotRequirePromoted = o.RequirePromoted + f.gotNoEgress = o.NoEgress + if f.claim == nil { + return &types.Sandbox{ID: "sb_1", Token: "tok", Volumes: slices.Clone(o.Volumes)}, nil + } + return f.claim(ctx, key, o.TTL) +} + type claimFunc func(context.Context, types.PoolKey, time.Duration) (*types.Sandbox, error) type sandboxVerbFunc func(string, string) error @@ -2602,6 +2429,9 @@ type checkpointClaimFunc func(string) (*types.Sandbox, error) type idActionFunc func(string) error type fakeManager struct { + getVolumes func(string, string) (types.SandboxVolumesResponse, error) + setVolumes func(string, string, []types.Volume) (types.SandboxVolumesResponse, error) + patchVolumes func(string, string, types.VolumePatch) (types.SandboxVolumesResponse, error) tenants []config.TenantSpec setTenants func(specs []config.TenantSpec) error putTenant func(spec config.TenantSpec) error @@ -2656,7 +2486,6 @@ type fakeManager struct { gotMetadata types.Metadata gotEnv types.Env gotOnExpire types.ExpireAction - gotVolumes []types.Volume gotWarmVolumes []types.Volume gotRequirePromoted bool gotNoEgress bool @@ -2666,14 +2495,24 @@ type fakeManager struct { gotNoForward bool tenantClaims map[string]int volumePlacement func(key types.PoolKey, tenant string, names []string) (bool, error) - placementCalls int - placementNames []string volumeCatalog func(tenant string, holders map[string]int) []types.VolumeInfo sandboxIndex func(tenant, claimRef string) []pool.SandboxSummary sandboxByID func(id string) (pool.SandboxSummary, bool) draining bool } +func (f *fakeManager) GetVolumes(id, tenant string) (types.SandboxVolumesResponse, error) { + return f.getVolumes(id, tenant) +} + +func (f *fakeManager) SetVolumes(_ context.Context, id, tenant string, v []types.Volume) (types.SandboxVolumesResponse, error) { + return f.setVolumes(id, tenant, v) +} + +func (f *fakeManager) PatchVolumes(_ context.Context, id, tenant string, p types.VolumePatch) (types.SandboxVolumesResponse, error) { + return f.patchVolumes(id, tenant, p) +} + func (f *fakeManager) ClaimWarm(ctx context.Context, key types.PoolKey, o pool.ClaimOptions) (*types.Sandbox, error) { f.warmCalls++ f.gotTenant, f.gotEgressClass = o.Tenant, o.EgressClass @@ -2837,9 +2676,7 @@ func (f *fakeManager) DeleteTenant(_ context.Context, name string) error { } func (f *fakeManager) VolumePlacement(key types.PoolKey, tenant string, names []string) (bool, error) { - f.placementCalls++ f.gotTenant = tenant - f.placementNames = slices.Clone(names) if f.volumePlacement == nil { return true, nil } @@ -3065,32 +2902,3 @@ type fakeProber struct { func (f *fakeProber) Owners(context.Context, string) []string { return f.owners } func (f *fakeProber) Forget(id string) { f.forgotten = append(f.forgotten, id) } - -func newPlacerTestServer(t *testing.T, apiToken string, mgr Manager, prober CheckpointProber) *httptest.Server { - t.Helper() - srv := New(apiToken, "node:7777", mgr, &fakeDialer{}, nil, prober, nil, nil) - ts := httptest.NewServer(srv.Handler()) - t.Cleanup(ts.Close) - return ts -} - -func postJSON(t *testing.T, url, token, body string) *http.Response { - t.Helper() - return doReq(t, http.MethodPost, url, token, body) -} - -func fakeClaimProvision(ctx context.Context, f *fakeManager, key types.PoolKey, o pool.ClaimOptions) (*types.Sandbox, error) { - f.provisionCalls++ - f.gotTenant = o.Tenant - f.gotClaimRef = o.ClaimRef - f.gotMetadata = o.Metadata - f.gotEnv = o.Env - f.gotOnExpire = o.OnExpire - f.gotVolumes = slices.Clone(o.Volumes) - f.gotRequirePromoted = o.RequirePromoted - f.gotNoEgress = o.NoEgress - if f.claim == nil { - return &types.Sandbox{ID: "sb_1", Token: "tok", Volumes: slices.Clone(o.Volumes)}, nil - } - return f.claim(ctx, key, o.TTL) -} diff --git a/sandboxd/server/volume_mutation.go b/sandboxd/server/volume_mutation.go new file mode 100644 index 00000000..fb7eca9e --- /dev/null +++ b/sandboxd/server/volume_mutation.go @@ -0,0 +1,69 @@ +package server + +import ( + "context" + "errors" + "net/http" + + "github.com/projecteru2/core/log" + + "github.com/cocoonstack/sandbox/sandboxd/engine" + "github.com/cocoonstack/sandbox/sandboxd/pool" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +func (s *Server) handleSandboxVolumes(w http.ResponseWriter, r *http.Request) { + id, tenant := r.PathValue("id"), tenantFrom(r.Context()) + var ( + view types.SandboxVolumesResponse + err error + ) + switch r.Method { + case http.MethodGet: + view, err = s.mgr.GetVolumes(id, tenant) + case http.MethodPut: + req, ok := decodeBodyStrict[types.SetVolumesRequest](w, r) + if !ok { + return + } + if req.Volumes == nil { + writeErr(w, http.StatusBadRequest, "volumes must be an array") + return + } + view, err = s.mgr.SetVolumes(r.Context(), id, tenant, req.Volumes) + case http.MethodPatch: + req, ok := decodeBodyStrict[types.VolumePatch](w, r) + if !ok { + return + } + view, err = s.mgr.PatchVolumes(r.Context(), id, tenant, req) + } + if errors.Is(err, pool.ErrUnknownSandbox) { + writePoolErr(w, err) + return + } + code := http.StatusOK + if err != nil { + code, view.Error = http.StatusInternalServerError, "volume convergence failed; inspect the volume states before retrying" + conflict, isConflict := errors.AsType[*engine.VolumeMountConflict](err) + unsupported, isUnsupported := errors.AsType[*engine.LiveVolumeUnsupported](err) + switch { + case isConflict: + code, view.Error = http.StatusConflict, conflict.Error() + case isUnsupported: + code, view.Error = http.StatusServiceUnavailable, unsupported.Error() + case errors.Is(err, context.DeadlineExceeded): + code, view.Error = http.StatusGatewayTimeout, "volume change timed out; inspect the volume states before retrying" + case errors.Is(err, context.Canceled): + code, view.Error = http.StatusRequestTimeout, "volume change canceled; inspect the volume states before retrying" + default: + if mapped, msg, ok := mapPoolErr(w, err); ok { + code, view.Error = mapped, msg + } + } + if code >= http.StatusInternalServerError { + log.WithFunc("server.handleSandboxVolumes").Errorf(r.Context(), err, "converge volumes for %s", id) + } + } + writeJSON(w, code, view) +} diff --git a/sandboxd/server/volume_mutation_test.go b/sandboxd/server/volume_mutation_test.go new file mode 100644 index 00000000..0bdfa22c --- /dev/null +++ b/sandboxd/server/volume_mutation_test.go @@ -0,0 +1,174 @@ +package server + +import ( + "context" + "encoding/json/v2" + "errors" + "fmt" + "io" + "net/http" + "slices" + "strings" + "testing" + + "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/engine" + "github.com/cocoonstack/sandbox/sandboxd/pool" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +func TestLiveVolumeRoutesAuthorizeAndDecode(t *testing.T) { + for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodPatch} { + for _, cred := range []struct{ token, tenant string }{{"acme-token", "acme"}, {"root-token", ""}} { + t.Run(method+"/"+cred.token, func(t *testing.T) { + called := false + check := func(id, tenant string) { + called = true + if id != "sb_1" || tenant != cred.tenant { + t.Errorf("scope %s %q", id, tenant) + } + } + view := types.SandboxVolumesResponse{VolumesWant: []types.Volume{{Name: "data"}}, States: []types.VolumeState{{Name: "data", State: "pending"}}} + mgr := &fakeManager{ + getVolumes: func(id, tenant string) (types.SandboxVolumesResponse, error) { + check(id, tenant) + return view, nil + }, + setVolumes: func(id, tenant string, v []types.Volume) (types.SandboxVolumesResponse, error) { + check(id, tenant) + if !slices.Equal(v, []types.Volume{{Name: "data"}}) { + t.Errorf("volumes %v", v) + } + return view, nil + }, + patchVolumes: func(id, tenant string, p types.VolumePatch) (types.SandboxVolumesResponse, error) { + check(id, tenant) + if !slices.Equal(p.Remove, []string{"data"}) { + t.Errorf("patch %v", p) + } + return view, nil + }, + } + ts := newTenantTestServer(t, "root-token", []config.TenantSpec{{Name: "acme", Token: "acme-token"}}, mgr, nil) + body := `{"volumes":[{"name":"data"}]}` + if method == http.MethodPatch { + body = `{"remove":["data"]}` + } + resp := doReq(t, method, ts.URL+"/v1/sandboxes/sb_1/volumes", cred.token, body) + defer resp.Body.Close() + var got types.SandboxVolumesResponse + if err := json.UnmarshalRead(resp.Body, &got); err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusOK || !called || len(got.VolumesWant) != 1 || got.States[0].State != "pending" { + t.Fatalf("status=%d called=%v view=%+v", resp.StatusCode, called, got) + } + }) + } + } +} + +func TestLiveVolumeRoutesErrors(t *testing.T) { + for _, tt := range []struct { + name, token, body string + err error + code int + }{ + {"missing token", "", `{"volumes":[]}`, nil, http.StatusUnauthorized}, + {"unknown field", "root", `{"volumes":[],"path":"/host"}`, nil, http.StatusBadRequest}, + {"missing volumes", "root", `{}`, nil, http.StatusBadRequest}, + {"null volumes", "root", `{"volumes":null}`, nil, http.StatusBadRequest}, + {"wrong sandbox", "root", `{"volumes":[]}`, pool.ErrUnknownSandbox, http.StatusNotFound}, + {"bad volume", "root", `{"volumes":[]}`, pool.ErrBadVolume, http.StatusBadRequest}, + {"paused", "root", `{"volumes":[]}`, pool.ErrPaused, http.StatusConflict}, + {"failed", "root", `{"volumes":[]}`, pool.ErrFailed, http.StatusConflict}, + {"sandbox token", "sandbox-token", `{"volumes":[]}`, nil, http.StatusUnauthorized}, + {"invalid token", "invalid", `{"volumes":[]}`, nil, http.StatusUnauthorized}, + {"foreign mount", "root", `{"volumes":[]}`, &engine.VolumeMountConflict{Message: `volume mount "/data" has a different device, root, or mode; restore the expected mount before retrying`}, http.StatusConflict}, + {"bind mount", "root", `{"volumes":[]}`, &engine.VolumeMountConflict{Message: `volume device has another mount at "/other"; remove guest mount references before retrying`}, http.StatusConflict}, + {"unmount refused", "root", `{"volumes":[]}`, &engine.VolumeMountConflict{Message: `cannot unmount "/data"; close guest references and retry`}, http.StatusConflict}, + {"unsupported runtime", "root", `{"volumes":[]}`, &engine.LiveVolumeUnsupported{Version: "v0.7.1"}, http.StatusServiceUnavailable}, + {"tenant store down", "root", `{"volumes":[]}`, fmt.Errorf("%w: connect host /private/pg", pool.ErrTenantStoreDown), http.StatusServiceUnavailable}, + {"unknown runtime", "root", `{"volumes":[]}`, &engine.LiveVolumeUnsupported{}, http.StatusServiceUnavailable}, + {"timeout", "root", `{"volumes":[]}`, context.DeadlineExceeded, http.StatusGatewayTimeout}, + {"cancel", "root", `{"volumes":[]}`, context.Canceled, http.StatusRequestTimeout}, + {"engine", "root", `{"volumes":[]}`, errors.New("host /private/image failed"), http.StatusInternalServerError}, + } { + t.Run(tt.name, func(t *testing.T) { + mgr := &fakeManager{setVolumes: func(string, string, []types.Volume) (types.SandboxVolumesResponse, error) { + return types.SandboxVolumesResponse{VolumesWant: []types.Volume{{Name: "data"}}, States: []types.VolumeState{{Name: "data", State: "pending", Error: "volume operation failed; retry after resolving the runtime error"}}}, tt.err + }} + ts := newTestServer(t, "root", mgr, nil) + resp := doReq(t, http.MethodPut, ts.URL+"/v1/sandboxes/sb_1/volumes", tt.token, tt.body) + defer resp.Body.Close() + body, _ := io.ReadAll(resp.Body) + if resp.StatusCode != tt.code || strings.Contains(string(body), "/private") { + t.Fatalf("status=%d body=%s", resp.StatusCode, body) + } + if errors.Is(tt.err, pool.ErrTenantStoreDown) && resp.Header.Get("Retry-After") == "" { + t.Fatal("store outage without Retry-After") + } + if tt.err != nil && tt.code != http.StatusNotFound { + var got types.SandboxVolumesResponse + if err := json.Unmarshal(body, &got); err != nil || len(got.VolumesWant) != 1 || len(got.States) != 1 || got.Error == "" || got.States[0].Error != "volume operation failed; retry after resolving the runtime error" { + t.Fatalf("lost partial state: %s %v", body, err) + } + } + }) + } +} + +func TestLiveVolumeRoutesHideOtherTenants(t *testing.T) { + for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodPatch} { + t.Run(method, func(t *testing.T) { + called := false + check := func(id, tenant string) (types.SandboxVolumesResponse, error) { + called = true + if id != "sb_1" || tenant != "other" { + t.Errorf("scope %s %q", id, tenant) + } + return types.SandboxVolumesResponse{}, pool.ErrUnknownSandbox + } + mgr := &fakeManager{ + getVolumes: check, + setVolumes: func(id, tenant string, _ []types.Volume) (types.SandboxVolumesResponse, error) { + return check(id, tenant) + }, + patchVolumes: func(id, tenant string, _ types.VolumePatch) (types.SandboxVolumesResponse, error) { + return check(id, tenant) + }, + } + ts := newTenantTestServer(t, "root", []config.TenantSpec{{Name: "other", Token: "other-token"}}, mgr, nil) + body := `{"volumes":[]}` + if method == http.MethodPatch { + body = `{}` + } + resp := doReq(t, method, ts.URL+"/v1/sandboxes/sb_1/volumes", "other-token", body) + defer resp.Body.Close() + if resp.StatusCode != http.StatusNotFound || !called { + t.Fatalf("status=%d called=%v", resp.StatusCode, called) + } + }) + } +} + +func TestLiveVolumeRoutesReportRefusalsOverRecordedDiagnostics(t *testing.T) { + const cause = "cannot unmount /data; close guest references and retry" + for _, refusal := range []error{pool.ErrVolumeBusy, pool.ErrVolumeCapture} { + t.Run(refusal.Error(), func(t *testing.T) { + mgr := &fakeManager{setVolumes: func(string, string, []types.Volume) (types.SandboxVolumesResponse, error) { + return types.SandboxVolumesResponse{VolumesWant: []types.Volume{{Name: "old"}}, States: []types.VolumeState{{Name: "old", State: "pending", Error: cause}}, Error: "volume change failed; retry the desired target"}, fmt.Errorf("%w: volume %q", refusal, "data") + }} + ts := newTestServer(t, "root", mgr, nil) + resp := doReq(t, http.MethodPut, ts.URL+"/v1/sandboxes/sb_1/volumes", "root", `{"volumes":[]}`) + defer resp.Body.Close() + var got types.SandboxVolumesResponse + if err := json.UnmarshalRead(resp.Body, &got); err != nil { + t.Fatal(err) + } + if resp.StatusCode != http.StatusConflict || !strings.HasPrefix(got.Error, refusal.Error()) || len(got.States) != 1 || got.States[0].Error != cause { + t.Fatalf("status=%d view=%+v", resp.StatusCode, got) + } + }) + } +} diff --git a/sandboxd/types/api.go b/sandboxd/types/api.go index f397db44..a3828440 100644 --- a/sandboxd/types/api.go +++ b/sandboxd/types/api.go @@ -5,6 +5,12 @@ import ( "time" ) +const ( + VolumeAttached VolumePhase = "attached" + VolumePending VolumePhase = "pending" + VolumeDetaching VolumePhase = "detaching" +) + // TTLField is the shared requested-lease field; zero means the server default. type TTLField struct { TTLSeconds int `json:"ttl_seconds,omitzero"` @@ -77,6 +83,36 @@ type VolumeListResponse struct { Volumes []VolumeInfo `json:"volumes"` } +// SetVolumesRequest is the wire body of PUT /v1/sandboxes/{id}/volumes. +type SetVolumesRequest struct { + Volumes []Volume `json:"volumes"` +} + +// VolumePatch is the wire body of PATCH /v1/sandboxes/{id}/volumes. +type VolumePatch struct { + Add []Volume `json:"add,omitempty"` + Remove []string `json:"remove,omitempty"` +} + +// VolumePhase is how far one held volume has converged. +type VolumePhase string + +// VolumeState reports one held volume and its last failure. +type VolumeState struct { + Name string `json:"name"` + State VolumePhase `json:"state"` + Error string `json:"error,omitempty"` +} + +// SandboxVolumesResponse is the wire reply of the sandbox volume routes, failures included. +type SandboxVolumesResponse struct { + VolumesWant []Volume `json:"volumes_want"` + Volumes []Volume `json:"volumes"` + States []VolumeState `json:"states"` + Converging bool `json:"converging"` + Error string `json:"error,omitempty"` +} + // ForkRequest is the wire body of POST /v1/sandboxes/{id}/fork. type ForkRequest struct { Token string `json:"token"` diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 699db9cc..755c312b 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -559,6 +559,8 @@ type Sandbox struct { OnExpire ExpireAction `json:"on_expire,omitempty"` // Volumes records the volumes successfully applied to this claim. Volumes []Volume `json:"volumes,omitempty"` + // VolumesWant is the durable target, including additions not yet confirmed. + VolumesWant []Volume `json:"volumes_want"` // Env is replaced whole through SetEnv under the manager mutex; host-only values are never served back. Env Env `json:"env,omitempty"` @@ -597,7 +599,7 @@ type Sandbox struct { // live is the env as of the last SetEnv, read by the egress proxy without the manager mutex. live atomic.Pointer[envView] - // Transition serializes hibernate/wake; lock it before (never under) the manager mutex. + // Transition serializes captures and live volume changes; lock it before (never under) the manager mutex. Transition sync.Mutex `json:"-"` } diff --git a/scripts/sandboxd-e2e.sh b/scripts/sandboxd-e2e.sh index 1e6f8cb6..2a4ae02b 100755 --- a/scripts/sandboxd-e2e.sh +++ b/scripts/sandboxd-e2e.sh @@ -16,6 +16,8 @@ VOLUME_DIRECTIO=${VOLUME_DIRECTIO:-off} # VOLUME_RW_IMAGE adds the writable leg; its own image is mutated by design, so it must not be VOLUME_IMAGE. VOLUME_RW_IMAGE=${VOLUME_RW_IMAGE:-} VOLUME_RW_NAME=${VOLUME_RW_NAME:-e2e-scratch} +# VOLUME_LIVE=1 adds the live volume leg, which needs cocoon v0.7.2. +VOLUME_LIVE=${VOLUME_LIVE:-} VOLUME_CHECKSUM="" if [[ -n $VOLUME_IMAGE ]]; then @@ -192,7 +194,7 @@ if [[ -n $VOLUME_IMAGE ]]; then done warm_before=$(warm_claims) "$DATA/volumesmoke" -addr "$ADDR" -token "$TOKEN" -template "$TEMPLATE" \ - -volume "$VOLUME_NAME" -probe "$VOLUME_PROBE" ${VOLUME_RW_IMAGE:+-rw-volume "$VOLUME_RW_NAME"} + -volume "$VOLUME_NAME" -probe "$VOLUME_PROBE" ${VOLUME_RW_IMAGE:+-rw-volume "$VOLUME_RW_NAME"} ${VOLUME_LIVE:+-live} warm_after=$(warm_claims) [[ $warm_after -ge $((warm_before + 2)) ]] || { echo "volume claims did not consume the two ready warm VMs: before=$warm_before after=$warm_after" diff --git a/sdk/go/client.go b/sdk/go/client.go index ac6be60d..ccb932fd 100644 --- a/sdk/go/client.go +++ b/sdk/go/client.go @@ -12,6 +12,7 @@ import ( "io" "net/http" "net/url" + "slices" "strings" "sync" "time" @@ -250,7 +251,7 @@ func (c *Client) ownerAt(ctx context.Context, addr, id, token string) (string, e func (c *Client) handleFrom(dialed string, cr claimResponse) *Sandbox { return &Sandbox{ - ID: cr.ID, Deadline: cr.Deadline, Volumes: cr.Volumes, + ID: cr.ID, Deadline: cr.Deadline, Volumes: cr.Volumes, VolumesWant: slices.Clone(cr.Volumes), FromCheckpoint: cr.FromCheckpoint, TemplateDigest: cr.TemplateDigest, NetRoute: cr.NetRoute, c: c, token: cr.Token, owner: cmp.Or(cr.OwnerAddr, dialed), } diff --git a/sdk/go/info.go b/sdk/go/info.go index b3a657ea..86879cd3 100644 --- a/sdk/go/info.go +++ b/sdk/go/info.go @@ -62,6 +62,7 @@ type SandboxSummary struct { Archived bool `json:"archived,omitzero"` FromCheckpoint string `json:"from_checkpoint,omitempty"` Volumes []Volume `json:"volumes,omitempty"` + VolumesWant []Volume `json:"volumes_want,omitempty"` ClaimRef string `json:"claim_ref,omitempty"` Metadata map[string]string `json:"metadata,omitempty"` CPUCount int `json:"cpu_count,omitzero"` diff --git a/sdk/go/sandbox.go b/sdk/go/sandbox.go index fea6b3e1..0a3ceb34 100644 --- a/sdk/go/sandbox.go +++ b/sdk/go/sandbox.go @@ -56,9 +56,10 @@ func (e *ExitError) Error() string { type Sandbox struct { ID string Deadline time.Time - // Volumes lists the volumes finalized on this claim; only a writable entry - // echoes a mode. + // Volumes lists confirmed applied volumes; only a writable entry echoes a mode. Volumes []Volume + // VolumesWant is the persisted desired set, which may differ after a failed convergence. + VolumesWant []Volume // TemplateDigest is the content identity of the promoted-template export // this sandbox was cloned from; empty for any other source. TemplateDigest string diff --git a/sdk/go/volume_mutation_test.go b/sdk/go/volume_mutation_test.go new file mode 100644 index 00000000..7f65427c --- /dev/null +++ b/sdk/go/volume_mutation_test.go @@ -0,0 +1,190 @@ +package sandbox + +import ( + "context" + "encoding/json/v2" + "errors" + "io" + "net/http" + "net/http/httptest" + "reflect" + "slices" + "testing" +) + +func TestLiveVolumesUseOwnerAndAPIToken(t *testing.T) { + for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodPatch} { + t.Run(method, func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != method || r.URL.Path != "/v1/sandboxes/sb_1/volumes" || r.Header.Get("Authorization") != "Bearer operator-token" { + t.Errorf("request: %s %s %s", r.Method, r.URL.Path, r.Header.Get("Authorization")) + } + body, _ := io.ReadAll(r.Body) + want := "" + switch method { + case http.MethodPut: + want = `{"volumes":[{"name":"data"}]}` + case http.MethodPatch: + want = `{"add":[{"name":"data"}],"remove":["old"]}` + } + if string(body) != want { + t.Errorf("body=%s want=%s", body, want) + } + _, _ = io.WriteString(w, `{"volumes_want":[{"name":"data","mount":"/data"}],"volumes":[{"name":"data","mount":"/data"}],"converging":true,"states":[{"name":"data","state":"attached"}]}`) + })) + t.Cleanup(ts.Close) + c := testClient(t, ts, WithAPIToken("operator-token")) + sb := &Sandbox{ID: "sb_1", token: "sandbox-token", owner: c.addr, c: c} + c.addr = "unused.invalid:7777" + got, err := callVolumes(t, sb, method) + if err != nil || !slices.Equal(got.Volumes, []Volume{{Name: "data", Mount: "/data"}}) || !slices.Equal(got.VolumesWant, got.Volumes) || got.States[0].State != VolumeAttached || !got.Converging { + t.Fatalf("got=%+v err=%v", got, err) + } + got.Volumes[0].Name, got.VolumesWant[0].Name = "caller copy", "caller copy" + if sb.Volumes[0].Name != "data" || sb.VolumesWant[0].Name != "data" { + t.Fatal("returned volumes alias public state") + } + }) + } +} + +func TestLiveVolumeFailureRetainsPartialView(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, `{"volumes_want":[{"name":"new","mount":"/new"}],"volumes":[{"name":"old","mount":"/old"}],"converging":false,"states":[{"name":"new","state":"pending","error":"retry"},{"name":"old","state":"detaching"}],"error":"convergence failed"}`) + })) + t.Cleanup(ts.Close) + c := testClient(t, ts) + for _, method := range []string{http.MethodGet, http.MethodPut, http.MethodPatch} { + sb := &Sandbox{ID: "sb_1", token: "tok", owner: c.addr, c: c} + got, err := callVolumes(t, sb, method) + failure, isVolume := errors.AsType[*VolumeError](err) + api, isAPI := errors.AsType[*APIError](err) + if !isVolume || !isAPI || api.Status != http.StatusInternalServerError || failure.Response == nil { + t.Fatalf("%s error lost response or APIError: %v", method, err) + } + if !reflect.DeepEqual(got, *failure.Response) || got.States[0].State != "pending" || got.States[1].State != "detaching" || sb.VolumesWant[0].Name != "new" || sb.Volumes[0].Name != "old" { + t.Fatalf("partial view=%+v handle=%+v", got, sb) + } + failure.Response.Volumes[0].Name = "error caller" + if got.Volumes[0].Name != "old" || sb.Volumes[0].Name != "old" { + t.Fatal("error response aliases returned or handle state") + } + } +} + +func TestLiveVolumeValidationFailurePreservesLastKnownSet(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusBadRequest) + _, _ = io.WriteString(w, `{"error":"invalid volume"}`) + })) + t.Cleanup(ts.Close) + c := testClient(t, ts) + sb := &Sandbox{ID: "sb_1", token: "tok", owner: c.addr, c: c, Volumes: []Volume{{Name: "old"}}, VolumesWant: []Volume{{Name: "wanted"}}} + _, err := sb.SetVolumes(t.Context(), []Volume{{Name: "new"}}) + if failure, ok := errors.AsType[*VolumeError](err); !ok || failure.Response != nil || failure.Status != http.StatusBadRequest { + t.Fatalf("error=%v", err) + } + if sb.Volumes[0].Name != "old" || sb.VolumesWant[0].Name != "wanted" { + t.Fatal("validation error replaced last known view") + } +} + +func TestLiveVolumesExplicitEmptyAndCancellation(t *testing.T) { + var bodies []map[string]any + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var body map[string]any + if err := json.UnmarshalRead(r.Body, &body); err != nil { + t.Error(err) + } + bodies = append(bodies, body) + _, _ = io.WriteString(w, `{"volumes_want":[],"volumes":[],"converging":false,"states":[]}`) + })) + t.Cleanup(ts.Close) + c := testClient(t, ts) + sb := &Sandbox{ID: "sb_1", owner: c.addr, c: c} + if _, err := sb.SetVolumes(t.Context(), nil); err == nil { + t.Fatal("nil desired set was accepted") + } + if _, err := sb.SetVolumes(t.Context(), []Volume{}); err != nil { + t.Fatal(err) + } + if _, err := sb.PatchVolumes(t.Context(), VolumePatch{}); err != nil { + t.Fatal(err) + } + if _, err := sb.PatchVolumes(t.Context(), VolumePatch{Add: []Volume{}, Remove: []string{}}); err != nil { + t.Fatal(err) + } + if len(bodies) != 3 || !reflect.DeepEqual(bodies[0], map[string]any{"volumes": []any{}}) || len(bodies[1]) != 0 || !reflect.DeepEqual(bodies[2], map[string]any{"add": []any{}, "remove": []any{}}) { + t.Fatalf("requests=%+v", bodies) + } + if sb.Volumes == nil || sb.VolumesWant == nil { + t.Fatal("explicitly empty response became absent") + } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if _, err := sb.GetVolumes(ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("canceled context returned %v", err) + } +} + +func TestLiveVolumesRetrySameDesiredSet(t *testing.T) { + var requests []string + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + requests = append(requests, string(body)) + if len(requests) == 1 { + w.WriteHeader(http.StatusInternalServerError) + _, _ = io.WriteString(w, `{"volumes_want":[{"name":"data","mount":"/data"}],"volumes":[],"converging":false,"states":[{"name":"data","state":"pending"}],"error":"retry"}`) + return + } + _, _ = io.WriteString(w, `{"volumes_want":[{"name":"data","mount":"/data"}],"volumes":[{"name":"data","mount":"/data"}],"converging":false,"states":[{"name":"data","state":"attached"}]}`) + })) + t.Cleanup(ts.Close) + c := testClient(t, ts) + sb := &Sandbox{ID: "sb_1", token: "tok", owner: c.addr, c: c} + target := []Volume{{Name: "data", Mount: "/data"}} + if _, err := sb.SetVolumes(t.Context(), target); err == nil || len(sb.Volumes) != 0 || !slices.Equal(sb.VolumesWant, target) { + t.Fatalf("failed convergence lost desired set: want=%v applied=%v error=%v", sb.VolumesWant, sb.Volumes, err) + } + view, err := sb.SetVolumes(t.Context(), target) + if err != nil || !slices.Equal(view.Volumes, target) || !slices.Equal(sb.Volumes, target) { + t.Fatalf("retry did not converge: %+v error=%v", view, err) + } + if len(requests) != 2 || requests[0] != requests[1] { + t.Fatalf("retry changed target: %v", requests) + } +} + +func TestLiveVolumeIncompleteViewPreservesHandle(t *testing.T) { + for _, status := range []int{http.StatusOK, http.StatusConflict} { + t.Run(http.StatusText(status), func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + _, _ = io.WriteString(w, `{"volumes_want":[],"volumes":[],"states":[]}`) + })) + t.Cleanup(ts.Close) + c := testClient(t, ts) + sb := &Sandbox{ID: "sb_1", owner: c.addr, c: c, Volumes: []Volume{{Name: "old"}}, VolumesWant: []Volume{{Name: "wanted"}}} + _, err := sb.GetVolumes(t.Context()) + if err == nil || sb.Volumes[0].Name != "old" || sb.VolumesWant[0].Name != "wanted" { + t.Fatalf("handle overwritten: %+v, %v", sb, err) + } + if failure, ok := errors.AsType[*VolumeError](err); status == http.StatusConflict && (!ok || failure.Response != nil) { + t.Fatalf("partial view accepted: %v", err) + } + }) + } +} + +func callVolumes(t *testing.T, sb *Sandbox, method string) (SandboxVolumesResponse, error) { + t.Helper() + switch method { + case http.MethodPut: + return sb.SetVolumes(t.Context(), []Volume{{Name: "data"}}) + case http.MethodPatch: + return sb.PatchVolumes(t.Context(), VolumePatch{Add: []Volume{{Name: "data"}}, Remove: []string{"old"}}) + default: + return sb.GetVolumes(t.Context()) + } +} diff --git a/sdk/go/volumes.go b/sdk/go/volumes.go new file mode 100644 index 00000000..69e1dea4 --- /dev/null +++ b/sdk/go/volumes.go @@ -0,0 +1,118 @@ +package sandbox + +import ( + "bytes" + "context" + "encoding/json/v2" + "errors" + "fmt" + "io" + "net/http" + "slices" +) + +const ( + VolumeAttached VolumePhase = "attached" + VolumePending VolumePhase = "pending" + VolumeDetaching VolumePhase = "detaching" +) + +// VolumePhase is the progress of a desired or applied volume. +type VolumePhase string + +// VolumeState reports the progress of one desired or applied volume. +type VolumeState struct { + Name string `json:"name"` + State VolumePhase `json:"state"` + Error string `json:"error,omitempty"` +} + +// SandboxVolumesResponse separates persisted intent from confirmed applied mounts. +type SandboxVolumesResponse struct { + VolumesWant []Volume `json:"volumes_want"` + Volumes []Volume `json:"volumes"` + States []VolumeState `json:"states"` + Converging bool `json:"converging"` + Error string `json:"error,omitempty"` +} + +// VolumePatch adds and removes volumes by name; a nil slice changes nothing. +type VolumePatch struct { + Add []Volume `json:"add,omitzero"` + Remove []string `json:"remove,omitzero"` +} + +// VolumeError reports a failed volume request; Response is nil unless the reply carried the full view. +type VolumeError struct { + *APIError + Response *SandboxVolumesResponse +} + +func (e *VolumeError) Unwrap() error { return e.APIError } + +// GetVolumes reads the owning node's view and updates public fields; callers must serialize volume methods. +func (s *Sandbox) GetVolumes(ctx context.Context) (SandboxVolumesResponse, error) { + return s.requestVolumes(ctx, http.MethodGet, nil, "get volumes") +} + +// SetVolumes converges the full desired set; an explicit empty slice clears mutable mounts, and nil is rejected. +func (s *Sandbox) SetVolumes(ctx context.Context, volumes []Volume) (SandboxVolumesResponse, error) { + if volumes == nil { + return SandboxVolumesResponse{}, errors.New("set volumes: use an explicit empty slice to clear volumes") + } + return s.requestVolumes(ctx, http.MethodPut, struct { + Volumes []Volume `json:"volumes"` + }{Volumes: volumes}, "set volumes") +} + +// PatchVolumes adds or removes read-only mounted entries by name, retaining immutable entries. +func (s *Sandbox) PatchVolumes(ctx context.Context, patch VolumePatch) (SandboxVolumesResponse, error) { + return s.requestVolumes(ctx, http.MethodPatch, patch, "patch volumes") +} + +func (s *Sandbox) requestVolumes(ctx context.Context, method string, request any, verb string) (SandboxVolumesResponse, error) { + var out SandboxVolumesResponse + var body io.Reader + if request != nil { + encoded, err := encodeBody(verb, request) + if err != nil { + return out, err + } + body = bytes.NewReader(encoded) + } + resp, err := s.c.roundTrip(ctx, method, s.owner, "/v1/sandboxes/"+s.ID+"/volumes", body, s.c.apiToken) + if err != nil { + return out, err + } + defer func() { _ = resp.Body.Close() }() + var wire struct { + VolumesWant *[]Volume `json:"volumes_want"` + Volumes *[]Volume `json:"volumes"` + States *[]VolumeState `json:"states"` + Converging *bool `json:"converging"` + Error string `json:"error"` + } + decodeErr := json.UnmarshalRead(resp.Body, &wire) + complete := decodeErr == nil && wire.VolumesWant != nil && wire.Volumes != nil && wire.States != nil && wire.Converging != nil + if complete { + out = SandboxVolumesResponse{VolumesWant: *wire.VolumesWant, Volumes: *wire.Volumes, States: *wire.States, Converging: *wire.Converging, Error: wire.Error} + s.VolumesWant = slices.Clone(out.VolumesWant) + s.Volumes = slices.Clone(out.Volumes) + } + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + failure := &VolumeError{APIError: &APIError{Verb: verb, Status: resp.StatusCode, Message: wire.Error}} + if complete { + view := out + view.VolumesWant, view.Volumes, view.States = slices.Clone(out.VolumesWant), slices.Clone(out.Volumes), slices.Clone(out.States) + failure.Response = &view + } + return out, failure + } + if decodeErr != nil { + return out, fmt.Errorf("decode %s response: %w", verb, decodeErr) + } + if !complete { + return out, fmt.Errorf("decode %s response: missing volume view", verb) + } + return out, nil +} diff --git a/sdk/python/cocoonsandbox/__init__.py b/sdk/python/cocoonsandbox/__init__.py index 0a14923e..449f37de 100644 --- a/sdk/python/cocoonsandbox/__init__.py +++ b/sdk/python/cocoonsandbox/__init__.py @@ -11,9 +11,18 @@ from .checkpoint import Checkpoint from .client import Client -from .errors import APIError, ExitError, ProtocolError, SandboxError, SandboxTimeout, SilkdError +from .errors import APIError, ExitError, ProtocolError, SandboxError, SandboxTimeout, SilkdError, VolumeError from .sandbox import Lsp, PortConn, Pty, Sandbox, Session, Watcher from .template import Template +from .volumes import ( + VOLUME_ATTACHED, + VOLUME_DETACHING, + VOLUME_PENDING, + SandboxVolumesResponse, + Volume, + VolumePhase, + VolumeState, +) __all__ = [ "APIError", @@ -27,8 +36,16 @@ "Sandbox", "SandboxError", "SandboxTimeout", + "SandboxVolumesResponse", "Session", "SilkdError", "Template", + "VOLUME_ATTACHED", + "VOLUME_DETACHING", + "VOLUME_PENDING", + "Volume", + "VolumeError", + "VolumePhase", + "VolumeState", "Watcher", ] diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index 6bc282c1..cbf29602 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/sdk/python/cocoonsandbox/client.py @@ -17,6 +17,7 @@ from .endpoint import _endpoint_url from .errors import APIError, SandboxTimeout, require_field from .sandbox import Sandbox +from .volumes import _volume_body T = TypeVar("T") @@ -212,7 +213,11 @@ def _request( raise SandboxTimeout(f"{verb} timed out") from None raise APIError(verb, 0, str(exc)) from None if status >= 400: - raise APIError(verb, status, _error_message(raw)) + try: + reply = json.loads(raw) + except (ValueError, TypeError): + reply = None + raise APIError(verb, status, _error_message(raw), reply=reply if isinstance(reply, dict) else None) if not raw: return {} try: @@ -299,29 +304,6 @@ def _claim_body( return claim -def _volume_body(volume: str | Mapping[str, str], mount: bool) -> dict[str, str]: - if isinstance(volume, str): - return {"name": volume} - if not isinstance(volume, Mapping): - raise TypeError("volume must be a name string or mapping") - unknown = sorted(set(volume) - {"name", "mount", "mode"}) - if unknown: - raise TypeError( - f"volume mapping accepts only name, mount, and mode, got unexpected key(s): {', '.join(unknown)}" - ) - if not mount and "mount" in volume: - raise TypeError("volume mount is meaningless with mount=False, which leaves mounting to the caller") - body = dict(volume) - mode = body.get("mode") - if mode in (None, "", "ro"): - body.pop("mode", None) - elif mode != "rw": - name = volume.get("name") - prefix = f"volume {name!r}: " if name is not None else "" - raise TypeError(f"{prefix}mode must be 'rw' or 'ro', got {mode!r}") - return body - - def _template_query(template: str, net: str, size: str) -> dict[str, str]: query = {"template": template} if net: diff --git a/sdk/python/cocoonsandbox/errors.py b/sdk/python/cocoonsandbox/errors.py index e81f1473..4e1ed4dc 100644 --- a/sdk/python/cocoonsandbox/errors.py +++ b/sdk/python/cocoonsandbox/errors.py @@ -1,6 +1,11 @@ """Error types raised by the SDK.""" -from typing import Any +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +if TYPE_CHECKING: + from .volumes import SandboxVolumesResponse class SandboxError(Exception): @@ -14,11 +19,20 @@ class SandboxTimeout(SandboxError, TimeoutError): class APIError(SandboxError): """A sandboxd control-plane call failed.""" - def __init__(self, verb: str, status: int, message: str) -> None: + def __init__(self, verb: str, status: int, message: str, *, reply: dict[str, Any] | None = None) -> None: super().__init__(f"{verb}: {message} (HTTP {status})") self.verb = verb self.status = status self.message = message + self.reply = reply + + +class VolumeError(APIError): + """A volume request failed; response holds the server's complete view when one came back.""" + + def __init__(self, error: APIError, response: SandboxVolumesResponse | None) -> None: + super().__init__(error.verb, error.status, error.message, reply=error.reply) + self.response = response class SilkdError(SandboxError): diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index 35ba426f..1a83fa86 100644 --- a/sdk/python/cocoonsandbox/sandbox.py +++ b/sdk/python/cocoonsandbox/sandbox.py @@ -7,14 +7,24 @@ import threading import time from collections import deque -from collections.abc import Callable, Iterator +from collections.abc import Callable, Iterator, Mapping from typing import TYPE_CHECKING, Any, cast from .checkpoint import Checkpoint from .conn import Conn, ConnPool, _Closeable, remaining_timeout -from .errors import APIError, ExitError, ProtocolError, SandboxError, SandboxTimeout, SilkdError, require_field +from .errors import ( + APIError, + ExitError, + ProtocolError, + SandboxError, + SandboxTimeout, + SilkdError, + VolumeError, + require_field, +) from .frames import BULK_CHUNK, FS_CHUNK, KEEP_ALIVE_PROTO from .template import Template +from .volumes import SandboxVolumesResponse, Volume, _volume_body, _volume_response if TYPE_CHECKING: from .client import Client @@ -45,6 +55,7 @@ def __init__( self.from_checkpoint = from_checkpoint self.template_digest = template_digest self.volumes = [dict(volume) for volume in volumes or []] + self.volumes_want = [dict(volume) for volume in self.volumes] self._pool = ConnPool(client.keep_alive) self._proto = 0 @@ -58,6 +69,37 @@ def __exit__(self, *exc: object) -> None: if exc[0] is None: # a clean block surfaces a real release failure raise + def get_volumes(self, *, timeout: float | None = None) -> SandboxVolumesResponse: + """Reads the desired and applied volumes from the owning node and records them on this handle.""" + return self._volumes_request("GET", None, "get volumes", timeout) + + def set_volumes( + self, volumes: list[str | Mapping[str, str] | Volume], *, timeout: float | None = None + ) -> SandboxVolumesResponse: + """Replaces the desired set and converges; an explicit empty list clears mutable mounts.""" + if volumes is None or isinstance(volumes, str): + raise TypeError("set volumes requires an explicit list; use [] to clear volumes") + return self._volumes_request( + "PUT", {"volumes": [_volume_body(volume, True) for volume in volumes]}, "set volumes", timeout + ) + + def patch_volumes( + self, + *, + add: list[str | Mapping[str, str] | Volume] | None = None, + remove: list[str] | None = None, + timeout: float | None = None, + ) -> SandboxVolumesResponse: + """Adds or removes mounted read-only entries by name; omitted arguments leave the set unchanged.""" + if isinstance(add, str) or isinstance(remove, str): + raise TypeError("patch volumes takes lists of volumes and names") + body: dict[str, Any] = {} + if add is not None: + body["add"] = [_volume_body(volume, True) for volume in add] + if remove is not None: + body["remove"] = list(remove) + return self._volumes_request("PATCH", body, "patch volumes", timeout) + def exec( self, *argv: str, @@ -375,6 +417,37 @@ def close(self) -> None: if exc.status != 404: raise + def _volumes_request( + self, method: str, body: dict[str, Any] | None, verb: str, timeout: float | None + ) -> SandboxVolumesResponse: + if timeout is not None and timeout <= 0: + raise ValueError("timeout must be positive") + try: + reply = self._client._request( + self.owner, + method, + f"/v1/sandboxes/{self.id}/volumes", + body, + verb, + timeout=timeout or 0.0, + ) + except APIError as exc: + if exc.status == 0: + raise + view = _volume_response(exc.reply) + if view is not None: + self._record_volumes(view) + raise VolumeError(exc, view) from exc + view = _volume_response(reply) + if view is None: + raise APIError(verb, 0, "reply without complete volume view") + self._record_volumes(view) + return view + + def _record_volumes(self, view: SandboxVolumesResponse) -> None: + self.volumes_want = [cast(dict[str, str], volume.copy()) for volume in view["volumes_want"]] + self.volumes = [cast(dict[str, str], volume.copy()) for volume in view["volumes"]] + def _dial(self, deadline: float | None = None) -> Conn: return self._client._dial(self.owner, self.id, self.token, deadline) diff --git a/sdk/python/cocoonsandbox/volumes.py b/sdk/python/cocoonsandbox/volumes.py new file mode 100644 index 00000000..ce156554 --- /dev/null +++ b/sdk/python/cocoonsandbox/volumes.py @@ -0,0 +1,79 @@ +"""Volume descriptors and live convergence views.""" + +from __future__ import annotations + +from collections.abc import Mapping +from typing import Any, Final, Literal, TypedDict, cast + +VolumePhase = Literal["attached", "pending", "detaching"] +VOLUME_ATTACHED: Final[VolumePhase] = "attached" +VOLUME_PENDING: Final[VolumePhase] = "pending" +VOLUME_DETACHING: Final[VolumePhase] = "detaching" + + +class _VolumeName(TypedDict): + name: str + + +class Volume(_VolumeName, total=False): + """A catalog volume by name, with its guest mount point and an `rw` mode for writable claims.""" + + mount: str + mode: str + + +class _VolumeState(TypedDict): + name: str + state: VolumePhase + + +class VolumeState(_VolumeState, total=False): + """One held volume's convergence phase and its last failure, if any.""" + + error: str + + +class _SandboxVolumesResponse(TypedDict): + volumes_want: list[Volume] + volumes: list[Volume] + states: list[VolumeState] + converging: bool + + +class SandboxVolumesResponse(_SandboxVolumesResponse, total=False): + """The desired and applied sets of a sandbox, with per-volume states; error names a refused request.""" + + error: str + + +def _volume_response(reply: dict[str, Any] | None) -> SandboxVolumesResponse | None: + if ( + reply is None + or not all(isinstance(reply.get(key), list) for key in ("volumes_want", "volumes", "states")) + or not isinstance(reply.get("converging"), bool) + ): + return None + return cast(SandboxVolumesResponse, reply) + + +def _volume_body(volume: str | Mapping[str, str] | Volume, mount: bool) -> dict[str, str]: + if isinstance(volume, str): + return {"name": volume} + if not isinstance(volume, Mapping): + raise TypeError("volume must be a name string or mapping") + unknown = sorted(set(volume) - {"name", "mount", "mode"}) + if unknown: + raise TypeError( + f"volume mapping accepts only name, mount, and mode, got unexpected key(s): {', '.join(unknown)}" + ) + if not mount and "mount" in volume: + raise TypeError("volume mount is meaningless with mount=False, which leaves mounting to the caller") + body = dict(cast(Mapping[str, str], volume)) + mode = body.get("mode") + if mode in (None, "", "ro"): + body.pop("mode", None) + elif mode != "rw": + name = volume.get("name") + prefix = f"volume {name!r}: " if name is not None else "" + raise TypeError(f"{prefix}mode must be 'rw' or 'ro', got {mode!r}") + return body diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index c8d1d892..171fca23 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -27,6 +27,12 @@ def do_GET(self): def do_DELETE(self): self._dispatch("DELETE") + def do_PUT(self): + self._dispatch("PUT") + + def do_PATCH(self): + self._dispatch("PATCH") + def log_message(self, *args): pass diff --git a/sdk/python/tests/test_volumes.py b/sdk/python/tests/test_volumes.py new file mode 100644 index 00000000..3226b82d --- /dev/null +++ b/sdk/python/tests/test_volumes.py @@ -0,0 +1,166 @@ +"""Live volume HTTP contract and partial convergence recovery.""" + +import pytest +from conftest import FakeNode, sandbox_at + +from cocoonsandbox import APIError, VolumeError + +PATH = "/v1/sandboxes/sb_1/volumes" +VIEW = { + "volumes_want": [{"name": "data", "mount": "/data"}], + "volumes": [{"name": "data", "mount": "/data"}], + "converging": True, + "states": [{"name": "data", "state": "attached"}], +} + + +@pytest.mark.parametrize("method", ["GET", "PUT", "PATCH"]) +def test_volume_requests_use_owner_and_api_token(spawn_node, method): + seen = [] + + def reply(body, path): + seen.append(body) + return 200, VIEW + + addr = spawn_node({(method, PATH): reply}) + sb = sandbox_at(addr, api_token="operator-token") + sb._client.addr = "unused.invalid:7777" + if method == "GET": + got = sb.get_volumes() + assert seen == [{}] + elif method == "PUT": + got = sb.set_volumes(["data"]) + assert seen == [{"volumes": [{"name": "data"}]}] + else: + got = sb.patch_volumes(add=["data"], remove=["old"]) + assert seen == [{"add": [{"name": "data"}], "remove": ["old"]}] + assert FakeNode.last_headers["Authorization"] == "Bearer operator-token" + assert got == VIEW + assert sb.volumes == VIEW["volumes"] + assert sb.volumes_want == VIEW["volumes_want"] + got["volumes"][0]["name"] = "caller copy" + got["volumes_want"][0]["name"] = "caller copy" + assert sb.volumes[0]["name"] == sb.volumes_want[0]["name"] == "data" + + +@pytest.mark.parametrize("method", ["GET", "PUT", "PATCH"]) +def test_volume_error_preserves_typed_partial_view(spawn_node, method): + partial = { + "volumes_want": [{"name": "new", "mount": "/new"}], + "volumes": [{"name": "old", "mount": "/old"}], + "converging": False, + "states": [ + {"name": "new", "state": "pending", "error": "retry"}, + {"name": "old", "state": "detaching"}, + ], + "error": "convergence failed", + } + sb = sandbox_at(spawn_node({(method, PATH): lambda body, path: (500, partial)})) + with pytest.raises(VolumeError) as caught: + if method == "GET": + sb.get_volumes() + elif method == "PUT": + sb.set_volumes(["new"]) + else: + sb.patch_volumes(remove=["old"]) + assert isinstance(caught.value, APIError) + assert caught.value.status == 500 + assert caught.value.response == partial + assert sb.volumes_want == partial["volumes_want"] + assert sb.volumes == partial["volumes"] + caught.value.response["volumes"][0]["name"] = "caller copy" + assert sb.volumes[0]["name"] == "old" + + +def test_validation_error_preserves_last_view(spawn_node): + sb = sandbox_at(spawn_node({("PUT", PATH): lambda body, path: (400, {"error": "invalid volume"})})) + sb.volumes = [{"name": "old"}] + sb.volumes_want = [{"name": "wanted"}] + with pytest.raises(VolumeError) as caught: + sb.set_volumes(["new"]) + assert caught.value.response is None + assert sb.volumes == [{"name": "old"}] + assert sb.volumes_want == [{"name": "wanted"}] + + +def test_explicit_empty_set_and_omitted_patch(spawn_node): + seen = [] + + def reply(body, path): + seen.append(body) + return 200, {"volumes_want": [], "volumes": [], "converging": False, "states": []} + + sb = sandbox_at(spawn_node({("PUT", PATH): reply, ("PATCH", PATH): reply})) + with pytest.raises(TypeError, match="explicit"): + sb.set_volumes(None) + sb.set_volumes([]) + sb.patch_volumes() + sb.patch_volumes(add=[], remove=[]) + assert seen == [{"volumes": []}, {}, {"add": [], "remove": []}] + assert sb.volumes == sb.volumes_want == [] + + +def test_identical_put_retries_partial_failure(spawn_node): + requests = [] + + def reply(body, path): + requests.append(body) + if len(requests) == 1: + return 500, { + "volumes_want": [{"name": "data", "mount": "/data"}], + "volumes": [], + "converging": False, + "states": [{"name": "data", "state": "pending"}], + "error": "retry", + } + return 200, VIEW + + sb = sandbox_at(spawn_node({("PUT", PATH): reply})) + with pytest.raises(VolumeError): + sb.set_volumes([{"name": "data", "mount": "/data"}]) + assert sb.volumes == [] + got = sb.set_volumes([{"name": "data", "mount": "/data"}]) + assert got == VIEW + assert requests[0] == requests[1] + + +def test_volume_timeout_bounds_the_call(black_hole): + sb = sandbox_at(black_hole, timeout=2) + with pytest.raises(APIError) as caught: + sb.get_volumes(timeout=0.02) + assert caught.value.status == 0 + + +@pytest.mark.parametrize("timeout", [0, -1]) +def test_volume_methods_reject_non_positive_timeout(black_hole, timeout): + sb = sandbox_at(black_hole) + with pytest.raises(ValueError): + sb.get_volumes(timeout=timeout) + + +@pytest.mark.parametrize("status", [200, 409]) +def test_incomplete_volume_view_preserves_handle(spawn_node, status): + incomplete = {"volumes_want": [], "volumes": [], "states": []} + sb = sandbox_at(spawn_node({("GET", PATH): lambda body, path: (status, incomplete)})) + sb.volumes = [{"name": "old"}] + sb.volumes_want = [{"name": "wanted"}] + with pytest.raises(APIError) as caught: + sb.get_volumes() + assert sb.volumes == [{"name": "old"}] + assert sb.volumes_want == [{"name": "wanted"}] + if status == 409: + assert isinstance(caught.value, VolumeError) + assert caught.value.response is None + + +@pytest.mark.parametrize( + "call", + [ + lambda sb: sb.set_volumes("data"), + lambda sb: sb.patch_volumes(add="data"), + lambda sb: sb.patch_volumes(remove="old"), + ], +) +def test_volume_methods_reject_a_bare_string(black_hole, call): + with pytest.raises(TypeError): + call(sandbox_at(black_hole))