diff --git a/.github/workflows/build-silkd.yml b/.github/workflows/build-silkd.yml index f419e37f..e3f9a757 100644 --- a/.github/workflows/build-silkd.yml +++ b/.github/workflows/build-silkd.yml @@ -18,6 +18,10 @@ on: description: sha-pinned silkd image ref this run pushed value: ${{ jobs.merge.outputs.image }} workflow_dispatch: + inputs: + no_cache: + type: boolean + default: false permissions: contents: read diff --git a/.github/workflows/images.yml b/.github/workflows/images.yml index 440a9aee..65825574 100644 --- a/.github/workflows/images.yml +++ b/.github/workflows/images.yml @@ -13,7 +13,6 @@ on: paths: - "boot/**" - "silkd/**" - - "protocol/**" - "os-image/**" - ".github/workflows/images.yml" - ".github/workflows/build-os-images.yml" @@ -72,7 +71,7 @@ jobs: # built must rebuild it, or the run goes green having built nothing. wf=.github/workflows changed boot/ $wf/build-boot.yml && boot=true || boot=false - changed silkd/ protocol/ $wf/build-silkd.yml && silkd=true || silkd=false + changed silkd/ $wf/build-silkd.yml && silkd=true || silkd=false changed os-image/ $wf/build-os-images.yml $wf/images.yml && osimage=true || osimage=false fi { diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 7687453e..41f719ed 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -30,7 +30,6 @@ jobs: run: | sudo apt-get update && sudo apt-get install -y --no-install-recommends musl-tools (cd silkd && cargo build --release --locked --target x86_64-unknown-linux-musl) - strip silkd/target/x86_64-unknown-linux-musl/release/silkd mkdir -p dist-extra tar -C silkd/target/x86_64-unknown-linux-musl/release \ -czf "dist-extra/silkd_${GITHUB_REF_NAME#v}_Linux_x86_64.tar.gz" silkd diff --git a/.github/workflows/sandboxd.yml b/.github/workflows/sandboxd.yml index ab627b58..94ef92d7 100644 --- a/.github/workflows/sandboxd.yml +++ b/.github/workflows/sandboxd.yml @@ -73,7 +73,7 @@ jobs: env: SANDBOXD_TEST_PG_URL: postgres://postgres:sbxci@localhost:5432/postgres?sslmode=disable run: | - GOWORK=off go test -race -count=1 -v ./tenants/... ./poolset/... ./archives/... ./usage/... | tee "$RUNNER_TEMP/pg-test.log" + GOWORK=off go test -race -count=1 -v ./tenants/... ./poolset/... ./archives/... ./usage/... ./moves/... | tee "$RUNNER_TEMP/pg-test.log" GOWORK=off go test -race -count=1 -v -run 'TestTwoNodesOnOneMetaStore|TestNodesOfACell|TestAMetaStoreNodeWithoutACell' ./pool/ | tee -a "$RUNNER_TEMP/pg-test.log" ! grep -- '--- SKIP' "$RUNNER_TEMP/pg-test.log" diff --git a/boot/Dockerfile b/boot/Dockerfile index 9df7cbaf..c6db6e39 100644 --- a/boot/Dockerfile +++ b/boot/Dockerfile @@ -9,7 +9,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ build-essential bc bison flex libelf-dev libssl-dev \ xz-utils curl ca-certificates python3 \ && rm -rf /var/lib/apt/lists/* -ARG KERNEL_VERSION=6.18.37 +ARG KERNEL_VERSION # Mirror chain: KERNEL_MIRROR (cdn.kernel.org by default), then kernel.org's # edge mirror and an rsync mirror. Download to a file first — a mid-stream # failure must not leave a half-extracted tree — and verify against the pinned SHA256 diff --git a/boot/init/src/cfg.rs b/boot/init/src/cfg.rs index 6e322058..94784bf8 100644 --- a/boot/init/src/cfg.rs +++ b/boot/init/src/cfg.rs @@ -14,7 +14,7 @@ pub struct BootCfg<'a> { pub layers: Vec<&'a str>, /// Writable ext4 COW disk ID (same resolution rules as layers). pub cow: &'a str, - /// Wait budget for the whole disk set; NICs get a fixed 200 ms. + /// Wait budget for the whole disk set. pub timeout: Duration, pub hostname: Option<&'a str>, /// Static per-NIC config, persisted as networkd units and never applied in the initramfs. diff --git a/docs/benchmarks.md b/docs/benchmarks.md index 09b04377..1c022c43 100644 --- a/docs/benchmarks.md +++ b/docs/benchmarks.md @@ -147,10 +147,9 @@ delta is the host, not the round's code (cocoon-specs | cold boot (unpooled ghcr.io/cocoonstack/sandbox/python:3.12) | 215.4 ms | 215.4 ms | 221.5 ms | 3 | | burst: 16 concurrent clones | 102.1 ms | 179.3 ms | 234.5 ms | 16 | -The clone-tier row measured warm hits this round: the pipelined refill -(clone and probe gated separately) refills between back-to-back claims, so -the adaptive watermark wins the race the harness assumes it loses. The -burst row is the real clone-path number. +The clone-tier row measured warm hits this round: before #185 the config +default turned the golden-only pool's `warm: 0` into 4, so its claims popped +warm VMs. The burst row is the real clone-path number. | data plane | measured | |---|---| diff --git a/docs/cluster.md b/docs/cluster.md index 490c337e..52695c81 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -141,9 +141,12 @@ curl -s -H "Authorization: Bearer $TOKEN" http://node-a:7777/v1/info | jq . {"key": {"template": "base:24.04", "net": "none", "size": "small"}, "warm": 4, "refilling": 0, "target": 4, "golden": true} ], + "templates": [], "claimed": 2, + "running": 2, "hibernated": 0, "archived": 0, + "advertise_addr": "10.0.0.5:7777", "peers": ["10.0.0.6:7777"] } ``` diff --git a/docs/deploy.md b/docs/deploy.md index 4c246e7d..7c5b6996 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -210,8 +210,7 @@ taken. Unpooled claims (template and checkpoint claims) always wake. connections to finish, and the next connection wakes the claim. A reader is a warm VM taken from the claim's own pool, so it boots the claim's -image with the claim's layers, and it has that pool's lane: no NIC on the none -lane, the bridge NIC the claim had on the bridge lane (egress-lane claims never +image with the claim's layers, and it has no NIC (egress-lane claims never hibernate). Taking one removes a VM from the pool until the refill replaces it, and an empty pool never cold-boots one: the call wakes the claim. A reader is never shared: it serves one claim, and only the claim's own token reaches it. The guest's own @@ -383,8 +382,8 @@ 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 +**Attach-only claims opt out of the marker, not out of admission.** A volume +claimed with `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 @@ -600,7 +599,7 @@ can delete only its own checkpoints and templates (root sees everything). Operator surfaces stay root-only — per-id sandbox reads and stats, `PUT/DELETE /v1/volumes/{name}`, `PUT /v1/sandboxes/{id}/instance-metadata`, `GET /v1/checkpoints/{id}/blob`, `GET /v1/info`, `PUT /v1/pools`, -`/v1/tenants`, `/v1/config/reload`, `POST/DELETE /v1/drain`, `/metrics`, `GET /v1/usage`; a +`/v1/tenants`, `/v1/config/reload`, `POST/DELETE /v1/drain`, `POST /v1/sandboxes/{id}/move` and the peer `/v1/moves/{id}` calls, `/metrics`, `GET /v1/usage`; a tenant token there is authenticated but not authorized, so it answers 403 (a wrong token stays 401). Per-sandbox tokens are unchanged: whoever holds a sandbox's token drives that sandbox. @@ -1213,7 +1212,7 @@ HTTP port under a signed, expiring shareable URL; a node that cannot bind it exits. The whole mechanism is in sandboxd: - **Minting** (`sb.PreviewURL(ctx, port, ttl)`): the owner node signs a token - embedding `{sandbox, port, owner, exp}` with `preview_secret`; the URL's + embedding `{id, port, owner, exp}` with `preview_secret`; the URL's life is clamped to the claim's lease. - **Serving**: any node's preview listener verifies the token (no shared state), then reverse-proxies to the guest port over the relay if it owns diff --git a/docs/e2b.md b/docs/e2b.md index 01274fc1..05c60d69 100644 --- a/docs/e2b.md +++ b/docs/e2b.md @@ -76,7 +76,7 @@ for the truth. `e2b-ci` is built from e2b's own recipe, `e2b-dev/code-interpreter` `template/` at the `CI_RECIPE_SHA` the Dockerfile pins: the build fetches that commit's archive, checks its sha256, and applies the one local change listed in -`os-image/e2b-ci/PATCHES`; e2b's license ships in the image under +`os-image/e2b-ci/PATCHES.md`; e2b's license ships in the image under `/usr/share/doc/e2b-code-interpreter/`. Moving to a newer recipe is a bump of the two `CI_RECIPE_*` args plus a refresh of the constraints files. On top of `e2b-rt`'s contents it runs a Jupyter server on loopback 8888 and the code-interpreter API on 49999, both enabled at boot, and diff --git a/docs/egress.md b/docs/egress.md index 2ef9eae7..25d6480e 100644 --- a/docs/egress.md +++ b/docs/egress.md @@ -462,7 +462,8 @@ it, each allowed request reads the claim's env snapshot, without a lock. tunnel or request when it ends, with the payload bytes it moved (`tx` guest to destination, `rx` back) and its upstream, so per-exit traffic can be billed. The counts come from the copies the proxy already runs; the cost is one more -usage-journal append per connection, which is why it is opt-in. +usage-journal append per connection, which is why it is opt-in. A tunnel that a +sandboxd stop cuts records no `egress_bytes` event. ## HTTPS interception diff --git a/docs/performance.md b/docs/performance.md index bb3cd9f3..cd8c1f96 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -67,7 +67,7 @@ keep an idle relay for 30 seconds and send later RPCs on it; use mode C of Every data-plane RPC used to pay a TCP dial, an HTTP Upgrade and, behind a TLS edge, a handshake. From proto 2 silkd serves RPCs back to back, and both -SDKs keep one relay connection per handle with a 30s idle window. +SDKs park up to 8 idle relay connections per handle for a 30s idle window. `e2e/cmd/rpcbench`, warm `rt:24.04` sandbox, `net=none`, n=200 `fs_stat` RPCs per arm, bare-metal host B, client on the node, 2026-09-16 at @@ -88,8 +88,7 @@ loaded node moved every arm together and left the ratio intact. The edge here is a local HTTP/1.1 TLS terminator on loopback, so its handshake carries no network RTT — a client across a WAN saves more, not -less. An old daemon reporting proto 1 makes the SDK dial per call, which is -the plain dial-per-RPC row. +less. The node's own hybrid-vsock `CONNECT` handshake read its `OK ` reply one byte per `read(2)` until #304 moved it to one buffered read: 13 fewer syscalls diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index 64a74a6c..daee9f94 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -9,7 +9,8 @@ All bodies are JSON. Three token kinds: preview mint) and the tenant-scoped listings/deletes below; everything a tenant creates is stamped with its name. Operator surfaces (the per-id sandbox reads, `PUT /v1/sandboxes/{id}/instance-metadata`, `GET /v1/info`, - `PUT /v1/pools`, `PUT/DELETE /v1/volumes/{name}`, `POST/DELETE /v1/drain`, + `PUT /v1/pools`, `POST /v1/config/reload`, the [tenant API](#tenants-v1tenants), + `POST /v1/sandboxes/{id}/move`, `PUT/DELETE /v1/volumes/{name}`, `POST/DELETE /v1/drain`, `GET /metrics`, `GET /v1/usage`, `GET /v1/checkpoints/{id}/blob`) answer a tenant token `403` — authenticated but not authorized; an unknown token stays `401`. On a node with a shared @@ -112,9 +113,6 @@ Auth: `Authorization: Bearer ` (when configured). entry's `writable: true` (see [deploy](deploy.md#dataset-volumes)). `attach_only: true` attaches that entry without mounting it and requires an empty or omitted `mount`. Mounted and attach-only entries can share a list -- `volumes_attach_only` (default `false`) is the legacy claim-wide form of - `attach_only`. The server expands it onto every entry before validation. - It requires at least one volume and rejects a nonempty `mount`. Success: @@ -250,7 +248,7 @@ an unknown and a forbidden volume return the same error text. Errors: 400 unknown template axis, `claim_ref` or metadata over a bound (the message names it), a bad `env` entry, an unknown `on_expire`, invalid/duplicate volumes, -`volumes_attach_only` with no volumes, or an attach-only entry with a nonempty `mount`, +an attach-only entry with a nonempty `mount`, `mode: "rw"` against a non-writable entry, or a volume that is unknown or forbidden (the latter two are deliberately indistinguishable), or @@ -448,9 +446,7 @@ The per-entry field requires coordinated daemon and SDK releases. Older daemons ignore `attach_only` in claims and reject it in live PUT and PATCH with 400. Older Python SDKs reject the field when returned descriptors are reused in requests. Responses without the field do -not safely roundtrip attach-only intent through new clients. The server still -accepts the legacy claim-wide request flag and normalizes old persisted empty -mounts when loading claims. +not safely roundtrip attach-only intent through new clients. ### Partial failure and recovery @@ -900,8 +896,8 @@ URL; releasing the sandbox ends it early either way. 400 a port of 0, 404 unknown id or wrong sandbox token, 501 when the node has no `preview_listen`. The signed token embeds the sandbox id, port, and owner `advertise_addr`, so any node's preview listener can serve it (forwarding -to the owner's main listener) and a released sandbox's URL simply stops -resolving — no revocation list. A browser request to the minted URL answers 503 +to the owner's main listener) and a released sandbox's URL answers 404 — +no revocation list. A browser request to the minted URL answers 503 `lease lost; retry` for a replicated claim while its node's lease is overdue, 503 `sandbox moving to another node; retry` while an [evacuation](#post-v1drain) has yet to move it, and 429 when it has to wake a sandbox while the disk reserve is low or the node is at `max_running`. See [deploy](deploy.md#preview-urls). ## POST /v1/sandboxes/{id}/checkpoint @@ -1236,9 +1232,8 @@ With it, the node answers the `101` before it touches the sandbox, reads the first frame, and serves `fs_stat`, `fs_list`, `fs_read` and `fs_pull` from a reader VM, a warm VM of the sandbox's pool that mounts a copy of the sandbox's disk read-only: the sandbox stays hibernated or archived, no `wake` usage event is recorded, and the read stamps -neither the idle nor the archive clock. An `info` frame pipelined ahead of the -call (the SDKs' first frames) rides along to the reader; one alone, with no call -behind it for 100 ms, wakes the sandbox. A second `info` frame before any call wakes it too. The first frame on the connection that +neither the idle nor the archive clock. An `info` frame wakes the sandbox, since a +reader would answer with its own processes and uptime. The first frame on the connection that is anything else, or any later one, wakes the sandbox and hands the same connection to its silkd with the unread bytes replayed, so a client sees one silkd and no close. When the sandbox sleeps again meanwhile (a new hibernate or diff --git a/docs/sdk-python.md b/docs/sdk-python.md index 44d96ce4..0b3f06ca 100644 --- a/docs/sdk-python.md +++ b/docs/sdk-python.md @@ -130,8 +130,8 @@ Volume sandboxes cannot hibernate, fork, checkpoint, or promote. Egress-lane sandboxes cannot either; each answers 409. Checkpoint branches do not accept volumes of either mode in this version. -`mount=False` on `client.new` or `template.new` leaves the mount to you: the -dictionaries in `sb.volumes` carry no `mount` key; see +`{"attach_only": True}` on a volume entry leaves the mount to you: that +entry in `sb.volumes` carries no `mount` key; see [attach-only volumes](sandboxd-api.md#attach-only-volumes) for finding the device and the consistency contract. For a root client managing a tenant claim, see the diff --git a/docs/sdk.md b/docs/sdk.md index 3cccb7f9..e09edf68 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -212,7 +212,6 @@ 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`, `XXLarge` | `Small` | resource tier: 1cpu/512M, 2cpu/1G, 4cpu/4G, 4cpu/8G, 8cpu/16G | | `WithVolumes(volumes...)` | `Volume{Name, Mount?, Mode?, AttachOnly?}` 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 | legacy claim-wide form of `Volume.AttachOnly`: attach every requested volume without mounting it; the workload finds each device and owns the mount. sandboxd rejects a `Volume.Mount` with 400 | | `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, @@ -232,9 +231,9 @@ moved to another node with a `Lookup`, also when that node is gone, releasing one already gone is not an error, and `Close` is bounded internally so it stays defer-friendly). Any other call on a handle answers 404 once its sandbox has moved; run `Lookup` to tell that from a released sandbox. -Volume sandboxes cannot hibernate, fork, checkpoint, or promote. Egress-lane -sandboxes cannot either, nor take `WithArchiveOnExpire`; each answers 409. Passing -`WithVolumes`, `WithVolumesAttachOnly` or `WithClaimRef` to `Checkpoint.New` returns a local error: +Volume sandboxes and egress-lane sandboxes cannot hibernate, fork, checkpoint, +or promote, nor take `WithArchiveOnExpire`; each answers 409. Passing +`WithVolumes` or `WithClaimRef` to `Checkpoint.New` returns a local error: checkpoint branches support neither volumes nor a claim reference in this version. `WithNetwork` or `WithSize` to `Checkpoint.New` or `Template.New` does too: the snapshot pins the lane and size. @@ -297,8 +296,8 @@ refusal, the release hand-off and a failed sandbox's release-only rule are in th [same-sandbox replacement tutorial](live-volumes.md). A settled retained entry receives no new attach or mount calls. -`WithVolumesAttachOnly()` leaves the mount to you: the entries in -`Sandbox.Volumes` carry an empty `Mount`; see +`Volume{AttachOnly: true}` leaves the mount to you: the entry in +`Sandbox.Volumes` carries an empty `Mount`; see [attach-only volumes](sandboxd-api.md#attach-only-volumes) for finding the device and the consistency contract. diff --git a/e2e/cmd/androidsmoke/main.go b/e2e/cmd/androidsmoke/main.go index 40e81665..aef2bcc4 100644 --- a/e2e/cmd/androidsmoke/main.go +++ b/e2e/cmd/androidsmoke/main.go @@ -116,8 +116,7 @@ func adbHandshake(ctx context.Context, sb *sandbox.Sandbox) error { } } -// dialAdb sends one ADB CNXN message; adbd replies CNXN (unauthenticated) -// or AUTH — either proves a live endpoint. +// dialAdb sends one ADB CNXN; a CNXN or AUTH reply proves a live adbd. func dialAdb(ctx context.Context, sb *sandbox.Sandbox) (string, error) { pc, err := sb.DialPort(ctx, adbPort) if err != nil { diff --git a/e2e/cmd/guestserver/main.go b/e2e/cmd/guestserver/main.go index e7cd2138..fa4958ee 100644 --- a/e2e/cmd/guestserver/main.go +++ b/e2e/cmd/guestserver/main.go @@ -18,35 +18,6 @@ import ( "sync" ) -func main() { - addr := flag.String("addr", "127.0.0.1:49983", "loopback address to listen on") - halfClose := flag.String("half-close-addr", "", "raw address that greets, shuts its write side, then reads") - flag.Parse() - - var seen halfCloseSink - if *halfClose != "" { - go seen.serve(*halfClose) - } - - mux := http.NewServeMux() - mux.HandleFunc("GET /health", func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusNoContent) - }) - mux.HandleFunc("GET /half-close-seen", func(w http.ResponseWriter, _ *http.Request) { - _, _ = io.WriteString(w, seen.read()) - }) - mux.HandleFunc("/", report) - - var protocols http.Protocols - protocols.SetHTTP1(true) - protocols.SetUnencryptedHTTP2(true) - srv := &http.Server{Addr: *addr, Handler: mux, Protocols: &protocols} //nolint:gosec // loopback test server inside a sandbox; no header timeout by design - if err := srv.ListenAndServe(); err != nil { - fmt.Fprintln(os.Stderr, "guestserver:", err) - os.Exit(1) - } -} - // halfCloseSink greets, shuts its write side, and keeps reading what the client sends after. type halfCloseSink struct { mu sync.Mutex @@ -96,6 +67,35 @@ func (h *halfCloseSink) read() string { return h.got } +func main() { + addr := flag.String("addr", "127.0.0.1:49983", "loopback address to listen on") + halfClose := flag.String("half-close-addr", "", "raw address that greets, shuts its write side, then reads") + flag.Parse() + + var seen halfCloseSink + if *halfClose != "" { + go seen.serve(*halfClose) + } + + mux := http.NewServeMux() + mux.HandleFunc("GET /health", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusNoContent) + }) + mux.HandleFunc("GET /half-close-seen", func(w http.ResponseWriter, _ *http.Request) { + _, _ = io.WriteString(w, seen.read()) + }) + mux.HandleFunc("/", report) + + var protocols http.Protocols + protocols.SetHTTP1(true) + protocols.SetUnencryptedHTTP2(true) + srv := &http.Server{Addr: *addr, Handler: mux, Protocols: &protocols} //nolint:gosec // loopback test server inside a sandbox; no header timeout by design + if err := srv.ListenAndServe(); err != nil { + fmt.Fprintln(os.Stderr, "guestserver:", err) + os.Exit(1) + } +} + // report answers with what the guest received, so a caller can assert the relay changed nothing. func report(w http.ResponseWriter, r *http.Request) { body := new(strings.Builder) diff --git a/e2e/cmd/rpcbench/main.go b/e2e/cmd/rpcbench/main.go index 4e6aa1a0..6e7e2874 100644 --- a/e2e/cmd/rpcbench/main.go +++ b/e2e/cmd/rpcbench/main.go @@ -2,7 +2,7 @@ // A dials+upgrades per RPC by hand; mode C is the SDK's own path, one kept // connection serving RPCs back to back. A and C interleave sample by sample // and swap which one leads, so node drift cannot land on one arm. Run by -// hand against a claimed sandbox: +// hand, or by scripts/bench.sh; it claims its own sandbox: // // rpcbench -addr -token -template -n 200 // @@ -29,6 +29,14 @@ import ( "github.com/cocoonstack/sandbox/sdk/go/silkd" ) +// bufferedConn reads through the handshake reader so bytes behind the 101 are kept. +type bufferedConn struct { + net.Conn + r *bufio.Reader +} + +func (b bufferedConn) Read(p []byte) (int, error) { return b.r.Read(p) } + func main() { addr := flag.String("addr", "127.0.0.1:7777", "sandboxd address") token := flag.String("token", "", "node api token") @@ -146,15 +154,6 @@ func statRPC(conn net.Conn) error { return nil } -// bufferedConn reads through the handshake reader so bytes coalesced behind -// the 101 are never lost. -type bufferedConn struct { - net.Conn - r *bufio.Reader -} - -func (b bufferedConn) Read(p []byte) (int, error) { return b.r.Read(p) } - // edgeTLS returns the client config for an https address, or nil for a plain one. func edgeTLS(addr, caCert string) (*tls.Config, error) { if !strings.HasPrefix(addr, "https://") { diff --git a/e2e/cmd/smoke/main.go b/e2e/cmd/smoke/main.go index b87d720b..cd2d4843 100644 --- a/e2e/cmd/smoke/main.go +++ b/e2e/cmd/smoke/main.go @@ -57,8 +57,7 @@ func run(addr, token, template, lspTemplate string, egress bool) error { return err } defer func() { _ = sb.Close() }() - // Info is node-local: on a cluster the claim may have been redirected, so - // hibernated-count assertions must ask the owner, not the entry node. + // Info is node-local; hibernated counts come from the owner, not the entry node. ownerClient := client if sb.Owner() != addr { if ownerClient, err = sandbox.Connect(sb.Owner(), sandbox.WithAPIToken(token)); err != nil { @@ -282,7 +281,6 @@ func smokeGit(ctx context.Context, sb *sandbox.Sandbox) error { return fmt.Errorf("after checkout: current=%q, want feature", br.Current) } - // The none lane answers push with the typed unimplemented error. if err := sb.GitPush(ctx, "/work", ""); !isSilkdKind(err, wire.KindUnimplemented) { return fmt.Errorf("push on none lane: %v, want unimplemented", err) } @@ -643,7 +641,6 @@ func smokeProcs(ctx context.Context, sb *sandbox.Sandbox) error { return fmt.Errorf("logs replay %q missing mark", out.String()) } - // A long-runner dies to kill; its pid must be gone from the next attach. pid, err = sb.Spawn(ctx, sandbox.Cmd{Argv: []string{"sleep", "30"}}) if err != nil { return fmt.Errorf("spawn sleeper: %w", err) diff --git a/e2e/cmd/volumesmoke/main.go b/e2e/cmd/volumesmoke/main.go index 2f968c8b..14f79780 100644 --- a/e2e/cmd/volumesmoke/main.go +++ b/e2e/cmd/volumesmoke/main.go @@ -191,8 +191,7 @@ func runLive(ctx context.Context, client *sandbox.Client, template, volume, prob 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. +// runWritable proves a writer's bytes outlive its release, exclude a concurrent writer, and reach the next reader. func runWritable(ctx context.Context, client *sandbox.Client, template, volume string) error { const ( mount = "/datasets/e2e-rw" diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go index cf17c404..0c987ca1 100644 --- a/e2e/e2e_test.go +++ b/e2e/e2e_test.go @@ -759,8 +759,7 @@ func TestAttachOnlyVolumeEndToEnd(t *testing.T) { []config.VolumeSpec{{Name: "scratch", Path: scratch, Writable: true}}) sb, err := st.client.New(t.Context(), "rt:24.04", - sandbox.WithVolumes(sandbox.Volume{Name: "scratch", Mode: "rw"}), - sandbox.WithVolumesAttachOnly()) + sandbox.WithVolumes(sandbox.Volume{Name: "scratch", Mode: "rw", AttachOnly: true})) if err != nil { t.Fatalf("attach-only claim: %v", err) } @@ -799,7 +798,7 @@ func TestAttachOnlyVolumeWireShape(t *testing.T) { }{ { "attach-only omits the mount", - `{"template":"rt:24.04","volumes_attach_only":true,"volumes":[{"name":"scratch","mode":"rw"}]}`, + `{"template":"rt:24.04","volumes":[{"name":"scratch","mode":"rw","attach_only":true}]}`, map[string]any{"name": "scratch", "mode": "rw", "attach_only": true}, }, { @@ -817,9 +816,6 @@ func TestAttachOnlyVolumeWireShape(t *testing.T) { 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"]) diff --git a/e2e/internal/harness/harness.go b/e2e/internal/harness/harness.go index 5a6cf007..ebf9e6ec 100644 --- a/e2e/internal/harness/harness.go +++ b/e2e/internal/harness/harness.go @@ -16,48 +16,6 @@ import ( const GuestProxy = "http://127.0.0.1:3128" -// Claim connects to a node and claims one sandbox; the caller owns Close. -func Claim(ctx context.Context, addr, token, template string, opts ...sandbox.Option) (*sandbox.Client, *sandbox.Sandbox, error) { - client, err := sandbox.Connect(addr, sandbox.WithAPIToken(token)) - if err != nil { - return nil, nil, err - } - sb, err := client.New(ctx, template, opts...) - if err != nil { - return nil, nil, fmt.Errorf("claim: %w", err) - } - return client, sb, nil -} - -// HTTPOverPort sends one HTTP/1.1 request over the port relay and returns a 200 body; the Host stays "localhost" because Chrome's DevTools allowlist accepts it but not proxied vhosts. -func HTTPOverPort(ctx context.Context, sb *sandbox.Sandbox, port uint16, method, path string, body []byte) ([]byte, error) { - pc, err := sb.DialPort(ctx, port) - if err != nil { - return nil, err - } - defer func() { _ = pc.Close() }() - head := fmt.Sprintf("%s %s HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n", method, path) - if body != nil { - head += fmt.Sprintf("Content-Type: application/json\r\nContent-Length: %d\r\n", len(body)) - } - if _, err = fmt.Fprintf(pc, "%s\r\n%s", head, body); err != nil { - return nil, err - } - resp, err := http.ReadResponse(bufio.NewReader(pc), nil) - if err != nil { - return nil, err - } - defer func() { _ = resp.Body.Close() }() - out, err := io.ReadAll(resp.Body) - if err != nil { - return nil, err - } - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("%s %s: %s: %s", method, path, resp.Status, strings.TrimSpace(string(out))) - } - return out, nil -} - type RelayConn interface { net.Conn CloseWrite() error @@ -109,6 +67,61 @@ func (p PortRelay) Dial(ctx context.Context, port uint16) (RelayConn, error) { return &portRelayConn{Conn: conn, reader: reader}, nil } +type portRelayConn struct { + net.Conn + reader *bufio.Reader +} + +func (c *portRelayConn) Read(p []byte) (int, error) { + return c.reader.Read(p) +} + +func (c *portRelayConn) CloseWrite() error { + return c.Conn.(*net.TCPConn).CloseWrite() +} + +// Claim connects to a node and claims one sandbox; the caller owns Close. +func Claim(ctx context.Context, addr, token, template string, opts ...sandbox.Option) (*sandbox.Client, *sandbox.Sandbox, error) { + client, err := sandbox.Connect(addr, sandbox.WithAPIToken(token)) + if err != nil { + return nil, nil, err + } + sb, err := client.New(ctx, template, opts...) + if err != nil { + return nil, nil, fmt.Errorf("claim: %w", err) + } + return client, sb, nil +} + +// HTTPOverPort sends one HTTP/1.1 request over the port relay and returns a 200 body; the Host stays "localhost" because Chrome's DevTools allowlist accepts it but not proxied vhosts. +func HTTPOverPort(ctx context.Context, sb *sandbox.Sandbox, port uint16, method, path string, body []byte) ([]byte, error) { + pc, err := sb.DialPort(ctx, port) + if err != nil { + return nil, err + } + defer func() { _ = pc.Close() }() + head := fmt.Sprintf("%s %s HTTP/1.1\r\nHost: localhost\r\nConnection: close\r\n", method, path) + if body != nil { + head += fmt.Sprintf("Content-Type: application/json\r\nContent-Length: %d\r\n", len(body)) + } + if _, err = fmt.Fprintf(pc, "%s\r\n%s", head, body); err != nil { + return nil, err + } + resp, err := http.ReadResponse(bufio.NewReader(pc), nil) + if err != nil { + return nil, err + } + defer func() { _ = resp.Body.Close() }() + out, err := io.ReadAll(resp.Body) + if err != nil { + return nil, err + } + if resp.StatusCode != http.StatusOK { + return nil, fmt.Errorf("%s %s: %s: %s", method, path, resp.Status, strings.TrimSpace(string(out))) + } + return out, nil +} + // Request builds one call to the endpoint, optionally asking for the upgrade. func Request(ctx context.Context, url, token string, upgrade bool) (*http.Request, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil) @@ -124,16 +137,3 @@ func Request(ctx context.Context, url, token string, upgrade bool) (*http.Reques } return req, nil } - -type portRelayConn struct { - net.Conn - reader *bufio.Reader -} - -func (c *portRelayConn) Read(p []byte) (int, error) { - return c.reader.Read(p) -} - -func (c *portRelayConn) CloseWrite() error { - return c.Conn.(*net.TCPConn).CloseWrite() -} diff --git a/e2e/wire_parity_test.go b/e2e/wire_parity_test.go index 012f172e..d7b0e5df 100644 --- a/e2e/wire_parity_test.go +++ b/e2e/wire_parity_test.go @@ -150,7 +150,5 @@ func parseSDKFields(t *testing.T) map[string]map[string]string { func splitJSONTag(tag string) (name, opts string) { name, rest, _ := strings.Cut(tag, ",") - options := strings.Split(rest, ",") - slices.Sort(options) - return name, strings.Join(options, ",") + return name, strings.Join(slices.Sorted(strings.SplitSeq(rest, ",")), ",") } diff --git a/mcp/server.go b/mcp/server.go index d30b2b5f..7c2264d4 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -198,6 +198,38 @@ func (s *server) dropCkpt(id string) { delete(s.ckpts, id) } +// parseAndBox decodes a tool's arguments and resolves their sandbox_id to a live handle, the prologue every sandbox-scoped tool shares. +func (s *server) parseAndBox[T sandboxIDer](raw jsontext.Value) (T, *sandbox.Sandbox, error) { + var args T + if err := parse(raw, &args); err != nil { + return args, nil, err + } + sb, err := s.box(args.id()) + return args, sb, err +} + +func (s *server) boxArg(raw jsontext.Value) (*sandbox.Sandbox, error) { + _, sb, err := s.parseAndBox[sandboxArg](raw) + return sb, err +} + +// checkpointArg resolves a checkpoint_id argument: a handle minted in this session when available, else a fresh one, because checkpoints outlive sessions. +func (s *server) checkpointArg(raw jsontext.Value) (*sandbox.Checkpoint, error) { + var args struct { + CheckpointID string `json:"checkpoint_id"` + } + if err := parse(raw, &args); err != nil { + return nil, err + } + if ckpt, ok := s.ckpt(args.CheckpointID); ok { + return ckpt, nil + } + if args.CheckpointID == "" { + return nil, fmt.Errorf("checkpoint_id is required") + } + return s.client.Checkpoint(args.CheckpointID), nil +} + func result(id jsontext.Value, v any) rpcResponse { return rpcResponse{JSONRPC: "2.0", ID: id, Result: v} } diff --git a/mcp/tools.go b/mcp/tools.go index 93d20954..06aa820f 100644 --- a/mcp/tools.go +++ b/mcp/tools.go @@ -88,6 +88,35 @@ type tool struct { handler func(context.Context, *server, jsontext.Value) (string, error) } +type sandboxArg struct { + SandboxID string `json:"sandbox_id"` +} + +func (a sandboxArg) id() string { return a.SandboxID } + +type sandboxIDer interface{ id() string } + +// cappedOutput keeps the first execOutputCap bytes: an agent that tails a firehose must not grow this process. +type cappedOutput struct { + strings.Builder + stopAtCap bool + truncated bool +} + +func (o *cappedOutput) Write(p []byte) (int, error) { + n := len(p) + if room := execOutputCap - o.Len(); len(p) > room { + if o.stopAtCap { + return 0, errOutputCap + } + p, o.truncated = p[:room], true + } + _, _ = o.Builder.Write(p) + return n, nil +} + +type props map[string]map[string]any + func toolSpecs() []map[string]any { specs := make([]map[string]any, len(tools)) for i, t := range tools { @@ -119,14 +148,6 @@ func toolCreateSandbox(ctx context.Context, s *server, raw jsontext.Value) (stri return jsonText(map[string]any{"sandbox_id": sb.ID, "deadline": sb.Deadline}), nil } -type sandboxArg struct { - SandboxID string `json:"sandbox_id"` -} - -func (a sandboxArg) id() string { return a.SandboxID } - -type sandboxIDer interface{ id() string } - type cmdArgs struct { sandboxArg Command string `json:"command"` @@ -141,25 +162,6 @@ func parseCommand(s *server, raw jsontext.Value) (cmdArgs, *sandbox.Sandbox, err return args, sb, err } -// cappedOutput keeps the first execOutputCap bytes: an agent that tails a firehose must not grow this process. -type cappedOutput struct { - strings.Builder - stopAtCap bool - truncated bool -} - -func (o *cappedOutput) Write(p []byte) (int, error) { - n := len(p) - if room := execOutputCap - o.Len(); len(p) > room { - if o.stopAtCap { - return 0, errOutputCap - } - p, o.truncated = p[:room], true - } - _, _ = o.Builder.Write(p) - return n, nil -} - func toolExec(ctx context.Context, s *server, raw jsontext.Value) (string, error) { args, sb, err := parseCommand(s, raw) if err != nil { @@ -418,38 +420,6 @@ func toolNodeInfo(ctx context.Context, s *server, _ jsontext.Value) (string, err return jsonText(info), nil } -// parseAndBox decodes a tool's arguments and resolves their sandbox_id to a live handle, the prologue every sandbox-scoped tool shares. -func (s *server) parseAndBox[T sandboxIDer](raw jsontext.Value) (T, *sandbox.Sandbox, error) { - var args T - if err := parse(raw, &args); err != nil { - return args, nil, err - } - sb, err := s.box(args.id()) - return args, sb, err -} - -func (s *server) boxArg(raw jsontext.Value) (*sandbox.Sandbox, error) { - _, sb, err := s.parseAndBox[sandboxArg](raw) - return sb, err -} - -// checkpointArg resolves a checkpoint_id argument: a handle minted in this session when available, else a fresh one, because checkpoints outlive sessions. -func (s *server) checkpointArg(raw jsontext.Value) (*sandbox.Checkpoint, error) { - var args struct { - CheckpointID string `json:"checkpoint_id"` - } - if err := parse(raw, &args); err != nil { - return nil, err - } - if ckpt, ok := s.ckpt(args.CheckpointID); ok { - return ckpt, nil - } - if args.CheckpointID == "" { - return nil, fmt.Errorf("checkpoint_id is required") - } - return s.client.Checkpoint(args.CheckpointID), nil -} - func parse(raw jsontext.Value, v any) error { if len(raw) == 0 { return nil @@ -469,8 +439,6 @@ func jsonText(v any) string { return string(out) } -type props map[string]map[string]any - func str(description string) map[string]any { return map[string]any{"type": "string", "description": description} } diff --git a/os-image/android/15/Dockerfile b/os-image/android/15/Dockerfile index 355d16f8..5a049b8b 100644 --- a/os-image/android/15/Dockerfile +++ b/os-image/android/15/Dockerfile @@ -7,9 +7,7 @@ # The base is public on ghcr (anonymous pull), pinned by tag; override for a # lineage bump. ARG ANDROID_BASE_IMAGE=ghcr.io/cocoonstack/cocoon/android:15.0 -# silkd carrier image (built from silkd/Dockerfile); CI passes the -# version-pinned tag. -ARG SILKD_IMAGE=ghcr.io/cocoonstack/sandbox/silkd:0.1.22 +ARG SILKD_IMAGE FROM ${SILKD_IMAGE} AS silkd diff --git a/os-image/android/silkd.rc b/os-image/android/silkd.rc index e508fe56..ece0a61a 100644 --- a/os-image/android/silkd.rc +++ b/os-image/android/silkd.rc @@ -11,7 +11,7 @@ service silkd /system/bin/silkd group root setenv SILKD_PORT 2048 setenv SHELL /system/bin/sh - # Mirrors the base:24.04 unit; forwarded into execs only on the no-NIC lane. + # Same relay URLs as the base:24.04 unit; forwarded into execs only on the no-NIC lane. setenv http_proxy http://127.0.0.1:3128 setenv https_proxy http://127.0.0.1:3128 setenv no_proxy localhost,127.0.0.1,::1 diff --git a/os-image/base/24.04/Dockerfile b/os-image/base/24.04/Dockerfile index c106ab7f..f835d1b9 100644 --- a/os-image/base/24.04/Dockerfile +++ b/os-image/base/24.04/Dockerfile @@ -1,11 +1,7 @@ # sandbox base: Ubuntu 24.04 userland on the sandbox boot chain. No distro # kernel, no initramfs-tools — /boot ships from the prebuilt boot artifact. -# CI overrides BOOT_IMAGE from boot/kernel/VERSION; the default is only a -# local-build fallback and may lag the pin. -ARG BOOT_IMAGE=ghcr.io/cocoonstack/sandbox/boot:6.18.37 -# silkd carrier image (built from silkd/Dockerfile). CI overrides with the -# version-pinned tag from images.yml; the default is only a local-build fallback. -ARG SILKD_IMAGE=ghcr.io/cocoonstack/sandbox/silkd:0.1.22 +ARG BOOT_IMAGE +ARG SILKD_IMAGE FROM ${BOOT_IMAGE} AS boot FROM ${SILKD_IMAGE} AS silkd diff --git a/os-image/e2b-ci/24.04/Dockerfile b/os-image/e2b-ci/24.04/Dockerfile index f1d80de3..4f420373 100644 --- a/os-image/e2b-ci/24.04/Dockerfile +++ b/os-image/e2b-ci/24.04/Dockerfile @@ -1,7 +1,7 @@ # e2b's code interpreter on the e2b data plane: envd, a Jupyter server and the # code-interpreter API on 49999, both started at boot. The recipe is # e2b-dev/code-interpreter template/ at CI_RECIPE_SHA, fetched and checksummed -# here, with PATCHES applied; constraints.txt and server-constraints.txt pin the +# here, with PATCHES.md applied; constraints.txt and server-constraints.txt pin the # full sets it resolves to, so a rebuild of one commit installs the same # packages. Python kernel only: the recipe's javascript, R and Java kernels # install from unpinned sources. diff --git a/os-image/e2b-ci/PATCHES b/os-image/e2b-ci/PATCHES.md similarity index 100% rename from os-image/e2b-ci/PATCHES rename to os-image/e2b-ci/PATCHES.md diff --git a/os-image/e2b-rt/24.04/Dockerfile b/os-image/e2b-rt/24.04/Dockerfile index 675771fe..f0d469ca 100644 --- a/os-image/e2b-rt/24.04/Dockerfile +++ b/os-image/e2b-rt/24.04/Dockerfile @@ -31,6 +31,7 @@ RUN curl -fsSL -o /usr/local/bin/envd \ ' }' \ '}' \ > /etc/nftables-envd.conf && \ + # [envd] Firecracker mode: silkd answers MMDS on the loopback alias the guard adds. printf '%s\n' \ '[Unit]' \ 'Description=drop non-loopback traffic to envd' \ @@ -46,7 +47,6 @@ RUN curl -fsSL -o /usr/local/bin/envd \ '[Install]' \ 'WantedBy=sysinit.target' \ > /etc/systemd/system/envd-guard.service && \ - # [envd] Firecracker mode: silkd answers MMDS on the loopback alias the guard adds. # -no-cgroups: envd's freeze/thaw is Firecracker's pause model; cocoon # hibernates the whole VM, and the guest's cgroup tree is systemd's. printf '%s\n' \ diff --git a/sandboxd/archives/archives.go b/sandboxd/archives/archives.go index f81f1dc6..83207282 100644 --- a/sandboxd/archives/archives.go +++ b/sandboxd/archives/archives.go @@ -31,7 +31,7 @@ type Record struct { // Registry keeps the rows; every write is a compare-and-set, and every epoch it hands out is new. type Registry interface { - // Put writes rec for rec.Owner as a live row at a new epoch and returns it; ErrLost when another owner holds the row. + // Put writes rec for rec.Owner as a live row at a new epoch and returns it; ErrLost when the row has another owner or none. Put(ctx context.Context, rec Record) (int64, error) // Replace writes rec over the live row of rec.Owner at epoch expect, at a new epoch; with expect 0 it is Put. Replace(ctx context.Context, rec Record, expect int64) (int64, error) diff --git a/sandboxd/egress/intercept_test.go b/sandboxd/egress/intercept_test.go index 78962397..547825d7 100644 --- a/sandboxd/egress/intercept_test.go +++ b/sandboxd/egress/intercept_test.go @@ -615,7 +615,7 @@ type echoReply struct { body string } -// seen decodes the upstream's /echo report; its hex survives the scrub, the query reflected after it does not. +// seen's hex report survives the scrub; the query reflected after it does not. func (r echoReply) seen(t *testing.T) (echoed, string) { t.Helper() report, reflected, _ := strings.Cut(r.body, "\n") diff --git a/sandboxd/egress/policy.go b/sandboxd/egress/policy.go index 42e5d54d..d9672cc6 100644 --- a/sandboxd/egress/policy.go +++ b/sandboxd/egress/policy.go @@ -101,7 +101,7 @@ func (r Rule) matchPort(port uint16) bool { // Policy is one sandbox's egress allow-list; a request matching no rule is denied. type Policy struct { Allow []Rule `json:"allow"` - Socks5 bool `json:"socks5,omitzero"` // opts the SOCKS5 door in; a rule must still admit CONNECT + Socks5 bool `json:"socks5,omitzero"` } // Intercepts reports whether any rule may terminate HTTPS; nil is false. diff --git a/sandboxd/egress/secrets.go b/sandboxd/egress/secrets.go index a85ac958..89773482 100644 --- a/sandboxd/egress/secrets.go +++ b/sandboxd/egress/secrets.go @@ -14,7 +14,7 @@ import ( // nonInjectable lists the secret headers the transport owns or strips. var nonInjectable = slices.Concat([]string{"Host", "Content-Length"}, hopHeaders) -// SecretSpec declares a credential the proxy injects, valued from the env ValueEnv names, or Name when that is empty. +// SecretSpec declares a credential the proxy injects, valued from the node env ValueEnv names, or from the claim env Name when ValueEnv is empty. type SecretSpec struct { Name string `json:"name"` Header string `json:"header"` diff --git a/sandboxd/engine/engine.go b/sandboxd/engine/engine.go index 73d111b9..fd604208 100644 --- a/sandboxd/engine/engine.go +++ b/sandboxd/engine/engine.go @@ -58,10 +58,10 @@ const ( connectTimeout = 10 * time.Second trimTimeout = 10 * time.Second probeInterval = 20 * time.Millisecond - connectMax = 64 // "OK " handshake reply cap - infoMax = 4096 // info response frame cap + connectMax = 64 // "OK " handshake reply cap + infoMax = 4096 outputTail = 400 - portForwardMax = 4096 // port_forward handshake reply line cap + portForwardMax = 4096 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 memoryDumps = 8 @@ -131,7 +131,7 @@ func (e *Engine) Version(ctx context.Context) (string, error) { return "", fmt.Errorf("cocoon version: no Version line in output") } -// VersionWarning returns a non-empty warning when cocoon's version is below RequiredCocoon. +// VersionWarning returns a non-empty warning when cocoon's version is below RequiredCocoon or cannot be read. func (e *Engine) VersionWarning(ctx context.Context) (version, warning string) { v, err := e.Version(ctx) if err != nil { diff --git a/sandboxd/engine/engine_test.go b/sandboxd/engine/engine_test.go index 705afbf0..478e2ff7 100644 --- a/sandboxd/engine/engine_test.go +++ b/sandboxd/engine/engine_test.go @@ -287,7 +287,7 @@ func TestAMemoryCommandGetsItsLongerTermGraceAndAColdBootTheCommandOne(t *testin defer cancel() done := make(chan error, 1) go func() { done <- tt.run(ctx, New(bin, nil, nil, false, false, "")) }() - waitFile(ready) + waitFile(t, ready) cancel() if err := <-done; err == nil || !strings.Contains(err.Error(), tt.want) { t.Errorf("got %v, want %q", err, tt.want) @@ -307,7 +307,7 @@ func TestCloseWaitsForARunningCommandAndRefusesNewOnes(t *testing.T) { e := New(bin, nil, nil, false, false, "") done := make(chan error, 1) go func() { done <- e.Hibernate(t.Context(), "vm", "snap") }() - waitFile(ready) + waitFile(t, ready) closed := make(chan struct{}) go func() { e.Close() @@ -471,12 +471,14 @@ func sockPath(t *testing.T) string { return filepath.Join(dir, "mux.sock") } -func waitFile(path string) { +func waitFile(t *testing.T, path string) { + t.Helper() for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) { if _, err := os.Stat(path); err == nil { return } } + t.Fatalf("%s not created within 5s", path) } func fakeCocoon(t *testing.T, script string) { diff --git a/sandboxd/engine/portconn.go b/sandboxd/engine/portconn.go index 23c47a3f..8f1220bc 100644 --- a/sandboxd/engine/portconn.go +++ b/sandboxd/engine/portconn.go @@ -48,7 +48,7 @@ func (g *guestPortConn) Read(p []byte) (int, error) { case "data": g.pending = frame.Data case "done", "": - return 0, io.EOF // the guest closed the forwarded port + return 0, io.EOF default: return 0, fmt.Errorf("port stream ended: %s", frame.Type) } @@ -72,7 +72,6 @@ func (g *guestPortConn) Write(p []byte) (int, error) { written := 0 for len(p) > 0 { n := min(len(p), wire.PortWriteChunk) - // The hot relay path reuses one buffer instead of allocating per chunk. g.wbuf = wire.AppendBulkRequest(g.wbuf, "data", p[:n]) if _, err := g.Conn.Write(g.wbuf); err != nil { return written, err diff --git a/sandboxd/engine/silkd.go b/sandboxd/engine/silkd.go index 67534515..a5dc6e26 100644 --- a/sandboxd/engine/silkd.go +++ b/sandboxd/engine/silkd.go @@ -18,7 +18,7 @@ const ( silkdReadMax = 64 << 10 mountInfoMax = 1 << 20 guestOutputMax = 4096 - // guestExecPATH is set because silkd starts the guest command with an empty environment. + // guestExecPATH overrides a PATH the claim's env sets for host-driven commands. guestExecPATH = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" ) diff --git a/sandboxd/mesh/mesh_test.go b/sandboxd/mesh/mesh_test.go index e3b3f7b3..fcbeac46 100644 --- a/sandboxd/mesh/mesh_test.go +++ b/sandboxd/mesh/mesh_test.go @@ -170,8 +170,8 @@ func TestCandidatesPowerOfTwo(t *testing.T) { } func TestTwoNodeClusterGossipsPools(t *testing.T) { - a := startNode(t, "127.0.0.1", 0, "node-a") - b := startNode(t, "127.0.0.1", 0, "node-b") + a := startNode(t, "node-a") + b := startNode(t, "node-b") if err := b.mesh.Join([]string{a.addr}); err != nil { t.Fatalf("join: %v", err) } @@ -193,8 +193,8 @@ func TestTwoNodeClusterGossipsPools(t *testing.T) { } func TestARuntimeDigestChangeReachesThePeer(t *testing.T) { - a := startNode(t, "127.0.0.1", 0, "node-a") - b := startNode(t, "127.0.0.1", 0, "node-b") + a := startNode(t, "node-a") + b := startNode(t, "node-b") if err := b.mesh.Join([]string{a.addr}); err != nil { t.Fatalf("join: %v", err) } @@ -299,12 +299,12 @@ type node struct { addr string } -func startNode(t *testing.T, host string, port int, id string) *node { +func startNode(t *testing.T, id string) *node { t.Helper() cfg := memberlist.DefaultLocalConfig() - cfg.BindAddr = host - cfg.BindPort = port - cfg.AdvertiseAddr = host + cfg.BindAddr = "127.0.0.1" + cfg.BindPort = 0 + cfg.AdvertiseAddr = "127.0.0.1" cfg.PushPullInterval = 200 * time.Millisecond cfg.Logger = discardLogger() m, err := New(t.Context(), cfg, NodeState{NodeID: id, Addr: id + ":7777", Digest: "shared"}, nil) @@ -312,5 +312,5 @@ func startNode(t *testing.T, host string, port int, id string) *node { t.Fatalf("new mesh %s: %v", id, err) } t.Cleanup(func() { _ = m.Shutdown() }) - return &node{mesh: m, addr: fmt.Sprintf("%s:%d", host, m.ml.LocalNode().Port)} + return &node{mesh: m, addr: fmt.Sprintf("127.0.0.1:%d", m.ml.LocalNode().Port)} } diff --git a/sandboxd/outbound/outbound.go b/sandboxd/outbound/outbound.go index ea399c08..cb48f5b5 100644 --- a/sandboxd/outbound/outbound.go +++ b/sandboxd/outbound/outbound.go @@ -159,7 +159,7 @@ func (h *Host) ArmProxy(ctx context.Context, sb *types.Sandbox) error { func (h *Host) Prebind(ctx context.Context, sb *types.Sandbox) { v := h.o.View() policy := v.poolEgress[sb.Key] - if !v.guarded || policy == nil || sb.VsockSocket == "" { + if policy == nil || sb.VsockSocket == "" { return } d, err := bindDoors(sb.VsockSocket, policy.ServesSocks()) @@ -344,10 +344,10 @@ func (h *Host) lockNIC(ctx context.Context, sb *types.Sandbox) error { func (h *Host) tapOf(ctx context.Context, vmName string) (string, error) { vm, ok, err := h.o.Engine.Inspect(ctx, vmName) if err != nil { - return "", fmt.Errorf("list %s: %w", vmName, err) + return "", fmt.Errorf("inspect %s: %w", vmName, err) } if !ok { - return "", fmt.Errorf("no NIC config for %s", vmName) + return "", fmt.Errorf("vm %s not found", vmName) } if tap := vm.TapDevice(); tap != "" { return tap, nil diff --git a/sandboxd/pool/archive_test.go b/sandboxd/pool/archive_test.go index 5dab0449..907de7d1 100644 --- a/sandboxd/pool/archive_test.go +++ b/sandboxd/pool/archive_test.go @@ -154,7 +154,7 @@ func TestArchiveCkHiddenAcrossNodes(t *testing.T) { if err := mB.DeleteCheckpoint(t.Context(), ck, "", DeleteFleet); !errors.Is(err, ErrUnknownCheckpoint) { t.Errorf("peer delete: %v, want ErrUnknownCheckpoint", err) } - tuneView(mB, func(v *configView) { v.ckptTTL = time.Nanosecond }) + editView(mB, func(v *configView) { v.ckptTTL = time.Nanosecond }) mB.sweepExpiredCheckpoints(t.Context()) if !ckExists(t, mA, ck) { t.Fatal("peer TTL sweep deleted the archive wake image") @@ -301,7 +301,7 @@ func TestSweepExpiredCheckpointsIsANoopWithoutTTL(t *testing.T) { if err != nil { t.Fatalf("Checkpoint: %v", err) } - tuneView(m, func(v *configView) { v.ckptTTL = 0 }) + editView(m, func(v *configView) { v.ckptTTL = 0 }) m.sweepExpiredCheckpoints(t.Context()) if !ckExists(t, m, ck.ID) { t.Fatal("a zero TTL sweep deleted a checkpoint") @@ -311,7 +311,7 @@ func TestSweepExpiredCheckpointsIsANoopWithoutTTL(t *testing.T) { func TestArchiveTTLSweepSparesLiveCheckpoint(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng, archivePool(3600)) - tuneView(m, func(v *configView) { v.ckptTTL = time.Nanosecond }) + editView(m, func(v *configView) { v.ckptTTL = time.Nanosecond }) sb := mustClaim(t, m, testKey) id, token := sb.ID, sb.Token mustArchive(t, m, sb) diff --git a/sandboxd/pool/capture.go b/sandboxd/pool/capture.go index 57da7e01..fd635aa3 100644 --- a/sandboxd/pool/capture.go +++ b/sandboxd/pool/capture.go @@ -70,6 +70,10 @@ func (m *Manager) exportSource(ctx context.Context, sb *types.Sandbox, exportDir sb.Transition.Unlock() return "", "", false, captureErr } + if settleErr := m.settleCapture(ctx, sb); settleErr != nil { + sb.Transition.Unlock() + return "", "", false, settleErr + } m.trimForCapture(ctx, sb) snap, cleanup, err := m.sourceSnap(ctx, sb) if err != nil { diff --git a/sandboxd/pool/checkpoint.go b/sandboxd/pool/checkpoint.go index efa26a61..4e177c7c 100644 --- a/sandboxd/pool/checkpoint.go +++ b/sandboxd/pool/checkpoint.go @@ -65,7 +65,6 @@ func (m *Manager) Checkpoint(ctx context.Context, id string, cred Cred, name, te // ClaimCheckpoint provisions a fresh claim cloned from a checkpoint, attributed to tenant. func (m *Manager) ClaimCheckpoint(ctx context.Context, ckptID string, o ClaimOptions) (*types.Sandbox, error) { - // A rejected id must not leave a lock-map entry, so resolve before recLock and quota ckpt, err := m.loadCheckpoint(ctx, ckptID) if err != nil { return nil, err @@ -101,8 +100,8 @@ func (m *Manager) Checkpoints(ctx context.Context, tenant string) ([]types.Check pinned := m.pinnedArchiveCks() ckpts := make([]types.Checkpoint, 0, len(metas)) for _, rec := range metas { - var ckpt types.Checkpoint - if err := json.Unmarshal(rec.Meta, &ckpt); err != nil || !tenantOwns(tenant, ckpt.Tenant) { + ckpt, err := parseCheckpoint(rec.Meta) + if err != nil || !tenantOwns(tenant, ckpt.Tenant) { continue } if _, archived := pinned[ckpt.ID]; archived || ckpt.Archive { @@ -130,7 +129,7 @@ func (m *Manager) DeleteCheckpoint(ctx context.Context, ckptID, tenant string, s } // ckpt.Archive guards wake images store-wide; the pin set guards uncommitted local ones if ckpt.Archive || m.archiveCkPinned(ckptID) { - return ErrUnknownCheckpoint // backs an archived sandbox, not a deletable checkpoint + return ErrUnknownCheckpoint } if err := m.ckpts.Delete(ctx, ckptID); err != nil { return fmt.Errorf("delete checkpoint: %w", err) diff --git a/sandboxd/pool/checkpoint_heal_test.go b/sandboxd/pool/checkpoint_heal_test.go index c7ece2b6..7611abe7 100644 --- a/sandboxd/pool/checkpoint_heal_test.go +++ b/sandboxd/pool/checkpoint_heal_test.go @@ -75,7 +75,7 @@ func TestClaimCheckpointHealDedupsConcurrentPulls(t *testing.T) { }) } waitFor(t, func() bool { return puller.count() >= 1 }) - time.Sleep(50 * time.Millisecond) + synctest.Wait() close(puller.release) wg.Wait() @@ -190,7 +190,7 @@ func TestRecLockEvictionWaitsForAllHolders(t *testing.T) { l1.Lock() l2 := m.recs.recLock(id) if l2 != l1 { - t.Fatalf("recLock returned a different mutex while the first holder is still outstanding") + t.Fatal("recLock returned a different mutex while the first holder is still outstanding") } l1.Unlock() m.recs.recDone(id) diff --git a/sandboxd/pool/checkpoint_test.go b/sandboxd/pool/checkpoint_test.go index 0bd2f5dc..061cf344 100644 --- a/sandboxd/pool/checkpoint_test.go +++ b/sandboxd/pool/checkpoint_test.go @@ -179,7 +179,7 @@ func TestReconcileSweepsCheckpointStaging(t *testing.T) { func TestRetentionSweepsExpiredCheckpoints(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - tuneView(m, func(v *configView) { v.ckptTTL = time.Hour }) + editView(m, func(v *configView) { v.ckptTTL = time.Hour }) plant := func(id string, age time.Duration) { t.Helper() dir := filepath.Join(m.dataDir, "checkpoints", id) @@ -203,7 +203,7 @@ func TestRunRefillsWhileTheRetentionSweepBlocks(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1}) m.pools[testKey].goldenDir = "/goldens/x" - tuneView(m, func(v *configView) { v.ckptTTL = time.Hour }) + editView(m, func(v *configView) { v.ckptTTL = time.Hour }) stall := &stallingMetasStore{Store: m.ckpts, entered: make(chan struct{}, 1), release: make(chan struct{})} m.ckpts = stall diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index c9c0feb4..93ac9943 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -492,9 +492,10 @@ func (m *Manager) finalize(ctx context.Context, sb *types.Sandbox, ttl time.Dura // finalizeBatch persists the batch as one write, all-or-nothing; one tenant per batch. func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl time.Duration, claimRefPrefix string, inherited bool) error { - now := time.Now() + now, lease := time.Now(), clampTTL(ttl) for _, sb := range sbs { - stampIdentity(sb, clampTTL(ttl)) + sb.ID, sb.Token = "sb_"+randHex(8), randHex(16) + sb.ClaimedAt, sb.Deadline, sb.LeaseSeconds = now, now.Add(lease), int(lease/time.Second) if claimRefPrefix != "" { sb.ClaimRef = claimRefPrefix + sb.ID } @@ -542,7 +543,7 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t Event: "claim", ID: sb.ID, VMName: sb.VMName, KeyHash: sb.Key.Hash(), Template: sb.Key.Template, Net: string(sb.Key.Net), Size: string(sb.Key.Size), - Tenant: sb.Tenant, Deadline: sb.ClaimedAt.Add(clampTTL(ttl)), + Tenant: sb.Tenant, Deadline: sb.ClaimedAt.Add(lease), Volumes: types.VolumeNames(sb.Volumes), VolumesRW: types.VolumeRWNames(sb.Volumes), }) } @@ -646,15 +647,6 @@ func (c *heldConn) CloseWrite() error { return cw.CloseWrite() } -func stampIdentity(sb *types.Sandbox, ttl time.Duration) { - now := time.Now() - sb.ID = "sb_" + randHex(8) - sb.Token = randHex(16) - sb.ClaimedAt = now - sb.Deadline = now.Add(ttl) - sb.LeaseSeconds = int(ttl / time.Second) -} - func clampTTL(ttl time.Duration) time.Duration { if ttl <= 0 { return defaultTTL diff --git a/sandboxd/pool/drain_test.go b/sandboxd/pool/drain_test.go index a44ec3f8..1c95c61c 100644 --- a/sandboxd/pool/drain_test.go +++ b/sandboxd/pool/drain_test.go @@ -327,7 +327,7 @@ func TestAQueuedFailedClaimStillRestarts(t *testing.T) { synctest.Test(t, func(t *testing.T) { c := newFailoverCluster(t) a := c.node(t, "a", t.TempDir(), keepAnHour) - tuneView(a, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) + editView(a, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) sb := mustClaim(t, a, testKey) killVMM(fakeOf(a), sb.VMName) a.recoverVMM(t.Context(), sb) diff --git a/sandboxd/pool/egress_bench_test.go b/sandboxd/pool/egress_bench_test.go index d17cb13d..6f4e8d3f 100644 --- a/sandboxd/pool/egress_bench_test.go +++ b/sandboxd/pool/egress_bench_test.go @@ -41,7 +41,8 @@ func BenchmarkArmEgress(b *testing.B) { id := "sb_" + strconv.FormatInt(n.Add(1), 10) sb := &types.Sandbox{ID: id, Key: testKey, Layer: types.LayerPooled, VsockSocket: filepath.Join(root, id)} if err := m.out.ArmProxy(b.Context(), sb); err != nil { - b.Fatal(err) + b.Error(err) + return } m.out.Disarm(id, true) } diff --git a/sandboxd/pool/expiry_test.go b/sandboxd/pool/expiry_test.go index da73900b..92e652a8 100644 --- a/sandboxd/pool/expiry_test.go +++ b/sandboxd/pool/expiry_test.go @@ -101,12 +101,12 @@ func TestExpiryArchiveRetriesOnlyTheExportAfterItFails(t *testing.T) { if !hibernated || archived || !kept { t.Fatalf("after a failed export: hibernated %t archived %t deadline kept %t, want a hibernated claim with its deadline", hibernated, archived, kept) } - hibernates := failing.publishes.Load() + exports := failing.publishes.Load() failing.fail.Store(false) m.reapOnce(t.Context()) waitFor(t, func() bool { return archivedCount(m) == 1 }) - if m.Counters().Hibernates != 1 || failing.publishes.Load() != hibernates+1 { - t.Errorf("hibernates %d, exports %d after %d: want one hibernate and one more export", m.Counters().Hibernates, failing.publishes.Load(), hibernates) + if m.Counters().Hibernates != 1 || failing.publishes.Load() != exports+1 { + t.Errorf("hibernates %d, exports %d after %d: want one hibernate and one more export", m.Counters().Hibernates, failing.publishes.Load(), exports) } } diff --git a/sandboxd/pool/fork.go b/sandboxd/pool/fork.go index 0536669f..87ba9fa1 100644 --- a/sandboxd/pool/fork.go +++ b/sandboxd/pool/fork.go @@ -82,6 +82,9 @@ func (m *Manager) forkSource(ctx context.Context, sb *types.Sandbox) (create vmP if err = m.captureAllowed(sb); err != nil { return nil, nil, false, err } + if err = m.settleCapture(ctx, 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/idle_test.go b/sandboxd/pool/idle_test.go index f23cc3a6..c1f0268b 100644 --- a/sandboxd/pool/idle_test.go +++ b/sandboxd/pool/idle_test.go @@ -45,7 +45,7 @@ func TestIdleHibernateDefaultAppliesOnlyToUnpooledKeys(t *testing.T) { synctest.Test(t, func(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1}) - tuneView(m, func(v *configView) { v.idleDefault = time.Second }) + editView(m, func(v *configView) { v.idleDefault = time.Second }) sb := mustClaim(t, m, testKey) backdate(m, sb, time.Hour) diff --git a/sandboxd/pool/interrupted_test.go b/sandboxd/pool/interrupted_test.go index 219104ee..bc427c55 100644 --- a/sandboxd/pool/interrupted_test.go +++ b/sandboxd/pool/interrupted_test.go @@ -467,6 +467,40 @@ func TestReconcileWithoutASnapshotListSettlesAStoppedHibernate(t *testing.T) { } } +func TestACaptureSettlesTheIntentAFailedHibernateLeft(t *testing.T) { + for _, tt := range []struct { + name string + capture func(m *Manager, sb *types.Sandbox) error + }{ + {"fork", func(m *Manager, sb *types.Sandbox) error { + _, err := m.Fork(t.Context(), sb.ID, Cred{Token: sb.Token}, 1, 0, "", "") + return err + }}, + {"checkpoint", func(m *Manager, sb *types.Sandbox) error { + _, err := m.Checkpoint(t.Context(), sb.ID, Cred{Token: sb.Token}, "", "") + return err + }}, + } { + t.Run(tt.name, func(t *testing.T) { + eng := newFakeEngine() + m := newTestManager(t, eng) + sb := mustClaim(t, m, testKey) + eng.mu.Lock() + eng.snapshots = append(eng.snapshots, interruptedHib) + eng.mu.Unlock() + m.mu.Lock() + sb.PendingSnap = interruptedHib + m.mu.Unlock() + if err := tt.capture(m, sb); err != nil { + t.Fatalf("capture: %v", err) + } + if len(eng.snapSaves) != 0 || sb.HibernateSnap != interruptedHib { + t.Errorf("saves %q hibernate snap %q, want the listed hibernate adopted and no save of the paused guest", eng.snapSaves, sb.HibernateSnap) + } + }) + } +} + func waitStarted(t *testing.T, eng *fakeEngine, n int) { t.Helper() waitFor(t, func() bool { diff --git a/sandboxd/pool/lease.go b/sandboxd/pool/lease.go index 41b8f3d1..adc1e3e9 100644 --- a/sandboxd/pool/lease.go +++ b/sandboxd/pool/lease.go @@ -133,12 +133,13 @@ func (m *Manager) lift(ctx context.Context, fenced *fenceState, start time.Time) if !m.fence.CompareAndSwap(fenced, &fenceState{ch: make(chan struct{}), lifted: time.Now()}) { return } + logger := log.WithFunc("pool.lift") if time.Since(start) > leaseWindow { - log.WithFunc("pool.unfence").Warn(ctx, "replicated claims stay fenced: confirming ownership outlasted the lease window") + logger.Warn(ctx, "replicated claims stay fenced: confirming ownership outlasted the lease window") m.trip(ctx) return } - log.WithFunc("pool.unfence").Info(ctx, "node lease renewed; replicated claims are served again") + logger.Info(ctx, "node lease renewed; replicated claims are served again") } func (m *Manager) takeOwnRows(ctx context.Context, held map[string]*types.Sandbox, rows map[string]archives.Record) (map[string]int64, error) { diff --git a/sandboxd/pool/liveness_test.go b/sandboxd/pool/liveness_test.go index 8ee93b9b..b1652e77 100644 --- a/sandboxd/pool/liveness_test.go +++ b/sandboxd/pool/liveness_test.go @@ -51,7 +51,7 @@ func TestLivenessRestartsExitedVMM(t *testing.T) { func TestLivenessNonePolicyFailsAndWakeRestarts(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - tuneView(m, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) + editView(m, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) sb := mustClaim(t, m, testKey) killVMM(eng, sb.VMName) @@ -471,7 +471,7 @@ func TestStaleReportsKeepALiveWarmVM(t *testing.T) { func TestRestartFailedRefusesAReleasedClaim(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - tuneView(m, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) + editView(m, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) sb := mustClaim(t, m, testKey) killVMM(eng, sb.VMName) m.recoverVMM(t.Context(), sb) @@ -543,7 +543,7 @@ func TestColdBootBlockerNamesWhyAVMCannotColdBoot(t *testing.T) { func TestExpiredFailedClaimIsDestroyedNotArchived(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - tuneView(m, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) + editView(m, func(v *configView) { v.vmmRestart = types.VMMRestartNone }) sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{OnExpire: types.ExpireArchive}) if err != nil { t.Fatalf("claim: %v", err) diff --git a/sandboxd/pool/memory_test.go b/sandboxd/pool/memory_test.go index 36a753df..1266b5d9 100644 --- a/sandboxd/pool/memory_test.go +++ b/sandboxd/pool/memory_test.go @@ -154,7 +154,7 @@ func TestRemovingTheMemoryLimitClearsTheState(t *testing.T) { m := memoryManager(t, config.MemoryLimit{Anon: "300G"}) observeAnon(t, m, 301*units.GiB) - tuneView(m, func(v *configView) { v.memoryAnon = 0 }) + editView(m, func(v *configView) { v.memoryAnon = 0 }) m.checkMemory(t.Context()) if _, g := m.Info(); g.MemoryLow || g.AnonBytes != 0 { t.Errorf("gauges %+v, want no memory state with the limit off", g) diff --git a/sandboxd/pool/move.go b/sandboxd/pool/move.go index 3e0c6612..7beb1efa 100644 --- a/sandboxd/pool/move.go +++ b/sandboxd/pool/move.go @@ -13,11 +13,10 @@ import ( ) const ( - // DefaultMoveDowntime is the final round's pause a move allows when the caller names none. - DefaultMoveDowntime = 2 * time.Second // MoveStartAllowance is what the target's receive and start add to the final round's transfer, so a smaller downtime budget never fits. MoveStartAllowance = 400 * time.Millisecond + defaultMoveDowntime = 2 * time.Second movesDir = "moves" moveReservationIdle = 2 * time.Minute moveRetryInterval = 2 * time.Second diff --git a/sandboxd/pool/move_out.go b/sandboxd/pool/move_out.go index 025b8f11..d6df65ae 100644 --- a/sandboxd/pool/move_out.go +++ b/sandboxd/pool/move_out.go @@ -36,7 +36,7 @@ func (m *Manager) Move(ctx context.Context, id, addr string, maxDowntime time.Du return MoveResult{}, err } ctx = context.WithoutCancel(ctx) - err := m.moveOut(ctx, sb, mv, cmp.Or(maxDowntime, DefaultMoveDowntime)) + err := m.moveOut(ctx, sb, mv, cmp.Or(maxDowntime, defaultMoveDowntime)) if err != nil { return m.failMoveOut(ctx, sb, mv, err) } @@ -152,10 +152,11 @@ func (m *Manager) beginMove(ctx context.Context, sb *types.Sandbox, to string) ( func (m *Manager) sendRound(ctx context.Context, id string, mv *outMove, round types.MigrateRound) (int64, error) { mv.rounds++ names := make([]string, len(round.Files)) + var size int64 for i, f := range round.Files { - names[i] = f.Name + names[i], size = f.Name, size+f.Size } - ctx, cancel := context.WithTimeout(ctx, roundTimeout(roundBytes(round))) + ctx, cancel := context.WithTimeout(ctx, moveRoundBase+time.Duration(size/moveRoundMinRate)*time.Second) defer cancel() return m.movePeer.SendRound(ctx, mv.Addr, id, mv.rounds, round.Dir, names) } @@ -198,7 +199,7 @@ func (m *Manager) moveBlocker(sb *types.Sandbox) string { return "it is waking" case m.midMoveLocked(sb): return "a move of it is already running" - case hasVolumes(sb) || m.hasVolumeHolds(sb): + case m.hasVolumeHolds(sb): return "it has volumes" case sb.Key.Net != types.NetNone: return "an egress-lane claim does not move" @@ -408,19 +409,6 @@ func (m *Manager) midMoveLocked(sb *types.Sandbox) bool { return moving || sb.MoveIn } -func roundBytes(round types.MigrateRound) int64 { - var n int64 - for _, f := range round.Files { - n += f.Size - } - return n -} - -func roundTimeout(size int64) time.Duration { - return moveRoundBase + time.Duration(size/moveRoundMinRate)*time.Second -} - -// finalPause estimates the final round's pause from the last delta's size at the rate the rounds so far reached. func finalPause(last, sent int64, sending time.Duration) time.Duration { return MoveStartAllowance + time.Duration(float64(sending)*float64(last)/float64(sent)) } diff --git a/sandboxd/pool/move_test.go b/sandboxd/pool/move_test.go index 42e31730..32c0a2ad 100644 --- a/sandboxd/pool/move_test.go +++ b/sandboxd/pool/move_test.go @@ -14,7 +14,6 @@ import ( "runtime" "slices" "strings" - "sync" "sync/atomic" "testing" "time" @@ -95,11 +94,11 @@ func TestMoveDisarmsTheSourceDoorsAndArmsTheTarget(t *testing.T) { func TestMoveRefusedByTheTargetLeavesTheClaimRunningOnTheSource(t *testing.T) { for name, refuse := range map[string]func(t *testing.T, mc *moveCluster){ "max_claims": func(t *testing.T, mc *moveCluster) { - tuneView(mc.t, func(v *configView) { v.maxClaims = 1 }) + editView(mc.t, func(v *configView) { v.maxClaims = 1 }) mustClaim(t, mc.t, testKey) }, "max_running": func(t *testing.T, mc *moveCluster) { - tuneView(mc.t, func(v *configView) { v.maxRunning = 1 }) + editView(mc.t, func(v *configView) { v.maxRunning = 1 }) mustClaim(t, mc.t, testKey) }, "tenant cap": func(t *testing.T, mc *moveCluster) { @@ -111,7 +110,7 @@ func TestMoveRefusedByTheTargetLeavesTheClaimRunningOnTheSource(t *testing.T) { } { t.Run(name, func(t *testing.T) { mc := newMoveCluster(t) - tuneView(mc.t, func(v *configView) { v.moveConcurrency = 1 }) + editView(mc.t, func(v *configView) { v.moveConcurrency = 1 }) sb := claimTenant(t, mc.s, "acme") refuse(t, mc) before := moveCounts(mc.t, "acme") @@ -270,7 +269,7 @@ func TestMoveRestartedSourceDropsTheTargetsReservation(t *testing.T) { func TestMoveTargetCountsAReservationLikeAClaim(t *testing.T) { mc := newMoveCluster(t) - tuneView(mc.t, func(v *configView) { v.maxClaims = 1 }) + editView(mc.t, func(v *configView) { v.maxClaims = 1 }) reserveMove(t, mc.t, "sb_r", "acme") if _, err := claimAny(t.Context(), mc.t, testKey, ClaimOptions{}); !errors.Is(err, ErrQuota) { t.Fatalf("claim beside a reservation at max_claims 1: %v, want ErrQuota", err) @@ -373,7 +372,7 @@ func TestMoveTargetRefusesARoundThatOutlivedItsReservation(t *testing.T) { func TestMoveTargetLetsTheSameSourceRetryAndRefusesAnother(t *testing.T) { mc := newMoveCluster(t) - tuneView(mc.t, func(v *configView) { v.moveConcurrency, v.maxClaims = 1, 1 }) + editView(mc.t, func(v *configView) { v.moveConcurrency, v.maxClaims = 1, 1 }) reserveMove(t, mc.t, "sb_r", "acme") first, _ := reservationOf(mc.t, "sb_r") stale := filepath.Join(mc.t.moveDir("sb_r"), "state.json") @@ -1100,13 +1099,7 @@ func TestMoveReceivedClaimWaitingOutAnOutageIsNotHibernatedOrServed(t *testing.T if !ok { t.Fatal("the target does not hold the received claim") } - waitFor(t, func() bool { - if !received.Transition.TryLock() { - return false - } - received.Transition.Unlock() - return true - }) + waitSettled(t, received) cred := Cred{Token: sb.Token} backdate(mc.t, received, time.Hour) received.Transition.Lock() @@ -1428,7 +1421,7 @@ func BenchmarkQuotaErr(b *testing.B) { for i := range incoming { m.incoming["sb_"+string(rune('a'+i))] = &inMove{tenant: "acme", touched: time.Now()} } - tuneView(m, func(v *configView) { v.maxClaims, v.maxRunning = 1000, 1000 }) + editView(m, func(v *configView) { v.maxClaims, v.maxRunning = 1000, 1000 }) for b.Loop() { m.mu.Lock() _ = m.quotaErr(1, "") @@ -1474,7 +1467,7 @@ func testMoveHoldsOffCallers(t *testing.T, archived bool) { if _, held := mc.s.byID(sb.ID); !held { t.Error("the reap took the moving claim") } - tuneView(mc.s, func(v *configView) { v.moveConcurrency = 1 }) + editView(mc.s, func(v *configView) { v.moveConcurrency = 1 }) if _, err := mc.s.Move(t.Context(), other.ID, "b", time.Minute); !errors.Is(err, ErrQuota) { t.Errorf("a second move past a lowered move_concurrency: %v, want ErrQuota", err) } @@ -1544,7 +1537,6 @@ type fakeMovePeer struct { c *failoverCluster srv *httptest.Server - mu sync.Mutex admitErr error roundHook func(round int) error commitHook func(round int) error @@ -1623,8 +1615,6 @@ func (p *fakeMovePeer) Drop(_ context.Context, addr, id string) error { } func (p *fakeMovePeer) target(addr string) *Manager { - p.mu.Lock() - defer p.mu.Unlock() i := slices.IndexFunc(p.c.nodes, func(m *Manager) bool { return m.nodeID == addr }) return p.c.nodes[i] } diff --git a/sandboxd/pool/operator.go b/sandboxd/pool/operator.go index 931f6a58..746fae19 100644 --- a/sandboxd/pool/operator.go +++ b/sandboxd/pool/operator.go @@ -21,7 +21,6 @@ func (m *Manager) Sandbox(id string) (SandboxSummary, bool) { if sb == nil { return SandboxSummary{}, false } - // summarize reads fields hibernate and archive mutate under m.mu out := summarize(sb) out.Token = sb.Token return out, true @@ -73,7 +72,7 @@ func (m *Manager) resolve(id string, cred Cred) (*types.Sandbox, bool) { return m.claim(id, cred.Token) } -// byID resolves a live claim by id alone; callers must have authorized the Operator credential. +// byID resolves a live claim by id alone; the caller authorizes the request. func (m *Manager) byID(id string) (*types.Sandbox, bool) { m.mu.Lock() defer m.mu.Unlock() diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index dc258622..0d432b0f 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -238,7 +238,7 @@ func TestReleaseDelayQueuesTeardown(t *testing.T) { synctest.Test(t, func(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - tuneView(m, func(v *configView) { v.releaseDelay = 2 * time.Second }) + editView(m, func(v *configView) { v.releaseDelay = 2 * time.Second }) sb := mustClaim(t, m, testKey) if err := m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { @@ -2331,12 +2331,6 @@ func settleGoldenBuilds(m *Manager) { } } -func tuneView(m *Manager, change func(*configView)) { - v := *m.view.Load() - change(&v) - m.view.Store(&v) -} - func assertRunningCount(t testing.TB, m *Manager) { t.Helper() m.mu.Lock() diff --git a/sandboxd/pool/provision.go b/sandboxd/pool/provision.go index 786ed1e5..72cc78df 100644 --- a/sandboxd/pool/provision.go +++ b/sandboxd/pool/provision.go @@ -127,7 +127,7 @@ func (m *Manager) cloneBatch(ctx context.Context, count int, key types.PoolKey, return children, nil } -// probeReady waits until a VM's silkd answers, polling `vm list` when sock is empty. +// probeReady waits until a VM's silkd answers, polling `vm inspect` when sock is empty. func (m *Manager) probeReady(ctx context.Context, name, sock string, timeout time.Duration) (string, error) { deadline := time.Now().Add(timeout) if sock == "" { diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index 68a2e368..f4b07427 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -25,7 +25,7 @@ func (m *Manager) Reconcile(ctx context.Context) error { if err != nil { return err } - fenced := m.fenceArchived(ctx, claims) + answered := m.fenceArchived(ctx, claims) vms, err := m.eng.List(ctx) if err != nil { return fmt.Errorf("list vms: %w", err) @@ -97,7 +97,7 @@ func (m *Manager) Reconcile(ctx context.Context) error { } // A failed save keeps the records, so the next boot records them if saveErr == nil { - m.recordReconciled(ctx, resumed, dropped, fenced) + m.recordReconciled(ctx, resumed, dropped, answered) } logger := log.WithFunc("pool.Reconcile") if err := m.ckpts.SweepStaging(); err != nil { @@ -112,7 +112,7 @@ func (m *Manager) Reconcile(ctx context.Context) error { _ = utils.RemoveDirEntries(filepath.Join(m.dataDir, readerDir), nil) _ = utils.RemoveDirEntries(filepath.Join(m.dataDir, movesDir), func(id string) bool { return claims[id] == nil || !claims[id].MoveIn }) - if fenced { + if answered { m.reclaimOrphanArchiveCks(ctx, claims) } m.retryArchiveDeletes(ctx) diff --git a/sandboxd/pool/refillkick_test.go b/sandboxd/pool/refillkick_test.go index 3643b263..556ac527 100644 --- a/sandboxd/pool/refillkick_test.go +++ b/sandboxd/pool/refillkick_test.go @@ -31,7 +31,6 @@ func TestAFreshGoldenStartsRefillWithoutWaitingForATick(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1}) ctx, cancel := context.WithCancel(t.Context()) - start := time.Now() go m.Run(ctx) synctest.Wait() @@ -39,14 +38,14 @@ func TestAFreshGoldenStartsRefillWithoutWaitingForATick(t *testing.T) { m.mu.Lock() golden := m.pools[testKey].goldenDir m.mu.Unlock() - clones, waited := eng.cloneCount(), time.Since(start) + clones := eng.cloneCount() cancel() synctest.Wait() if golden == "" { t.Fatal("no golden built, so the test proves nothing") } - if clones == 0 || waited >= refillInterval { - t.Errorf("%d warm clones %v after Run started, want the first one before the %v refill tick", clones, waited, refillInterval) + if clones == 0 { + t.Errorf("no warm clone before the %v refill tick", refillInterval) } }) } diff --git a/sandboxd/pool/remove.go b/sandboxd/pool/remove.go index 06bb7af8..5480cc8b 100644 --- a/sandboxd/pool/remove.go +++ b/sandboxd/pool/remove.go @@ -21,14 +21,10 @@ func (m *Manager) removeVM(ctx context.Context, name string) bool { if err == nil { return true } - log.WithFunc("pool.removeVM").Warnf(ctx, "remove vm %s: %v; verifying", name, err) - return m.confirmGone(ctx, name) -} - -func (m *Manager) confirmGone(ctx context.Context, name string) bool { + logger := log.WithFunc("pool.removeVM") + logger.Warnf(ctx, "remove vm %s: %v; verifying", name, err) ctx, cancel := context.WithTimeout(ctx, removeVerifyTimeout) defer cancel() - logger := log.WithFunc("pool.confirmGone") _, present, err := m.eng.Inspect(ctx, name) if err != nil { // Cannot tell: a false "gone" leaks a running VM, a retry only costs a sweep diff --git a/sandboxd/pool/running_test.go b/sandboxd/pool/running_test.go index 7326ecb2..bfe0e58b 100644 --- a/sandboxd/pool/running_test.go +++ b/sandboxd/pool/running_test.go @@ -15,7 +15,7 @@ import ( func TestMaxRunningRefusesPastCapAndIgnoresSleepingClaims(t *testing.T) { m := newTestManager(t, newFakeEngine()) - tuneView(m, func(v *configView) { v.maxRunning = 2 }) + editView(m, func(v *configView) { v.maxRunning = 2 }) first, second := mustClaim(t, m, testKey), mustClaim(t, m, testKey) if _, err := claimAny(t.Context(), m, testKey, ClaimOptions{TTL: time.Hour}); !errors.Is(err, ErrQuota) { t.Fatalf("claim past cap: %v, want ErrQuota", err) @@ -43,7 +43,7 @@ func TestMaxRunningLeavesTheWarmPoolAlone(t *testing.T) { m := newTestManager(t, newFakeEngine(), config.PoolSpec{PoolKey: testKey, Warm: 1}) mustClaim(t, m, testKey) fillOneWarm(t, m) - tuneView(m, func(v *configView) { v.maxRunning = 1 }) + editView(m, func(v *configView) { v.maxRunning = 1 }) if _, err := m.ClaimWarm(t.Context(), testKey, ClaimOptions{TTL: time.Hour}); !errors.Is(err, ErrQuota) { t.Fatalf("warm claim past cap: %v, want ErrQuota", err) } @@ -57,7 +57,7 @@ func TestANodeAtACapGossipsNoPools(t *testing.T) { if _, served := m.WarmCounts()[testKey.Hash()]; !served { t.Fatal("a node under its caps names no pool") } - tuneView(m, func(v *configView) { v.maxRunning = 1 }) + editView(m, func(v *configView) { v.maxRunning = 1 }) mustClaim(t, m, testKey) if counts := m.WarmCounts(); len(counts) != 0 { t.Errorf("counts %v at the cap, want none", counts) @@ -66,7 +66,7 @@ func TestANodeAtACapGossipsNoPools(t *testing.T) { func TestMaxRunningCountsEveryForkChild(t *testing.T) { m := newTestManager(t, newFakeEngine()) - tuneView(m, func(v *configView) { v.maxRunning = 2 }) + editView(m, func(v *configView) { v.maxRunning = 2 }) parent := mustClaim(t, m, testKey) if _, err := m.Fork(t.Context(), parent.ID, Cred{Token: parent.Token}, 2, 0, "", ""); !errors.Is(err, ErrQuota) { t.Fatalf("fork past cap: %v, want ErrQuota", err) @@ -85,7 +85,7 @@ func TestARefusedWakeOfAnArchivedClaimKeepsItsRow(t *testing.T) { if err != nil || !ok { t.Fatalf("row before the wake: %v %v", ok, err) } - tuneView(a, func(v *configView) { v.maxRunning = 1 }) + editView(a, func(v *configView) { v.maxRunning = 1 }) if wakeErr := wakeClaim(t, a, sb); !errors.Is(wakeErr, ErrQuota) { t.Fatalf("wake past cap: %v, want ErrQuota", wakeErr) } @@ -112,7 +112,7 @@ func TestConcurrentWakesStayUnderMaxRunning(t *testing.T) { claims[i] = mustClaim(t, m, testKey) hibernateClaim(t, m, claims[i]) } - tuneView(m, func(v *configView) { v.maxRunning = 2 }) + editView(m, func(v *configView) { v.maxRunning = 2 }) eng.restoreStall = make(chan struct{}) var refused, woken atomic.Int32 var wg sync.WaitGroup @@ -147,7 +147,7 @@ func TestAFailedWakeGivesBackItsSlot(t *testing.T) { m := newTestManager(t, eng) sb := mustClaim(t, m, testKey) hibernateClaim(t, m, sb) - tuneView(m, func(v *configView) { v.maxRunning = 1 }) + editView(m, func(v *configView) { v.maxRunning = 1 }) eng.restoreErr = errors.New("restore failed") if err := wakeClaim(t, m, sb); err == nil || errors.Is(err, ErrQuota) { t.Fatalf("wake: %v, want the restore error", err) @@ -160,7 +160,7 @@ func TestAFailedWakeGivesBackItsSlot(t *testing.T) { func TestAFailedClaimHoldsNoSlotUntilItsWake(t *testing.T) { m := newTestManager(t, newFakeEngine()) - tuneView(m, func(v *configView) { v.maxRunning = 1 }) + editView(m, func(v *configView) { v.maxRunning = 1 }) sb := mustClaim(t, m, testKey) sb.Transition.Lock() m.failLocked(t.Context(), sb, "vmm exited") diff --git a/sandboxd/pool/setpools.go b/sandboxd/pool/setpools.go index 2b4f7913..0c7e9878 100644 --- a/sandboxd/pool/setpools.go +++ b/sandboxd/pool/setpools.go @@ -74,7 +74,6 @@ func (m *Manager) applyPools(ctx context.Context, desired map[types.PoolKey]conf return nil } m.poolVersion = version - // Sequenced under the mutex (apply order) so commit drops a write a later apply superseded. seq := m.poolStore.seq.Add(1) now := time.Now() for key, p := range m.pools { diff --git a/sandboxd/pool/telemetry_test.go b/sandboxd/pool/telemetry_test.go index abbaf6d2..7b50d328 100644 --- a/sandboxd/pool/telemetry_test.go +++ b/sandboxd/pool/telemetry_test.go @@ -144,7 +144,7 @@ func TestJournalRotationKeepsOneBackup(t *testing.T) { func TestQuotaRefusesClaimsPastCap(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 2}) - tuneView(m, func(v *configView) { v.maxClaims = 1 }) + editView(m, func(v *configView) { v.maxClaims = 1 }) first := mustClaim(t, m, testKey) if _, err := claimAny(t.Context(), m, testKey, ClaimOptions{TTL: time.Hour}); !errors.Is(err, ErrQuota) { diff --git a/sandboxd/pool/volume_attach_test.go b/sandboxd/pool/volume_attach_test.go index b25c2d56..eccf5c9a 100644 --- a/sandboxd/pool/volume_attach_test.go +++ b/sandboxd/pool/volume_attach_test.go @@ -199,7 +199,7 @@ 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}}) - tuneView(m, func(v *configView) { v.maxClaims = 1 }) + editView(m, func(v *configView) { v.maxClaims = 1 }) attaches := rendezvousAttaches(eng, 2) writable := []types.Volume{{Name: "scratch", Mode: types.VolumeModeRW}} @@ -272,7 +272,6 @@ func TestFinalizeTenantQuotaFailureQuiescesAndUncountsTenant(t *testing.T) { claimErr := make(chan error, 1) go func() { _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Tenant: "acme", Volumes: []types.Volume{{Name: "scratch", Mode: types.VolumeModeRW}}}) - claimErr <- err }() waitFor(t, func() bool { return slices.Contains(eng.volumeOpsLog(), "attach:scratch") }) diff --git a/sandboxd/pool/volume_cleanup_test.go b/sandboxd/pool/volume_cleanup_test.go index 5fd1b809..519157d6 100644 --- a/sandboxd/pool/volume_cleanup_test.go +++ b/sandboxd/pool/volume_cleanup_test.go @@ -59,7 +59,6 @@ func TestOwnershipTransferReleasesRunningVolumeHoldAfterRemoval(t *testing.T) { } } -// 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() for range cap(m.maintSem) { diff --git a/sandboxd/pool/volume_mutation_test.go b/sandboxd/pool/volume_mutation_test.go index 1dc2cc6d..cdfbd2ca 100644 --- a/sandboxd/pool/volume_mutation_test.go +++ b/sandboxd/pool/volume_mutation_test.go @@ -513,12 +513,12 @@ func TestLiveVolumesRespectOwningTenantAndRemoval(t *testing.T) { } } -func TestDesiredVolumesJournalMigration(t *testing.T) { +func TestVolumesWantOmittedWhenEqualToApplied(t *testing.T) { for _, tt := range []struct { name, field string want []types.Volume }{ - {"legacy", "", []types.Volume{{Name: "data", Mount: "/data"}}}, + {"omitted", "", []types.Volume{{Name: "data", Mount: "/data"}}}, {"explicit empty", `,"volumes_want":[]`, nil}, {"desired", `,"volumes_want":[{"name":"other","mount":"/other"}]`, []types.Volume{{Name: "other", Mount: "/other"}}}, } { diff --git a/sandboxd/pool/volume_rw_test.go b/sandboxd/pool/volume_rw_test.go index 01f92bbf..24d8b5dd 100644 --- a/sandboxd/pool/volume_rw_test.go +++ b/sandboxd/pool/volume_rw_test.go @@ -278,7 +278,6 @@ func TestVolumeAdmissionMatrix(t *testing.T) { wantErr error }{ {"writer excludes writer", types.VolumeModeRW, types.VolumeModeRW, ErrVolumeBusy}, - {"writer excludes reader", types.VolumeModeRW, "", ErrVolumeBusy}, {"reader excludes writer", "", types.VolumeModeRW, ErrVolumeBusy}, {"readers share", "", "", nil}, diff --git a/sandboxd/pool/volume_test.go b/sandboxd/pool/volume_test.go index a7c43070..4a054ca5 100644 --- a/sandboxd/pool/volume_test.go +++ b/sandboxd/pool/volume_test.go @@ -255,21 +255,20 @@ func TestClaimProvisionRejectsInvalidVolumesBeforeProvision(t *testing.T) { } for _, tt := range []struct { name string - key types.PoolKey catalog []config.VolumeSpec volumes []types.Volume }{ - {"invalid name", testKey, []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "Data"}}}, - {"reserved name", testKey, []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "cocoon-data"}}}, - {"duplicate", testKey, []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "data"}, {Name: "data"}}}, - {"too many", testKey, nil, tooMany}, - {"unknown", testKey, []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "other"}}}, - {"invalid mount", testKey, []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "data", Mount: "relative"}}}, + {"invalid name", []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "Data"}}}, + {"reserved name", []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "cocoon-data"}}}, + {"duplicate", []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "data"}, {Name: "data"}}}, + {"too many", nil, tooMany}, + {"unknown", []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "other"}}}, + {"invalid mount", []config.VolumeSpec{{Name: "data", Path: path}}, []types.Volume{{Name: "data", Mount: "relative"}}}, } { t.Run(tt.name, func(t *testing.T) { eng := newFakeEngine() m := newVolumeManager(t, eng, tt.catalog) - if _, err := m.ClaimProvision(t.Context(), tt.key, ClaimOptions{Volumes: tt.volumes}); !errors.Is(err, ErrBadVolume) { + if _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: tt.volumes}); !errors.Is(err, ErrBadVolume) { t.Errorf("got %v, want ErrBadVolume", err) } if len(eng.colds)+len(eng.clones) != 0 { diff --git a/sandboxd/server/api.go b/sandboxd/server/api.go index f26864d2..675d8c4d 100644 --- a/sandboxd/server/api.go +++ b/sandboxd/server/api.go @@ -1,8 +1,6 @@ package server import ( - "errors" - "slices" "time" "github.com/cocoonstack/sandbox/sandboxd/types" @@ -24,8 +22,6 @@ type ClaimRequest struct { Net types.NetShape `json:"net,omitempty"` Size types.Size `json:"size,omitempty"` Volumes []types.Volume `json:"volumes,omitempty"` - // VolumesAttachOnly attaches every requested volume without mounting it. - VolumesAttachOnly bool `json:"volumes_attach_only,omitzero"` TTLField NoRedirect bool `json:"no_redirect,omitzero"` // RequirePromoted makes the target refuse a cold-image fallback. @@ -44,20 +40,6 @@ func (r ClaimRequest) Key() types.PoolKey { return types.PoolKey{Template: r.Template, Net: r.Net, Size: r.Size}.Defaulted() } -func (r ClaimRequest) normalizedVolumes() ([]types.Volume, error) { - if !r.VolumesAttachOnly { - return types.ValidateVolumes(r.Volumes) - } - if len(r.Volumes) == 0 { - return nil, errors.New("volumes_attach_only requires at least one volume") - } - volumes := slices.Clone(r.Volumes) - for i := range volumes { - volumes[i].AttachOnly = true - } - return types.ValidateVolumes(volumes) -} - // ClaimResponse is the wire reply of POST /v1/claim; success and Redirect are exclusive. type ClaimResponse struct { ID string `json:"id,omitempty"` diff --git a/sandboxd/server/client_endpoint_test.go b/sandboxd/server/client_endpoint_test.go index 611a9ae7..c329b5ec 100644 --- a/sandboxd/server/client_endpoint_test.go +++ b/sandboxd/server/client_endpoint_test.go @@ -3,6 +3,7 @@ package server import ( "encoding/json/jsontext" "encoding/json/v2" + "net/http" "net/http/httptest" "strings" "testing" @@ -12,11 +13,11 @@ import ( func TestClientAddressesInResponses(t *testing.T) { for _, tt := range []struct{ method, path, body, field string }{ - {"POST", "/v1/claim", `{"template":"rt"}`, "redirect"}, - {"POST", "/v1/checkpoints/ck_1/claim", `{}`, "redirect"}, - {"DELETE", "/v1/templates?template=rt", "", "redirect"}, - {"GET", "/v1/peers", "", "peers"}, - {"GET", "/v1/info", "", "peers"}, + {http.MethodPost, "/v1/claim", `{"template":"rt"}`, "redirect"}, + {http.MethodPost, "/v1/checkpoints/ck_1/claim", `{}`, "redirect"}, + {http.MethodDelete, "/v1/templates?template=rt", "", "redirect"}, + {http.MethodGet, "/v1/peers", "", "peers"}, + {http.MethodGet, "/v1/info", "", "peers"}, } { t.Run(tt.path, func(t *testing.T) { p := &clientPlacer{addrs: []string{"private:7777"}, owners: []string{"private:7777"}} @@ -43,7 +44,7 @@ func TestClientOwner(t *testing.T) { if got := s.claimResponse(&types.Sandbox{ID: "sb_1"}).OwnerAddr; got != "https://self.example" { t.Fatalf("owner = %q", got) } - r := httptest.NewRequest("GET", "/v1/sandboxes/sb_1/owner", nil) + r := httptest.NewRequest(http.MethodGet, "/v1/sandboxes/sb_1/owner", nil) r.Header.Set("Authorization", "Bearer tok") w := httptest.NewRecorder() s.Handler().ServeHTTP(w, r) diff --git a/sandboxd/server/exec.go b/sandboxd/server/exec.go index 299bb70f..a48c936a 100644 --- a/sandboxd/server/exec.go +++ b/sandboxd/server/exec.go @@ -165,8 +165,9 @@ func collectExec(guest net.Conn, reserve func(int) bool) (resp ExecResponse, pid return nil } sc := wire.NewFrameScanner(guest) + var scratch []byte for sc.Scan() { - frame, err := wire.DecodeResponse(sc.Bytes()) + frame, err := wire.DecodeResponseInto(sc.Bytes(), &scratch) if err != nil { return ExecResponse{}, pid, err } diff --git a/sandboxd/server/preview.go b/sandboxd/server/preview.go index 8b57f99f..7ca909ea 100644 --- a/sandboxd/server/preview.go +++ b/sandboxd/server/preview.go @@ -6,7 +6,6 @@ import ( "crypto/sha256" "encoding/base64" "encoding/json/v2" - "errors" "fmt" "net" "net/http" @@ -132,15 +131,10 @@ func (p *PreviewServer) proxyLocal(w http.ResponseWriter, r *http.Request, claim }, Transport: p.transport, ErrorHandler: func(w http.ResponseWriter, r *http.Request, err error) { - if errors.Is(err, pool.ErrLeaseLost) || errors.Is(err, pool.ErrEvacuating) { - code, msg, _ := mapPoolErr(w, err) + if code, msg, ok := mapPoolErr(w, err); ok { http.Error(w, msg, code) return } - if errors.Is(err, pool.ErrQuota) { - http.Error(w, "node at capacity; retry", http.StatusTooManyRequests) - return - } log.WithFunc("server.proxyLocal").Errorf(r.Context(), err, "proxy %s:%d", claims.ID, claims.Port) http.Error(w, "preview target unreachable", http.StatusBadGateway) }, diff --git a/sandboxd/server/preview_test.go b/sandboxd/server/preview_test.go index aecbf033..06d349c6 100644 --- a/sandboxd/server/preview_test.go +++ b/sandboxd/server/preview_test.go @@ -97,6 +97,23 @@ func TestPreviewRevokedWhenDialFails(t *testing.T) { } } +func TestPreviewAnswers404ForAReleasedSandbox(t *testing.T) { + ps := NewPreviewServer("secret", "node:9000", "node:7777", &fakePreviewMgr{ + dial: func(string, uint16) (net.Conn, error) { return nil, pool.ErrUnknownSandbox }, + }) + ts := httptest.NewServer(ps.Handler()) + t.Cleanup(ts.Close) + + resp, err := http.Get(ts.URL + "/p/" + mintToken(ps, "sb_released", 3000, time.Hour) + "/") + if err != nil { + t.Fatalf("get: %v", err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusNotFound { + t.Errorf("status %d, want 404 for a released sandbox", resp.StatusCode) + } +} + func TestPreviewAnswers503WhileAClaimCannotServe(t *testing.T) { for _, dialErr := range []error{pool.ErrLeaseLost, pool.ErrEvacuating} { t.Run(dialErr.Error(), func(t *testing.T) { diff --git a/sandboxd/server/reader.go b/sandboxd/server/reader.go index 8269b56d..bf8eab2f 100644 --- a/sandboxd/server/reader.go +++ b/sandboxd/server/reader.go @@ -19,11 +19,7 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/pool" ) -const ( - // infoWait bounds how long a lone info frame waits for the call a client pipelines behind it. - infoWait = 100 * time.Millisecond - routeBuf = 64 << 10 -) +const routeBuf = 64 << 10 var ( readOnlyOps = map[string]bool{ @@ -32,7 +28,6 @@ var ( wire.FsRead{}.Op(): true, wire.FsPull{}.Op(): true, } - infoOp = wire.Info{}.Op() errReaderGone = errors.New("reader connection lost") errUnreachable = errors.New("guest agent unreachable") @@ -52,8 +47,7 @@ type router struct { pumped chan struct{} superseded atomic.Bool - pending []byte - replay []byte + replay []byte } func (r *router) run() (net.Conn, func(), bool) { @@ -74,19 +68,7 @@ func (r *router) run() (net.Conn, func(), bool) { if err != nil { return nil, nil, false } - case op == infoOp && r.reader == nil: - if len(r.pending) > 0 { - return r.handover(head) - } - r.pending = append(r.pending, head...) - followed, waitErr := r.infoFollowed() - if waitErr != nil { - return nil, nil, false - } - if !followed { - return r.handover(nil) - } - case readOnlyOps[op] || op == infoOp: + case readOnlyOps[op]: switch err = r.toReader(head, partial); { case err == nil: case errors.Is(err, errReaderGone): @@ -129,16 +111,6 @@ func (r *router) dialReader() error { } r.reader, r.source = conn, source r.startPump() - if len(r.pending) == 0 { - return nil - } - if r.s.mgr.AuditEnabled() { - r.auditPending() - } - if _, err = conn.Write(r.pending); err != nil { - return errReaderGone - } - r.pending = nil return nil } @@ -149,7 +121,7 @@ func (r *router) handover(head []byte) (net.Conn, func(), bool) { r.fail(err) return nil, nil, false } - r.replay = slices.Concat(r.pending, head) + r.replay = slices.Clone(head) return guest, done, true } @@ -178,19 +150,6 @@ func (r *router) pass(head []byte, partial bool, w io.Writer) error { return nil } -func (r *router) infoFollowed() (bool, error) { - if r.br.Buffered() > 0 { - return true, nil - } - _ = r.client.SetReadDeadline(time.Now().Add(infoWait)) - _, err := r.br.Peek(1) - _ = r.client.SetReadDeadline(time.Time{}) - if timeout, ok := errors.AsType[net.Error](err); ok && timeout.Timeout() { - return false, nil - } - return err == nil, err -} - func (r *router) startPump() { r.superseded.Store(false) r.pumped = make(chan struct{}) @@ -221,14 +180,14 @@ func (r *router) fail(err error) { switch _, mapped, ok := lookupPoolErr(err); { case errors.Is(err, errUnreachable): msg = errUnreachable.Error() - log.WithFunc("server.fail").Errorf(r.ctx, err, "dial silkd for %s", r.id) + log.WithFunc("server.router.fail").Errorf(r.ctx, err, "dial silkd for %s", r.id) case ok: msg = mapped if errors.Is(err, pool.ErrUnknownSandbox) { kind = wire.KindNotFound } default: - log.WithFunc("server.fail").Errorf(r.ctx, err, "agent socket for %s", r.id) + log.WithFunc("server.router.fail").Errorf(r.ctx, err, "agent socket for %s", r.id) } frame, encodeErr := wire.EncodeResponse(&wire.ErrorResp{Kind: kind, Message: msg}) if encodeErr != nil { @@ -237,12 +196,6 @@ func (r *router) fail(err error) { _, _ = r.client.Write(append(frame, '\n')) } -func (r *router) auditPending() { - for line := range bytes.SplitSeq(bytes.TrimSuffix(r.pending, newline), newline) { - r.s.mgr.Audit(r.ctx, r.id, line) - } -} - func (r *router) audit(head []byte, partial bool) { if partial { r.s.mgr.Audit(r.ctx, r.id, head) @@ -288,5 +241,6 @@ func (s *Server) openGuest(ctx context.Context, id, token string, wake agentWake done() return nil, nil, fmt.Errorf("%w: %w", errUnreachable, err) } - return guest, fenceGuest(guest, fenced, done), nil + stop := pool.CloseOnFence(fenced, guest) + return guest, func() { stop(); done() }, nil } diff --git a/sandboxd/server/reader_test.go b/sandboxd/server/reader_test.go index fac1be27..ec18eb22 100644 --- a/sandboxd/server/reader_test.go +++ b/sandboxd/server/reader_test.go @@ -44,24 +44,7 @@ func TestSleepingReadIsServedByTheReaderWithoutWaking(t *testing.T) { } } -func TestSleepingFirstDialPipelinesInfoBehindTheRead(t *testing.T) { - env := newSleepingEnv(t) - conn, r := env.upgrade(t) - - send(t, conn, infoFrame) - time.Sleep(10 * time.Millisecond) - send(t, conn, readFrame) - expectLine(t, r, "reader") - expectLine(t, r, "reader") - if wakes, _ := env.reader.counts(); wakes != 0 { - t.Errorf("wakes=%d, want the info and the read served by the reader", wakes) - } - if got := env.frames.all(); !slices.Equal(got, []string{"reader:" + infoFrame, "reader:" + readFrame}) { - t.Errorf("frames %q, want info then read at the reader in order", got) - } -} - -func TestSleepingLoneInfoWakes(t *testing.T) { +func TestSleepingInfoWakes(t *testing.T) { env := newSleepingEnv(t) conn, r := env.upgrade(t) @@ -75,32 +58,16 @@ func TestSleepingLoneInfoWakes(t *testing.T) { } } -func TestSleepingSecondInfoBeforeACallWakes(t *testing.T) { +func TestSleepingInfoAfterAReadWakes(t *testing.T) { env := newSleepingEnv(t) conn, r := env.upgrade(t) - send(t, conn, infoFrame+infoFrame) - stop := make(chan struct{}) - defer close(stop) - go func() { - tick := time.Tick(10 * time.Millisecond) - for { - select { - case <-stop: - return - case <-tick: - _, _ = io.WriteString(conn, infoFrame) - } - } - }() - _ = conn.SetReadDeadline(time.Now().Add(time.Second)) - expectLine(t, r, "guest") + send(t, conn, statFrame) + expectLine(t, r, "reader") + send(t, conn, infoFrame) expectLine(t, r, "guest") - if wakes, dials := env.reader.counts(); wakes != 1 || dials != 0 { - t.Errorf("wakes=%d reader dials=%d, want a wake and no reader", wakes, dials) - } - if got := env.frames.all(); len(got) < 2 || !slices.Equal(got[:2], []string{"guest:" + infoFrame, "guest:" + infoFrame}) { - t.Errorf("frames %q, want both infos replayed to the guest first", got) + if wakes, _ := env.reader.counts(); wakes != 1 { + t.Errorf("wakes=%d, want the info to wake the claim rather than ask the reader", wakes) } } @@ -291,17 +258,14 @@ func TestSleepingFramesAreAuditedOnceOnEitherPath(t *testing.T) { mu.Unlock() } conn, r := env.upgrade(t) - send(t, conn, infoFrame) - time.Sleep(10 * time.Millisecond) send(t, conn, statFrame) expectLine(t, r, "reader") - expectLine(t, r, "reader") send(t, conn, execFrame) expectLine(t, r, "guest") mu.Lock() defer mu.Unlock() - want := []string{strings.TrimSpace(infoFrame), strings.TrimSpace(statFrame), strings.TrimSpace(execFrame)} + want := []string{strings.TrimSpace(statFrame), strings.TrimSpace(execFrame)} if !slices.Equal(audited, want) { t.Errorf("audited %q, want each opening frame once: %q", audited, want) } diff --git a/sandboxd/server/relay.go b/sandboxd/server/relay.go index 468e0327..e46a6804 100644 --- a/sandboxd/server/relay.go +++ b/sandboxd/server/relay.go @@ -199,11 +199,6 @@ func (t *auditTee) endLine() { t.skip = false } -func fenceGuest(guest net.Conn, fenced <-chan struct{}, done func()) func() { - stop := pool.CloseOnFence(fenced, guest) - return func() { stop(); done() } -} - func copyBoth(client net.Conn, clientR io.Reader, guest net.Conn, ends relayEnds, noSplice bool) { done := make(chan struct{}) go func() { diff --git a/sandboxd/server/server.go b/sandboxd/server/server.go index 2ba424aa..aa0f4f7c 100644 --- a/sandboxd/server/server.go +++ b/sandboxd/server/server.go @@ -371,7 +371,7 @@ func (s *Server) handleClaim(w http.ResponseWriter, r *http.Request) { } key := req.Key() tenant := tenantFrom(r.Context()) - if len(req.Volumes) > 0 || req.VolumesAttachOnly { + if len(req.Volumes) > 0 { s.handleVolumeClaim(w, r, req, key, key.Hash(), tenant) return } @@ -400,7 +400,7 @@ func (s *Server) handleClaim(w http.ResponseWriter, r *http.Request) { } func (s *Server) handleVolumeClaim(w http.ResponseWriter, r *http.Request, req ClaimRequest, key types.PoolKey, hash, tenant string) { - volumes, err := req.normalizedVolumes() + volumes, err := types.ValidateVolumes(req.Volumes) if err != nil { writeErr(w, http.StatusBadRequest, fmt.Sprintf("%v: %v", pool.ErrBadVolume, err)) return diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index d883a3e6..d3643eb3 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -1694,9 +1694,6 @@ 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"}} @@ -2200,7 +2197,7 @@ type fakeManager struct { tenants []tenants.TenantSpec setTenants func(specs []tenants.TenantSpec) error putTenant func(spec tenants.TenantSpec) error - deleteTenant func(name string) error + deleteTenant idActionFunc tenantList []pool.TenantInfo tenantErr error gotEgressClass string @@ -2232,7 +2229,6 @@ type fakeManager struct { claimCheckpoint checkpointClaimFunc healCheckpoint checkpointClaimFunc healCalls int - checkpoints []types.Checkpoint deleteCheckpoint idActionFunc setPools func(pools []config.PoolSpec) error setInstanceMetadata func(id string, doc []byte) error @@ -2574,7 +2570,7 @@ func (f *fakeManager) ClaimCheckpointHeal(_ context.Context, ckptID string, o po func (f *fakeManager) Checkpoints(_ context.Context, tenant string) ([]types.Checkpoint, error) { f.gotTenant = tenant - return f.checkpoints, nil + return nil, nil } func (f *fakeManager) DeleteCheckpoint(_ context.Context, ckptID, tenant string, scope pool.DeleteScope) error { diff --git a/sandboxd/server/volume_mutation_test.go b/sandboxd/server/volume_mutation_test.go index cbaad515..cd7432fe 100644 --- a/sandboxd/server/volume_mutation_test.go +++ b/sandboxd/server/volume_mutation_test.go @@ -172,27 +172,3 @@ func TestLiveVolumeRoutesReportRefusalsOverRecordedDiagnostics(t *testing.T) { }) } } - -func TestClaimVolumeCompatibilityExpansion(t *testing.T) { - for _, tt := range []struct { - name string - req ClaimRequest - want []types.Volume - err bool - }{ - {"legacy all devices", ClaimRequest{VolumesAttachOnly: true, Volumes: []types.Volume{{Name: "data"}, {Name: "work", Mode: types.VolumeModeRW}}}, []types.Volume{{Name: "data", AttachOnly: true}, {Name: "work", Mode: types.VolumeModeRW, AttachOnly: true}}, false}, - {"empty legacy", ClaimRequest{VolumesAttachOnly: true}, nil, true}, - {"legacy mount conflict", ClaimRequest{VolumesAttachOnly: true, Volumes: []types.Volume{{Name: "data", Mount: "/data"}}}, nil, true}, - } { - t.Run(tt.name, func(t *testing.T) { - before := slices.Clone(tt.req.Volumes) - got, err := tt.req.normalizedVolumes() - if (err != nil) != tt.err || !slices.Equal(got, tt.want) { - t.Fatalf("normalized volumes=%+v err=%v, want %+v error=%t", got, err, tt.want, tt.err) - } - if !slices.Equal(tt.req.Volumes, before) { - t.Fatal("legacy expansion mutated caller input") - } - }) - } -} diff --git a/sandboxd/store/peer/peer.go b/sandboxd/store/peer/peer.go index 759482a8..f0a3ea3a 100644 --- a/sandboxd/store/peer/peer.go +++ b/sandboxd/store/peer/peer.go @@ -38,10 +38,6 @@ func (h *Healer) Pull(ctx context.Context, id, staging string, validate Validate // A client hanging up must not abandon a started pull; the budget bounds it. ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), healBudget) defer cancel() - return h.pullFrom(ctx, id, staging, addrs, validate) -} - -func (h *Healer) pullFrom(ctx context.Context, id, staging string, addrs []string, validate Validate) error { deadline, _ := ctx.Deadline() var errs []error for i, addr := range addrs { diff --git a/sandboxd/store/s3/s3_test.go b/sandboxd/store/s3/s3_test.go index 0fae5e5d..a04501ac 100644 --- a/sandboxd/store/s3/s3_test.go +++ b/sandboxd/store/s3/s3_test.go @@ -834,7 +834,7 @@ func TestFetchFileDownloadsOnlyTheRequestedFile(t *testing.T) { if n := fake.requestCount(http.MethodGet, memoryKey); n != 0 { t.Errorf("GET %s = %d requests, want 0", memoryKey, n) } - if _, err := os.Stat(filepath.Join(st.staging, "cache")); !errors.Is(err, os.ErrNotExist) { + if _, err := os.Stat(filepath.Join(st.staging, cacheDir)); !errors.Is(err, os.ErrNotExist) { t.Errorf("FetchFile touched the shared fetch cache: %v", err) } } diff --git a/sandboxd/store/store.go b/sandboxd/store/store.go index a8126188..f75501fd 100644 --- a/sandboxd/store/store.go +++ b/sandboxd/store/store.go @@ -76,7 +76,7 @@ type Store interface { // SetLabels replaces a record's labels; nil removes them. The caller holds the record lock. SetLabels(ctx context.Context, id string, labels []byte) error Delete(ctx context.Context, id string) error - // SweepStaging removes abandoned staging left by a crash mid-publish. + // SweepStaging clears abandoned staging, and a backend's local fetch cache, at startup. SweepStaging() error // SweepGenerations runs generation retention, sparing staging and fetch caches used within GenerationGrace. SweepGenerations(ctx context.Context) error diff --git a/sandboxd/types/env.go b/sandboxd/types/env.go index 894874c1..115da278 100644 --- a/sandboxd/types/env.go +++ b/sandboxd/types/env.go @@ -224,11 +224,11 @@ func (e Env) Redacted() Env { // GuestFile renders the guest entries as a systemd EnvironmentFile, sorted by name. func (e Env) GuestFile() []byte { - var b strings.Builder + var b []byte for _, name := range slices.Sorted(maps.Keys(e.guest())) { - fmt.Fprintf(&b, "%s=\"%s\"\n", name, guestFileEscaper.Replace(e[name].Value)) + b = fmt.Appendf(b, "%s=\"%s\"\n", name, guestFileEscaper.Replace(e[name].Value)) } - return []byte(b.String()) + return b } func (e Env) guest() Env { diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 999b189a..87b45de7 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -241,6 +241,14 @@ type namedVar struct { v EnvVar } +// MoveOut is the source's journal of a live move: the record's first epoch, cocoon's series and the target. +type MoveOut struct { + Epoch int64 `json:"epoch"` + Series string `json:"series,omitempty"` + Addr string `json:"addr,omitempty"` + To string `json:"to,omitempty"` +} + // Sandbox is the node-local record of one pooled or claimed VM. type Sandbox struct { ID string `json:"id"` @@ -298,7 +306,7 @@ type Sandbox struct { FromCheckpoint string `json:"from_checkpoint,omitempty"` // TemplateDigest identifies the promoted-template export this sandbox was cloned from. - TemplateDigest string `json:"template_digest,omitempty"` + TemplateDigest string `json:"-"` // Restarts counts the cold boots that replaced an exited VMM; each loses guest memory. Restarts int `json:"restarts,omitzero"` @@ -459,14 +467,6 @@ func (r VMRecord) TapDevice() string { return r.NetworkConfigs[0].TAP } -// MoveOut is the source's journal of a live move: the record's first epoch, cocoon's series and the target. -type MoveOut struct { - Epoch int64 `json:"epoch"` - Series string `json:"series,omitempty"` - Addr string `json:"addr,omitempty"` - To string `json:"to,omitempty"` -} - // MigrateFile is one file a migrate round wrote, named relative to the round's Dir. type MigrateFile struct { Name string `json:"name"` diff --git a/sandboxd/usage/ship.go b/sandboxd/usage/ship.go index 827d1434..207941d3 100644 --- a/sandboxd/usage/ship.go +++ b/sandboxd/usage/ship.go @@ -50,7 +50,7 @@ type shipper struct { } func (s *shipper) drain(ctx context.Context) { - logger := log.WithFunc("usage.Ship") + logger := log.WithFunc("usage.shipper.drain") for { n, err := s.shipOnce(ctx) switch { @@ -98,7 +98,7 @@ func (s *shipper) shipOnce(ctx context.Context) (int, error) { return 0, nil // rotated between the two opens } } - f, off := s.locate(live, backup) + f, off := s.locate(ctx, live, backup) files := []*os.File{f} if f == backup { files = append(files, live) @@ -117,7 +117,7 @@ func (s *shipper) shipOnce(ctx context.Context) (int, error) { } // locate returns where the first unlanded line starts: past the anchor when a file still holds it, else past the marked line, else at the oldest line. -func (s *shipper) locate(live, backup *os.File) (*os.File, int64) { +func (s *shipper) locate(ctx context.Context, live, backup *os.File) (*os.File, int64) { for _, f := range []*os.File{live, backup} { if f != nil && s.at.in(f) { return f, s.at.end @@ -139,6 +139,7 @@ func (s *shipper) locate(live, backup *os.File) (*os.File, int64) { } } if found == nil { + log.WithFunc("usage.shipper.locate").Warn(ctx, "the last landed usage line rotated out of the journal; the lines between it and the oldest kept line are lost") return oldest, 0 } return found, s.at.end @@ -157,7 +158,7 @@ func (s *shipper) read(ctx context.Context, files []*os.File, off int64) ([]Reco last = newAnchor(line, end) ev, err := decode(line) if err != nil { - log.WithFunc("usage.Ship").Warnf(ctx, "skip undecodable line ending at %s:%d: %v", f.Name(), end, err) + log.WithFunc("usage.shipper.read").Warnf(ctx, "skip undecodable line ending at %s:%d: %v", f.Name(), end, err) } else { recs = append(recs, Record{Node: s.node, Digest: last.digest, Event: ev}) } diff --git a/sdk/go/checkpoint.go b/sdk/go/checkpoint.go index 04483994..57d3874f 100644 --- a/sdk/go/checkpoint.go +++ b/sdk/go/checkpoint.go @@ -60,8 +60,8 @@ func (ck *Checkpoint) New(ctx context.Context, opts ...Option) (*Sandbox, error) for _, opt := range opts { opt(&claim) } - if len(claim.Volumes) > 0 || claim.VolumesAttachOnly || claim.ClaimRef != "" { - return nil, errors.New("checkpoint claims do not accept WithVolumes, WithVolumesAttachOnly or WithClaimRef") + if len(claim.Volumes) > 0 || claim.ClaimRef != "" { + return nil, errors.New("checkpoint claims do not accept WithVolumes or WithClaimRef") } if err := claim.rejectPinnedAxes(); err != nil { return nil, err diff --git a/sdk/go/checkpoint_test.go b/sdk/go/checkpoint_test.go index 76d22817..888ceee9 100644 --- a/sdk/go/checkpoint_test.go +++ b/sdk/go/checkpoint_test.go @@ -51,7 +51,6 @@ func TestCheckpointNewRejectsVolumeOptionsLocally(t *testing.T) { opt Option }{ {"WithClaimRef", WithClaimRef("ns/claim")}, - {"WithVolumesAttachOnly", WithVolumesAttachOnly()}, {"WithVolumes", WithVolumes(Volume{Name: "data"})}, } for _, tt := range tests { diff --git a/sdk/go/client.go b/sdk/go/client.go index 587f2422..453aa1b0 100644 --- a/sdk/go/client.go +++ b/sdk/go/client.go @@ -89,18 +89,17 @@ func (e *APIError) Error() string { } type claimRequest struct { - 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,omitzero"` - TTLSeconds int `json:"ttl_seconds,omitzero"` - NoRedirect bool `json:"no_redirect,omitzero"` - RequirePromoted bool `json:"require_promoted,omitzero"` - ClaimRef string `json:"claim_ref,omitempty"` - Metadata map[string]string `json:"metadata,omitempty"` - OnExpire string `json:"on_expire,omitempty"` - Env map[string]EnvVar `json:"env,omitempty"` + Template string `json:"template"` + Net string `json:"net,omitempty"` + Size string `json:"size,omitempty"` + Volumes []Volume `json:"volumes,omitempty"` + TTLSeconds int `json:"ttl_seconds,omitzero"` + NoRedirect bool `json:"no_redirect,omitzero"` + RequirePromoted bool `json:"require_promoted,omitzero"` + ClaimRef string `json:"claim_ref,omitempty"` + Metadata map[string]string `json:"metadata,omitempty"` + OnExpire string `json:"on_expire,omitempty"` + Env map[string]EnvVar `json:"env,omitempty"` } func (r claimRequest) rejectPinnedAxes() error { diff --git a/sdk/go/files_test.go b/sdk/go/files_test.go index b2082da2..eb9dc704 100644 --- a/sdk/go/files_test.go +++ b/sdk/go/files_test.go @@ -155,7 +155,7 @@ func TestReadFileMultiChunk(t *testing.T) { } } -func TestUploadToAnOldDaemonReturnsItsRejection(t *testing.T) { +func TestAnUploadReturnsItsEarlyRejection(t *testing.T) { sb := testSandbox(t, newAgentServer(t, func(conn net.Conn) { defer func() { _ = conn.Close() }() line, err := bufio.NewReader(conn).ReadBytes('\n') @@ -166,16 +166,15 @@ func TestUploadToAnOldDaemonReturnsItsRejection(t *testing.T) { if err != nil { return } - var resp wire.Response = &wire.ErrorResp{Kind: wire.KindNotFound, Message: "create /missing/f: no such file or directory"} - if _, ok := req.(*wire.Info); ok { - resp = &wire.InfoResp{Proto: 1} + if _, ok := req.(*wire.FsWrite); !ok { + return } - frame, _ := wire.EncodeResponse(resp) + frame, _ := wire.EncodeResponse(&wire.ErrorResp{Kind: wire.KindNotFound, Message: "create /missing/f: no such file or directory"}) _, _ = conn.Write(append(frame, '\n')) })) err := sb.WriteFile(t.Context(), "/missing/f", make([]byte, 64<<20), nil) if e, ok := errors.AsType[*wire.ErrorResp](err); !ok || e.Kind != wire.KindNotFound { - t.Fatalf("upload rejected by a proto 1 daemon: %v, want its not_found frame", err) + t.Fatalf("upload rejected at open: %v, want its not_found frame", err) } } @@ -193,8 +192,6 @@ func uploadAgent(t *testing.T) (*Sandbox, <-chan int) { } var resp wire.Response switch req.(type) { - case *wire.Info: - resp = &wire.InfoResp{Proto: wire.KeepAliveProto} case *wire.Data: data++ case *wire.DataEnd: diff --git a/sdk/go/keepalive.go b/sdk/go/keepalive.go index d5dd1e5f..85a81cdd 100644 --- a/sdk/go/keepalive.go +++ b/sdk/go/keepalive.go @@ -1,7 +1,6 @@ package sandbox import ( - "context" "errors" "slices" "sync" @@ -18,14 +17,6 @@ const ( keepAliveConns = 8 ) -// WithKeepAlive bounds how long a handle keeps an idle relay connection for -// its next call (default 30s; 0 dials per call). An open connection holds -// the sandbox's idle-hibernate clock, so keep it below the deployment's -// idle_hibernate_seconds. -func WithKeepAlive(idle time.Duration) ClientOption { - return func(c *Client) { c.keepAlive = idle } -} - // agentPool parks a handle's idle relay connections between calls. type agentPool struct { mu sync.Mutex @@ -120,23 +111,6 @@ func (c *agentConn) sent(req wire.Request) (*agentConn, error) { return c, nil } -// probeProto pipelines info ahead of req and reads the daemon's answer. -func (c *agentConn) probeProto(ctx context.Context, req wire.Request) (uint32, error) { - if err := c.Send(wire.Info{}); err != nil { - return 0, err - } - if err := c.Send(req); err != nil { - return 0, err - } - stop := context.AfterFunc(ctx, func() { _ = c.Close() }) - defer stop() - info, err := expect[wire.InfoResp](ctx, c.Conn) - if err != nil { - return 0, err - } - return max(info.Proto, 1), nil -} - // lease is one RPC's hold on a relay connection: close drops it, reuse parks it for the next call. type lease struct { s *Sandbox @@ -159,7 +133,7 @@ func (l *lease) reuse() { if !l.over.CompareAndSwap(false, true) { return } - if !l.stop() || !canProbe || l.s.c.keepAlive <= 0 || l.s.proto.Load() < wire.KeepAliveProto { + if !l.stop() || !canProbe || l.s.c.keepAlive <= 0 { _ = l.conn.Close() return } @@ -173,3 +147,11 @@ func (l *lease) close() { l.stop() _ = l.conn.Close() } + +// WithKeepAlive bounds how long a handle keeps an idle relay connection for +// its next call (default 30s; 0 dials per call). An open connection holds +// the sandbox's idle-hibernate clock, so keep it below the deployment's +// idle_hibernate_seconds. +func WithKeepAlive(idle time.Duration) ClientOption { + return func(c *Client) { c.keepAlive = idle } +} diff --git a/sdk/go/keepalive_test.go b/sdk/go/keepalive_test.go index c574988d..ca4a2610 100644 --- a/sdk/go/keepalive_test.go +++ b/sdk/go/keepalive_test.go @@ -8,8 +8,6 @@ import ( "testing" "time" - "github.com/cocoonstack/sandbox/protocol/wire" - "github.com/cocoonstack/sandbox/sdk/go/silkd" "github.com/cocoonstack/sandbox/sdk/go/silkd/silkdtest" ) @@ -56,61 +54,6 @@ func TestRefusedStreamOpenKeepsItsConnection(t *testing.T) { } } -func TestOldDaemonDialsPerCall(t *testing.T) { - var upgrades atomic.Int32 - sb := testSandbox(t, newAgentServer(t, func(c net.Conn) { - upgrades.Add(1) - silkdtest.ServeConnOnce(c) - })) - for range 3 { - if out, err := sb.Exec(t.Context(), "echo", "42"); err != nil || out != "42\n" { - t.Fatalf("exec: %q, %v", out, err) - } - } - if got := upgrades.Load(); got != 4 { - t.Errorf("upgrades = %d, want 4: the proto probe plus one per call", got) - } -} - -func TestKeepAliveOffSkipsTheProbe(t *testing.T) { - var upgrades atomic.Int32 - sb := testSandbox(t, newAgentServer(t, func(c net.Conn) { - upgrades.Add(1) - silkdtest.ServeConnOnce(c) - }), WithKeepAlive(0)) - for range 3 { - if out, err := sb.Exec(t.Context(), "echo", "42"); err != nil || out != "42\n" { - t.Fatalf("exec: %q, %v", out, err) - } - } - if got := upgrades.Load(); got != 3 { - t.Errorf("upgrades = %d, want 3: nothing can park, so no probe dial", got) - } -} - -func TestOldDaemonConnectionIsNotParked(t *testing.T) { - if !canProbe { - t.Skip("no parked-connection probe on this platform") - } - client, server := net.Pipe() - t.Cleanup(func() { _ = server.Close() }) - sb := &Sandbox{c: &Client{keepAlive: time.Minute}} - t.Cleanup(sb.pool.drain) - sb.proto.Store(1) - l := lease{ - s: sb, - conn: &agentConn{Conn: silkd.NewConn(client)}, - stop: func() bool { return true }, - } - l.reuse() - sb.pool.mu.Lock() - parked := len(sb.pool.idle) - sb.pool.mu.Unlock() - if parked != 0 { - t.Errorf("parked connections = %d, want 0", parked) - } -} - func TestKeepAliveOffDialsPerCall(t *testing.T) { var upgrades atomic.Int32 sb := testSandbox(t, newAgentServer(t, func(c net.Conn) { @@ -175,7 +118,6 @@ func TestPeerHangUpIsNoticedBeforeReuse(t *testing.T) { fake.ServeConn(&hangUpAfterReply{Conn: c}) gone <- struct{}{} })) - sb.proto.Store(wire.KeepAliveProto) if _, err := sb.Stat(t.Context(), "/"); err != nil { t.Fatalf("stat: %v", err) } diff --git a/sdk/go/options.go b/sdk/go/options.go index a1bca0e6..89d2b8d6 100644 --- a/sdk/go/options.go +++ b/sdk/go/options.go @@ -82,11 +82,6 @@ func WithVolumes(volumes ...Volume) Option { return func(r *claimRequest) { r.Volumes = volumes } } -// WithVolumesAttachOnly is the legacy claim-wide form of Volume.AttachOnly. -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/pools.go b/sdk/go/pools.go index 4ce157c6..fae83e08 100644 --- a/sdk/go/pools.go +++ b/sdk/go/pools.go @@ -6,7 +6,7 @@ import ( "net/http" ) -// PoolSpec is a desired warm pool for SetPools; egress, warmup, capture_trim, storage and egress_upstream_env are config-owned, so it has no field for them. +// PoolSpec is a desired warm pool for SetPools; egress, warmup, capture_trim, storage, reader, replicate_seconds and egress_upstream_env are config-owned, so it has no field for them. type PoolSpec struct { Template string `json:"template"` Net NetShape `json:"net,omitempty"` @@ -18,9 +18,6 @@ type PoolSpec struct { ArchiveDeleteAfterSeconds int `json:"archive_delete_after_seconds,omitzero"` } -// PoolResult is one node's outcome from SetPoolsCluster. -type PoolResult = NodeResult - type poolUpdate struct { Pools []PoolSpec `json:"pools"` } @@ -35,7 +32,7 @@ func (c *Client) SetPools(ctx context.Context, pools []PoolSpec) (*NodeInfo, err // per-node result; retrying failed nodes is the whole protocol (idempotent // replace). A non-nil error means peer discovery failed and only the entry node // was reached — an incomplete apply to retry, not a single-node cluster (nil). -func (c *Client) SetPoolsCluster(ctx context.Context, pools []PoolSpec) ([]PoolResult, error) { +func (c *Client) SetPoolsCluster(ctx context.Context, pools []PoolSpec) ([]NodeResult, error) { return c.eachNode(ctx, func(addr string) (*NodeInfo, error) { return c.setPoolsAt(ctx, addr, pools) }) } diff --git a/sdk/go/port.go b/sdk/go/port.go index 74e993dd..885835ab 100644 --- a/sdk/go/port.go +++ b/sdk/go/port.go @@ -162,14 +162,14 @@ func (s *Sandbox) proxyConn(ctx context.Context, local net.Conn, port uint16) { <-done } +type portAddr string + +func (a portAddr) Network() string { return "silkd" } +func (a portAddr) String() string { return string(a) } + // closeWrite half-closes so the peer sees EOF while the tail still drains. func closeWrite(conn net.Conn) { if cw, ok := conn.(interface{ CloseWrite() error }); ok { _ = cw.CloseWrite() } } - -type portAddr string - -func (a portAddr) Network() string { return "silkd" } -func (a portAddr) String() string { return string(a) } diff --git a/sdk/go/proc_test.go b/sdk/go/proc_test.go index ac0d87d5..65c92f00 100644 --- a/sdk/go/proc_test.go +++ b/sdk/go/proc_test.go @@ -24,7 +24,7 @@ func TestProcVerbs(t *testing.T) { `{"type":"exit","code":7}`, }, } - sb := legacySandbox(t, newAgentServer(t, procServe(script))) + sb := testSandbox(t, newAgentServer(t, procServe(script)), WithKeepAlive(0)) ctx := t.Context() pid, err := sb.Spawn(ctx, Cmd{Argv: []string{"sleep", "30"}}) @@ -68,7 +68,7 @@ func TestProcVerbsUnknownPid(t *testing.T) { script := map[string][]string{ "logs": {`{"type":"error","kind":"not_found","message":"no such pid"}`}, } - sb := legacySandbox(t, newAgentServer(t, procServe(script))) + sb := testSandbox(t, newAgentServer(t, procServe(script)), WithKeepAlive(0)) _, _, err := sb.Logs(t.Context(), 999, nil, nil) if err == nil || !strings.Contains(err.Error(), "not_found") { t.Fatalf("Logs unknown pid: %v, want typed not_found", err) diff --git a/sdk/go/pty_test.go b/sdk/go/pty_test.go index 08c4ec90..ef2c8be2 100644 --- a/sdk/go/pty_test.go +++ b/sdk/go/pty_test.go @@ -67,7 +67,7 @@ func TestPtyReleasesTheRelayWhenTheShellExits(t *testing.T) { _, _ = r.ReadByte() close(released) }) - pty, err := legacySandbox(t, ts).OpenPty(t.Context(), wire.PtyOpen{Cols: 80, Rows: 24}) + pty, err := testSandbox(t, ts, WithKeepAlive(0)).OpenPty(t.Context(), wire.PtyOpen{Cols: 80, Rows: 24}) if err != nil { t.Fatalf("OpenPty: %v", err) } diff --git a/sdk/go/sandbox.go b/sdk/go/sandbox.go index 181e4e68..4ea6fce0 100644 --- a/sdk/go/sandbox.go +++ b/sdk/go/sandbox.go @@ -8,7 +8,6 @@ import ( "io" "net/http" "strings" - "sync/atomic" "time" "github.com/cocoonstack/sandbox/protocol/wire" @@ -75,8 +74,7 @@ type Sandbox struct { c *Client token string - owner string // data-plane address (owner node), from the claim - proto atomic.Uint32 + owner string pool agentPool } @@ -261,7 +259,7 @@ func (s *Sandbox) call(ctx context.Context, req wire.Request) (*silkd.Conn, *lea return c.Conn, &lease{s: s, conn: c, stop: context.AfterFunc(ctx, func() { _ = c.Close() })}, nil } -// connect sends req on a parked connection or a fresh dial; a handle's first dial pipelines info ahead of req, and a daemon before proto 2 closes after answering it without reading req, hence the second dial. +// connect sends req on a parked connection or a fresh dial. func (s *Sandbox) connect(ctx context.Context, req wire.Request) (*agentConn, error) { if c := s.pool.take(); c != nil { return c.sent(req) @@ -270,22 +268,6 @@ func (s *Sandbox) connect(ctx context.Context, req wire.Request) (*agentConn, er if err != nil { return nil, err } - if s.proto.Load() != 0 || !canProbe || s.c.keepAlive <= 0 { - return c.sent(req) - } - proto, err := c.probeProto(ctx, req) - if err != nil { - _ = c.Close() - return nil, err - } - s.proto.Store(proto) - if proto >= wire.KeepAliveProto { - return c, nil - } - _ = c.Close() - if c, err = s.dial(ctx); err != nil { - return nil, err - } return c.sent(req) } diff --git a/sdk/go/sandbox_test.go b/sdk/go/sandbox_test.go index 2521d271..a40a8975 100644 --- a/sdk/go/sandbox_test.go +++ b/sdk/go/sandbox_test.go @@ -145,7 +145,7 @@ func TestUpgradeKeepsCoalescedBytes(t *testing.T) { t.Cleanup(ts.Close) var out strings.Builder - code, err := legacySandbox(t, ts).Run(t.Context(), Cmd{Argv: []string{"echo", "42"}, Stdout: &out}) + code, err := testSandbox(t, ts, WithKeepAlive(0)).Run(t.Context(), Cmd{Argv: []string{"echo", "42"}, Stdout: &out}) if err != nil { t.Fatalf("Run: %v", err) } @@ -166,7 +166,7 @@ func TestFirstCallSendsItsFramesAheadOfThe101(t *testing.T) { defer conn.Close() _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) var got []string - for range 2 { + for range 1 { line, readErr := brw.ReadBytes('\n') if readErr != nil { break @@ -176,7 +176,7 @@ func TestFirstCallSendsItsFramesAheadOfThe101(t *testing.T) { } } ops <- got - _, _ = io.WriteString(conn, upgrade101+`{"type":"info","proto":2}`+"\n"+`{"type":"done"}`+"\n") + _, _ = io.WriteString(conn, upgrade101+`{"type":"done"}`+"\n") _ = conn.SetReadDeadline(time.Time{}) _, _ = io.Copy(io.Discard, brw) }) @@ -186,8 +186,8 @@ func TestFirstCallSendsItsFramesAheadOfThe101(t *testing.T) { if err := testSandbox(t, ts).Mkdir(t.Context(), "/w", false); err != nil { t.Fatalf("Mkdir: %v", err) } - if got := <-ops; !slices.Equal(got, []string{"info", "fs_mkdir"}) { - t.Errorf("frames ahead of the 101 = %q, want [info fs_mkdir]", got) + if got := <-ops; !slices.Equal(got, []string{"fs_mkdir"}) { + t.Errorf("frames ahead of the 101 = %q, want [fs_mkdir]", got) } } @@ -313,10 +313,3 @@ func testSandbox(t testing.TB, ts *httptest.Server, opts ...ClientOption) *Sandb t.Cleanup(sb.pool.drain) return sb } - -func legacySandbox(t *testing.T, ts *httptest.Server) *Sandbox { - t.Helper() - sb := testSandbox(t, ts) - sb.proto.Store(1) - return sb -} diff --git a/sdk/go/silkd/conn_test.go b/sdk/go/silkd/conn_test.go index 6dbffdbc..8475a4a0 100644 --- a/sdk/go/silkd/conn_test.go +++ b/sdk/go/silkd/conn_test.go @@ -88,7 +88,7 @@ func TestConnExecStdinEcho(t *testing.T) { func TestRecvKeepsDataWhileRecvTransientReusesIt(t *testing.T) { client, server := net.Pipe() - t.Cleanup(func() { client.Close(); server.Close() }) + t.Cleanup(func() { _ = client.Close(); _ = server.Close() }) go func() { for _, payload := range []string{"aGVsbG8=", "d29ybGQ=", "Zmlyc3Q=", "c2Vjb25k"} { _, _ = io.WriteString(server, `{"type":"data","data":"`+payload+`"}`+"\n") @@ -118,7 +118,7 @@ func TestRecvKeepsDataWhileRecvTransientReusesIt(t *testing.T) { func TestConnRejectsOversizedFrame(t *testing.T) { client, server := net.Pipe() - t.Cleanup(func() { client.Close(); server.Close() }) + t.Cleanup(func() { _ = client.Close(); _ = server.Close() }) conn := silkd.NewConn(client) go func() { huge := strings.Repeat("a", wire.MaxFrame+2) diff --git a/sdk/go/silkd/silkdtest/fake.go b/sdk/go/silkd/silkdtest/fake.go index 576b4f5e..4c699a60 100644 --- a/sdk/go/silkd/silkdtest/fake.go +++ b/sdk/go/silkd/silkdtest/fake.go @@ -39,7 +39,7 @@ func NewFake(root string) *Fake { // ServeConn serves RPCs on an already-open connection (e.g. after an HTTP // hijack) until the peer closes it. func (f *Fake) ServeConn(conn net.Conn) { - serveConn(conn, bufio.NewReader(conn), wire.KeepAliveProto, f.serve) + serveConn(conn, bufio.NewReader(conn), f.serve) } func (f *Fake) serve(conn net.Conn, r *bufio.Reader, req wire.Request) { diff --git a/sdk/go/silkd/silkdtest/silkdtest.go b/sdk/go/silkd/silkdtest/silkdtest.go index 76376376..cfc2010c 100644 --- a/sdk/go/silkd/silkdtest/silkdtest.go +++ b/sdk/go/silkd/silkdtest/silkdtest.go @@ -12,6 +12,13 @@ import ( "github.com/cocoonstack/sandbox/protocol/wire" ) +type hijackedConn struct { + net.Conn + r *bufio.Reader +} + +func (c *hijackedConn) Read(p []byte) (int, error) { return c.r.Read(p) } + // Serve accepts connections until l closes, serving RPCs back to back on each like silkd. func Serve(l net.Listener) { acceptLoop(l, ServeConn) @@ -19,12 +26,7 @@ func Serve(l net.Listener) { // ServeConn serves RPCs on an open connection until the peer closes it. func ServeConn(conn net.Conn) { - serveConn(conn, bufio.NewReader(conn), wire.KeepAliveProto, serveStateless) -} - -// ServeConnOnce serves one RPC and closes, like a daemon from before proto 2. -func ServeConnOnce(conn net.Conn) { - serveConn(conn, bufio.NewReader(conn), 1, serveStateless) + serveConn(conn, bufio.NewReader(conn), serveStateless) } // Upgrade hijacks an agent request and answers the 101, handing back the relay connection with any frames sent ahead of the 101. @@ -57,20 +59,13 @@ func ListenHybrid(sockPath string, port int) (io.Closer, error) { _ = c.Close() return } - serveConn(c, r, wire.KeepAliveProto, serveStateless) + serveConn(c, r, serveStateless) }) return l, nil } -type hijackedConn struct { - net.Conn - r *bufio.Reader -} - -func (c *hijackedConn) Read(p []byte) (int, error) { return c.r.Read(p) } - -// serveConn answers info with proto and dispatches the other requests until the peer closes, or after one when proto predates keep-alive; input frames outside an RPC are dropped as silkd drops them. -func serveConn(conn net.Conn, r *bufio.Reader, proto uint32, serve func(net.Conn, *bufio.Reader, wire.Request)) { +// serveConn answers info and dispatches the other requests until the peer closes; input frames outside an RPC are dropped as silkd drops them. +func serveConn(conn net.Conn, r *bufio.Reader, serve func(net.Conn, *bufio.Reader, wire.Request)) { defer func() { _ = conn.Close() }() for { req, err := recvRequest(r) @@ -81,13 +76,10 @@ func serveConn(conn net.Conn, r *bufio.Reader, proto uint32, serve func(net.Conn case *wire.Stdin, *wire.StdinClose, *wire.Data, *wire.DataEnd: continue case *wire.Info: - send(conn, &wire.InfoResp{Version: "silkdtest", Proto: proto}) + send(conn, &wire.InfoResp{Version: "silkdtest", Proto: wire.KeepAliveProto}) default: serve(conn, r, req) } - if proto < wire.KeepAliveProto { - return - } } } @@ -149,7 +141,7 @@ func serveExec(conn net.Conn, r *bufio.Reader, req *wire.Exec) { case "false": send(conn, &wire.Exit{Code: 1}) case "sleep": - _, _ = io.Copy(io.Discard, r) // hold the RPC open until disconnect + _, _ = io.Copy(io.Discard, r) default: errFrame(conn, wire.KindNotFound, "silkdtest: no such command "+req.Argv[0]) } diff --git a/sdk/go/volumes_test.go b/sdk/go/volumes_test.go index 57dda6ce..7e4a50dc 100644 --- a/sdk/go/volumes_test.go +++ b/sdk/go/volumes_test.go @@ -226,39 +226,6 @@ 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", AttachOnly: true}, {Name: "scratch", Mode: "rw", AttachOnly: true}} - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if err := json.UnmarshalRead(r.Body, &raw); err != nil { - t.Errorf("decode body: %v", err) - } - _ = json.MarshalWrite(w, 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 TestVolumePolicyErrorsComeFromServer(t *testing.T) { tests := []struct { name string diff --git a/sdk/go/watch_test.go b/sdk/go/watch_test.go index 3a1e53a9..a44c256e 100644 --- a/sdk/go/watch_test.go +++ b/sdk/go/watch_test.go @@ -57,7 +57,7 @@ func TestWatchReleasesTheRelayWhenTheSandboxEnds(t *testing.T) { _, _ = r.ReadByte() close(released) }) - sb := legacySandbox(t, ts) + sb := testSandbox(t, ts, WithKeepAlive(0)) w, err := sb.Watch(t.Context(), "/work", true) if err != nil { t.Fatalf("Watch: %v", err) diff --git a/sdk/langchain/cocoonsandbox_langchain/toolkit.py b/sdk/langchain/cocoonsandbox_langchain/toolkit.py index f164fe13..f4fd34a9 100644 --- a/sdk/langchain/cocoonsandbox_langchain/toolkit.py +++ b/sdk/langchain/cocoonsandbox_langchain/toolkit.py @@ -62,7 +62,7 @@ def __exit__(self, *exc: object) -> None: raise def get_tools(self) -> list[StructuredTool]: - """The sandbox tool set; sync-native (_run), async via to_thread.""" + """The sandbox tool set; async calls run on a worker thread.""" return [ self._tool( "sandbox_exec", diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index c3737de9..6c1c4881 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/sdk/python/cocoonsandbox/client.py @@ -62,12 +62,11 @@ def new( ttl_seconds: int = 0, claim_ref: str = "", volumes: list[str | Volume] | None = None, - mount: bool = True, *, deadline: float | None = None, ) -> Sandbox: """Claims a sandbox; a warm hit is milliseconds. deadline is a time.monotonic() bound over the whole claim.""" - claim = _claim_body(template, net, size, ttl_seconds, volumes, mount, claim_ref) + claim = _claim_body(template, net, size, ttl_seconds, volumes, claim_ref) return self._claim_from(self.addr, claim, deadline=deadline) def delete_template(self, template: str, net: str = "", size: str = "") -> None: @@ -75,7 +74,7 @@ def delete_template(self, template: str, net: str = "", size: str = "") -> None: query = _template_query(template, net, size) path = "/v1/templates?" + urllib.parse.urlencode(query) reply = self._request(self.addr, "DELETE", path, None, "delete template") - candidates = (reply or {}).get("redirect") or [] + candidates = reply.get("redirect") or [] if not candidates: return query["no_redirect"] = "1" diff --git a/sdk/python/cocoonsandbox/frames.py b/sdk/python/cocoonsandbox/frames.py index 4a2ea58c..45e41cd4 100644 --- a/sdk/python/cocoonsandbox/frames.py +++ b/sdk/python/cocoonsandbox/frames.py @@ -13,9 +13,9 @@ # the info proto from which silkd serves RPCs back to back on one connection KEEP_ALIVE_PROTO = 2 MAX_FRAME = 8 * 1024 * 1024 -FS_CHUNK = 256 * 1024 +BULK_CHUNK = 256 * 1024 # tar and port streams chunk at 1 MiB, still under MAX_FRAME after base64 -BULK_CHUNK = 1 << 20 +STREAM_CHUNK = 1 << 20 _FAST_DATA = sys.version_info >= (3, 11) @@ -36,7 +36,6 @@ def encode_request(op: str, **fields: object) -> bytes: def decode_response(line: bytes) -> dict[str, Any]: """Decodes one response with binary payloads under data.""" - # base64 is JSON-escape-free, so an exactly-shaped data frame slices without json.loads. if _FAST_DATA and line.startswith(b'{"type":"'): te = line.find(b'"', 9) if te > 0 and line[9:te] in (b"stdout", b"stderr", b"data") and line.startswith(b'","data":"', te): diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index 3e059b95..3e7d8c60 100644 --- a/sdk/python/cocoonsandbox/sandbox.py +++ b/sdk/python/cocoonsandbox/sandbox.py @@ -22,7 +22,7 @@ VolumeError, require_field, ) -from .frames import BULK_CHUNK, FS_CHUNK, KEEP_ALIVE_PROTO +from .frames import BULK_CHUNK, STREAM_CHUNK from .template import Template from .volumes import SandboxVolumesResponse, Volume, _volume_body, _volume_response @@ -57,7 +57,6 @@ def __init__( self.volumes = [volume.copy() for volume in volumes or []] self.volumes_want = [volume.copy() for volume in self.volumes] self._pool = ConnPool(client.keep_alive) - self._proto = 0 def __enter__(self) -> Sandbox: return self @@ -227,7 +226,7 @@ def attach( def write_file(self, path: str, data: bytes, mode: int | None = None) -> None: """Writes data to path atomically (temp + rename on the guest).""" with self._lease(self._connect("fs_write", path=path, mode=mode)) as conn: - _upload(conn, data, FS_CHUNK) + _upload(conn, data, BULK_CHUNK) def read_file(self, path: str) -> bytes: with self._lease(self._connect("fs_read", path=path)) as conn: @@ -256,7 +255,7 @@ def rename(self, src: str, dst: str) -> None: def push(self, dest: str, tar_stream: bytes) -> None: """Extracts a tar stream into dest; a truncated stream leaves dest untouched.""" with self._lease(self._connect("fs_push", dest=dest)) as conn: - _upload(conn, tar_stream, BULK_CHUNK) + _upload(conn, tar_stream, STREAM_CHUNK) def pull(self, path: str) -> bytes: """Returns path (file or tree) as a tar archive.""" @@ -420,11 +419,9 @@ def close(self) -> None: unreachable = exc if exc.status == 0 else None try: moved = self._client.lookup(self.id, self.token) - except APIError as exc: + except APIError: if unreachable is not None: raise unreachable from None - if exc.status != 404: - raise return try: self._release_at(moved.owner) @@ -470,29 +467,20 @@ def _dial(self, deadline: float | None = None) -> Conn: return self._client._dial(self.owner, self.id, self.token, deadline) def _connect(self, op: str, deadline: float | None = None, /, **fields: object) -> Conn: - """Sends the request on a parked or new connection, behind an info probe on a handle's first kept dial.""" + """Sends the request on a parked or new connection.""" conn = self._pool.take() if conn is None: conn = self._dial(deadline) - probe = not self._proto and self._client.keep_alive > 0 - bounded = probe or deadline is not None try: - if bounded: + if deadline is not None: conn.settimeout(remaining_timeout(self._client.timeout, deadline, "agent request")) - if probe: - conn.send("info") conn.send(op, **fields) - if probe: - self._proto = int(_expect(conn, "info").get("proto") or 1) - if bounded: + if deadline is not None: conn.settimeout(None) except BaseException: conn.close() raise - if not probe or self._proto >= KEEP_ALIVE_PROTO: - return conn - conn.close() - return self._connect(op, deadline, **fields) + return conn @contextlib.contextmanager def _lease(self, conn: Conn) -> Iterator[Conn]: @@ -500,25 +488,19 @@ def _lease(self, conn: Conn) -> Iterator[Conn]: try: yield conn except SilkdError: - self._park(conn) + self._pool.park(conn) raise except BaseException: conn.close() raise - self._park(conn) - - def _park(self, conn: Conn) -> None: - if self._proto >= KEEP_ALIVE_PROTO: - self._pool.park(conn) - else: - conn.close() + self._pool.park(conn) def _open_stream(self, op: str, expect: str = "ready", **fields: object) -> tuple[Conn, dict[str, Any]]: conn = self._connect(op, **fields) try: frame = _expect(conn, expect) except SilkdError: - self._park(conn) + self._pool.park(conn) raise except BaseException: conn.close() @@ -558,7 +540,7 @@ def pump_out() -> None: try: with contextlib.suppress(SandboxError, OSError): while True: - chunk = local.recv(FS_CHUNK) + chunk = local.recv(BULK_CHUNK) if not chunk: break guest.send(chunk) @@ -699,7 +681,7 @@ def __init__(self, conn: Conn) -> None: self._eof = False def send(self, data: bytes) -> None: - _send_chunks(self._conn, data, chunk=BULK_CHUNK) + _send_chunks(self._conn, data, chunk=STREAM_CHUNK) def recv(self) -> bytes: """Returns the next chunk from the guest; b'' on stream end.""" @@ -719,7 +701,7 @@ def close(self) -> None: self._conn.close() -def _send_chunks(conn: Conn, data: bytes, op: str = "data", chunk: int = FS_CHUNK) -> None: +def _send_chunks(conn: Conn, data: bytes, op: str = "data", chunk: int = BULK_CHUNK) -> None: view = memoryview(data) for off in range(0, len(view), chunk): conn.send(op, data=view[off : off + chunk]) diff --git a/sdk/python/cocoonsandbox/template.py b/sdk/python/cocoonsandbox/template.py index 205e5763..16f4f213 100644 --- a/sdk/python/cocoonsandbox/template.py +++ b/sdk/python/cocoonsandbox/template.py @@ -27,12 +27,11 @@ def new( self, ttl_seconds: int = 0, volumes: list[str | Volume] | None = None, - mount: bool = True, *, deadline: float | None = None, ) -> Sandbox: - """Claims the template, following placement when volumes require it; mount=False attaches them unmounted.""" - claim = _claim_body(self.name, self.net, self.size, ttl_seconds, volumes, mount) + """Claims the template, following placement when volumes require it.""" + claim = _claim_body(self.name, self.net, self.size, ttl_seconds, volumes) claim["require_promoted"] = True if volumes: return self._client._claim_from(self._addr, claim, deadline=deadline) @@ -54,7 +53,6 @@ def _claim_body( size: str, ttl_seconds: int, volumes: list[str | Volume] | None = None, - mount: bool = True, claim_ref: str = "", ) -> dict[str, Any]: claim: dict[str, Any] = {"template": template} @@ -68,8 +66,6 @@ def _claim_body( raise TypeError("volumes must be a list") if volumes: claim["volumes"] = [_volume_body(volume) for volume in volumes] - if not mount: - claim["volumes_attach_only"] = True if claim_ref: claim["claim_ref"] = claim_ref return claim diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index f05fa03c..3d55fa48 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -114,43 +114,14 @@ def test_claim_preserves_explicit_volume_mode(node): assert seen[1]["volumes"] == [{"name": "imagenet", "mode": ""}] -def test_claim_attaches_volumes_without_mounting(node): - seen = recording_claim( - {"id": "sb_1", "token": "tok", "volumes": [{"name": "imagenet"}, {"name": "scratch", "mode": "rw"}]} - ) - 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 = recording_claim({"id": "sb_2", "token": "tok", "volumes": [{"name": "imagenet"}]}) - 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_leaves_volume_policy_to_server(node): seen = recording_claim({"id": "sb_1", "token": "tok"}) entry = {"name": "imagenet", "mount": "/datasets", "attach_only": True, "mode": "future-mode"} - Client(node).new("rt:24.04", volumes=[entry], mount=False) - Template(Client(node), node, "task:v1", "none", "small").new(volumes=[entry], mount=False) + Client(node).new("rt:24.04", volumes=[entry]) + Template(Client(node), node, "task:v1", "none", "small").new(volumes=[entry]) assert [request["volumes"] for request in seen] == [[entry], [entry]] -def test_claim_keeps_mounting_by_default(node): - seen = recording_claim({"id": "sb_1", "token": "tok"}) - Client(node).new("rt:24.04", volumes=["imagenet"]) - assert "volumes_attach_only" not in seen[0] - - def test_template_claim_asks_for_the_promoted_template(node): seen = recording_claim({"id": "sb_9", "token": "tok"}) Template(Client(node), node, "task:v1", "none", "small").new() diff --git a/sdk/python/tests/test_hardening.py b/sdk/python/tests/test_hardening.py index 2686aa89..f2b5e8a2 100644 --- a/sdk/python/tests/test_hardening.py +++ b/sdk/python/tests/test_hardening.py @@ -22,7 +22,7 @@ Watcher, ) from cocoonsandbox.conn import Conn, dial_agent, remaining_timeout -from cocoonsandbox.frames import FS_CHUNK +from cocoonsandbox.frames import BULK_CHUNK def test_dial_agent_rejects_control_chars_in_identity(): @@ -125,10 +125,10 @@ def drain(): reader_thread = threading.Thread(target=drain, daemon=True) reader_thread.start() - pty.write(b"x" * (3 * FS_CHUNK)) + pty.write(b"x" * (3 * BULK_CHUNK)) reader_thread.join(5) assert [f["op"] for f in frames] == ["stdin"] * 3 - assert all(len(base64.b64decode(f["data"])) == FS_CHUNK for f in frames) + assert all(len(base64.b64decode(f["data"])) == BULK_CHUNK for f in frames) def test_session_and_lsp_close_twice(monkeypatch): diff --git a/sdk/python/tests/test_keepalive.py b/sdk/python/tests/test_keepalive.py index 6b4d5a4e..7491c65c 100644 --- a/sdk/python/tests/test_keepalive.py +++ b/sdk/python/tests/test_keepalive.py @@ -16,10 +16,9 @@ class FakeAgent: - """A relay-side silkd stand-in; proto 1 half-closes after one RPC and drains; slam closes as a failed relay does.""" + """A relay-side silkd stand-in; hang_up_after half-closes after N RPCs, slam closes as a failed relay does.""" - def __init__(self, proto: int = KEEP_ALIVE_PROTO, hang_up_after: int = 0, slam: bool = False) -> None: - self.proto = proto + def __init__(self, hang_up_after: int = 0, slam: bool = False) -> None: self.hang_up_after = hang_up_after self.slam = slam self.upgrades = 0 @@ -55,7 +54,7 @@ def _serve(self, conn: socket.socket) -> None: replies += 1 if self.slam: break - if self.proto < KEEP_ALIVE_PROTO or replies == self.hang_up_after: + if replies == self.hang_up_after: conn.shutdown(socket.SHUT_WR) self.hangups += 1 while reader.readline(): @@ -65,7 +64,7 @@ def _serve(self, conn: socket.socket) -> None: def _answer(self, op: str) -> list: if op == "info": - return [{"type": "info", "version": "fake", "proto": self.proto, "uptime_secs": 0, "procs": 0}] + return [{"type": "info", "version": "fake", "proto": KEEP_ALIVE_PROTO, "uptime_secs": 0, "procs": 0}] if op == "fs_stat": return [{"type": "stat", "info": {"kind": "dir", "size": 0, "mode": 0o755, "mtime_epoch_secs": 0}}] if op == "exec": @@ -91,20 +90,6 @@ def test_a_refused_stream_open_keeps_its_connection(relay): assert agent.upgrades == 1 -def test_old_daemon_dials_per_call(relay): - agent, sb = relay(proto=1) - for _ in range(3): - assert sb.exec("echo", "42") == "42\n" - assert agent.upgrades == 4, "the proto probe plus one dial per call" - - -def test_old_daemon_that_closes_after_info_gets_the_request_on_a_redial(relay): - agent, sb = relay(proto=1, slam=True) - for _ in range(2): - assert sb.stat("/")["kind"] == "dir" - assert agent.upgrades == 3, "the proto probe plus one dial per call" - - def test_keep_alive_off_dials_per_call(relay): agent, sb = relay(keep_alive=0) for _ in range(3): @@ -119,14 +104,14 @@ def test_idle_connection_closes(relay): def test_peer_hang_up_is_noticed_before_reuse(fd_floor, relay): - agent, sb = relay(hang_up_after=2) + agent, sb = relay(hang_up_after=1) sb.stat("/") wait_until(lambda: agent.hangups == 1, "the fake never hung up") assert sb.stat("/")["kind"] == "dir" assert agent.upgrades == 2 -def test_run_timeout_bounds_the_proto_probe(): +def test_run_timeout_bounds_an_unanswered_request(): server = socket.create_server(("127.0.0.1", 0)) cut = threading.Event() @@ -144,7 +129,7 @@ def serve(): try: with pytest.raises(TimeoutError): sb.run(["true"], timeout=0.5) - assert cut.wait(1), "the unanswered info probe outlived the run timeout" + assert cut.wait(1), "the unanswered request outlived the run timeout" finally: server.close() @@ -191,8 +176,8 @@ def test_close_drains_the_parked_connection(monkeypatch, relay): wait_until(lambda: agent.closed == 1, "parked connection survived close") -def test_an_upload_an_old_daemon_rejects_raises_its_error_frame(relay): - _, sb = relay(proto=1, slam=True) +def test_an_upload_the_daemon_rejects_at_open_raises_its_error_frame(relay): + _, sb = relay(slam=True) with pytest.raises(SilkdError): sb.write_file("/missing/f", bytes(64 << 20)) @@ -201,8 +186,8 @@ def test_an_upload_an_old_daemon_rejects_raises_its_error_frame(relay): def relay(): started = [] - def start(proto=KEEP_ALIVE_PROTO, hang_up_after=0, slam=False, **client_kwargs): - agent = FakeAgent(proto, hang_up_after, slam) + def start(hang_up_after=0, slam=False, **client_kwargs): + agent = FakeAgent(hang_up_after, slam) sb = sandbox_at(agent.addr, **client_kwargs) started.append((agent, sb)) return agent, sb diff --git a/sdk/python/tests/test_proc.py b/sdk/python/tests/test_proc.py index b7169223..dda18cde 100644 --- a/sdk/python/tests/test_proc.py +++ b/sdk/python/tests/test_proc.py @@ -6,7 +6,7 @@ from conftest import FakeConn, sandbox_at from cocoonsandbox import ProtocolError -from cocoonsandbox.frames import FS_CHUNK +from cocoonsandbox.frames import BULK_CHUNK class BlockingStdinConn(FakeConn): @@ -75,7 +75,7 @@ def test_run_pumps_stdin_while_reading_output(monkeypatch): blocking = BlockingStdinConn([{"type": "exit", "code": 0}], buffer_frames=1) monkeypatch.setattr(sb, "_dial", lambda deadline=None: blocking) - assert sb.run(["cat"], stdin=b"x" * (FS_CHUNK * 3)) == 0 + assert sb.run(["cat"], stdin=b"x" * (BULK_CHUNK * 3)) == 0 assert [op for op, _ in blocking.sent].count("stdin") == 3 assert blocking.sent[-1][0] == "stdin_close" diff --git a/sdk/python/tests/test_upgrade.py b/sdk/python/tests/test_upgrade.py index 6d033fa7..a68cf561 100644 --- a/sdk/python/tests/test_upgrade.py +++ b/sdk/python/tests/test_upgrade.py @@ -48,18 +48,16 @@ def _serve(self) -> None: conn.sendall(json.dumps(REPLIES[json.loads(line)["op"]]).encode() + b"\n") -@pytest.mark.parametrize( - ("keep_alive", "early"), [(0, ["fs_mkdir"]), (30, ["info", "fs_mkdir"])], ids=["no probe", "proto probe"] -) -def test_first_frames_ride_ahead_of_the_101(keep_alive, early): - agent = EarlyFrameAgent(frames=len(early)) +@pytest.mark.parametrize("keep_alive", [0, 30], ids=["dial per call", "kept connection"]) +def test_the_first_frame_rides_ahead_of_the_101(keep_alive): + agent = EarlyFrameAgent(frames=1) sb = sandbox_at(agent.addr, timeout=5, keep_alive=keep_alive) try: sb.mkdir("/w") finally: sb._pool.drain() agent.stop() - assert agent.early == early, agent.early + assert agent.early == ["fs_mkdir"], agent.early def test_rejected_upgrade_is_an_api_error(): diff --git a/silkd/Dockerfile b/silkd/Dockerfile index cf68dd28..26ee3550 100644 --- a/silkd/Dockerfile +++ b/silkd/Dockerfile @@ -14,6 +14,7 @@ WORKDIR /silkd # compiler. A digest bump that moves ahead of the pin then costs a toolchain # download instead of silently building the guest binary with another rustc. COPY rust-toolchain.toml ./ +RUN sed -i '/^components/d;/^targets/d' rust-toolchain.toml RUN case "$TARGETARCH" in \ amd64) echo x86_64-unknown-linux-musl > /musl-target ;; \ arm64) echo aarch64-unknown-linux-musl > /musl-target ;; \ @@ -24,9 +25,8 @@ RUN case "$TARGETARCH" in \ rm -rf /var/lib/apt/lists/* COPY Cargo.toml Cargo.lock ./ COPY src ./src -RUN cargo build --release --locked && strip target/release/silkd +RUN cargo build --release --locked RUN cargo build --release --locked --target "$(cat /musl-target)" && \ - strip "target/$(cat /musl-target)/release/silkd" && \ cp "target/$(cat /musl-target)/release/silkd" /silkd-static FROM scratch diff --git a/silkd/src/exec.rs b/silkd/src/exec.rs index c3582dc7..dfdeeea7 100644 --- a/silkd/src/exec.rs +++ b/silkd/src/exec.rs @@ -89,7 +89,7 @@ where let mut pump = tokio::spawn(pump_out(Arc::clone(&proc), stdout, stderr, pump_fg)); let pump_abort = pump.abort_handle(); // stdin rides its own task so a slow write cannot stall output or reaping. - let stdin_task = tokio::spawn(pump_stdin(stdin, client, req.detach)); + let stdin_task = tokio::spawn(pump_stdin(stdin.filter(|_| !req.detach), client)); // record the reaped code before the drain: an abort mid-drain must publish the real code, not -1. let reaped: Arc> = Arc::new(OnceLock::new()); @@ -208,12 +208,8 @@ where async fn pump_stdin( stdin: Option, mut client: mpsc::Receiver, - detach: bool, ) { let Some(mut sink) = stdin else { return }; - if detach { - return; - } while let Some(frame) = client.recv().await { match frame { Request::Stdin { data } => { diff --git a/silkd/src/find.rs b/silkd/src/find.rs index d2839e05..a9aaa3b0 100644 --- a/silkd/src/find.rs +++ b/silkd/src/find.rs @@ -5,7 +5,7 @@ use std::os::unix::fs::OpenOptionsExt; use std::path::{Path, PathBuf}; use std::sync::Arc; -use regex::{Regex, Replacer}; +use regex::{Captures, Regex, Replacer}; use tokio::io::{AsyncBufRead, AsyncBufReadExt, AsyncWrite}; use tokio::runtime::Handle; use tokio::sync::{Semaphore, SemaphorePermit, mpsc}; @@ -138,8 +138,8 @@ struct Counting<'a> { count: u64, } -impl regex::Replacer for Counting<'_> { - fn replace_append(&mut self, caps: ®ex::Captures<'_>, dst: &mut String) { +impl Replacer for Counting<'_> { + fn replace_append(&mut self, caps: &Captures<'_>, dst: &mut String) { self.count += 1; caps.expand(self.replacement, dst); } diff --git a/silkd/src/lsp.rs b/silkd/src/lsp.rs index 4e2bcb86..3f305d84 100644 --- a/silkd/src/lsp.rs +++ b/silkd/src/lsp.rs @@ -217,17 +217,10 @@ fn manifest_dir() -> Cow<'static, str> { } async fn read_manifest(language: &str) -> Option { - if language.is_empty() || language.contains(['/', '\\', '\0']) || language.contains("..") { - return None; // never let a language name escape the manifest dir + if language.contains('/') { + return None; } - // O_NOFOLLOW: a symlink in lsp.d must not read a target outside it. - let mut f = tokio::fs::OpenOptions::new() - .read(true) - .custom_flags(libc::O_NOFOLLOW) - .open(format!("{}/{language}", manifest_dir())) + tokio::fs::read_to_string(format!("{}/{language}", manifest_dir())) .await - .ok()?; - let mut raw = String::new(); - f.read_to_string(&mut raw).await.ok()?; - Some(raw) + .ok() } diff --git a/silkd/src/proto.rs b/silkd/src/proto.rs index d7ac4679..da9b76dd 100644 --- a/silkd/src/proto.rs +++ b/silkd/src/proto.rs @@ -235,7 +235,7 @@ pub enum Response { code: i32, }, Done, - /// Acknowledges an armed watch (later events are guaranteed captured) or a connected port_forward. + /// Acknowledges an armed watch (later events are guaranteed captured), a connected port_forward or an attached lsp_request. Ready, Error { kind: ErrorKind, @@ -564,7 +564,6 @@ pub async fn write_feed_error(w: &mut W, err: FeedError) } } -#[inline] pub(crate) fn parse_request(frame: &[u8], buf: &mut Vec) -> serde_json::Result { if let Some(data) = decode_bulk_frame(frame, DATA_FRAME_HEAD, buf) { return Ok(Request::Data { data }); @@ -597,7 +596,7 @@ fn cap_check(len: usize) -> io::Result<()> { fn render_frame(buf: &mut Vec, resp: &Response) -> io::Result<()> { let start = buf.len(); serde_json::to_writer(&mut *buf, resp).map_err(io::Error::other)?; - if let Err(e) = cap_check(buf.len() - start) { + if let Err(e) = cap_check(buf.len() - start + 1) { buf.truncate(start); return Err(e); } @@ -735,6 +734,24 @@ mod tests { assert!(sent.contains("a.txt"), "{sent}"); } + #[test] + fn render_frame_counts_the_newline_against_the_cap() { + let frame = |content: String| Response::Match { + file: "a".into(), + line: 1, + content, + }; + let overhead = serde_json::to_vec(&frame(String::new())) + .expect("encode") + .len(); + let mut buf = Vec::new(); + render_frame(&mut buf, &frame("a".repeat(MAX_FRAME - overhead))) + .expect_err("a body of MAX_FRAME bytes plus its newline exceeds the cap"); + render_frame(&mut buf, &frame("a".repeat(MAX_FRAME - overhead - 1))) + .expect("a body one byte shorter fits"); + assert_eq!(buf.len(), MAX_FRAME); + } + #[tokio::test] async fn frame_reader_caps_oversized_lines() { let big = vec![b'a'; MAX_FRAME + 2]; diff --git a/silkd/src/pty.rs b/silkd/src/pty.rs index 16328230..ab8b226e 100644 --- a/silkd/src/pty.rs +++ b/silkd/src/pty.rs @@ -187,7 +187,7 @@ async fn pump_stdin( } } -/// Reads the master's buffered tail after the shell exits, bounded by POST_EXIT_DRAIN. +/// Reads the master's buffered tail after the shell exits. async fn drain( master: &AsyncFd, proc: &Proc, diff --git a/silkd/src/server.rs b/silkd/src/server.rs index 7d3460e0..79f394ea 100644 --- a/silkd/src/server.rs +++ b/silkd/src/server.rs @@ -215,7 +215,7 @@ impl State { match self.sessions.get(sid) { Some(sess) => { if !sess.run(&e.argv, writer).await { - self.sessions.remove(sid); + self.sessions.remove_if(sid, &sess); } } None => proto::error_frame(writer, ErrorKind::NotFound, "no such session").await?, diff --git a/silkd/src/session.rs b/silkd/src/session.rs index 9f2e1b3b..3b824f7a 100644 --- a/silkd/src/session.rs +++ b/silkd/src/session.rs @@ -122,6 +122,14 @@ impl Table { removed.is_some() } + pub fn remove_if(&self, id: &str, sess: &Arc) { + let mut map = sysutil::lock(&self.inner); + if map.get(id).is_some_and(|cur| Arc::ptr_eq(cur, sess)) { + map.remove(id); + sysutil::kill_group(sess.pid); + } + } + /// Removes and kills sessions idle longer than `ttl`; a session holding the io lock is never idle. pub fn reap_idle(&self, ttl: Duration) -> usize { let mut map = sysutil::lock(&self.inner); diff --git a/silkd/src/sysutil.rs b/silkd/src/sysutil.rs index 1458dc50..a8e1097c 100644 --- a/silkd/src/sysutil.rs +++ b/silkd/src/sysutil.rs @@ -204,7 +204,6 @@ pub async fn wait_code(child: &mut tokio::process::Child) -> i32 { child.wait().await.map_or(-1, exit_code) } -/// Makes the calling process a session leader and adopts fd 0 as its controlling terminal. fn make_controlling_tty() -> std::io::Result<()> { // SAFETY: setsid and ioctl take no pointers, so no memory precondition applies. unsafe { diff --git a/silkd/src/tree.rs b/silkd/src/tree.rs index 3b7811c4..497f1dba 100644 --- a/silkd/src/tree.rs +++ b/silkd/src/tree.rs @@ -104,7 +104,7 @@ where }; let feed = proto::feed_data_frames(reader, &mut sink).await; - drop(sink); // EOF to tar regardless of outcome + drop(sink); let status = child.wait().await?; let msg = err_task.await.unwrap_or_default(); diff --git a/silkd/src/vsock.rs b/silkd/src/vsock.rs index e6292e3b..fe458cf2 100644 --- a/silkd/src/vsock.rs +++ b/silkd/src/vsock.rs @@ -10,7 +10,6 @@ pub async fn serve(port: u32, state: Arc) -> std::io::Result<()> { let listener = VsockListener::bind(VsockAddr::new(VMADDR_CID_ANY, port))?; loop { - // a transient accept error must not tear down the daemon and lose every session. let conn = match listener.accept().await { Ok((conn, peer)) if peer.cid() == VMADDR_CID_HOST => conn, // only the host may drive silkd; a guest-side vsock loopback peer would exec as root @@ -18,6 +17,7 @@ pub async fn serve(port: u32, state: Arc) -> std::io::Result<()> { eprintln!("silkd: refused peer cid {}", peer.cid()); continue; } + // a transient accept error must not tear down the daemon and lose every session. Err(e) => { eprintln!("silkd: accept: {e}"); tokio::time::sleep(std::time::Duration::from_millis(50)).await; diff --git a/silkd/tests/session_e2e.rs b/silkd/tests/session_e2e.rs index 1e73f40f..76280c0c 100644 --- a/silkd/tests/session_e2e.rs +++ b/silkd/tests/session_e2e.rs @@ -284,3 +284,32 @@ async fn session_merges_stderr_into_stdout() { "stderr not merged: {frames:?}" ); } + +#[tokio::test] +async fn a_dead_session_command_leaves_a_recreated_session_alone() { + let state = Arc::new(State::new()); + let id = create_with(&state, json!({"op":"session_create","id":"s"})).await; + let bg = sh( + &state, + &id, + &["sh", "-c", "perl -MPOSIX -e 'setsid(); sleep 2' &"], + ) + .await; + assert_eq!(type_of(bg.last().unwrap()), "exit", "{bg:?}"); + let busy = spawn_sh(&state, &id, &["sleep", "3600"]); + tokio::time::sleep(Duration::from_millis(200)).await; + let rm = one(&state, &json!({"op":"session_rm","id":id}).to_string()).await; + assert_eq!(type_of(&rm[0]), "done"); + let again = create_with(&state, json!({"op":"session_create","id":"s"})).await; + let frames = tokio::time::timeout(Duration::from_secs(10), busy) + .await + .expect("the removed session's command never saw its shell die") + .unwrap(); + assert_eq!(type_of(frames.last().unwrap()), "error"); + let got = sh(&state, &again, &["echo", "alive"]).await; + assert_eq!( + stdout_body(&got).trim(), + "alive", + "the recreated session died: {got:?}" + ); +}