diff --git a/docs/deploy.md b/docs/deploy.md index 58eaa18f4..1ba503659 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -120,7 +120,7 @@ gets the intersection of the two. | `client_advertise` | unset | client-facing HTTP(S) origin for this node, e.g. `https://node-a.sandbox.example.com`; published in owner, redirect, and peer responses. No path, query, fragment, or userinfo | | `bridges` / `networks` | unset | egress-lane attachment: a list of host bridge devices, or a list of CNI conflist names. Mutually exclusive; with neither set the node serves only the no-network lane. A Linux bridge holds at most 1024 ports (kernel `BR_MAX_PORTS`), so an N-entry list raises the node's egress ceiling to N×1024 — VMs spread over the list by a stable hash of the VM name, so size it with headroom (the spread is statistical, not exact). `bridges` keeps the raw TAP-on-bridge attachment (taps in the root netns, no per-VM network namespace or CNI plugin execution); `networks` runs the CNI chain per VM. [Guarded egress](egress.md) on the egress lane (an egress-lane pool policy or any tenant policy) needs `bridges` and rejects a CNI network at load; none-lane pool policies ride the proxy on either | | `volumes` | unset | node-local catalog of operator-managed dataset images: `[ {"name":"imagenet","path":"/srv/datasets/imagenet.img","directio":"off","tenants":["acme"]}, {"name":"scratch-db","path":"/srv/datasets/scratch.img","writable":true} ]`. Names match `^[a-z][a-z0-9_-]{0,19}$` and cannot start with `cocoon-`; paths are absolute; `directio` is `on`, `off`, or `auto` and defaults to `off` for both read-only and writable entries. `tenants` is an optional [access list](sandboxd-api.md#volume-access-lists); a listed name the tenant set does not hold grants nothing and logs a warning at boot (the [tenant API](sandboxd-api.md#tenants-v1tenants) may add it later). `writable` (default `false`) lets a claim request `mode: "rw"` on that entry — see [Dataset volumes](#dataset-volumes). The catalog is intentionally not part of the cluster digest | -| `volume_roots` | unset | absolute directories that bound [runtime registration](sandboxd-api.md#put-v1volumesname): the registered path, once cleaned, must equal a root or sit under one, or the PUT returns 400. The check is lexical: `..` is resolved but symlinks are not followed, so a symlink under a root can point outside it. Static `volumes` entries are not checked. A saved registration outside the roots prevents startup. Unregister it before narrowing the roots, or widen the roots to include it. DELETE always rewrites the catalog, including for an absent name. Keep `data_dir` outside every root. Unset or an empty list accepts any absolute path | +| `volume_roots` | unset | absolute directories that bound [runtime registration](sandboxd-api.md#put-v1volumesname): the registered path, once cleaned, must equal a root or sit under one, or the PUT returns 400. The check is lexical: `..` is resolved but symlinks are not followed, so a symlink under a root can point outside it. Static `volumes` entries are not checked. A saved registration outside the roots prevents startup. Unregister it before narrowing the roots, or widen the roots to include it. Keep `data_dir` outside every root. Unset or an empty list accepts any absolute path | | `secrets` | unset | credentials the egress proxy injects by name: `[{"name": "gh", "header": "Authorization", "value_env": "GH_TOKEN"}]`. A pool or egress-class rule references the name (on a claim where both apply, only the pool rule's secret is injected); the value comes from the claim's `guest: false` env, then the node's environment, never this file. Without `value_env` the secret's name is the env to read, and only claims supply it. See [egress](egress.md#claim-env-and-secrets) | | `egress_internal_allow` | unset | CIDR prefixes re-admitted through the egress proxy's SSRF guard, node-wide (every pool and tenant); `prefix:port,port` scopes an entry to those ports, a bare prefix admits every port. Prefer the port form. Prefixes, not a permit-private switch — the guest bridges are themselves ULA/RFC1918. See [egress](egress.md) | | `egress_upstream` | unset | per-claim upstream proxies: `{"claim_env": "EGRESS_UPSTREAM", "allow": ["res.example.com", "10.1.0.0/16"]}`. A claim's `guest: false` entry under `claim_env` (`http://` or `socks5://`, optional `user:pass@`, or `direct`) routes its egress through that upstream, which must be on `allow` (host names, IPs, CIDR prefixes). See [upstream proxies](egress.md#upstream-proxies) | @@ -325,8 +325,7 @@ set, promoted-template intersection, and redirect follow the fleet-wide rule in [cluster](cluster.md#volumes-and-placement). Catalog `tenants` lists follow the -[access-list rule](sandboxd-api.md#volume-access-lists). A listed tenant the -set no longer holds grants nothing and logs a warning at boot. +[access-list rule](sandboxd-api.md#volume-access-lists). `GET /v1/volumes` reports the caller-visible fleet union and holder count, plus the answering node's current local availability and whether the entry is `writable`, without exposing host paths or node addresses. Applied names, diff --git a/docs/live-volumes.md b/docs/live-volumes.md index 43fa04cba..589e504d0 100644 --- a/docs/live-volumes.md +++ b/docs/live-volumes.md @@ -9,12 +9,8 @@ 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. -A configured node root token can -[register backing paths](sandboxd-api.md#put-v1volumesname) without editing -static config. Registration persists only node-local catalog metadata and does -not manage disk files. Runtime `tenants` lists follow the -[access-list rule](sandboxd-api.md#volume-access-lists). Live changes remain -limited to mounted read-only entries. +Entries [registered at runtime](sandboxd-api.md#put-v1volumesname) work the +same way. Pause application reads before replacement. Close files, mmap mappings, cwd, and bind mount references on the old disk. Replacement unmounts the old disk diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index e16ef79ac..3262ad0e2 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -278,7 +278,9 @@ duplicate fields, and trailing JSON return 400. - `tenants` is the entry's [access list](#volume-access-lists). Success is 204. An identical normalized registration is idempotent. A different -binding, changed fields, or a static name or cleaned path collision returns 409. +binding, changed fields, a static name or cleaned path collision, or a name a +claim still holds returns 409. Each change is an `op:"volumes"` audit record +naming the volume added or removed. Invalid fields return 400. GET keeps its existing projection without host paths or tenant lists. Runtime entries support the existing live volume API. @@ -297,27 +299,25 @@ cleanup directories: `/readers`, `/goldens`, and ### Volume access lists Static and runtime entries share one `tenants` rule. An omitted or empty list -permits every authenticated scope. A nonempty list permits only claims owned -by the listed tenants, and root-owned claims can use any entry. Live additions -check the claim owner's list even when root sends the request; held names -finish repair or removal without a new grant. Runtime registration rejects -tenant names that do not match the tenant API syntax. Static config preserves -its supplied tenant lists. An invalid static name matches no tenant, so a -nonempty list containing only invalid names permits no tenant. A valid name -for a tenant that does not exist yet grants nothing until the tenant is created. +permits every authenticated scope. A nonempty list permits only claims owned by +the listed tenants, and root-owned claims can use any entry. Runtime +registration rejects tenant names that do not match the tenant API syntax. +Static config preserves its supplied tenant lists. An invalid static name +matches no tenant, so a nonempty list containing only invalid names permits no +tenant. A valid name for a tenant that does not exist yet grants nothing until +the tenant is created. ## DELETE /v1/volumes/{name} Auth and name validation match PUT. Deletes only runtime registration metadata. -Success, including an absent unreferenced name, is 204. Static names and names -referenced by a claim, an in-flight operation, or cleanup return 409. -DELETE always writes the absence, including when the name is already absent. +Static names and names referenced by a claim, an in-flight operation, or +cleanup return 409. Otherwise DELETE writes the absence and returns 204, also +for an absent name. Persistence failure returns 503 with `Retry-After`. Retry the same DELETE, even if GET no longer lists the name; a restart can restore the old registration. -Before downgrading to a daemon without runtime registration, drain runtime -references or supply equivalent static volume config and finish pending live -volume changes. +Before downgrading to a daemon without runtime registration, release claims +that use runtime names or declare those names in static `volumes`. ## GET /v1/sandboxes/{id}/volumes @@ -341,8 +341,7 @@ 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 while a request worker -is running. +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 diff --git a/docs/security.md b/docs/security.md index 0cd239c7d..c89258ba9 100644 --- a/docs/security.md +++ b/docs/security.md @@ -105,7 +105,7 @@ exposes host image paths or access lists. Runtime [registration and unregistration](sandboxd-api.md#put-v1volumesname) require a configured root `api_token`; these endpoints return 403 when the token is unset. New catalog records use -mode 0600. Reads and replacements use the daemon's filesystem permissions. +mode 0600. Registration uses static configuration's path validation plus, when set, the [`volume_roots`](deploy.md#configuration) bound. It does not verify backing type or pin backing identity. Without diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index 1301cded2..67334f2f1 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -381,7 +381,7 @@ func (m *Manager) admitVolumes(tenant string, volumes []resolvedVolume) ([]types return applied, nil } -// admitClaim refuses a busy volume or an over-quota tenant before any VM is built. +// admitClaim refuses a busy or retired volume or an over-quota tenant before any VM is built. func (m *Manager) admitClaim(tenant string, volumes []resolvedVolume, applied []types.Volume) error { m.mu.Lock() defer m.mu.Unlock() diff --git a/sandboxd/pool/failover.go b/sandboxd/pool/failover.go index 05d30a20c..da3c8c058 100644 --- a/sandboxd/pool/failover.go +++ b/sandboxd/pool/failover.go @@ -394,26 +394,29 @@ func (m *Manager) forgetAdopted(ctx context.Context, sb *types.Sandbox) func() { logger.Warnf(ctx, "clear archive ck %s: %v", ck, err) } } - if !m.forgetClaim(ctx, sb) { + holds, ok := m.forgetClaim(ctx, sb) + if !ok { return nil } return func() { if vm != "" { - m.destroy(ctx, vm) + m.removeOrRetry(ctx, vm, "", volumeTeardown{holds: holds}) } m.dropSnap(ctx, snap) logger.Infof(ctx, "sandbox %s moved to another node", sb.ID) } } -func (m *Manager) forgetClaim(ctx context.Context, sb *types.Sandbox) bool { +func (m *Manager) forgetClaim(ctx context.Context, sb *types.Sandbox) ([]types.Volume, bool) { m.mu.Lock() if m.claimed[sb.ID] != sb { m.mu.Unlock() - return false + return nil, false } + holds := m.cancelVolumeRun(sb) if sb.ArchiveCk != "" { - m.releaseVolumes(heldVolumes(sb)) + m.releaseVolumes(holds) + holds = nil } m.dropClaimLocked(sb) js := m.store.del(sb.ID) @@ -421,7 +424,7 @@ func (m *Manager) forgetClaim(ctx context.Context, sb *types.Sandbox) bool { if err := m.store.commit(js); err != nil { m.store.recommit(ctx, js) } - return true + return holds, true } func (m *Manager) sweepOrphanArchives(ctx context.Context) { diff --git a/sandboxd/pool/volume_bench_test.go b/sandboxd/pool/volume_bench_test.go index 561afd8dc..2b4e39123 100644 --- a/sandboxd/pool/volume_bench_test.go +++ b/sandboxd/pool/volume_bench_test.go @@ -1,8 +1,6 @@ package pool import ( - "os" - "path/filepath" "testing" "github.com/cocoonstack/sandbox/sandboxd/config" @@ -45,11 +43,7 @@ func volumeBenchManager(b *testing.B) *Manager { if err := m.store.save(m.claimed); err != nil { b.Fatal(err) } - path := filepath.Join(b.TempDir(), "data.raw") - if err := os.WriteFile(path, []byte("benchmark backing"), 0o600); err != nil { - b.Fatal(err) - } - if err := m.RegisterVolume(b.Context(), "bench-data", types.VolumeRegistration{Path: path}); err != nil { + if err := m.RegisterVolume(b.Context(), "bench-data", types.VolumeRegistration{Path: writeVolumeImage(b, "data.raw", "benchmark backing")}); err != nil { b.Fatal(err) } return m diff --git a/sandboxd/pool/volume_catalog.go b/sandboxd/pool/volume_catalog.go index feeffb00f..5ffd9e43d 100644 --- a/sandboxd/pool/volume_catalog.go +++ b/sandboxd/pool/volume_catalog.go @@ -34,15 +34,36 @@ func (c *volumeCatalog) withinRoots(path string) bool { }) } +func (c *volumeCatalog) normalize(name string, spec types.VolumeRegistration) (*catalogVolume, error) { + spec.DirectIO = cmp.Or(spec.DirectIO, types.DirectIOOff) + for _, tenant := range spec.Tenants { + if !types.NameRe.MatchString(tenant) { + return nil, fmt.Errorf("%w: volume %q tenant %q must match %s", ErrBadVolume, name, tenant, types.NameRe) + } + } + if err := config.ValidateVolumeCatalog([]config.VolumeSpec{{ + Name: name, Path: spec.Path, DirectIO: spec.DirectIO, Writable: spec.Writable, Tenants: spec.Tenants, + }}); err != nil { + return nil, fmt.Errorf("%w: %w", ErrBadVolume, err) + } + volume := &catalogVolume{ + runtime: true, + disk: engine.VolumeSpec{Name: name, Path: filepath.Clean(spec.Path), DirectIO: spec.DirectIO}, + tenants: slices.Compact(slices.Sorted(slices.Values(spec.Tenants))), + writable: spec.Writable, + } + if !c.withinRoots(volume.disk.Path) { + return nil, fmt.Errorf("%w: volume %q path is outside volume_roots", ErrBadVolume, name) + } + return volume, nil +} + // RegisterVolume durably binds a root-managed name to a backing path. func (m *Manager) RegisterVolume(ctx context.Context, name string, spec types.VolumeRegistration) error { - volume, err := normalizeRegistration(name, spec) + volume, err := m.catalog.normalize(name, spec) if err != nil { return err } - if !m.catalog.withinRoots(volume.disk.Path) { - return fmt.Errorf("%w: volume %q path is outside volume_roots", ErrBadVolume, name) - } m.catalog.writeMu.Lock() defer m.catalog.writeMu.Unlock() m.mu.Lock() @@ -110,24 +131,19 @@ func (m *Manager) loadRuntimeVolumes() error { if err = json.Unmarshal(raw, &record); err != nil { return fmt.Errorf("parse runtime volume catalog: %w", err) } - entries := maps.Clone(m.catalog.entries) for name, spec := range record { - volume, normalizeErr := normalizeRegistration(name, spec) + volume, normalizeErr := m.catalog.normalize(name, spec) if normalizeErr != nil { return fmt.Errorf("runtime volume catalog: %w", normalizeErr) } - if !m.catalog.withinRoots(volume.disk.Path) { - return fmt.Errorf("runtime volume catalog: volume %q path is outside volume_roots", name) - } if m.catalog.entries[name] != nil { return fmt.Errorf("runtime volume catalog duplicates static name %q", name) } - if err := catalogPathConflict(entries, volume.disk.Path); err != nil { + if err := catalogPathConflict(m.catalog.entries, volume.disk.Path); err != nil { return fmt.Errorf("runtime volume catalog: %w", err) } - entries[name] = volume + m.catalog.entries[name] = volume } - m.catalog.entries = entries return nil } @@ -144,7 +160,7 @@ func (m *Manager) persistRuntimeCatalog(entries map[string]*catalogVolume) error } raw, err := json.Marshal(record) if err != nil { - return fmt.Errorf("%w: marshal runtime volume catalog: %w", ErrVolumeCatalogUnavailable, err) + return fmt.Errorf("marshal runtime volume catalog: %w", err) } if err := utils.WriteFileSync(filepath.Join(m.dataDir, runtimeVolumesFile), raw, 0o600); err != nil { return fmt.Errorf("%w: %w", ErrVolumeCatalogUnavailable, err) @@ -160,23 +176,3 @@ func catalogPathConflict(entries map[string]*catalogVolume, path string) error { } return nil } - -func normalizeRegistration(name string, spec types.VolumeRegistration) (*catalogVolume, error) { - spec.DirectIO = cmp.Or(spec.DirectIO, types.DirectIOOff) - for _, tenant := range spec.Tenants { - if !types.NameRe.MatchString(tenant) { - return nil, fmt.Errorf("%w: volume %q tenant %q must match %s", ErrBadVolume, name, tenant, types.NameRe) - } - } - if err := config.ValidateVolumeCatalog([]config.VolumeSpec{{ - Name: name, Path: spec.Path, DirectIO: spec.DirectIO, Writable: spec.Writable, Tenants: spec.Tenants, - }}); err != nil { - return nil, fmt.Errorf("%w: %w", ErrBadVolume, err) - } - return &catalogVolume{ - runtime: true, - disk: engine.VolumeSpec{Name: name, Path: filepath.Clean(spec.Path), DirectIO: spec.DirectIO}, - tenants: slices.Compact(slices.Sorted(slices.Values(spec.Tenants))), - writable: spec.Writable, - }, nil -} diff --git a/sandboxd/pool/volume_catalog_test.go b/sandboxd/pool/volume_catalog_test.go index 7c0c42078..0ee286259 100644 --- a/sandboxd/pool/volume_catalog_test.go +++ b/sandboxd/pool/volume_catalog_test.go @@ -120,26 +120,7 @@ func TestRuntimeCatalogRecoversLegacyArchivedVolumeReference(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { synctest.Test(t, func(t *testing.T) { - m := newVolumeManager(t, newFakeEngine(), nil) - path := writeVolumeImage(t, "data", "bytes") - if err := m.RegisterVolume(t.Context(), "data", types.VolumeRegistration{Path: path, Writable: true}); err != nil { - t.Fatal(err) - } - sb := &types.Sandbox{ - ID: "sb_legacy", Key: testKey, Token: "legacy-token", ArchiveCk: "ck_00000000000000aa", - Volumes: []types.Volume{{Name: "data", Mount: "/volumes/data"}}, - } - if err := m.store.save(map[string]*types.Sandbox{sb.ID: sb}); err != nil { - t.Fatal(err) - } - m = newVolumeManagerAt(t, newFakeEngine(), m.dataDir, nil) - if err := m.Reconcile(t.Context()); err != nil { - t.Fatal(err) - } - recovered, ok := m.byID(sb.ID) - if !ok { - t.Fatal("archived claim did not survive restart") - } + m, recovered := recoverLegacyArchived(t) if err := m.UnregisterVolume(t.Context(), "data"); !errors.Is(err, ErrVolumeRegistrationConflict) { t.Fatalf("unregister recovered reference: %v", err) } @@ -156,7 +137,7 @@ func TestRuntimeCatalogRecoversLegacyArchivedVolumeReference(t *testing.T) { t.Fatal(err) } synctest.Wait() - if _, ok := m.byID(sb.ID); ok { + if _, ok := m.byID(recovered.ID); ok { t.Fatal("archived claim survived retirement") } if _, err := m.ClaimProvision(t.Context(), testKey, writer); !errors.Is(err, ErrVolumeBusy) { @@ -207,31 +188,12 @@ func TestRuntimeCatalogRetainsArchivedVolumeReferenceAfterFailedReap(t *testing. } { t.Run(tc.name, func(t *testing.T) { synctest.Test(t, func(t *testing.T) { - m := newVolumeManager(t, newFakeEngine(), nil) - path := writeVolumeImage(t, "data", "bytes") - if err := m.RegisterVolume(t.Context(), "data", types.VolumeRegistration{Path: path, Writable: true}); err != nil { - t.Fatal(err) - } - sb := &types.Sandbox{ - ID: "sb_legacy", Key: testKey, Token: "legacy-token", ArchiveCk: "ck_00000000000000aa", - Volumes: []types.Volume{{Name: "data", Mount: "/volumes/data"}}, - } - if err := m.store.save(map[string]*types.Sandbox{sb.ID: sb}); err != nil { - t.Fatal(err) - } - m = newVolumeManagerAt(t, newFakeEngine(), m.dataDir, nil) - if err := m.Reconcile(t.Context()); err != nil { - t.Fatal(err) - } - recovered, ok := m.byID(sb.ID) - if !ok { - t.Fatal("archived claim did not survive restart") - } + m, recovered := recoverLegacyArchived(t) restore := sync.OnceFunc(tc.breakStore(t, m)) defer restore() expire(m, recovered) m.reapOnce(t.Context()) - if _, ok := m.byID(sb.ID); !ok { + if _, ok := m.byID(recovered.ID); !ok { t.Fatal("failed reap removed the archived claim") } if err := m.UnregisterVolume(t.Context(), "data"); !errors.Is(err, ErrVolumeRegistrationConflict) { @@ -384,8 +346,8 @@ func TestRuntimeCatalogMarshalFailureDoesNotPublish(t *testing.T) { m := newVolumeManager(t, newFakeEngine(), nil) err := m.RegisterVolume(t.Context(), "runtime", types.VolumeRegistration{Path: "/srv/\xff"}) _, wrapped := errors.AsType[*jsontext.SyntacticError](err) - if !errors.Is(err, ErrVolumeCatalogUnavailable) || !wrapped { - t.Fatalf("marshal error was not preserved: %v", err) + if errors.Is(err, ErrVolumeCatalogUnavailable) || !wrapped { + t.Fatalf("marshal error was not reported as permanent: %v", err) } if _, err := os.Stat(filepath.Join(m.dataDir, runtimeVolumesFile)); !errors.Is(err, os.ErrNotExist) || m.catalog.entries["runtime"] != nil { t.Fatalf("failed registration persisted: stat=%v entry=%+v", err, m.catalog.entries["runtime"]) @@ -473,7 +435,7 @@ func TestRuntimeCatalogFailedWriteLeavesUnrelatedAdmissionsAvailable(t *testing. m, sb := newTenantVolumeManager(t, eng, liveCatalog(t)) specs := map[string]types.VolumeRegistration{ "healthy": {Path: writeVolumeImage(t, "healthy", "bytes")}, - "affected": {Path: writeVolumeImage(t, "affected", "bytes"), Tenants: []string{"other"}}, + "affected": {Path: writeVolumeImage(t, "affected", "bytes")}, } for _, name := range tc.before { if err := m.RegisterVolume(t.Context(), name, specs[name]); err != nil { @@ -498,15 +460,10 @@ func TestRuntimeCatalogFailedWriteLeavesUnrelatedAdmissionsAvailable(t *testing. t.Errorf("root claim %v = %v, want %v", volumes, err, tc.want) } } - _, deniedErr := m.SetVolumes(t.Context(), sb.ID, "acme", []types.Volume{{Name: "data"}, {Name: "affected"}}) - _, missingErr := m.SetVolumes(t.Context(), sb.ID, "acme", []types.Volume{{Name: "data"}, {Name: "missing"}}) - if !errors.Is(deniedErr, ErrVolumeUnavailable) || !errors.Is(missingErr, ErrVolumeUnavailable) || deniedErr.Error() != missingErr.Error() { - t.Errorf("private name distinguishable: denied=%v missing=%v", deniedErr, missingErr) - } if len(eng.volumeSpecs) != 0 { t.Fatal("refused request reached device attachment") } - for _, name := range []string{"data", "healthy", "affected", "missing"} { + for _, name := range []string{"data", "healthy", "affected"} { if got := volumeHoldersOf(m, name); got != (volumeHolders{}) { t.Fatalf("refused request reserved %s: %+v", name, got) } @@ -614,3 +571,27 @@ func claimAndRelease(t *testing.T, m *Manager, volumes ...types.Volume) error { } return m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) } + +func recoverLegacyArchived(t *testing.T) (*Manager, *types.Sandbox) { + t.Helper() + m := newVolumeManager(t, newFakeEngine(), nil) + if err := m.RegisterVolume(t.Context(), "data", types.VolumeRegistration{Path: writeVolumeImage(t, "data", "bytes"), Writable: true}); err != nil { + t.Fatal(err) + } + sb := &types.Sandbox{ + ID: "sb_legacy", Key: testKey, Token: "legacy-token", ArchiveCk: "ck_00000000000000aa", + Volumes: []types.Volume{{Name: "data", Mount: "/volumes/data"}}, + } + if err := m.store.save(map[string]*types.Sandbox{sb.ID: sb}); err != nil { + t.Fatal(err) + } + m = newVolumeManagerAt(t, newFakeEngine(), m.dataDir, nil) + if err := m.Reconcile(t.Context()); err != nil { + t.Fatal(err) + } + recovered, ok := m.byID(sb.ID) + if !ok { + t.Fatal("archived claim did not survive restart") + } + return m, recovered +} diff --git a/sandboxd/pool/volume_cleanup_test.go b/sandboxd/pool/volume_cleanup_test.go index 8728f50cb..3cba011d9 100644 --- a/sandboxd/pool/volume_cleanup_test.go +++ b/sandboxd/pool/volume_cleanup_test.go @@ -36,6 +36,29 @@ func TestDispatchedCleanupRetainsCatalogReference(t *testing.T) { } } +func TestOwnershipTransferReleasesRunningVolumeHoldAfterRemoval(t *testing.T) { + base := newFakeEngine() + m := newVolumeManager(t, base, nil) + if err := m.RegisterVolume(t.Context(), "data", types.VolumeRegistration{Path: writeVolumeImage(t, "runtime", "bytes"), Writable: true}); err != nil { + t.Fatal(err) + } + sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: []types.Volume{{Name: "data"}}}) + if err != nil { + t.Fatal(err) + } + writer := ClaimOptions{Volumes: []types.Volume{{Name: "data", Mode: types.VolumeModeRW}}} + base.removeErrFor = sb.VMName + m.dropAdopted(t.Context(), sb) + if _, err := m.ClaimProvision(t.Context(), testKey, writer); !errors.Is(err, ErrVolumeBusy) { + t.Fatalf("writer beside surviving VM: %v", err) + } + base.removeErrFor = "" + m.retryRemovals(t.Context()).Wait() + if err := m.UnregisterVolume(t.Context(), "data"); err != nil { + t.Fatalf("unregister after transferred VM removal: %v", err) + } +} + // dispatchBlocked moves the removal queue into workers parked on a full maintenance semaphore. func dispatchBlocked(ctx context.Context, t *testing.T, m *Manager) *sync.WaitGroup { t.Helper() diff --git a/sandboxd/pool/volume_test.go b/sandboxd/pool/volume_test.go index 9367ae043..f79b637a3 100644 --- a/sandboxd/pool/volume_test.go +++ b/sandboxd/pool/volume_test.go @@ -607,7 +607,7 @@ func newVolumePoolManager(t *testing.T, eng *fakeEngine, dataDir string, volumes return m } -func writeVolumeImage(t *testing.T, name, content string) string { +func writeVolumeImage(t testing.TB, name, content string) string { t.Helper() path := filepath.Join(t.TempDir(), name) if err := os.WriteFile(path, []byte(content), 0o600); err != nil {