From 4bfe9b910acb1e30ef886ec3ca1efceaf679de03 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:05:51 +0800 Subject: [PATCH 01/19] pool: settle a failed capture's intent before a fork or checkpoint captures the claim A capture waiting on the transition can win it before the failed capture's background settle; it then saved the paused guest, so a later adoption of the hibernate snapshot restored memory older than the disk. --- sandboxd/pool/capture.go | 4 ++++ sandboxd/pool/fork.go | 3 +++ sandboxd/pool/interrupted_test.go | 34 +++++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+) 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/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/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 { From 8ff8f78c9f0b5d029aebab6e396c95c8572846e7 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:06:36 +0800 Subject: [PATCH 02/19] server: answer a preview of a released sandbox 404 through the shared pool error map The preview proxy mapped three pool errors by hand and answered every other one 502 with an Error log, so a tab polling a released sandbox logged an error per request. --- docs/sandboxd-api.md | 4 ++-- sandboxd/server/preview.go | 8 +------- sandboxd/server/preview_test.go | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index 64a74a6c..df9ebb60 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -900,8 +900,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 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) { From 0bae2a0a24efb6992d11b63c0b118f9c5c2d1b39 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:07:50 +0800 Subject: [PATCH 03/19] silkd: drop a dead session command's session only if its id still names that session A removed session's command can see its shell die long after the removal when a setsid child keeps the pipe open; removing by id then killed a session recreated under the same id. --- silkd/src/server.rs | 2 +- silkd/src/session.rs | 8 ++++++++ silkd/tests/session_e2e.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) 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/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:?}" + ); +} From 983ea1412bfeafc2c7d40178f341b68ad2088bb5 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:08:31 +0800 Subject: [PATCH 04/19] silkd: count a frame's newline against the cap every client enforces A body of exactly MAX_FRAME bytes passed render_frame but overflowed the Go scanner and the Python reader, which count the newline, killing the stream instead of answering an error frame. --- silkd/src/proto.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/silkd/src/proto.rs b/silkd/src/proto.rs index d7ac4679..a80931cf 100644 --- a/silkd/src/proto.rs +++ b/silkd/src/proto.rs @@ -597,7 +597,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 +735,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]; From 3f8a2e08dd62269e2eb1a84fa9369d5a1e56ff45 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:09:18 +0800 Subject: [PATCH 05/19] usage: warn when the last landed line rotated out of the journal The shipper then lands from the oldest kept line and the lines in between are lost; the API doc promises a collector never loses a window silently. --- sandboxd/usage/ship.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sandboxd/usage/ship.go b/sandboxd/usage/ship.go index 827d1434..2ecb16c8 100644 --- a/sandboxd/usage/ship.go +++ b/sandboxd/usage/ship.go @@ -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 From ccc31d0c5c97f67bfbfa9ccee247e55a6f50730e Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:09:44 +0800 Subject: [PATCH 06/19] ci: run the moves registry's PostgreSQL tests with the other meta-store suites --- .github/workflows/sandboxd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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" From 08ae7bf90d0748ade2b3f89db3b61dad4fc05e34 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:12:17 +0800 Subject: [PATCH 07/19] docs: match the API, deploy, cluster, SDK and benchmark docs and the godocs to the code --- boot/init/src/cfg.rs | 2 +- docs/benchmarks.md | 7 +++---- docs/cluster.md | 3 +++ docs/deploy.md | 7 +++---- docs/egress.md | 3 ++- docs/performance.md | 2 +- docs/sandboxd-api.md | 6 +++--- docs/sdk.md | 4 ++-- e2e/cmd/rpcbench/main.go | 2 +- sandboxd/archives/archives.go | 2 +- sandboxd/egress/secrets.go | 2 +- sandboxd/engine/engine.go | 2 +- sandboxd/engine/silkd.go | 2 +- sandboxd/pool/operator.go | 2 +- sandboxd/pool/provision.go | 2 +- sandboxd/store/store.go | 2 +- sdk/go/pools.go | 2 +- sdk/langchain/cocoonsandbox_langchain/toolkit.py | 2 +- silkd/src/proto.rs | 2 +- 19 files changed, 29 insertions(+), 27 deletions(-) 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..253d00fc 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 @@ -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/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..3373d497 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 diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index df9ebb60..4944403c 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 @@ -449,8 +450,7 @@ 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. +accepts the legacy claim-wide request flag. ### Partial failure and recovery diff --git a/docs/sdk.md b/docs/sdk.md index 3cccb7f9..af964de9 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -232,8 +232,8 @@ 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 +Volume sandboxes and egress-lane sandboxes cannot hibernate, fork, checkpoint, +or promote, nor take `WithArchiveOnExpire`; each answers 409. Passing `WithVolumes`, `WithVolumesAttachOnly` 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 diff --git a/e2e/cmd/rpcbench/main.go b/e2e/cmd/rpcbench/main.go index 4e6aa1a0..63fbf1af 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 // 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/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..a95e69d6 100644 --- a/sandboxd/engine/engine.go +++ b/sandboxd/engine/engine.go @@ -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/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/pool/operator.go b/sandboxd/pool/operator.go index 931f6a58..bcfa8d2e 100644 --- a/sandboxd/pool/operator.go +++ b/sandboxd/pool/operator.go @@ -73,7 +73,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/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/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/sdk/go/pools.go b/sdk/go/pools.go index 4ce157c6..63de035a 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"` 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/silkd/src/proto.rs b/silkd/src/proto.rs index a80931cf..b823e566 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, From dc1b411b549a939facce2d9e7ac9f3cdeb3a0d07 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:13:17 +0800 Subject: [PATCH 08/19] review: delete restating, stale and narrative comments and fold two-line docs to one --- e2e/cmd/androidsmoke/main.go | 3 +-- e2e/cmd/rpcbench/main.go | 3 +-- e2e/cmd/smoke/main.go | 5 +---- e2e/cmd/volumesmoke/main.go | 3 +-- sandboxd/egress/intercept_test.go | 2 +- sandboxd/egress/policy.go | 2 +- sandboxd/engine/engine.go | 6 +++--- sandboxd/engine/portconn.go | 3 +-- sandboxd/pool/checkpoint.go | 3 +-- sandboxd/pool/move_out.go | 1 - sandboxd/pool/operator.go | 1 - sandboxd/pool/setpools.go | 1 - sandboxd/pool/volume_cleanup_test.go | 1 - sdk/go/sandbox.go | 2 +- sdk/go/silkd/silkdtest/silkdtest.go | 2 +- sdk/python/cocoonsandbox/frames.py | 1 - silkd/src/pty.rs | 2 +- silkd/src/sysutil.rs | 1 - silkd/src/tree.rs | 2 +- silkd/src/vsock.rs | 2 +- 20 files changed, 16 insertions(+), 30 deletions(-) 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/rpcbench/main.go b/e2e/cmd/rpcbench/main.go index 63fbf1af..1d7a9ac8 100644 --- a/e2e/cmd/rpcbench/main.go +++ b/e2e/cmd/rpcbench/main.go @@ -146,8 +146,7 @@ func statRPC(conn net.Conn) error { return nil } -// bufferedConn reads through the handshake reader so bytes coalesced behind -// the 101 are never lost. +// bufferedConn reads through the handshake reader so bytes behind the 101 are kept. type bufferedConn struct { net.Conn r *bufio.Reader 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/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/engine/engine.go b/sandboxd/engine/engine.go index a95e69d6..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 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/pool/checkpoint.go b/sandboxd/pool/checkpoint.go index efa26a61..03045618 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 @@ -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/move_out.go b/sandboxd/pool/move_out.go index 025b8f11..32000746 100644 --- a/sandboxd/pool/move_out.go +++ b/sandboxd/pool/move_out.go @@ -420,7 +420,6 @@ 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/operator.go b/sandboxd/pool/operator.go index bcfa8d2e..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 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/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/sdk/go/sandbox.go b/sdk/go/sandbox.go index 181e4e68..df03ad9e 100644 --- a/sdk/go/sandbox.go +++ b/sdk/go/sandbox.go @@ -75,7 +75,7 @@ type Sandbox struct { c *Client token string - owner string // data-plane address (owner node), from the claim + owner string proto atomic.Uint32 pool agentPool } diff --git a/sdk/go/silkd/silkdtest/silkdtest.go b/sdk/go/silkd/silkdtest/silkdtest.go index 76376376..c1dae0a5 100644 --- a/sdk/go/silkd/silkdtest/silkdtest.go +++ b/sdk/go/silkd/silkdtest/silkdtest.go @@ -149,7 +149,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/python/cocoonsandbox/frames.py b/sdk/python/cocoonsandbox/frames.py index 4a2ea58c..5df83d08 100644 --- a/sdk/python/cocoonsandbox/frames.py +++ b/sdk/python/cocoonsandbox/frames.py @@ -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/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/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; From f92b29ccac7d882c1f4466b5d47ae5c8721ed672 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:15:42 +0800 Subject: [PATCH 09/19] review: order declarations per /code, name each logger after its function, use stdlib names and imports in tests Moves only: utility funcs below type blocks (sdk keepalive, port, silkdtest, e2e harness, guestserver, rpcbench, mcp tools with the server methods in server.go), MoveOut ahead of Sandbox. Logger tags pool.lift, server.router.fail, usage.shipper.drain/read; tapOf names the inspect call. --- e2e/cmd/guestserver/main.go | 58 ++++++------- e2e/cmd/rpcbench/main.go | 16 ++-- e2e/internal/harness/harness.go | 110 ++++++++++++------------ e2e/wire_parity_test.go | 4 +- mcp/server.go | 32 +++++++ mcp/tools.go | 90 +++++++------------ sandboxd/outbound/outbound.go | 4 +- sandboxd/pool/checkpoint_heal_test.go | 2 +- sandboxd/pool/expiry_test.go | 6 +- sandboxd/pool/lease.go | 5 +- sandboxd/pool/volume_attach_test.go | 1 - sandboxd/pool/volume_rw_test.go | 1 - sandboxd/server/client_endpoint_test.go | 13 +-- sandboxd/server/reader.go | 4 +- sandboxd/server/server_test.go | 2 +- sandboxd/types/types.go | 16 ++-- sandboxd/usage/ship.go | 4 +- sdk/go/keepalive.go | 16 ++-- sdk/go/port.go | 10 +-- sdk/go/silkd/conn_test.go | 4 +- sdk/go/silkd/silkdtest/silkdtest.go | 14 +-- silkd/src/find.rs | 6 +- 22 files changed, 208 insertions(+), 210 deletions(-) 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 1d7a9ac8..6e7e2874 100644 --- a/e2e/cmd/rpcbench/main.go +++ b/e2e/cmd/rpcbench/main.go @@ -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,14 +154,6 @@ func statRPC(conn net.Conn) error { return nil } -// 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) } - // 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/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/sandboxd/outbound/outbound.go b/sandboxd/outbound/outbound.go index ea399c08..1b8c7187 100644 --- a/sandboxd/outbound/outbound.go +++ b/sandboxd/outbound/outbound.go @@ -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/checkpoint_heal_test.go b/sandboxd/pool/checkpoint_heal_test.go index c7ece2b6..382ffeca 100644 --- a/sandboxd/pool/checkpoint_heal_test.go +++ b/sandboxd/pool/checkpoint_heal_test.go @@ -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/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/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/volume_attach_test.go b/sandboxd/pool/volume_attach_test.go index b25c2d56..979917cb 100644 --- a/sandboxd/pool/volume_attach_test.go +++ b/sandboxd/pool/volume_attach_test.go @@ -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_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/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/reader.go b/sandboxd/server/reader.go index 8269b56d..390b62ec 100644 --- a/sandboxd/server/reader.go +++ b/sandboxd/server/reader.go @@ -221,14 +221,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 { diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index d883a3e6..38235625 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -2200,7 +2200,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 diff --git a/sandboxd/types/types.go b/sandboxd/types/types.go index 999b189a..e1d42791 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"` @@ -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 2ecb16c8..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 { @@ -158,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/keepalive.go b/sdk/go/keepalive.go index d5dd1e5f..1b7990ca 100644 --- a/sdk/go/keepalive.go +++ b/sdk/go/keepalive.go @@ -18,14 +18,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 @@ -173,3 +165,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/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/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/silkdtest.go b/sdk/go/silkd/silkdtest/silkdtest.go index c1dae0a5..9d29a94c 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) @@ -62,13 +69,6 @@ func ListenHybrid(sockPath string, port int) (io.Closer, error) { 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)) { defer func() { _ = conn.Close() }() 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); } From 27a87e7783ee6ff3531b30ebd3126bef448f038f Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:22:20 +0800 Subject: [PATCH 10/19] cut: inline single-caller helpers, drop a dead guard, a dead JSON tag and the PoolResult alias stampIdentity, confirmGone, roundBytes/roundTimeout, fenceGuest and pullFrom fold into their one caller; Prebind's guarded check cannot fail once a pool policy exists; TemplateDigest is never encoded; DefaultMoveDowntime has no reader outside pool. Buffered exec decodes into one scratch buffer, GuestFile appends without a copy, and the volume patch slices omit when empty. --- sandboxd/outbound/outbound.go | 2 +- sandboxd/pool/checkpoint.go | 4 ++-- sandboxd/pool/claim.go | 16 ++++------------ sandboxd/pool/move.go | 3 +-- sandboxd/pool/move_out.go | 21 +++++---------------- sandboxd/pool/reconcile.go | 6 +++--- sandboxd/pool/remove.go | 8 ++------ sandboxd/server/exec.go | 3 ++- sandboxd/server/reader.go | 3 ++- sandboxd/server/relay.go | 5 ----- sandboxd/store/peer/peer.go | 4 ---- sandboxd/types/api.go | 4 ++-- sandboxd/types/env.go | 6 +++--- sandboxd/types/types.go | 2 +- sdk/go/pools.go | 5 +---- 15 files changed, 29 insertions(+), 63 deletions(-) diff --git a/sandboxd/outbound/outbound.go b/sandboxd/outbound/outbound.go index 1b8c7187..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()) diff --git a/sandboxd/pool/checkpoint.go b/sandboxd/pool/checkpoint.go index 03045618..4e177c7c 100644 --- a/sandboxd/pool/checkpoint.go +++ b/sandboxd/pool/checkpoint.go @@ -100,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 { 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/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 32000746..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,18 +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 -} - 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/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/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/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/reader.go b/sandboxd/server/reader.go index 390b62ec..5f8f8025 100644 --- a/sandboxd/server/reader.go +++ b/sandboxd/server/reader.go @@ -288,5 +288,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/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/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/types/api.go b/sandboxd/types/api.go index 0d81b540..218e0f54 100644 --- a/sandboxd/types/api.go +++ b/sandboxd/types/api.go @@ -32,8 +32,8 @@ type VolumeRegistration struct { // VolumePatch is the wire body of PATCH /v1/sandboxes/{id}/volumes. type VolumePatch struct { - Add []Volume `json:"add,omitzero"` - Remove []string `json:"remove,omitzero"` + Add []Volume `json:"add,omitempty"` + Remove []string `json:"remove,omitempty"` } // VolumePhase is how far one held volume has converged. 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 e1d42791..87b45de7 100644 --- a/sandboxd/types/types.go +++ b/sandboxd/types/types.go @@ -306,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"` diff --git a/sdk/go/pools.go b/sdk/go/pools.go index 63de035a..fae83e08 100644 --- a/sdk/go/pools.go +++ b/sdk/go/pools.go @@ -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) }) } From 48c33513671f0c28510bb5ef728b4d064700fd33 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:22:20 +0800 Subject: [PATCH 11/19] cut: silkd drops the detached stdin task's flag, a no-op inline hint and the lsp name filters a guest root caller can bypass anyway --- silkd/src/exec.rs | 6 +----- silkd/src/lsp.rs | 15 ++++----------- silkd/src/proto.rs | 1 - 3 files changed, 5 insertions(+), 17 deletions(-) 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/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 b823e566..da9b76dd 100644 --- a/silkd/src/proto.rs +++ b/silkd/src/proto.rs @@ -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 }); From ac58b2cd678fe95c440bbf8ff667afdcd59aec3c Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:22:20 +0800 Subject: [PATCH 12/19] cut: python SDK drops an unreachable status check and names its chunk sizes as Go and Rust do --- sdk/python/cocoonsandbox/client.py | 2 +- sdk/python/cocoonsandbox/frames.py | 4 ++-- sdk/python/cocoonsandbox/sandbox.py | 16 +++++++--------- sdk/python/tests/test_hardening.py | 6 +++--- sdk/python/tests/test_proc.py | 4 ++-- 5 files changed, 15 insertions(+), 17 deletions(-) diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index c3737de9..7375a788 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/sdk/python/cocoonsandbox/client.py @@ -75,7 +75,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 5df83d08..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) diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index 3e059b95..4decddaf 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, KEEP_ALIVE_PROTO, STREAM_CHUNK from .template import Template from .volumes import SandboxVolumesResponse, Volume, _volume_body, _volume_response @@ -227,7 +227,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 +256,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 +420,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) @@ -558,7 +556,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 +697,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 +717,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/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_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" From ceb0fb934ef096e433b2ca2c956f34a6d777ae65 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:22:20 +0800 Subject: [PATCH 13/19] ci: build the silkd carrier only when silkd changes, and let a manual run skip the cache --- .github/workflows/build-silkd.yml | 4 ++++ .github/workflows/images.yml | 3 +-- 2 files changed, 5 insertions(+), 2 deletions(-) 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 { From ea61debc710bc1bb728b2249f6c63094c34eab96 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:25:06 +0800 Subject: [PATCH 14/19] test: drop assertions that cannot fail, fail loudly where a wait times out, and reuse the shared helpers refillkick's elapsed check is always zero under synctest; a RunParallel worker reports with Error, not Fatal; waitFile fails the test instead of returning; the fetch-cache check names cacheDir; tuneView's shallow copy gives way to editView; dead fields, parameters and a mutex that guards nothing go. --- sandboxd/engine/engine_test.go | 8 +++++--- sandboxd/mesh/mesh_test.go | 18 +++++++++--------- sandboxd/pool/archive_test.go | 6 +++--- sandboxd/pool/checkpoint_heal_test.go | 2 +- sandboxd/pool/checkpoint_test.go | 4 ++-- sandboxd/pool/drain_test.go | 2 +- sandboxd/pool/egress_bench_test.go | 3 ++- sandboxd/pool/idle_test.go | 2 +- sandboxd/pool/liveness_test.go | 6 +++--- sandboxd/pool/memory_test.go | 2 +- sandboxd/pool/move_test.go | 26 ++++++++------------------ sandboxd/pool/pool_test.go | 8 +------- sandboxd/pool/refillkick_test.go | 7 +++---- sandboxd/pool/running_test.go | 16 ++++++++-------- sandboxd/pool/telemetry_test.go | 2 +- sandboxd/pool/volume_attach_test.go | 2 +- sandboxd/pool/volume_mutation_test.go | 4 ++-- sandboxd/pool/volume_test.go | 15 +++++++-------- sandboxd/server/server_test.go | 3 +-- sandboxd/store/s3/s3_test.go | 2 +- 20 files changed, 61 insertions(+), 77 deletions(-) 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/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/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/checkpoint_heal_test.go b/sandboxd/pool/checkpoint_heal_test.go index 382ffeca..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() 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/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/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/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_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/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/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/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/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 979917cb..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}} 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_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/server_test.go b/sandboxd/server/server_test.go index 38235625..deb0b7d1 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -2232,7 +2232,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 +2573,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/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) } } From ef52a34c4ecd9a0e3643b8905111f7930b2945de Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:31:13 +0800 Subject: [PATCH 15/19] review: keep the volume patch tags the SDK pins through the wire-parity test --- sandboxd/types/api.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sandboxd/types/api.go b/sandboxd/types/api.go index 218e0f54..0d81b540 100644 --- a/sandboxd/types/api.go +++ b/sandboxd/types/api.go @@ -32,8 +32,8 @@ type VolumeRegistration struct { // VolumePatch is the wire body of PATCH /v1/sandboxes/{id}/volumes. type VolumePatch struct { - Add []Volume `json:"add,omitempty"` - Remove []string `json:"remove,omitempty"` + Add []Volume `json:"add,omitzero"` + Remove []string `json:"remove,omitzero"` } // VolumePhase is how far one held volume has converged. From b8a3be1f9a12c98075d1f35383dd4063b6afe49e Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:39:02 +0800 Subject: [PATCH 16/19] cut: drop the info probe kept for silkd before proto 2; info on a sleeping claim wakes it Every silkd since #198 serves RPCs back to back, so the SDKs send the request on their first dial and park the connection without asking. The reader router no longer holds a lone info frame for a pipelined call: info always wakes the claim, since a reader would answer with its own processes and uptime. --- docs/performance.md | 3 +- docs/sandboxd-api.md | 5 +-- sandboxd/server/reader.go | 55 ++------------------------- sandboxd/server/reader_test.go | 52 ++++---------------------- sdk/go/files_test.go | 13 +++---- sdk/go/keepalive.go | 20 +--------- sdk/go/keepalive_test.go | 58 ----------------------------- sdk/go/proc_test.go | 4 +- sdk/go/pty_test.go | 2 +- sdk/go/sandbox.go | 20 +--------- sdk/go/sandbox_test.go | 17 +++------ sdk/go/silkd/silkdtest/fake.go | 2 +- sdk/go/silkd/silkdtest/silkdtest.go | 18 +++------ sdk/go/watch_test.go | 2 +- sdk/python/cocoonsandbox/sandbox.py | 32 ++++------------ sdk/python/tests/test_keepalive.py | 37 ++++++------------ sdk/python/tests/test_upgrade.py | 10 ++--- 17 files changed, 60 insertions(+), 290 deletions(-) diff --git a/docs/performance.md b/docs/performance.md index 3373d497..cd8c1f96 100644 --- a/docs/performance.md +++ b/docs/performance.md @@ -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 4944403c..88589512 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -1236,9 +1236,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/sandboxd/server/reader.go b/sandboxd/server/reader.go index 5f8f8025..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{}) @@ -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) 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/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 1b7990ca..85a81cdd 100644 --- a/sdk/go/keepalive.go +++ b/sdk/go/keepalive.go @@ -1,7 +1,6 @@ package sandbox import ( - "context" "errors" "slices" "sync" @@ -112,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 @@ -151,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 } 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/proc_test.go b/sdk/go/proc_test.go index ac0d87d5..67ebb39a 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))) 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))) _, _, 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..abc20c8f 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).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 df03ad9e..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" @@ -76,7 +75,6 @@ type Sandbox struct { c *Client token string owner string - proto atomic.Uint32 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..1684433d 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).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/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 9d29a94c..cfc2010c 100644 --- a/sdk/go/silkd/silkdtest/silkdtest.go +++ b/sdk/go/silkd/silkdtest/silkdtest.go @@ -26,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. @@ -64,13 +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 } -// 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 - } } } diff --git a/sdk/go/watch_test.go b/sdk/go/watch_test.go index 3a1e53a9..58c4910a 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) w, err := sb.Watch(t.Context(), "/work", true) if err != nil { t.Fatalf("Watch: %v", err) diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index 4decddaf..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, KEEP_ALIVE_PROTO, STREAM_CHUNK +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 @@ -468,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]: @@ -498,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() 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_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(): From cf34ade8a32236ac941a726dd37700cc58e54c9a Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:42:27 +0800 Subject: [PATCH 17/19] cut: drop the claim-wide volumes_attach_only flag, WithVolumesAttachOnly and the Python mount=False Per-entry attach_only gives the same claim; nothing outside this repo sends the legacy flag. --- docs/deploy.md | 4 +-- docs/sandboxd-api.md | 8 ++---- docs/sdk-python.md | 4 +-- docs/sdk.md | 7 +++--- e2e/e2e_test.go | 8 ++---- sandboxd/server/api.go | 18 -------------- sandboxd/server/server.go | 4 +-- sandboxd/server/server_test.go | 3 --- sandboxd/server/volume_mutation_test.go | 24 ------------------ sdk/go/checkpoint.go | 4 +-- sdk/go/checkpoint_test.go | 1 - sdk/go/client.go | 23 +++++++++-------- sdk/go/options.go | 5 ---- sdk/go/volumes_test.go | 33 ------------------------- sdk/python/cocoonsandbox/client.py | 3 +-- sdk/python/cocoonsandbox/template.py | 8 ++---- sdk/python/tests/test_client.py | 33 ++----------------------- 17 files changed, 31 insertions(+), 159 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index 253d00fc..7c5b6996 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -382,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 diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index 88589512..daee9f94 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -113,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: @@ -251,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 @@ -449,8 +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. +not safely roundtrip attach-only intent through new clients. ### Partial failure and recovery 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 af964de9..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, @@ -234,7 +233,7 @@ call on a handle answers 404 once its sandbox has moved; run `Lookup` to tell that from a released sandbox. Volume sandboxes and egress-lane sandboxes cannot hibernate, fork, checkpoint, or promote, nor take `WithArchiveOnExpire`; each answers 409. Passing -`WithVolumes`, `WithVolumesAttachOnly` or `WithClaimRef` to `Checkpoint.New` returns a local error: +`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/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/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/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 deb0b7d1..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"}} 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/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/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/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/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index 7375a788..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: 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() From 3417bb200943d394851829b55f4cba12ce388a02 Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:44:52 +0800 Subject: [PATCH 18/19] build: drop the silkd build's unused toolchain downloads, the strips cargo already does, and the image version defaults silkd builds with the pinned toolchain minus rustfmt, clippy and the toolchain file's x86_64 target, as boot/init does since #456; the release profile strips, so the Dockerfile and release workflow no longer strip again. boot, base and android take their version args from the Makefile and CI only, so a bare build fails loudly instead of building from a stale pin. PATCHES becomes PATCHES.md, which the image hash already skips. --- .github/workflows/release.yml | 1 - boot/Dockerfile | 2 +- docs/e2b.md | 2 +- os-image/android/15/Dockerfile | 4 +--- os-image/android/silkd.rc | 2 +- os-image/base/24.04/Dockerfile | 8 ++------ os-image/e2b-ci/24.04/Dockerfile | 2 +- os-image/e2b-ci/{PATCHES => PATCHES.md} | 0 os-image/e2b-rt/24.04/Dockerfile | 2 +- silkd/Dockerfile | 4 ++-- 10 files changed, 10 insertions(+), 17 deletions(-) rename os-image/e2b-ci/{PATCHES => PATCHES.md} (100%) 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/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/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/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/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 From 68209725d7c861e94415b9bdc93fa2e8788432fd Mon Sep 17 00:00:00 2001 From: CMGS Date: Thu, 5 Nov 2026 14:52:36 +0800 Subject: [PATCH 19/19] test: keep one-RPC fakes off connection reuse now that the probe no longer did it legacySandbox kept these fakes on a dial per call; without it a second call could take the parked connection the fake is about to close. --- sdk/go/proc_test.go | 4 ++-- sdk/go/pty_test.go | 2 +- sdk/go/sandbox_test.go | 2 +- sdk/go/watch_test.go | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/go/proc_test.go b/sdk/go/proc_test.go index 67ebb39a..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 := testSandbox(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 := testSandbox(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 abc20c8f..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 := testSandbox(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_test.go b/sdk/go/sandbox_test.go index 1684433d..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 := testSandbox(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) } diff --git a/sdk/go/watch_test.go b/sdk/go/watch_test.go index 58c4910a..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 := testSandbox(t, ts) + sb := testSandbox(t, ts, WithKeepAlive(0)) w, err := sb.Watch(t.Context(), "/work", true) if err != nil { t.Fatalf("Watch: %v", err)