diff --git a/docs/deploy.md b/docs/deploy.md index 15099a0..d92a0da 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -219,6 +219,18 @@ with concurrent readers. proceed; a crashed writer leaves it, and `ro` claims are refused until one `rw` claim replays and releases cleanly. +**Attach-only claims opt out of the marker, not out of admission.** A claim +sent with `volumes_attach_only` gets the device attached and nothing else, so +sandboxd neither writes nor clears `.dirty` for it — it cannot verify a +mount it did not perform. Everything in this section still applies to the +default, mounting claims, and the exclusion rules above apply to attach-only +claims exactly the same way, which is what keeps other tenants safe. The +operator-visible difference: an image whose attach-only writer released +without unmounting cleanly carries no marker, so the next `ro` claim is +admitted and fails at mount time (500) instead of being refused early (409). +Whoever hands out attach-only `rw` access owns that trade; see +[sandboxd-api](sandboxd-api.md#attach-only-volumes). + ### A fuller config The block above is the minimum. A production node with tenants, guarded diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index c1bb360..76fe4ba 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -47,6 +47,12 @@ Auth: `Authorization: Bearer ` (when configured). `mode` is `"ro"` (default, omitted) or `"rw"`; `"rw"` requires the catalog entry's `writable: true` (see [deploy](deploy.md#dataset-volumes)). Volumes require Cloud Hypervisor +- `volumes_attach_only` (default `false`) attaches every requested volume + without mounting it, handing the whole mount contract to the workload. It + requires at least one volume, and rejects any entry carrying a `mount` — + the path is meaningless when the caller mounts the device. Everything below + describes the default, eager behaviour, which is completely unchanged; the + attach-only contract is in [its own section](#attach-only-volumes) Success: @@ -75,6 +81,51 @@ filesystem — read-only, unless the entry requested and was granted `rw` — before returning. A custom mount may shadow an existing populated guest directory for the claim's life. +### Attach-only volumes + +`"volumes_attach_only": true` stops after the attach. The device appears in +the guest, nothing is mounted, and the echoed entries carry no `mount` key: + +```json +{"id": "sb_…", "token": "…", + "volumes": [{"name": "imagenet"}, {"name": "scratch-db", "mode": "rw"}]} +``` + +Find each device by its virtio serial, which is the catalog name: + +```sh +for dev in /sys/block/*/serial; do + [ "$(cat "$dev")" = scratch-db ] && echo "/dev/$(basename "$(dirname "$dev")")" +done +``` + +Then mount it however the workload needs. A `ro` entry is attached +`--readonly` and stays read-only at the guest block layer no matter who +mounts it, so the guarantee does not depend on your mount flags. + +The whole consistency contract moves to the caller with the mount: + +- sandboxd writes no dirty marker for an attach-only `rw` claim and clears + none, because it cannot verify your unmount and therefore promises nothing. + Releasing without a clean unmount of your own leaves the image exactly as a + filesystem crash would. The next *eager* `ro` claim then fails at mount time + with a 500 — loud and attributable to the claim that skipped its unmount — + instead of the marker's 409. Recovery is any `rw` cycle that replays the + journal. +- a marker left by an earlier *eager* `rw` crash is not cleared by attach-only + cycles, and still refuses eager `ro` claims with 409 until a `rw` claim + releases cleanly. +- writes straight to the block device bypass the filesystem journal entirely + and are outside marker protection in either mode. + +What still protects other claims is unchanged: admission excludes an +attach-only `rw` claim against every other claim of the name (and readers +against a writer), an attach-only `ro` claim of a marker-bearing image is +refused with 409, and a claim with volumes still refuses checkpoint, fork and +hibernate. An attach-only claim costs one disk attach per volume — no device +settle, no mount round-trip — and release costs nothing at all: removing the +VM closes the devices. + Redirects (mutually exclusive with the fields above) name peers to retry at — sent on a warm miss with warm peers, when the node lacks a golden for the key but gossip names a template owner, and when the node is at @@ -102,7 +153,9 @@ name (attributed in the usage journal and counted against the tenant's `max_claims`). A catalog access list may restrict an entry to named tenants; an unknown and a forbidden volume return the same error text. -Errors: 400 unknown template axis, invalid/duplicate volumes, `mode: "rw"` +Errors: 400 unknown template axis, invalid/duplicate volumes, +`volumes_attach_only` with no volumes or with an entry carrying a `mount`, +`mode: "rw"` against a non-writable entry, or a volume that is unknown or forbidden (the latter two are deliberately indistinguishable), Firecracker with volumes, or bad body; 401 bad api token; 409 egress requested on a node without an egress diff --git a/docs/sdk-python.md b/docs/sdk-python.md index b44b70c..81af4ee 100644 --- a/docs/sdk-python.md +++ b/docs/sdk-python.md @@ -96,6 +96,7 @@ sb = client.new("ghcr.io/cocoonstack/sandbox/rt:24.04", | `net` | `"none"`, `"egress"` | `"none"` | Cloud Hypervisor network shape: `none` disables the NIC and uses vsock-only I/O; `egress` attaches a bridge/CNI NIC | | `size` | `"small"`, `"medium"`, `"large"`, `"xlarge"` | `"small"` | resource tier: 1cpu/512M, 2cpu/1G, 4cpu/4G, 4cpu/8G | | `volumes` | bare names or `{name, mount?, mode?}` mappings | `None` | attach and mount up to eight unique catalog dataset disks; an omitted mount defaults to `/volumes/`; `mode` is `"ro"` (default) or `"rw"` — `"rw"` requires the catalog entry's `writable: true`; accepted by `Client.new` and `Template.new` | +| `mount` | bool | `True` | mount every requested volume. `False` attaches the devices and leaves the mounting to the workload; a mapping carrying `mount` is then a `TypeError` | | `ttl_seconds` | int | server default 5m | sandbox TTL, server-capped at 24h. The node reaps the sandbox after the TTL even if the client vanishes | `new` returns when the sandbox's silkd answers: a warm hit is milliseconds, @@ -113,6 +114,16 @@ an error — double-release and reap races stay silent). Volume sandboxes cannot hibernate, fork, checkpoint, or promote. Checkpoint branches do not accept volumes of either mode in this version. +`mount=False` on `client.new` or `template.new` claims the same volumes +without mounting them: the dictionaries in `sb.volumes` carry no `mount` key, +and the workload finds each device by polling `/sys/block/*/serial` for the +catalog name. Everything above describes the default (`mount=True`) and is +unchanged by it. What changes is that the mount and its consistency are +entirely yours: sandboxd writes and clears no dirty marker for an attach-only +`rw` claim, because it cannot verify your unmount, so releasing without +unmounting cleanly leaves the image as a crash would — see +[sandboxd-api](sandboxd-api.md#attach-only-volumes) for the full contract. + The caller-visible constraints are deliberate: volume claims may consume a warm VM, remain non-capturable, mount read-only by default, and require Cloud Hypervisor. diff --git a/docs/sdk.md b/docs/sdk.md index c0b86af..5f18612 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -144,6 +144,7 @@ defer sb.Close() | `WithNetwork(n)` | `NetNone`, `NetEgress` | `NetNone` | Cloud Hypervisor network shape: `NetNone` disables the NIC and uses vsock-only I/O; `NetEgress` attaches a bridge/CNI NIC | | `WithSize(s)` | `Small`, `Medium`, `Large`, `XLarge` | `Small` | resource tier: 1cpu/512M, 2cpu/1G, 4cpu/4G, 4cpu/8G | | `WithVolumes(volumes...)` | `Volume{Name, Mount?, Mode?}` entries | none | attach and mount up to eight unique catalog dataset disks; `Mount` defaults to `/volumes/`; `Mode` is `"ro"` (default) or `"rw"` — `"rw"` requires the catalog entry's `writable: true`; supported by `Client.New` and `Template.New` | +| `WithVolumesAttachOnly()` | — | mount | attach the requested volumes without mounting them; the workload finds each device and owns the mount. Rejects a `Volume.Mount` locally | | `WithTimeout(d)` | duration | server default 5m | sandbox TTL, rounded up to seconds, server-capped at 24h. The node reaps the sandbox after the TTL even if the client vanishes | `New` returns when the sandbox's silkd answers: a warm hit is milliseconds, @@ -163,6 +164,16 @@ Volume sandboxes cannot hibernate, fork, checkpoint, or promote. Passing `WithVolumes` to `Checkpoint.New` returns a local error because checkpoint branches do not support volumes of either mode in this version. +`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. Everything +above describes the default and is unchanged by this option. What changes is +that the mount and its consistency are entirely yours: sandboxd writes and +clears no dirty marker for an attach-only `rw` claim, because it cannot verify +your unmount, so releasing without unmounting cleanly leaves the image as a +crash would — see +[sandboxd-api](sandboxd-api.md#attach-only-volumes) for the full contract. + The caller-visible constraints are deliberate: volume claims may consume a warm VM, remain non-capturable, mount read-only by default, and require Cloud Hypervisor. diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index 84916a1..be7bb87 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -6,6 +6,7 @@ package e2e import ( "encoding/json" + "errors" "fmt" "io" "maps" @@ -426,6 +427,99 @@ func TestVolumeModeWireShape(t *testing.T) { } } +// TestAttachOnlyVolumeEndToEnd drives one attach-only writable claim through +// the whole stack: the device is attached writable and nothing else happens — +// no mount, no marker, no unmount at release — while admission still excludes +// every other claim on the name, which is what protects third parties. +func TestAttachOnlyVolumeEndToEnd(t *testing.T) { + scratch := writeVolumeImage(t, "scratch.img", "scratch-bytes") + stack := startTenantStack(t, "node-token", nil, + []config.VolumeSpec{{Name: "scratch", Path: scratch, Writable: true}}) + + sb, err := stack.client.New(t.Context(), "rt:24.04", + sandbox.WithVolumes(sandbox.Volume{Name: "scratch", Mode: "rw"}), + sandbox.WithVolumesAttachOnly()) + if err != nil { + t.Fatalf("attach-only claim: %v", err) + } + if want := []sandbox.Volume{{Name: "scratch", Mode: "rw"}}; !slices.Equal(sb.Volumes, want) { + t.Errorf("claim volumes %+v, want %+v", sb.Volumes, want) + } + applied := []string{"attach:scratch:rw"} + if got := stack.eng.volumeOpsLog(); !slices.Equal(got, applied) { + t.Errorf("engine ops %v, want %v", got, applied) + } + assertNoDirtyMarker(t, scratch, "apply") + for _, requested := range []string{`{"name":"scratch"}`, `{"name":"scratch","mode":"rw"}`} { + if status, _ := rawClaim(t, stack, requested); status != http.StatusConflict { + t.Errorf("claim %s under a live attach-only writer: %d, want 409", requested, status) + } + } + + if err := sb.Close(); err != nil { + t.Fatalf("Close: %v", err) + } + want := slices.Concat(applied, []string{"remove"}) + if got := stack.eng.volumeOpsLog(); !slices.Equal(got, want) { + t.Errorf("engine ops after release %v, want %v", got, want) + } + assertNoDirtyMarker(t, scratch, "release") +} + +// TestAttachOnlyVolumeWireShape pins both claim replies' volume bytes: an +// attach-only entry echoes without a mount, the eager entry is unchanged, and +// the request flag never rides back in either. +func TestAttachOnlyVolumeWireShape(t *testing.T) { + scratch := writeVolumeImage(t, "scratch.img", "scratch-bytes") + stack := startTenantStack(t, "node-token", nil, + []config.VolumeSpec{{Name: "scratch", Path: scratch, Writable: true}}) + for _, tt := range []struct { + name string + request string + want map[string]any + }{ + { + "attach-only omits the mount", + `{"template":"rt:24.04","volumes_attach_only":true,"volumes":[{"name":"scratch","mode":"rw"}]}`, + map[string]any{"name": "scratch", "mode": "rw"}, + }, + { + "eager claim is unchanged", + `{"template":"rt:24.04","volumes":[{"name":"scratch","mode":"rw"}]}`, + map[string]any{"name": "scratch", "mount": "/volumes/scratch", "mode": "rw"}, + }, + } { + t.Run(tt.name, func(t *testing.T) { + status, body := rawJSON(t, stack, http.MethodPost, "/v1/claim", tt.request) + if status != http.StatusOK { + t.Fatalf("claim: %d %s, want 200", status, body) + } + var reply map[string]any + if err := json.Unmarshal(body, &reply); err != nil { + t.Fatalf("decode claim %s: %v", body, err) + } + if _, leaked := reply["volumes_attach_only"]; leaked { + t.Errorf("reply %s carries the request flag", body) + } + entries, _ := reply["volumes"].([]any) + if len(entries) != 1 { + t.Fatalf("reply volumes %v, want one entry", reply["volumes"]) + } + entry, _ := entries[0].(map[string]any) + if !maps.Equal(entry, tt.want) { + t.Errorf("reply volume %v, want %v", entry, tt.want) + } + var claimed rawClaimResponse + if err := json.Unmarshal(body, &claimed); err != nil { + t.Fatalf("decode claim %s: %v", body, err) + } + if err := stack.client.Attach(stack.addr, claimed.ID, claimed.Token).Close(); err != nil { + t.Fatalf("release: %v", err) + } + }) + } +} + // TestDirtyVolumeRefusesReader: the marker a crashed writer leaves behind // (pre-created here) turns read-only claims into 409s over the wire. func TestDirtyVolumeRefusesReader(t *testing.T) { @@ -504,6 +598,15 @@ func writeVolumeImage(t *testing.T, name, content string) string { return image } +// assertNoDirtyMarker fails if the image carries the write-ahead marker: an +// attach-only claim makes no consistency promise, so it must never write one. +func assertNoDirtyMarker(t *testing.T, image, when string) { + t.Helper() + if _, err := os.Stat(image + ".dirty"); !errors.Is(err, os.ErrNotExist) { + t.Errorf("dirty marker at %s: stat=%v, want no marker", when, err) + } +} + // rawClaimResponse decodes the volume entries generically, so the assertion // is the server's own JSON rather than the SDK's mirror of it. type rawClaimResponse struct { diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index bc3d861..f96eb95 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -56,7 +56,7 @@ func (m *Manager) ClaimWarm(ctx context.Context, key types.PoolKey, ttl time.Dur } m.kickRefill() if volumeErr := m.applyVolumes(ctx, sb, volumeSpecs, applied); volumeErr != nil { - m.destroy(ctx, sb.VMName) + m.abortVolumeClaim(ctx, sb.VMName, &reserved) return nil, volumeErr } sb.Tenant = tenant @@ -284,10 +284,9 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t if quotaErr := m.quotaErr(len(sbs), sbs[0].Tenant); quotaErr != nil { m.mu.Unlock() for _, sb := range sbs { - // No quiesce: the claim was never handed out, so nothing wrote to - // the guest and the marker converges on the next writable claim. - m.unreserveVolumes(sb.Volumes) - m.destroy(ctx, sb.VMName) + // No quiesce: never handed out, so nothing wrote to the guest and the + // marker waits for the next writable claim; the holds ride the removal. + m.abortVolumeClaim(ctx, sb.VMName, &sb.Volumes) } return quotaErr } @@ -338,6 +337,13 @@ func (m *Manager) rollbackClaim(ctx context.Context, sbs []*types.Sandbox) { } } +// abortVolumeClaim removes a failed claim's VM, holds riding the teardown; +// clearing reserved stops the deferred unreserve from double-releasing them. +func (m *Manager) abortVolumeClaim(ctx context.Context, vmName string, reserved *[]types.Volume) { + m.removeOrRetry(ctx, vmName, "", "", volumeTeardown{holds: *reserved}) + *reserved = nil +} + func (m *Manager) reapOnce(ctx context.Context) { now := time.Now() type victim struct { @@ -529,7 +535,7 @@ func (m *Manager) claimProvision(ctx context.Context, key types.PoolKey, ttl tim return nil, err } if volumeErr := m.applyVolumes(ctx, sb, volumeSpecs, applied); volumeErr != nil { - m.destroy(ctx, sb.VMName) + m.abortVolumeClaim(ctx, sb.VMName, &reserved) return nil, volumeErr } sb.TemplateDigest = golden.templateDigest diff --git a/sandboxd/pool/volume.go b/sandboxd/pool/volume.go index 1984457..826751d 100644 --- a/sandboxd/pool/volume.go +++ b/sandboxd/pool/volume.go @@ -139,7 +139,9 @@ func (m *Manager) resolveVolumes(key types.PoolKey, tenant string, requested []t if len(requested) == 0 { return nil, nil } - applied, err := types.ValidateVolumes(requested) + // An attach-only claim arrives with no mounts to default: re-defaulting + // them here would mount what the caller asked to mount itself. + applied, err := types.ValidateVolumes(requested, types.VolumesAttachOnly(requested)) if err != nil { return nil, fmt.Errorf("%w: %v", ErrBadVolume, err) } @@ -160,6 +162,7 @@ func (m *Manager) resolveVolumes(key types.PoolKey, tenant string, requested []t } 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}) } return resolved, nil @@ -204,8 +207,10 @@ func (m *Manager) applyVolumes(ctx context.Context, sb *types.Sandbox, volumes [ // applyVolume keeps one volume's steps strictly ordered; siblings overlap freely. func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume resolvedVolume) 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. - if volume.disk.RW { + if volume.disk.RW && !attachOnly { if err := markVolumeDirty(volume.disk.Path); err != nil { return fmt.Errorf("mark volume %q dirty: %w", volume.applied.Name, err) } @@ -213,6 +218,9 @@ func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume res if err := m.eng.DiskAttach(ctx, sb.VMName, volume.disk); err != nil { return fmt.Errorf("attach volume %q: %w", volume.applied.Name, err) } + if attachOnly { + return nil + } if err := m.eng.MountVolume(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) } @@ -222,10 +230,12 @@ func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume res // quiesceVolumes unmounts a claim's writable mounts in reverse order and // returns the teardown its VM removal must finish. A failed unmount never // blocks teardown: the image keeps its marker and waits for a recovering -// writer. Runs while the guest is still live, before the VM is removed. +// writer. Attach-only entries have no mount to bring down — removing the VM +// closes their devices. Runs while the guest is still live, before the VM is +// removed. func (m *Manager) quiesceVolumes(ctx context.Context, sb *types.Sandbox) volumeTeardown { td := volumeTeardown{holds: sb.Volumes} - mounts := len(types.VolumeRWNames(sb.Volumes)) + mounts := writableMounts(sb.Volumes) if mounts == 0 { return td } @@ -235,7 +245,7 @@ func (m *Manager) quiesceVolumes(ctx context.Context, sb *types.Sandbox) volumeT defer cancel() stuck := false for _, volume := range slices.Backward(sb.Volumes) { - if !volume.RW() { + if !volume.RW() || volume.Mount == "" { continue } if err := m.eng.UnmountVolume(ctx, sb.VsockSocket, volume.Mount); err != nil { @@ -330,6 +340,16 @@ func quiesceBudget(mounts int) time.Duration { return min(time.Duration(mounts+1)*engine.VolumeCallTimeout, volumeQuiesceMax) } +func writableMounts(volumes []types.Volume) int { + mounts := 0 + for _, volume := range volumes { + if volume.RW() && volume.Mount != "" { + mounts++ + } + } + return mounts +} + func appliedVolumes(volumes []resolvedVolume) []types.Volume { applied := make([]types.Volume, len(volumes)) for i, volume := range volumes { diff --git a/sandboxd/pool/volume_attach_test.go b/sandboxd/pool/volume_attach_test.go new file mode 100644 index 0000000..2018c8f --- /dev/null +++ b/sandboxd/pool/volume_attach_test.go @@ -0,0 +1,386 @@ +package pool + +import ( + "errors" + "slices" + "strings" + "sync" + "testing" + + "github.com/cocoonstack/sandbox/sandboxd/config" + "github.com/cocoonstack/sandbox/sandboxd/types" +) + +// TestAttachOnlyClaimAttachesWithoutMounting: the whole mount contract belongs +// to the caller, so a writable attach-only claim leaves no marker to clear and +// no mount to quiesce — the device only goes away with the VM. +func TestAttachOnlyClaimAttachesWithoutMounting(t *testing.T) { + path := writeVolumeImage(t, "scratch.img", "data") + eng := newFakeEngine() + m := newVolumeManager(t, eng, []config.VolumeSpec{{Name: "scratch", Path: path, Writable: true}}) + + sb, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW, AttachOnly: true}, + }) + if err != nil { + t.Fatalf("ClaimProvision: %v", err) + } + want := []types.Volume{{Name: "scratch", Mode: types.VolumeModeRW}} + if !slices.Equal(sb.Volumes, want) { + t.Errorf("applied volumes=%v, want %v", sb.Volumes, want) + } + if specs := eng.volumeSpecs; len(specs) != 1 || !specs[0].RW || specs[0].Path != path { + t.Errorf("attached specs=%+v, want one writable disk at %s", specs, path) + } + if len(eng.volumeMounts) != 0 { + t.Errorf("mounts=%v, want none", eng.volumeMounts) + } + if volumeDirty(path) { + t.Error("attach-only claim marked the image dirty, promising a flush it cannot make") + } + if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{writers: 1}) { + t.Errorf("registry=%+v, want the writer counted", holders) + } + + if err := m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Fatalf("Release: %v", err) + } + if ops := eng.volumeOpsLog(); slices.ContainsFunc(ops, func(op string) bool { + return strings.HasPrefix(op, "umount:") || op == "sync" + }) { + t.Errorf("release ran %v, want no unmount or sync", ops) + } + if got := eng.syncCount(); got != 0 { + t.Errorf("guest syncs=%d, want none", got) + } + if volumeDirty(path) { + t.Error("release of an attach-only claim left a marker") + } + if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{}) { + t.Errorf("registry after release=%+v, want empty", holders) + } +} + +func TestAttachOnlyClaimAttachesEveryVolumeConcurrently(t *testing.T) { + dataset := writeVolumeImage(t, "dataset.img", "dataset") + scratch := writeVolumeImage(t, "scratch.img", "scratch") + cache := writeVolumeImage(t, "cache.img", "cache") + eng := newFakeEngine() + m := newVolumeManager(t, eng, []config.VolumeSpec{ + {Name: "dataset", Path: dataset}, + {Name: "scratch", Path: scratch, Writable: true}, + {Name: "cache", Path: cache, Writable: true}, + }) + requested := []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW, AttachOnly: true}, + {Name: "dataset", AttachOnly: true}, + {Name: "cache", Mode: types.VolumeModeRW, AttachOnly: true}, + } + // Every attach must be in flight at once: a sequential apply blocks here. + var attaches sync.WaitGroup + attaches.Add(len(requested)) + eng.attachRendezvous = &attaches + + sb, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", requested) + if err != nil { + t.Fatalf("ClaimProvision: %v", err) + } + want := []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW}, + {Name: "dataset"}, + {Name: "cache", Mode: types.VolumeModeRW}, + } + if !slices.Equal(sb.Volumes, want) { + t.Errorf("volumes=%v, want the request order %v", sb.Volumes, want) + } + wantOps := []string{"attach:cache", "attach:dataset", "attach:scratch", "probe", "provision"} + if ops := slices.Sorted(slices.Values(eng.volumeOpsLog())); !slices.Equal(ops, wantOps) { + t.Errorf("volume ops=%v, want exactly %v", eng.volumeOpsLog(), wantOps) + } + if volumeDirty(scratch) || volumeDirty(cache) { + t.Error("attach-only writers marked their images dirty") + } +} + +func TestClaimProvisionAttachFailureKeepsHoldsUntilRemoval(t *testing.T) { + scratch := writeVolumeImage(t, "scratch.img", "scratch") + broken := writeVolumeImage(t, "broken.img", "broken") + eng := newFakeEngine() + eng.diskAttachErrFor = "broken" + eng.removeStall = make(chan struct{}) + var attaches sync.WaitGroup + attaches.Add(2) + eng.attachRendezvous = &attaches + m := newVolumeManager(t, eng, []config.VolumeSpec{ + {Name: "scratch", Path: scratch, Writable: true}, + {Name: "broken", Path: broken}, + }) + requested := []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW, AttachOnly: true}, + {Name: "broken", AttachOnly: true}, + } + + claimErr := make(chan error, 1) + go func() { + _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", requested) + claimErr <- err + }() + attaches.Wait() + eng.mu.Lock() + vmCount := len(eng.vms) + var vmName string + for name := range eng.vms { + vmName = name + } + eng.removeErrFor = vmName + eng.attachRendezvous = nil + eng.mu.Unlock() + close(eng.removeStall) + err := <-claimErr + + if vmCount != 1 { + t.Fatalf("live VMs before cleanup=%d, want 1", vmCount) + } + if err == nil || !strings.Contains(err.Error(), `attach volume "broken"`) { + t.Fatalf("ClaimProvision: %v, want the failing volume's attach error", err) + } + if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{writers: 1}) { + t.Errorf("scratch registry after failed removal=%+v, want writer retained", holders) + } + if holders := volumeHoldersOf(m, "broken"); holders != (volumeHolders{readers: 1}) { + t.Errorf("broken registry after failed removal=%+v, want reader retained", holders) + } + if _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", requested[:1]); !errors.Is(err, ErrVolumeBusy) { + t.Errorf("claim against surviving VM: %v, want ErrVolumeBusy", err) + } + + eng.removeErrFor = "" + m.retryRemovals(t.Context()).Wait() + + if !eng.removed(vmName) { + t.Errorf("removes=%v, want %s drained", eng.removedNames(), vmName) + } + for _, name := range []string{"scratch", "broken"} { + if holders := volumeHoldersOf(m, name); holders != (volumeHolders{}) { + t.Errorf("registry for %s after retry=%+v, want empty", name, holders) + } + } +} + +func TestClaimWarmAttachFailureKeepsHoldsUntilRemoval(t *testing.T) { + scratch := writeVolumeImage(t, "scratch.img", "scratch") + broken := writeVolumeImage(t, "broken.img", "broken") + eng := newFakeEngine() + eng.diskAttachErrFor = "broken" + eng.removeErrFor = "sbx-warm" + eng.vms["sbx-warm"] = "/vsock/warm" + var attaches sync.WaitGroup + attaches.Add(2) + eng.attachRendezvous = &attaches + m := newVolumePoolManager(t, eng, t.TempDir(), []config.VolumeSpec{ + {Name: "scratch", Path: scratch, Writable: true}, + {Name: "broken", Path: broken}, + }) + warm := &types.Sandbox{VMName: "sbx-warm", Key: testKey, VsockSocket: "/vsock/warm"} + m.pools[testKey].warm = append(m.pools[testKey].warm, warm) + requested := []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW, AttachOnly: true}, + {Name: "broken", AttachOnly: true}, + } + + _, err := m.ClaimWarm(t.Context(), testKey, 0, "", "", requested) + if err == nil || !strings.Contains(err.Error(), `attach volume "broken"`) { + t.Fatalf("ClaimWarm: %v, want the failing volume's attach error", err) + } + if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{writers: 1}) { + t.Errorf("scratch registry after failed removal=%+v, want writer retained", holders) + } + if holders := volumeHoldersOf(m, "broken"); holders != (volumeHolders{readers: 1}) { + t.Errorf("broken registry after failed removal=%+v, want reader retained", holders) + } + if _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", requested[:1]); !errors.Is(err, ErrVolumeBusy) { + t.Errorf("claim against surviving warm VM: %v, want ErrVolumeBusy", err) + } + + eng.removeErrFor = "" + m.retryRemovals(t.Context()).Wait() + + if !eng.removed(warm.VMName) { + t.Errorf("removes=%v, want %s drained", eng.removedNames(), warm.VMName) + } + for _, name := range []string{"scratch", "broken"} { + if holders := volumeHoldersOf(m, name); holders != (volumeHolders{}) { + t.Errorf("registry for %s after retry=%+v, want empty", name, holders) + } + } +} + +// TestFinalizeQuotaFailureKeepsHoldsUntilRemoval: the finalize re-check loses +// the quota race with the volumes already mounted, so its holds must outlive +// the claim exactly as an attach failure's do — until the VM is confirmed gone. +func TestFinalizeQuotaFailureKeepsHoldsUntilRemoval(t *testing.T) { + scratch := writeVolumeImage(t, "scratch.img", "scratch") + eng := newFakeEngine() + m := newVolumeManager(t, eng, []config.VolumeSpec{{Name: "scratch", Path: scratch, Writable: true}}) + m.maxClaims = 1 + // The volume claim parks in its attach, so the volume-less claim below always + // takes the last slot and the volume claim always loses the finalize re-check. + var attaches sync.WaitGroup + attaches.Add(2) + eng.attachRendezvous = &attaches + writable := []types.Volume{{Name: "scratch", Mode: types.VolumeModeRW}} + + claimErr := make(chan error, 1) + go func() { + _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", writable) + claimErr <- err + }() + waitFor(t, func() bool { return slices.Contains(eng.volumeOpsLog(), "attach:scratch") }) + eng.mu.Lock() + vmCount := len(eng.vms) + var vmName string + for name := range eng.vms { + vmName = name + } + eng.mu.Unlock() + filler, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", nil) + if err != nil { + t.Fatalf("volume-less claim: %v", err) + } + eng.mu.Lock() + eng.removeErrFor = vmName // the quota loser's VM survives its removal + eng.attachRendezvous = nil + eng.mu.Unlock() + attaches.Done() + err = <-claimErr + + if vmCount != 1 { + t.Fatalf("live VMs at the attach=%d, want only the volume claim's", vmCount) + } + if !errors.Is(err, ErrQuota) { + t.Fatalf("volume claim: %v, want ErrQuota from the finalize re-check", err) + } + if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{writers: 1}) { + t.Errorf("registry after failed removal=%+v, want writer retained", holders) + } + if err := m.Release(t.Context(), filler.ID, Cred{Token: filler.Token}); err != nil { + t.Fatalf("release the volume-less claim: %v", err) + } + if _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", writable); !errors.Is(err, ErrVolumeBusy) { + t.Errorf("claim against the surviving VM: %v, want ErrVolumeBusy", err) + } + + eng.mu.Lock() + eng.removeErrFor = "" + eng.mu.Unlock() + m.retryRemovals(t.Context()).Wait() + + if !eng.removed(vmName) { + t.Errorf("removes=%v, want %s drained", eng.removedNames(), vmName) + } + if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{}) { + t.Errorf("registry after the drain=%+v, want empty", holders) + } + // Accepted residual: no quiesce runs, so the marker waits for a writable claim. + if !volumeDirty(scratch) { + t.Error("the unquiesced mount cleared its marker") + } + if _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", writable); err != nil { + t.Errorf("writable claim after the drain: %v", err) + } +} + +// TestAttachOnlyClaimKeepsAdmissionExclusion: what protects other claims is +// unchanged — only this claim's own mount contract moved to the caller. +func TestAttachOnlyClaimKeepsAdmissionExclusion(t *testing.T) { + path := writeVolumeImage(t, "scratch.img", "data") + eng := newFakeEngine() + m := newVolumeManager(t, eng, []config.VolumeSpec{{Name: "scratch", Path: path, Writable: true}}) + if _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW, AttachOnly: true}, + }); err != nil { + t.Fatalf("attach-only writer: %v", err) + } + + for _, tt := range []struct { + name string + request []types.Volume + }{ + {"mounted writer", []types.Volume{{Name: "scratch", Mode: types.VolumeModeRW}}}, + {"mounted reader", []types.Volume{{Name: "scratch"}}}, + {"attach-only reader", []types.Volume{{Name: "scratch", AttachOnly: true}}}, + } { + t.Run(tt.name, func(t *testing.T) { + if _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", tt.request); !errors.Is(err, ErrVolumeBusy) { + t.Errorf("claim under an attach-only writer: %v, want ErrVolumeBusy", err) + } + }) + } +} + +func TestAttachOnlyReadOnlyClaimRefusesDirtyImage(t *testing.T) { + path := writeVolumeImage(t, "scratch.img", "data") + m := newVolumeManager(t, newFakeEngine(), []config.VolumeSpec{{Name: "scratch", Path: path, Writable: true}}) + if err := markVolumeDirty(path); err != nil { + t.Fatalf("mark dirty: %v", err) + } + + _, err := m.ClaimProvision(t.Context(), testKey, 0, "", "", []types.Volume{{Name: "scratch", AttachOnly: true}}) + if !errors.Is(err, ErrVolumeNeedsRecovery) { + t.Errorf("attach-only read-only claim of a dirty image: %v, want ErrVolumeNeedsRecovery", err) + } +} + +func TestQuiesceVolumesSkipsAttachOnlyEntries(t *testing.T) { + scratch := writeVolumeImage(t, "scratch.img", "scratch") + mounted := writeVolumeImage(t, "mounted.img", "mounted") + eng := newFakeEngine() + eng.vms["sbx-mixed"] = "/vsock/mixed" + m := newVolumeManager(t, eng, []config.VolumeSpec{ + {Name: "scratch", Path: scratch, Writable: true}, + {Name: "mounted", Path: mounted, Writable: true}, + }) + sb := &types.Sandbox{ + ID: "sb_mixed", VMName: "sbx-mixed", Key: testKey, VsockSocket: "/vsock/mixed", + Volumes: []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW}, + {Name: "mounted", Mount: "/volumes/mounted", Mode: types.VolumeModeRW}, + }, + } + + td := m.quiesceVolumes(t.Context(), sb) + + if want := []string{"umount:/volumes/mounted"}; !slices.Equal(eng.volumeOpsLog(), want) { + t.Errorf("quiesce ops=%v, want %v", eng.volumeOpsLog(), want) + } + if !slices.Equal(td.clears, []string{mounted}) { + t.Errorf("marker clears=%v, want only %s", td.clears, mounted) + } + if !slices.Equal(td.holds, sb.Volumes) { + t.Errorf("holds=%v, want every entry %v", td.holds, sb.Volumes) + } +} + +func TestWritableMountsCountsOnlyQuiescableEntries(t *testing.T) { + for _, tt := range []struct { + name string + volumes []types.Volume + want int + }{ + {"none", nil, 0}, + {"read-only mount", []types.Volume{{Name: "a", Mount: "/a"}}, 0}, + {"attach-only writer", []types.Volume{{Name: "a", Mode: types.VolumeModeRW}}, 0}, + {"writable mount", []types.Volume{{Name: "a", Mount: "/a", Mode: types.VolumeModeRW}}, 1}, + {"mixed", []types.Volume{ + {Name: "a", Mount: "/a", Mode: types.VolumeModeRW}, + {Name: "b", Mode: types.VolumeModeRW}, + {Name: "c", Mount: "/c"}, + }, 1}, + } { + t.Run(tt.name, func(t *testing.T) { + if got := writableMounts(tt.volumes); got != tt.want { + t.Errorf("writableMounts(%v)=%d, want %d", tt.volumes, got, tt.want) + } + }) + } +} diff --git a/sandboxd/server/server.go b/sandboxd/server/server.go index d313dd8..2c085c5 100644 --- a/sandboxd/server/server.go +++ b/sandboxd/server/server.go @@ -237,7 +237,7 @@ func (s *Server) handleClaim(w http.ResponseWriter, r *http.Request) { key := req.Key() hash := key.Hash() tenant := tenantFrom(r.Context()) - if len(req.Volumes) > 0 { + if len(req.Volumes) > 0 || req.VolumesAttachOnly { s.handleVolumeClaim(w, r, req, key, hash, tenant) return } @@ -265,7 +265,7 @@ func (s *Server) handleClaim(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleVolumeClaim(w http.ResponseWriter, r *http.Request, req types.ClaimRequest, key types.PoolKey, hash, tenant string) { - volumes, err := types.ValidateVolumes(req.Volumes) + volumes, err := types.ValidateVolumes(req.Volumes, req.VolumesAttachOnly) if err == nil && key.Engine != types.EngineCH { err = errors.New("volumes require engine ch") } diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index b7f07a4..f71fae9 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -1318,6 +1318,9 @@ func TestVolumeClaimRejectsShapeBeforePlacement(t *testing.T) { `{"template":"rt:24.04","volumes":[{"name":"data","mount":"relative"}]}`, `{"template":"rt:24.04","volumes":[{"name":"data","mount":"/datasets"},{"name":"other","mount":"/datasets/nested"}]}`, `{"template":"rt:24.04","volumes":[{"name":"a"},{"name":"b"},{"name":"c"},{"name":"d"},{"name":"e"},{"name":"f"},{"name":"g"},{"name":"h"},{"name":"i"}]}`, + `{"template":"rt:24.04","volumes_attach_only":true}`, + `{"template":"rt:24.04","volumes_attach_only":true,"volumes":[{"name":"data","mount":"/datasets"}]}`, + `{"template":"rt:24.04","volumes_attach_only":true,"volumes":[{"name":"data"},{"name":"other","mount":"/datasets"}]}`, } { mgr := &fakeManager{} placer := &fakePlacer{addrs: []string{"warm-peer:7777"}, owners: []string{"owner:7777"}} diff --git a/sandboxd/types/api.go b/sandboxd/types/api.go index 1ea2f7b..49b578f 100644 --- a/sandboxd/types/api.go +++ b/sandboxd/types/api.go @@ -24,6 +24,9 @@ type ClaimRequest struct { Size Size `json:"size,omitempty"` Engine Engine `json:"engine,omitempty"` Volumes []Volume `json:"volumes,omitempty"` + // VolumesAttachOnly attaches every requested volume without mounting it: + // the workload finds the device by its serial and owns the mount contract. + VolumesAttachOnly bool `json:"volumes_attach_only,omitempty"` TTLField NoRedirect bool `json:"no_redirect,omitempty"` // RequirePromoted is carried from a promoted-volume redirect to make the diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 5bb3684..aa09ca0 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -9,6 +9,7 @@ import ( "fmt" "path/filepath" "regexp" + "slices" "strings" "sync" "sync/atomic" @@ -275,13 +276,16 @@ type VMConfig struct { Name string `json:"name"` } -// Volume is one requested or applied dataset mount. Mount is empty only -// before request validation; persisted and response entries are effective. -// Mode is normalized to "" (read-only) or VolumeModeRW. +// Volume is one requested or applied dataset mount. Mount is empty before +// request validation and on an attach-only entry, whose device the caller +// mounts itself. Mode is normalized to "" (read-only) or VolumeModeRW. type Volume struct { Name string `json:"name"` Mount string `json:"mount,omitempty"` Mode string `json:"mode,omitempty"` + // AttachOnly carries the claim's flag into the pool's own validation pass; + // past it the empty Mount is the signal, so it is never stored or sent. + AttachOnly bool `json:"-"` } // RW reports whether the entry asks for write access. @@ -325,12 +329,23 @@ func VolumeRWNames(volumes []Volume) []string { return names } +// VolumesAttachOnly reports whether request validation marked the set as the +// caller's to mount. +func VolumesAttachOnly(volumes []Volume) bool { + return slices.ContainsFunc(volumes, func(v Volume) bool { return v.AttachOnly }) +} + // ValidateVolumes validates a request and returns detached entries with every -// default mount filled and every mode normalized. The input is not modified. -func ValidateVolumes(volumes []Volume) ([]Volume, error) { +// default mount filled and every mode normalized. attachOnly instead leaves +// every mount empty for the caller to mount itself, and rejects an entry that +// carries one. The input is not modified. +func ValidateVolumes(volumes []Volume, attachOnly bool) ([]Volume, error) { if len(volumes) > MaxClaimVolumes { return nil, fmt.Errorf("volumes must contain at most %d entries, got %d", MaxClaimVolumes, len(volumes)) } + if attachOnly && len(volumes) == 0 { + return nil, errors.New("volumes_attach_only requires at least one volume") + } applied := make([]Volume, len(volumes)) names := make(map[string]struct{}, len(volumes)) for i, volume := range volumes { @@ -349,6 +364,13 @@ func ValidateVolumes(volumes []Volume) ([]Volume, error) { default: return nil, fmt.Errorf("volumes[%d] mode %q must be %s or %s", i, mode, VolumeModeRO, VolumeModeRW) } + if attachOnly { + if volume.Mount != "" { + return nil, fmt.Errorf("volumes[%d] mount %q is meaningless when the caller mounts the device itself", i, volume.Mount) + } + applied[i] = Volume{Name: volume.Name, Mode: mode, AttachOnly: true} + continue + } mount := volume.Mount if mount == "" { mount = DefaultVolumeMount(volume.Name) diff --git a/sandboxd/types/volume_test.go b/sandboxd/types/volume_test.go index 6af7878..9549af4 100644 --- a/sandboxd/types/volume_test.go +++ b/sandboxd/types/volume_test.go @@ -31,7 +31,7 @@ func TestValidVolumeName(t *testing.T) { func TestValidateVolumes(t *testing.T) { volumes := []Volume{{Name: "dataset"}, {Name: "weights-1", Mount: "/models"}} wantInput := slices.Clone(volumes) - got, err := ValidateVolumes(volumes) + got, err := ValidateVolumes(volumes, false) if err != nil { t.Fatalf("ValidateVolumes: %v", err) } @@ -54,7 +54,7 @@ func TestValidateVolumes(t *testing.T) { {Name: "f"}, {Name: "g"}, {Name: "h"}, - }); err != nil || len(got) != MaxClaimVolumes { + }, false); err != nil || len(got) != MaxClaimVolumes { t.Errorf("ValidateVolumes at limit: got=%v err=%v", got, err) } @@ -75,7 +75,7 @@ func TestValidateVolumes(t *testing.T) { {"uppercase-mode", []Volume{{Name: "dataset", Mode: "RW"}}}, } { t.Run(tt.name, func(t *testing.T) { - if _, err := ValidateVolumes(tt.volumes); err == nil { + if _, err := ValidateVolumes(tt.volumes, false); err == nil { t.Fatal("ValidateVolumes succeeded") } }) @@ -87,7 +87,7 @@ func TestValidateVolumesNormalizesMode(t *testing.T) { {Name: "shared"}, {Name: "explicit", Mode: VolumeModeRO}, {Name: "writable", Mode: VolumeModeRW}, - }) + }, false) if err != nil { t.Fatalf("ValidateVolumes: %v", err) } @@ -110,21 +110,65 @@ func TestValidateVolumesNormalizesMode(t *testing.T) { } } +func TestValidateVolumesAttachOnly(t *testing.T) { + got, err := ValidateVolumes([]Volume{ + {Name: "dataset"}, + {Name: "scratch", Mode: VolumeModeRW}, + {Name: "explicit", Mode: VolumeModeRO}, + }, true) + if err != nil { + t.Fatalf("ValidateVolumes: %v", err) + } + want := []Volume{ + {Name: "dataset", AttachOnly: true}, + {Name: "scratch", Mode: VolumeModeRW, AttachOnly: true}, + {Name: "explicit", AttachOnly: true}, + } + if !slices.Equal(got, want) { + t.Errorf("volumes %v, want %v", got, want) + } + if !VolumesAttachOnly(got) { + t.Error("validated attach-only set does not report itself") + } + if VolumesAttachOnly(nil) || VolumesAttachOnly([]Volume{{Name: "dataset", Mount: "/volumes/dataset"}}) { + t.Error("mounted set reports itself attach-only") + } + + for _, tt := range []struct { + name string + volumes []Volume + }{ + {"no volumes", nil}, + {"default mount spelled out", []Volume{{Name: "dataset", Mount: "/volumes/dataset"}}}, + {"custom mount", []Volume{{Name: "dataset", Mount: "/datasets"}}}, + {"one entry of many", []Volume{{Name: "dataset"}, {Name: "scratch", Mount: "/scratch"}}}, + {"duplicate name", []Volume{{Name: "dataset"}, {Name: "dataset"}}}, + {"invalid name", []Volume{{Name: "bad/name"}}}, + {"unknown mode", []Volume{{Name: "dataset", Mode: "readwrite"}}}, + } { + t.Run(tt.name, func(t *testing.T) { + if _, err := ValidateVolumes(tt.volumes, true); err == nil { + t.Fatal("ValidateVolumes succeeded") + } + }) + } +} + func TestValidateVolumesRejectsGuestOSMounts(t *testing.T) { for _, root := range append([]string{"/"}, guestOSMountRoots...) { t.Run(root, func(t *testing.T) { - if _, err := ValidateVolumes([]Volume{{Name: "dataset", Mount: root}}); err == nil { + if _, err := ValidateVolumes([]Volume{{Name: "dataset", Mount: root}}, false); err == nil { t.Fatalf("accepted OS mount %q", root) } if root != "/" { - if _, err := ValidateVolumes([]Volume{{Name: "dataset", Mount: root + "/child"}}); err == nil { + if _, err := ValidateVolumes([]Volume{{Name: "dataset", Mount: root + "/child"}}, false); err == nil { t.Fatalf("accepted mount under OS root %q", root) } } }) } for _, mount := range []string{"/data", "/home/dataset", "/opt-data", "/usrdata"} { - if _, err := ValidateVolumes([]Volume{{Name: "dataset", Mount: mount}}); err != nil { + if _, err := ValidateVolumes([]Volume{{Name: "dataset", Mount: mount}}, false); err != nil { t.Errorf("rejected allowed mount %q: %v", mount, err) } } diff --git a/sdk/go/client.go b/sdk/go/client.go index 0ec48d5..5692e48 100644 --- a/sdk/go/client.go +++ b/sdk/go/client.go @@ -426,13 +426,14 @@ func apiError(verb string, resp *http.Response) error { // claimRequest mirrors sandboxd's wire type; duplicated so the SDK stays // dependency-free — the e2e module guards against drift. type claimRequest struct { - Template string `json:"template"` - Net string `json:"net,omitempty"` - Size string `json:"size,omitempty"` - Volumes []Volume `json:"volumes,omitempty"` - TTLSeconds int `json:"ttl_seconds,omitempty"` - NoRedirect bool `json:"no_redirect,omitempty"` - RequirePromoted bool `json:"require_promoted,omitempty"` + Template string `json:"template"` + Net string `json:"net,omitempty"` + Size string `json:"size,omitempty"` + Volumes []Volume `json:"volumes,omitempty"` + VolumesAttachOnly bool `json:"volumes_attach_only,omitempty"` + TTLSeconds int `json:"ttl_seconds,omitempty"` + NoRedirect bool `json:"no_redirect,omitempty"` + RequirePromoted bool `json:"require_promoted,omitempty"` } // rejectPinnedAxes fails a snapshot claim (checkpoint, template) that passed @@ -444,13 +445,16 @@ func (r claimRequest) rejectPinnedAxes() error { return nil } -// validateVolumes rejects a volume mode outside the wire's vocabulary before -// it reaches the network; WithVolumes already normalizes "ro" to "". +// validateVolumes rejects a mode outside the wire's vocabulary and a mount +// attach-only makes meaningless; WithVolumes already normalizes "ro" to "". func (r claimRequest) validateVolumes() error { for _, v := range r.Volumes { if v.Mode != "" && v.Mode != volumeModeRW { return fmt.Errorf("volume %q: mode must be \"\", %q, or %q, got %q", v.Name, volumeModeRO, volumeModeRW, v.Mode) } + if r.VolumesAttachOnly && v.Mount != "" { + return fmt.Errorf("volume %q: mount %q is meaningless with WithVolumesAttachOnly, which leaves mounting to the caller", v.Name, v.Mount) + } } return nil } diff --git a/sdk/go/options.go b/sdk/go/options.go index bead197..9851af8 100644 --- a/sdk/go/options.go +++ b/sdk/go/options.go @@ -70,6 +70,12 @@ func WithVolumes(volumes ...Volume) Option { return func(r *claimRequest) { r.Volumes = volumes } } +// WithVolumesAttachOnly attaches the claim's volumes without mounting them; +// the workload finds each device by its serial and owns the mount from there. +func WithVolumesAttachOnly() Option { + return func(r *claimRequest) { r.VolumesAttachOnly = true } +} + // WithTimeout bounds the sandbox's lifetime: the owning node reaps it after // d (rounded up to seconds) even if the client vanishes. func WithTimeout(d time.Duration) Option { diff --git a/sdk/go/volumes_test.go b/sdk/go/volumes_test.go index 4133254..fd76180 100644 --- a/sdk/go/volumes_test.go +++ b/sdk/go/volumes_test.go @@ -208,6 +208,52 @@ func TestWithVolumesEncodesMode(t *testing.T) { } } +func TestWithVolumesAttachOnlySendsFlagAndDecodesMountlessEcho(t *testing.T) { + var raw struct { + Volumes []map[string]any `json:"volumes"` + VolumesAttachOnly bool `json:"volumes_attach_only"` + } + echo := []Volume{{Name: "imagenet"}, {Name: "scratch", Mode: "rw"}} + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := json.NewDecoder(r.Body).Decode(&raw); err != nil { + t.Errorf("decode body: %v", err) + } + _ = json.NewEncoder(w).Encode(claimResponse{ID: "sb_1", Token: "tok", Volumes: echo}) + })) + t.Cleanup(ts.Close) + + sb, err := testClient(t, ts).New(t.Context(), "rt:24.04", + WithVolumes(Volume{Name: "imagenet"}, Volume{Name: "scratch", Mode: "rw"}), + WithVolumesAttachOnly()) + if err != nil { + t.Fatalf("New: %v", err) + } + if !raw.VolumesAttachOnly { + t.Error("volumes_attach_only missing from the claim body") + } + for _, entry := range raw.Volumes { + if mount, present := entry["mount"]; present { + t.Errorf("volume %v carries mount %v, want none", entry["name"], mount) + } + } + if !slices.Equal(sb.Volumes, echo) { + t.Errorf("volumes = %+v, want %+v", sb.Volumes, echo) + } +} + +func TestWithVolumesAttachOnlyRejectsMountLocally(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + t.Error("server should not be contacted for a locally invalid claim") + })) + t.Cleanup(ts.Close) + + _, err := testClient(t, ts).New(t.Context(), "rt:24.04", + WithVolumes(Volume{Name: "imagenet", Mount: "/datasets"}), WithVolumesAttachOnly()) + if err == nil || !strings.Contains(err.Error(), "meaningless with WithVolumesAttachOnly") { + t.Errorf("err = %v, want local mount rejection", err) + } +} + func TestRejectsInvalidVolumeModeLocally(t *testing.T) { tests := []struct { name string diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index b4dc483..7c60a53 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/sdk/python/cocoonsandbox/client.py @@ -29,12 +29,13 @@ def __init__(self, addr: str, api_token: str = "", timeout: float = 120.0): self.timeout = timeout def new(self, template: str, net: str = "", size: str = "", ttl_seconds: int = 0, - volumes: list[str | Mapping[str, str]] | None = None) -> Sandbox: + volumes: list[str | Mapping[str, str]] | None = None, mount: bool = True) -> Sandbox: """Claims a sandbox; a warm hit is milliseconds. On a cluster a warm miss may redirect to a peer, followed transparently; if every candidate fails transiently, the claim falls back to the origin - once so it provisions or heals locally.""" - claim = _claim_body(template, net, size, ttl_seconds, volumes) + once so it provisions or heals locally. mount=False attaches the + volumes without mounting them, leaving that to the workload.""" + claim = _claim_body(template, net, size, ttl_seconds, volumes, mount) return self._claim_from(self.addr, claim) def delete_template(self, template: str, net: str = "", size: str = "") -> None: @@ -163,7 +164,7 @@ def _request(self, addr: str, method: str, path: str, body, verb: str, bearer: s def _claim_body(template: str, net: str, size: str, ttl_seconds: int, - volumes: list[str | Mapping[str, str]] | None = None) -> dict: + volumes: list[str | Mapping[str, str]] | None = None, mount: bool = True) -> dict: claim = {"template": template} if net: claim["net"] = net @@ -172,17 +173,21 @@ def _claim_body(template: str, net: str, size: str, ttl_seconds: int, if ttl_seconds: claim["ttl_seconds"] = ttl_seconds if volumes: - claim["volumes"] = [_volume_body(volume) for volume in volumes] + claim["volumes"] = [_volume_body(volume, mount) for volume in volumes] + if not mount: + claim["volumes_attach_only"] = True return claim -def _volume_body(volume: str | Mapping[str, str]) -> dict: +def _volume_body(volume: str | Mapping[str, str], mount: bool = True) -> dict: if isinstance(volume, str): return {"name": volume} if not isinstance(volume, Mapping): raise TypeError("volume must be a name string or mapping") if set(volume) - {"name", "mount", "mode"}: raise TypeError("volume mapping accepts only name, mount, and mode") + 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"): diff --git a/sdk/python/cocoonsandbox/template.py b/sdk/python/cocoonsandbox/template.py index 3158aef..ad9c9cd 100644 --- a/sdk/python/cocoonsandbox/template.py +++ b/sdk/python/cocoonsandbox/template.py @@ -22,13 +22,15 @@ def __init__(self, client: Client, addr: str, name: str, net: str, size: str, co self.size = size self.content_digest = content_digest - def new(self, ttl_seconds: int = 0, volumes: list[str | Mapping[str, str]] | None = None) -> Sandbox: - """Claims the template, following placement when volumes require it.""" + def new(self, ttl_seconds: int = 0, volumes: list[str | Mapping[str, str]] | None = None, + mount: bool = True) -> Sandbox: + """Claims the template, following placement when volumes require it. + mount=False attaches the volumes without mounting them.""" # Local import: a top-level one would close the client → sandbox → # template cycle. from .client import _claim_body - claim = _claim_body(self.name, self.net, self.size, ttl_seconds, volumes) + claim = _claim_body(self.name, self.net, self.size, ttl_seconds, volumes, mount) if volumes: return self._client._claim_from(self._addr, claim) claim["no_redirect"] = True diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index d5dc24f..2965a35 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -130,6 +130,60 @@ def test_claim_rejects_unknown_volume_key(node): Client(node).new("rt:24.04", volumes=[{"name": "imagenet", "bogus": "x"}]) +def test_claim_attaches_volumes_without_mounting(node): + seen = [] + + def claim(body, path): + seen.append(body) + return 200, {"id": "sb_1", "token": "tok", "volumes": [ + {"name": "imagenet"}, {"name": "scratch", "mode": "rw"}, + ]} + + FakeNode.routes[("POST", "/v1/claim")] = claim + sb = Client(node).new("rt:24.04", volumes=["imagenet", {"name": "scratch", "mode": "rw"}], mount=False) + assert seen == [{ + "template": "rt:24.04", + "volumes": [{"name": "imagenet"}, {"name": "scratch", "mode": "rw"}], + "volumes_attach_only": True, + }] + assert sb.volumes == [{"name": "imagenet"}, {"name": "scratch", "mode": "rw"}] + + +def test_template_claim_attaches_volumes_without_mounting(node): + seen = [] + + def claim(body, path): + seen.append(body) + return 200, {"id": "sb_2", "token": "tok", "volumes": [{"name": "imagenet"}]} + + FakeNode.routes[("POST", "/v1/claim")] = claim + sb = Template(Client(node), node, "task:v1", "none", "small").new( + volumes=["imagenet"], mount=False) + assert seen[0]["volumes_attach_only"] is True + assert seen[0]["volumes"] == [{"name": "imagenet"}] + assert sb.volumes == [{"name": "imagenet"}] + + +def test_claim_rejects_mount_without_mounting(node): + with pytest.raises(TypeError, match="meaningless with mount=False"): + Client(node).new("rt:24.04", volumes=[{"name": "imagenet", "mount": "/datasets"}], mount=False) + with pytest.raises(TypeError, match="meaningless with mount=False"): + Template(Client(node), node, "task:v1", "none", "small").new( + volumes=[{"name": "imagenet", "mount": "/datasets"}], mount=False) + + +def test_claim_keeps_mounting_by_default(node): + seen = [] + + def claim(body, path): + seen.append(body) + return 200, {"id": "sb_1", "token": "tok"} + + FakeNode.routes[("POST", "/v1/claim")] = claim + Client(node).new("rt:24.04", volumes=["imagenet"]) + assert "volumes_attach_only" not in seen[0] + + def test_template_claim_sends_volumes(node): seen = []