From a0505e86c58692e4003abcff3829a2fd57f57371 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:07:50 +0800 Subject: [PATCH 01/40] silkd: end a watch with an error when the kernel event queue overflows notify reports an inotify overflow as an Other event with the Rescan flag, not as an error, so to_frames dropped it and the watch went on with a gap that docs/silkd.md says never happens. --- silkd/src/watch.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/silkd/src/watch.rs b/silkd/src/watch.rs index 7d08ad3ca..c68594984 100644 --- a/silkd/src/watch.rs +++ b/silkd/src/watch.rs @@ -101,6 +101,10 @@ fn to_frames(res: notify::Result, out: &mut Vec) { return; } }; + if event.need_rescan() { + out.push(Response::error(ErrorKind::Internal, OVERFLOW_MESSAGE)); + return; + } let kind = match event.kind { N::Create(_) => EventKind::Created, N::Modify(notify::event::ModifyKind::Name(_)) => EventKind::Renamed, @@ -141,6 +145,19 @@ mod tests { } } + #[test] + fn kernel_queue_overflow_becomes_terminal_error_frame() { + let event = + notify::Event::new(notify::EventKind::Other).set_flag(notify::event::Flag::Rescan); + let mut frames = Vec::new(); + to_frames(Ok(event), &mut frames); + assert_eq!(frames.len(), 1); + match &frames[0] { + Response::Error { message, .. } => assert_eq!(message, OVERFLOW_MESSAGE), + other => panic!("expected error frame, got {other:?}"), + } + } + #[test] fn create_event_becomes_event_frames() { let event = notify::Event::new(notify::EventKind::Create(notify::event::CreateKind::File)) From b601d2c64d3bbbb99fb47cf28655a5bf65e000dd Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:08:43 +0800 Subject: [PATCH 02/40] silkd: answer bad_request for a session cwd the guest cannot enter, as exec does The session shell ran 'cd || exit 1' in its init line, so a missing cwd surfaced as 'internal: shell exited'. The cwd is now the child's working directory and spawn errors share exec's mapping. --- silkd/src/exec.rs | 5 +++-- silkd/src/server.rs | 6 +----- silkd/src/session.rs | 8 +++----- silkd/tests/session_e2e.rs | 12 ++++++++++++ 4 files changed, 19 insertions(+), 12 deletions(-) diff --git a/silkd/src/exec.rs b/silkd/src/exec.rs index dfdeeea72..35c89c606 100644 --- a/silkd/src/exec.rs +++ b/silkd/src/exec.rs @@ -138,11 +138,12 @@ where } /// A missing or unrunnable argv or cwd is the caller's mistake; anything else is the guest's. -fn spawn_kind(e: &std::io::Error) -> ErrorKind { +pub(crate) fn spawn_kind(e: &std::io::Error) -> ErrorKind { match e.kind() { std::io::ErrorKind::NotFound | std::io::ErrorKind::NotADirectory - | std::io::ErrorKind::PermissionDenied => ErrorKind::BadRequest, + | std::io::ErrorKind::PermissionDenied + | std::io::ErrorKind::InvalidInput => ErrorKind::BadRequest, _ => ErrorKind::Internal, } } diff --git a/silkd/src/server.rs b/silkd/src/server.rs index 85a66bfce..918440a89 100644 --- a/silkd/src/server.rs +++ b/silkd/src/server.rs @@ -89,11 +89,7 @@ impl State { match self.sessions.create(id, cwd, &env).await { Ok(id) => proto::write_frame(writer, &Response::SessionCreated { id }).await, Err(e) => { - let kind = if e.kind() == std::io::ErrorKind::InvalidInput { - ErrorKind::BadRequest - } else { - ErrorKind::Internal - }; + let kind = crate::exec::spawn_kind(&e); proto::error_frame(writer, kind, format!("session create: {e}")).await } } diff --git a/silkd/src/session.rs b/silkd/src/session.rs index f6bb9e495..3ad1e06bd 100644 --- a/silkd/src/session.rs +++ b/silkd/src/session.rs @@ -47,6 +47,9 @@ impl Table { let mut cmd = Command::new("bash"); cmd.env_clear().envs(sysutil::base_env().iter().copied()); sysutil::apply_claim_env(&mut cmd); + if let Some(dir) = &cwd { + cmd.current_dir(dir); + } let mut child = cmd .stdin(Stdio::piped()) .stdout(Stdio::piped()) @@ -74,11 +77,6 @@ impl Table { // sync on a sentinel so the first real command reads a clean stream. let mut init = String::from("exec 2>&1\n"); - if let Some(dir) = cwd { - init.push_str("cd "); - shell_quote_into(&mut init, &dir); - init.push_str(" || exit 1\n"); - } for (k, v) in env { init.push_str("export "); shell_quote_into(&mut init, k); diff --git a/silkd/tests/session_e2e.rs b/silkd/tests/session_e2e.rs index 8ae5677d9..5235e0c43 100644 --- a/silkd/tests/session_e2e.rs +++ b/silkd/tests/session_e2e.rs @@ -71,6 +71,18 @@ async fn session_applies_cwd_and_env_at_create() { assert!(dir.path().join("marker").exists(), "cwd was not applied"); } +#[tokio::test] +async fn session_create_rejects_a_missing_cwd_like_exec() { + let state = Arc::new(State::new()); + let f = one( + &state, + &json!({"op":"session_create","cwd":"/nonexistent/silkd-cwd"}).to_string(), + ) + .await; + assert_error(&f[0], "bad_request"); + assert!(state.sessions.list().is_empty()); +} + #[tokio::test] async fn session_create_rejects_an_env_name_bash_cannot_export() { let state = Arc::new(State::new()); From b998aa81a8835e77e7b9dd4c16c477b90670c44a Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:13:11 +0800 Subject: [PATCH 03/40] silkd: run each exec in its own process group and signal the group kill, the SDKs' cancel and timeout kill, and a foreground client that hangs up reached only the direct child, so 'sh -c "a | b"' left a and b running after every timeout. Sessions already ran in their own group; exec and its kill now do the same. --- docs/sdk-python.md | 3 ++- docs/sdk.md | 3 ++- docs/silkd.md | 4 ++-- silkd/src/exec.rs | 4 ++++ silkd/src/server.rs | 2 +- silkd/src/session.rs | 6 +++--- silkd/src/sysutil.rs | 13 +++---------- silkd/tests/exec_e2e.rs | 41 ++++++++++++++++++++++++++++++++++++++++- 8 files changed, 57 insertions(+), 19 deletions(-) diff --git a/docs/sdk-python.md b/docs/sdk-python.md index 038bef7a8..d5f1b680c 100644 --- a/docs/sdk-python.md +++ b/docs/sdk-python.md @@ -406,7 +406,8 @@ sb.kill(pid) # default SIGKILL `spawn` starts the command detached with a bounded output ring; `logs` replays it, `attach` follows live output until exit (replay and live stream -hand off atomically). Killing an already-exited process is a no-op success. +hand off atomically). `kill` signals the process group the pid leads, so a +pipeline or a forked child goes with it. Killing an already-exited process is a no-op success. An exited process leaves the table after 5 minutes; from then on its pid raises `SilkdError` `not_found`. diff --git a/docs/sdk.md b/docs/sdk.md index 83238a0cc..1b05df8ab 100644 --- a/docs/sdk.md +++ b/docs/sdk.md @@ -573,7 +573,8 @@ err = sb.Kill(ctx, pid, 0) // 0 = SIGKILL bounded output ring. `Logs` replays the ring and reports the exit code once the process has ended (`exited=false` while running); `Attach` follows live output until exit — the replay and the live stream hand off atomically, so -no chunk is lost or doubled between them. Killing an already-exited process +no chunk is lost or doubled between them. `Kill` signals the process group +the pid leads, so a pipeline or a forked child goes with it. Killing an already-exited process is a no-op success (its OS pid may be recycled; silkd never signals a reaped child). An exited process leaves the table after 5 minutes; from then on its pid answers `not_found` to every verb. diff --git a/docs/silkd.md b/docs/silkd.md index 4726b4c17..31cb326d2 100644 --- a/docs/silkd.md +++ b/docs/silkd.md @@ -33,7 +33,7 @@ fields are ignored (the forward-compatibility story). Sessions and detached processes are server-side state addressed by id — a dropped connection loses nothing (`attach` resumes). A foreground `exec` is bound to its connection, but silkd learns of a drop only when a write to the -client fails: a child that is producing output is killed and its pid freed, a +client fails: a child that is producing output is killed with its process group and its pid freed, a silent one runs to completion — which is why the SDKs' `Run`/`run` answer a canceled ctx or a timeout with a `kill` of the pid `started` reported. The other connection-bound verbs are `fs_watch`, `pty_open`, `lsp_request`, and @@ -48,7 +48,7 @@ suites — a frame only one side can parse fails CI. | group | ops | response flow | |---|---|---| | exec | `exec {argv, cwd?, env?, user?, detach?, session?}` | `started{pid}` → `stdout/stderr{data}`… → `exit{code}`; the client may stream `stdin{data}` / `stdin_close`. `detach` returns after `started`; the process keeps a bounded output ring for later `logs`/`attach` and leaves the table 5 minutes after it exits, after which its pid answers `not_found`; not combinable with `session`. With `session` only `argv` is used — `cwd`, `env`, `user` and `stdin` frames are ignored (the session owns them, and its commands read `/dev/null`) — no `started` is sent, and stderr arrives merged into `stdout`. A child's environment is cleared to `PATH`, `TERM` and the lane's proxy variables, then the [claim env](#claim-env), then `env`; `HOME` and `USER` are set only with `user` | -| procs | `ps` / `kill {pid, signal?}` / `attach {pid}` / `logs {pid}` | `ps` answers one `procs{procs}` frame, `kill` a bare `done`; `logs` replays `stdout/stderr{data}`…, adds `exit{code}` once the process has exited, and closes with `done`; `attach` replays and follows `stdout/stderr{data}`… to `exit{code}`. Handles are guest pids; any connection can list, signal, replay, or re-attach live | +| procs | `ps` / `kill {pid, signal?}` / `attach {pid}` / `logs {pid}` | `ps` answers one `procs{procs}` frame, `kill` signals the process group the pid leads (every exec and pty runs in its own) and answers a bare `done`; `logs` replays `stdout/stderr{data}`…, adds `exit{code}` once the process has exited, and closes with `done`; `attach` replays and follows `stdout/stderr{data}`… to `exit{code}`. Handles are guest pids; any connection can list, signal, replay, or re-attach live | | sessions | `session_create {id?, cwd?, env?}` / `session_list` / `session_rm {id}` | `session_created{id}` / `sessions{sessions}` / `done`. A session is a real persistent bash; `exec` with `session` runs inside it. Idle sessions are reaped after 30 minutes | | fs | `fs_write {path, mode?}` (+`data`/`data_end` frames) / `fs_read` / `fs_list` / `fs_stat` / `fs_mkdir {parents?}` / `fs_rm {recursive?}` / `fs_rename {from, to}` | `fs_read` streams `data{data}`… → `done` for a regular file and answers `bad_request` for a directory, device or FIFO, `fs_list` streams 4096-entry `entries{entries}` batches → `done`, `fs_stat` answers one `stat{info}`, and the mutating verbs terminate with `done`. Streaming runs both directions; write commits atomically via temp+rename and inherits an overwritten file's mode and, under root, its owner | | tree | `fs_push {dest}` (+tar as `data` frames) / `fs_pull {path}` | whole trees as tar streams through the guest tar: `fs_pull` streams `data{data}`… → `done`, `fs_push` terminates with `done` | diff --git a/silkd/src/exec.rs b/silkd/src/exec.rs index 35c89c606..5d7ba6da1 100644 --- a/silkd/src/exec.rs +++ b/silkd/src/exec.rs @@ -48,6 +48,7 @@ where .stdin(Stdio::piped()) .stdout(Stdio::piped()) .stderr(Stdio::piped()) + .process_group(0) .kill_on_drop(!req.detach); if let Some(cwd) = &req.cwd { cmd.current_dir(cwd); @@ -121,6 +122,9 @@ where if let Err(e) = stream_to_client(&mut fg_rx, out).await { // client gone: publish the terminal state the abort pre-empts, else an attacher waits forever. + if proc.exit_code().is_none() { + sysutil::signal_group(pid, libc::SIGKILL); + } supervise.abort(); pump_abort.abort(); stdin_task.abort(); diff --git a/silkd/src/server.rs b/silkd/src/server.rs index 918440a89..a44297d9b 100644 --- a/silkd/src/server.rs +++ b/silkd/src/server.rs @@ -261,7 +261,7 @@ impl State { }; // an exited pid may already be recycled, so signalling it would hit an unrelated process. if proc.exit_code().is_none() { - crate::sysutil::signal_pid(proc.pid, signal.unwrap_or(libc::SIGKILL)); + crate::sysutil::signal_group(proc.pid, signal.unwrap_or(libc::SIGKILL)); } proto::write_frame(w, &Response::Done).await } diff --git a/silkd/src/session.rs b/silkd/src/session.rs index 3ad1e06bd..7d12b1159 100644 --- a/silkd/src/session.rs +++ b/silkd/src/session.rs @@ -115,7 +115,7 @@ impl Table { pub fn remove(&self, id: &str) -> bool { let removed = sysutil::lock(&self.inner).remove(id); if let Some(s) = &removed { - sysutil::kill_group(s.pid); + sysutil::signal_group(s.pid, libc::SIGKILL); } removed.is_some() } @@ -124,7 +124,7 @@ impl Table { 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); + sysutil::signal_group(sess.pid, libc::SIGKILL); } } @@ -135,7 +135,7 @@ impl Table { map.retain(|_, s| { let idle = s.io.try_lock().is_ok() && sysutil::lock(&s.last_active).elapsed() > ttl; if idle { - sysutil::kill_group(s.pid); + sysutil::signal_group(s.pid, libc::SIGKILL); } !idle }); diff --git a/silkd/src/sysutil.rs b/silkd/src/sysutil.rs index 04d378696..01927695c 100644 --- a/silkd/src/sysutil.rs +++ b/silkd/src/sysutil.rs @@ -64,17 +64,10 @@ pub fn rand_token() -> std::io::Result { .collect()) } -/// SIGKILLs the group led by `pgid`, so a session's external command dies with its shell. -pub fn kill_group(pgid: u32) { +/// Sends `sig` to the group led by `pgid`, so a command's children share its fate; ESRCH is ignored. +pub fn signal_group(pgid: u32, sig: i32) { if is_valid_pid(pgid) { - send_signal(-(pgid as libc::pid_t), libc::SIGKILL); - } -} - -/// Sends `sig` to `pid`, ignoring ESRCH against a just-exited pid. -pub fn signal_pid(pid: u32, sig: i32) { - if is_valid_pid(pid) { - send_signal(pid as libc::pid_t, sig); + send_signal(-(pgid as libc::pid_t), sig); } } diff --git a/silkd/tests/exec_e2e.rs b/silkd/tests/exec_e2e.rs index 1651c4ed3..5a0f81875 100644 --- a/silkd/tests/exec_e2e.rs +++ b/silkd/tests/exec_e2e.rs @@ -246,6 +246,38 @@ async fn kill_actually_terminates_a_live_process() { wait_for_exit(&state, pid).await; } +#[tokio::test] +async fn kill_ends_the_whole_command_not_only_its_shell() { + let dir = tempfile::tempdir().unwrap(); + let marker = dir.path().join("child"); + let state = Arc::new(State::new()); + let script = format!("sleep 300 & echo $! > {}; wait", marker.display()); + let pid = detached_pid(&state, &script).await; + wait_listed(&state, pid).await; + let child = tokio::time::timeout(common::DEADLINE, async { + loop { + if let Ok(s) = std::fs::read_to_string(&marker) + && let Ok(n) = s.trim().parse::() + { + return n; + } + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("the shell never wrote its child's pid"); + let killed = one(&state, &format!(r#"{{"op":"kill","pid":{pid}}}"#)).await; + assert_eq!(type_of(&killed[0]), "done"); + wait_for_exit(&state, pid).await; + tokio::time::timeout(common::DEADLINE, async { + while alive(child) { + tokio::time::sleep(Duration::from_millis(10)).await; + } + }) + .await + .expect("the command's child outlived the kill"); +} + #[tokio::test] async fn exec_unknown_user_is_rejected() { let frames = @@ -388,7 +420,7 @@ async fn a_client_gone_mid_output_releases_the_stdin_a_forked_child_holds() { &mut cw, json!({ "op": "exec", - "argv": ["/bin/sh", "-c", "exec 3<&0; (sleep 1; wc -c <&3 >\"$OUT.tmp\"; mv \"$OUT.tmp\" \"$OUT\") >/dev/null 2>&1 & while :; do echo x; sleep 0.05; done"], + "argv": ["/bin/sh", "-c", "exec 3<&0; perl -e 'setpgrp; exec @ARGV' /bin/sh -c 'sleep 1; wc -c <&3 >\"$OUT.tmp\"; mv \"$OUT.tmp\" \"$OUT\"' >/dev/null 2>&1 & while :; do echo x; sleep 0.05; done"], "env": {"OUT": out.to_str().unwrap()} }), ) @@ -419,3 +451,10 @@ async fn a_client_gone_mid_output_releases_the_stdin_a_forked_child_holds() { "the forked child read {read} bytes: stdin kept flowing after its client left" ); } + +fn alive(pid: u32) -> bool { + std::process::Command::new("/bin/sh") + .args(["-c", &format!("kill -0 {pid} 2>/dev/null")]) + .status() + .is_ok_and(|s| s.success()) +} From 762bc91ebaa7fe4c3c18155dced62630863ed2a6 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:14:02 +0800 Subject: [PATCH 04/40] sdk/go: send nothing for a call whose ctx is already done A parked keepalive connection skipped the dial, the one step that checked the ctx, so a canceled call still ran its verb in the guest while the caller saw context.Canceled; Run started a command it would never kill. --- sdk/go/keepalive_test.go | 17 +++++++++++++++++ sdk/go/sandbox.go | 3 +++ 2 files changed, 20 insertions(+) diff --git a/sdk/go/keepalive_test.go b/sdk/go/keepalive_test.go index ca4a2610e..ef89a96a7 100644 --- a/sdk/go/keepalive_test.go +++ b/sdk/go/keepalive_test.go @@ -2,6 +2,8 @@ package sandbox import ( "bufio" + "context" + "errors" "io" "net" "sync/atomic" @@ -54,6 +56,21 @@ func TestRefusedStreamOpenKeepsItsConnection(t *testing.T) { } } +func TestCanceledCallSendsNothingOnAParkedConnection(t *testing.T) { + sb := testSandbox(t, newAgentServer(t, silkdtest.NewFake(t.TempDir()).ServeConn)) + if _, err := sb.Stat(t.Context(), "/"); err != nil { + t.Fatalf("stat: %v", err) + } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if err := sb.Mkdir(ctx, "/d", false); !errors.Is(err, context.Canceled) { + t.Fatalf("mkdir on a canceled ctx: %v, want context.Canceled", err) + } + if _, err := sb.Stat(t.Context(), "/d"); err == nil { + t.Fatal("a canceled mkdir still ran in the guest") + } +} + func TestKeepAliveOffDialsPerCall(t *testing.T) { var upgrades atomic.Int32 sb := testSandbox(t, newAgentServer(t, func(c net.Conn) { diff --git a/sdk/go/sandbox.go b/sdk/go/sandbox.go index 4ea6fce0e..706c8d583 100644 --- a/sdk/go/sandbox.go +++ b/sdk/go/sandbox.go @@ -261,6 +261,9 @@ func (s *Sandbox) call(ctx context.Context, req wire.Request) (*silkd.Conn, *lea // connect sends req on a parked connection or a fresh dial. func (s *Sandbox) connect(ctx context.Context, req wire.Request) (*agentConn, error) { + if err := ctx.Err(); err != nil { + return nil, err + } if c := s.pool.take(); c != nil { return c.sent(req) } From 239a78d09e960315fd85899f94da84d992553431 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:16:00 +0800 Subject: [PATCH 05/40] pool, config: one list of config-owned pool fields, so PUT /v1/pools refuses tun The list lived in three copies (the API refusal, the seed hash, reload's diff) and #478 added tun to only one: PUT /v1/pools answered 200 for "tun": true and dropped it, and a tun edit made every restart warn that the pools differ. --- docs/advanced-network.md | 2 +- docs/sandboxd-api.md | 2 +- sandboxd/config/config.go | 22 ++++++++++++++++++++++ sandboxd/config/reload.go | 25 ++++++++----------------- sandboxd/pool/pool_test.go | 1 + sandboxd/pool/poolstore.go | 10 +++------- sandboxd/pool/poolstore_test.go | 1 + sandboxd/pool/setpools.go | 18 +++--------------- sdk/go/pools.go | 2 +- 9 files changed, 41 insertions(+), 42 deletions(-) diff --git a/docs/advanced-network.md b/docs/advanced-network.md index 837ab32b3..0202352a7 100644 --- a/docs/advanced-network.md +++ b/docs/advanced-network.md @@ -194,7 +194,7 @@ guest -> tun0 -> silkd --vsock 2051--> sandboxd --VXLAN--> exit (vxlan0, forward ### Configuration The node's `tun` block turns the mode on; a pool opts in with `"tun": true` -(the `none` lane only). +(the `none` lane only). Like `egress`, the flag is config-owned: `PUT /v1/pools` rejects it. ```json "tun": { diff --git a/docs/sandboxd-api.md b/docs/sandboxd-api.md index bae014037..f93ef3648 100644 --- a/docs/sandboxd-api.md +++ b/docs/sandboxd-api.md @@ -715,7 +715,7 @@ is deleted; a pool kept with `warm: 0` keeps its golden. Answers the fresh `GET /v1/info` payload. 400 bad key, negative warm/idle, `warm_max` below `warm`, `idle_hibernate_seconds` on an egress pool, a negative archive duration or an `archive_after_seconds` not above the pool's -`idle_hibernate_seconds`, duplicate pool, or a config-owned `egress`/`warmup`/`capture_trim`/`storage`/`replicate_seconds`/`reader`/`egress_upstream_env` +`idle_hibernate_seconds`, duplicate pool, or a config-owned `egress`/`warmup`/`capture_trim`/`storage`/`replicate_seconds`/`reader`/`egress_upstream_env`/`tun` field; 401 bad api token; 409 egress pool on a node without an egress attachment; 503 the `meta_store` cannot take a cell's write. diff --git a/sandboxd/config/config.go b/sandboxd/config/config.go index 0156c14b1..36a4a78f6 100644 --- a/sandboxd/config/config.go +++ b/sandboxd/config/config.go @@ -12,6 +12,7 @@ import ( "net/url" "os" "path/filepath" + "reflect" "runtime" "slices" "strconv" @@ -30,6 +31,8 @@ import ( const ( DefaultMaxForkCount = 16 + fieldReplicateSeconds = "replicate_seconds" + defaultListen = ":7777" defaultDataDir = "/var/lib/sandboxd" defaultCocoonBin = "cocoon" @@ -125,6 +128,25 @@ func (s PoolSpec) ValidateLimits() error { return validateArchiveWindow(s.IdleHibernateSeconds, s.ArchiveAfterSeconds, s.ArchiveDeleteAfterSeconds) } +type ConfigField struct { + Name string + Value reflect.Value +} + +// ConfigOwned lists s's config-only fields; PUT /v1/pools refuses them, the seed hash drops them, and reload diffs them. +func (s *PoolSpec) ConfigOwned() []ConfigField { + return []ConfigField{ + {"egress", reflect.ValueOf(&s.Egress).Elem()}, + {"warmup", reflect.ValueOf(&s.Warmup).Elem()}, + {"capture_trim", reflect.ValueOf(&s.CaptureTrim).Elem()}, + {"storage", reflect.ValueOf(&s.Storage).Elem()}, + {fieldReplicateSeconds, reflect.ValueOf(&s.ReplicateSeconds).Elem()}, + {"reader", reflect.ValueOf(&s.Reader).Elem()}, + {"egress_upstream_env", reflect.ValueOf(&s.EgressUpstreamEnv).Elem()}, + {"tun", reflect.ValueOf(&s.Tun).Elem()}, + } +} + // StoreConfig selects a checkpoint backend. type StoreConfig struct { Kind string `json:"kind"` diff --git a/sandboxd/config/reload.go b/sandboxd/config/reload.go index 190bcc525..9e2dc276e 100644 --- a/sandboxd/config/reload.go +++ b/sandboxd/config/reload.go @@ -68,28 +68,19 @@ func diffPools(cur, next []PoolSpec) (changed, ignored, restart []string) { x, inA := a[k] y, inB := b[k] label := poolLabel(k) - for _, f := range []struct { - name string - same bool - }{ - {"egress", reflect.DeepEqual(x.Egress, y.Egress)}, - {"warmup", slices.Equal(x.Warmup, y.Warmup)}, - {"capture_trim", x.CaptureTrim == y.CaptureTrim}, - {"storage", x.Storage == y.Storage}, - {"reader", x.Reader == y.Reader}, - {"egress_upstream_env", x.EgressUpstreamEnv == y.EgressUpstreamEnv}, - {"tun", x.Tun == y.Tun}, - } { - if !f.same { - changed = append(changed, label+"."+f.name) + ys := y.ConfigOwned() + for i, f := range x.ConfigOwned() { + switch { + case reflect.DeepEqual(f.Value.Interface(), ys[i].Value.Interface()): + case f.Name == fieldReplicateSeconds: + restart = append(restart, label+"."+f.Name) + default: + changed = append(changed, label+"."+f.Name) } } if inA != inB || targetsOf(x) != targetsOf(y) { ignored = append(ignored, label+" targets (PUT /v1/pools owns them)") } - if x.ReplicateSeconds != y.ReplicateSeconds { - restart = append(restart, label+".replicate_seconds") - } } return changed, ignored, restart } diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index bf02b4af6..5abdb35f9 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -650,6 +650,7 @@ func TestSetPoolsRejectsConfigOwnedFields(t *testing.T) { {"replicate_seconds", config.PoolSpec{PoolKey: testKey, Warm: 1, ReplicateSeconds: 60}}, {"reader", config.PoolSpec{PoolKey: testKey, Warm: 1, Reader: true}}, {"egress_upstream_env", config.PoolSpec{PoolKey: testKey, Warm: 1, EgressUpstreamEnv: "UPSTREAM"}}, + {"tun", config.PoolSpec{PoolKey: testKey, Warm: 1, Tun: true}}, } { err := m.SetPools(t.Context(), []config.PoolSpec{tt.spec}) if !errors.Is(err, ErrBadKey) || !strings.Contains(err.Error(), tt.field+" is set in the config file") { diff --git a/sandboxd/pool/poolstore.go b/sandboxd/pool/poolstore.go index 0a7e35972..b510f4555 100644 --- a/sandboxd/pool/poolstore.go +++ b/sandboxd/pool/poolstore.go @@ -121,13 +121,9 @@ func (m *Manager) adoptPools(ctx context.Context) error { func poolSeedHash(specs []config.PoolSpec) string { shaped := slices.Clone(specs) for i := range shaped { - shaped[i].Egress = nil - shaped[i].Warmup = nil - shaped[i].CaptureTrim = false - shaped[i].Storage = "" - shaped[i].ReplicateSeconds = 0 - shaped[i].Reader = false - shaped[i].EgressUpstreamEnv = "" + for _, f := range shaped[i].ConfigOwned() { + f.Value.SetZero() + } } slices.SortFunc(shaped, func(a, b config.PoolSpec) int { return a.Compare(b.PoolKey) }) return utils.DigestHex(shaped) diff --git a/sandboxd/pool/poolstore_test.go b/sandboxd/pool/poolstore_test.go index 3df1f5861..49f42fe69 100644 --- a/sandboxd/pool/poolstore_test.go +++ b/sandboxd/pool/poolstore_test.go @@ -90,6 +90,7 @@ func TestPoolSeedHashIgnoresConfigOwnedFields(t *testing.T) { "replicate_seconds": func(s *config.PoolSpec) { s.ReplicateSeconds = 60 }, "reader": func(s *config.PoolSpec) { s.Reader = true }, "egress_upstream_env": func(s *config.PoolSpec) { s.EgressUpstreamEnv = "POOL_UPSTREAM" }, + "tun": func(s *config.PoolSpec) { s.Tun = true }, } { spec := base edit(&spec) diff --git a/sandboxd/pool/setpools.go b/sandboxd/pool/setpools.go index 9cc121c1e..13363e408 100644 --- a/sandboxd/pool/setpools.go +++ b/sandboxd/pool/setpools.go @@ -40,21 +40,9 @@ func (m *Manager) desiredPools(specs []config.PoolSpec) (map[types.PoolKey]confi if err := spec.ValidateLimits(); err != nil { return nil, fmt.Errorf("%w: pool %s: %w", ErrBadConfig, spec.Template, err) } - // Config-owned fields; accepting one here would silently drop it - for _, f := range []struct { - name string - set bool - }{ - {"egress", spec.Egress != nil}, - {"warmup", spec.Warmup != nil}, - {"capture_trim", spec.CaptureTrim}, - {"reader", spec.Reader}, - {"storage", spec.Storage != ""}, - {"replicate_seconds", spec.ReplicateSeconds != 0}, - {"egress_upstream_env", spec.EgressUpstreamEnv != ""}, - } { - if f.set { - return nil, fmt.Errorf("%w: pool %q: %s is set in the config file, not via the API", ErrBadKey, spec.Template, f.name) + for _, f := range spec.ConfigOwned() { + if !f.Value.IsZero() { + return nil, fmt.Errorf("%w: pool %q: %s is set in the config file, not via the API", ErrBadKey, spec.Template, f.Name) } } if _, ok := desired[spec.PoolKey]; ok { diff --git a/sdk/go/pools.go b/sdk/go/pools.go index fae83e08d..97099aeee 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, reader, replicate_seconds 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, egress_upstream_env and tun are config-owned, so it has no field for them. type PoolSpec struct { Template string `json:"template"` Net NetShape `json:"net,omitempty"` From 79bcb376701eb119f805271aa2bcaf302bc00432 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:17:30 +0800 Subject: [PATCH 06/40] pool: record a claim's usage event right after its commit The event waited for the egress arm, so a crash between the claims.json commit and the arm restored a served claim that no event billed. A batch the arm rolls back now records a release for each claim. --- sandboxd/pool/claim.go | 17 ++++++++++------- sandboxd/pool/egress_test.go | 13 +++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index 62dc97010..daabbfefe 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -609,13 +609,7 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t m.rollbackClaim(ctx, sbs) return fmt.Errorf("persist claim: %w", saveErr) } - for _, sb := range sbs { - if armErr := m.out.Arm(ctx, sb); armErr != nil { - m.rollbackClaim(ctx, sbs) - return fmt.Errorf("arm egress %s: %w", sb.ID, armErr) - } - } - // Usage lands only after the batch armed, so a rollback leaves no unterminated claim event + // The claim event follows the commit at once, so a crash cannot restore a claim nothing billed for _, sb := range sbs { m.recordUsage(ctx, usage.Event{ Event: "claim", @@ -625,6 +619,15 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t Volumes: types.VolumeNames(sb.Volumes), VolumesRW: types.VolumeRWNames(sb.Volumes), }) } + for _, sb := range sbs { + if armErr := m.out.Arm(ctx, sb); armErr != nil { + m.rollbackClaim(ctx, sbs) + for _, sb := range sbs { + m.recordUsage(ctx, usage.Event{Event: "release", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant}) + } + return fmt.Errorf("arm egress %s: %w", sb.ID, armErr) + } + } return nil } diff --git a/sandboxd/pool/egress_test.go b/sandboxd/pool/egress_test.go index 3bb89c1b8..2665483f2 100644 --- a/sandboxd/pool/egress_test.go +++ b/sandboxd/pool/egress_test.go @@ -17,7 +17,6 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/outbound" "github.com/cocoonstack/sandbox/sandboxd/outbound/outboundtest" "github.com/cocoonstack/sandbox/sandboxd/types" - "github.com/cocoonstack/sandbox/sandboxd/usage" ) func TestLaneVerdictReachesGuestOnColdBoots(t *testing.T) { @@ -220,7 +219,7 @@ func TestLockUsesProvisionedTapWithoutList(t *testing.T) { } } -func TestBatchArmFailureRecordsNoUsage(t *testing.T) { +func TestBatchArmFailureEndsEveryClaimEvent(t *testing.T) { eng := newFakeEngine() m := egressManager(t, eng, config.PoolSpec{PoolKey: egKey, Egress: egPolicy}) @@ -238,12 +237,10 @@ func TestBatchArmFailureRecordsNoUsage(t *testing.T) { if claimed != 0 { t.Errorf("rollback left %d claims", claimed) } - raw, err := os.ReadFile(filepath.Join(m.dataDir, usage.JournalFile)) - if err != nil && !errors.Is(err, os.ErrNotExist) { - t.Fatalf("read usage journal: %v", err) - } - if strings.Contains(string(raw), `"ev":"claim"`) { - t.Errorf("rolled-back batch left claim usage events:\n%s", raw) + for _, sb := range sbs { + if got := usageEventsOf(t, m, sb.ID); got != "claim,release" { + t.Errorf("%s usage = %q, want a claim the rollback ends", sb.VMName, got) + } } if !eng.removed("sbx-ok") || !eng.removed("sbx-eg") { t.Error("rollback must destroy every batch VM") From 9e3672103e51cadb516b52f19866bab2cbc56a77 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:21:02 +0800 Subject: [PATCH 07/40] pool: bill each transition at the moment it is published Release takes no Transition lock, and wake, hibernate, archive and unarchive stamped their usage event after the claims commit, so a release committing in between was journaled first and the late event reopened a span to the deadline, or forever for an archive kept forever. A graceful stop had the same gap: eng.Close returned when the last hibernate CLI exited and main exited before the hibernate event. Hibernate, wake and a settled intent now record their event inside the m.mu section that publishes them; archive and unarchive stamp the time there. An archived wake also records unarchive and arms its egress door before the checkpoint store delete, not after it. --- sandboxd/pool/archive.go | 21 +++++++++-------- sandboxd/pool/hibernate.go | 19 ++++++--------- sandboxd/pool/hibernate_test.go | 42 +++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 22 deletions(-) diff --git a/sandboxd/pool/archive.go b/sandboxd/pool/archive.go index 1414b9861..2bc0123d7 100644 --- a/sandboxd/pool/archive.go +++ b/sandboxd/pool/archive.go @@ -152,7 +152,7 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error { sb.HibernateSnap, sb.VsockSocket = "", "" m.setVMNameLocked(sb, "") sb.Deadline = m.archiveDeadline(sb) - js := m.store.set(sb) + js, archivedAt := m.store.set(sb), time.Now() m.mu.Unlock() if saveErr := m.store.commit(js); saveErr != nil { m.mu.Lock() @@ -178,7 +178,7 @@ func (m *Manager) archive(ctx context.Context, sb *types.Sandbox) error { return fmt.Errorf("archive %s: persist claims: %w", sb.ID, saveErr) } m.counters.archives.Add(1) - m.recordUsage(ctx, usage.Event{Event: "archive", ID: sb.ID, Reference: ck.ID, Tenant: sb.Tenant, Deadline: sb.Deadline}) + m.recordUsage(ctx, usage.Event{Time: archivedAt, Event: "archive", ID: sb.ID, Reference: ck.ID, Tenant: sb.Tenant, Deadline: sb.Deadline}) // Disarm under Transition so a wake right after the release is not clobbered by a late disarm m.out.Disarm(sb.ID, true) sb.Transition.Unlock() @@ -235,7 +235,12 @@ func (m *Manager) wakeArchived(ctx context.Context, sb *types.Sandbox) (string, m.destroy(ctx, built.VMName) return "", err } + m.counters.unarchives.Add(1) logger := log.WithFunc("pool.wakeArchived") + // Only the none lane reaches here, so there is no NIC to re-lock + if proxyErr := m.out.ArmProxy(ctx, sb); proxyErr != nil { + logger.Errorf(ctx, proxyErr, "arm egress proxy %s", sb.ID) + } // The pre-commit marker keeps a failed delete retryable after the journal update. if delErr := m.ckpts.Delete(ctx, ck); delErr != nil { logger.Warnf(ctx, "delete consumed archive ck %s: %v", ck, delErr) @@ -245,15 +250,9 @@ func (m *Manager) wakeArchived(ctx context.Context, sb *types.Sandbox) (string, } m.dropTombstone(ctx, sb.ID, consumed) } - // Only the none lane reaches here, so there is no NIC to re-lock - if proxyErr := m.out.ArmProxy(ctx, sb); proxyErr != nil { - logger.Errorf(ctx, proxyErr, "arm egress proxy %s", sb.ID) - } if m.disarmIfReleased(sb) { return "", ErrUnknownSandbox } - m.counters.unarchives.Add(1) - m.recordUsage(ctx, usage.Event{Event: "unarchive", ID: sb.ID, Reference: ck, Tenant: sb.Tenant, Deadline: m.deadlineOf(sb)}) return built.VsockSocket, nil } @@ -266,8 +265,9 @@ func (m *Manager) commitWake(ctx context.Context, sb *types.Sandbox, vmName, soc ck, deadline, epoch, safe := sb.ArchiveCk, sb.Deadline, sb.ArchiveEpoch, sb.ReaderSafe sb.VsockSocket, sb.ArchiveCk, sb.ArchiveEpoch, sb.ReaderSafe = sock, "", 0, false m.setVMNameLocked(sb, vmName) - sb.Deadline = time.Now().Add(clampTTL(types.Seconds(sb.LeaseSeconds))) // a woken sandbox is a fresh lease - js := m.store.set(sb) + wokeAt := time.Now() + sb.Deadline = wokeAt.Add(clampTTL(types.Seconds(sb.LeaseSeconds))) // a woken sandbox is a fresh lease + js, lease := m.store.set(sb), sb.Deadline m.mu.Unlock() if err := m.store.commit(js); err != nil { m.mu.Lock() @@ -282,6 +282,7 @@ func (m *Manager) commitWake(ctx context.Context, sb *types.Sandbox, vmName, soc return fmt.Errorf("wake %s: persist claims: %w", sb.ID, err) } sb.Touch() // restart the idle→hibernate→archive clock + m.recordUsage(ctx, usage.Event{Time: wokeAt, Event: "unarchive", ID: sb.ID, Reference: ck, Tenant: sb.Tenant, Deadline: lease}) return nil } diff --git a/sandboxd/pool/hibernate.go b/sandboxd/pool/hibernate.go index 319555743..b66be743d 100644 --- a/sandboxd/pool/hibernate.go +++ b/sandboxd/pool/hibernate.go @@ -140,8 +140,6 @@ func (m *Manager) hibernateLocked(ctx context.Context, sb *types.Sandbox) error m.dropSnap(ctx, snap) return ErrUnknownSandbox } - // The VM is hibernated either way, so the billing window closes here. - m.recordHibernate(ctx, sb) m.out.Disarm(sb.ID, true) if err != nil { return fmt.Errorf("hibernate %s: persist claims: %w", sb.ID, err) @@ -216,7 +214,6 @@ func (m *Manager) wakeOrRead(ctx context.Context, sb *types.Sandbox, readerOK bo } m.counters.wakes.Add(1) m.counters.wakeNanos.Add(uint64(time.Since(wakeStart))) //nolint:gosec // durations are positive - m.recordUsage(ctx, usage.Event{Event: "wake", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant, Deadline: m.deadlineOf(sb)}) if proxyErr := m.out.ArmProxy(ctx, sb); proxyErr != nil { log.WithFunc("pool.wakeOrRead").Errorf(ctx, proxyErr, "arm egress proxy %s", sb.ID) } @@ -310,6 +307,12 @@ func (m *Manager) commitTransition(ctx context.Context, sb *types.Sandbox, snap, if now := time.Now(); snap == "" && (sb.OnExpire == types.ExpireArchive || m.archiveAfterFor(sb.Key) > 0) && !m.tenantGone(sb.Tenant) && now.After(sb.Deadline) { sb.Deadline = now.Add(clampTTL(types.Seconds(sb.LeaseSeconds))) } + // Under m.mu, so a racing release's event lands after this one and a shutdown cannot keep the record without it + if snap != "" { + m.recordHibernate(ctx, sb) + } else { + m.recordUsage(ctx, usage.Event{Event: "wake", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant, Deadline: sb.Deadline}) + } }) } @@ -333,14 +336,6 @@ func (m *Manager) dropStale(ctx context.Context, sb *types.Sandbox) { // settlePendingSnap settles a dangling hibernate intent and bills an adopted one, which never was. func (m *Manager) settlePendingSnap(ctx context.Context, sb *types.Sandbox) (adopted bool, err error) { - adopted, err = m.resolvePendingSnap(ctx, sb) - if adopted { - m.recordHibernate(ctx, sb) - } - return adopted, err -} - -func (m *Manager) resolvePendingSnap(ctx context.Context, sb *types.Sandbox) (adopted bool, err error) { if sb.PendingSnap == "" { return false, nil } @@ -356,6 +351,7 @@ func (m *Manager) resolvePendingSnap(ctx context.Context, sb *types.Sandbox) (ad if live { if exists { sb.HibernateSnap = pending + m.recordHibernate(ctx, sb) } sb.PendingSnap = "" js = m.store.set(sb) @@ -370,7 +366,6 @@ func (m *Manager) resolvePendingSnap(ctx context.Context, sb *types.Sandbox) (ad } if err := m.store.commit(js); err != nil { m.store.recommit(ctx, js) - // The transition is decided in memory, so report the adoption for the caller to bill return exists, fmt.Errorf("resolve hibernate intent %s: persist: %w", sb.ID, err) } return exists, nil diff --git a/sandboxd/pool/hibernate_test.go b/sandboxd/pool/hibernate_test.go index 77ebe54ef..4c4df3ad6 100644 --- a/sandboxd/pool/hibernate_test.go +++ b/sandboxd/pool/hibernate_test.go @@ -79,6 +79,48 @@ func TestWakePersistFailureSurfaces(t *testing.T) { }) } +func TestAReleaseRacingAWakeBillsTheWakeFirst(t *testing.T) { + m := newTestManager(t, newFakeEngine()) + sb := mustClaim(t, m, testKey) + hibernateClaim(t, m, sb) + m.store.writeMu.Lock() + locked := true + t.Cleanup(func() { + if locked { + m.store.writeMu.Unlock() + } + }) + woke := make(chan error, 1) + go func() { + _, _, _, err := m.WakeAgentSocket(t.Context(), sb.ID, sb.Token) + woke <- err + }() + waitFor(t, func() bool { + m.mu.Lock() + defer m.mu.Unlock() + return sb.HibernateSnap == "" + }) + if got := usageEventsOf(t, m, sb.ID); got != "claim,hibernate,wake" { + t.Fatalf("usage once the wake is published = %q, want its wake already recorded", got) + } + released := make(chan error, 1) + go func() { released <- m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) }() + waitFor(t, func() bool { + m.mu.Lock() + defer m.mu.Unlock() + return m.claimed[sb.ID] == nil + }) + m.store.writeMu.Unlock() + locked = false + <-woke + if err := <-released; err != nil { + t.Fatalf("release: %v", err) + } + if got := usageEventsOf(t, m, sb.ID); got != "claim,hibernate,wake,release" { + t.Errorf("usage = %q, want the release to end the span the wake opened", got) + } +} + func TestHibernateAmbiguousErrorTreatedAsDone(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) From 76aeb1461bf051ebf184b48e490b764f3191fe2d Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:26:10 +0800 Subject: [PATCH 08/40] pool: bill a cold-booted guest from its start, and end a failed claim's span A VMM restart recorded vmm_restart only after the probe and the claims commit, so a stop or crash during the probe (up to 90 s) left a running guest billed as ended. vmm_restart now follows the start under m.mu, and every stop after a start records vmm_exit. failLocked records vmm_exit before vmm_failed when the exit was not recorded yet, so an interrupted capture whose guest does not resume no longer stays billed running until release (docs/deploy.md promises both events). --- docs/deploy.md | 2 +- sandboxd/pool/interrupted_test.go | 5 ++++- sandboxd/pool/liveness.go | 30 ++++++++++++++++++++++-------- sandboxd/pool/liveness_test.go | 22 ++++++++++++++++++++-- 4 files changed, 47 insertions(+), 12 deletions(-) diff --git a/docs/deploy.md b/docs/deploy.md index e67e29472..3f9a463fd 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -743,7 +743,7 @@ claimed `on_expire: archive`, which needs a running VM to archive: | outcome | when | client sees | |---|---|---| | cold restart | `cold` (default), none-lane claim without volumes, within 3 restarts in 10 min | `restarts` +1 and `restarted_at` on the sandbox row, `vmm_exit` then `vmm_restart` usage events; a call before the next pass gets `502`, one during the boot waits for it, open relays see EOF and redial | -| failed | `none`, an egress-lane or volume claim, the VM record gone, the restart budget spent, or 3 failed boots | `failed: ""` on the row, `vmm_exit` then `vmm_failed` usage events, `409` from exec, relays, ports, hibernate, fork, checkpoint and promote | +| failed | `none`, an egress-lane or volume claim, the VM record gone, the restart budget spent, or 3 failed boots | `failed: ""` on the row, `vmm_exit` then `vmm_failed` usage events (a boot whose guest setup failed adds its own `vmm_restart` and `vmm_exit`), `409` from exec, relays, ports, hibernate, fork, checkpoint and promote | A cold restart boots the claim's own copy-on-write disk: files the guest synced survive, memory and processes do not. The claim's guest env entries diff --git a/sandboxd/pool/interrupted_test.go b/sandboxd/pool/interrupted_test.go index d46f2d0a0..78185591a 100644 --- a/sandboxd/pool/interrupted_test.go +++ b/sandboxd/pool/interrupted_test.go @@ -213,12 +213,15 @@ func TestInterruptedCaptureFailsTheClaim(t *testing.T) { synctest.Test(t, func(t *testing.T) { eng := newFakeEngine() tt.edit(eng) - _, sb := reconcileInterrupted(t, eng, interruptedHib, "", nil) + m, sb := reconcileInterrupted(t, eng, interruptedHib, "", nil) time.Sleep(captureResumeBudget + 2*captureResumeRetry) synctest.Wait() if !strings.Contains(sb.Failed, "did not resume") { t.Errorf("failed %q, want the resume failure", sb.Failed) } + if events := usageEventsOf(t, m, sb.ID); !strings.HasSuffix(events, "vmm_exit,vmm_failed") { + t.Errorf("events %q, want the failure to end the running span", events) + } }) }) } diff --git a/sandboxd/pool/liveness.go b/sandboxd/pool/liveness.go index 40ceab0ca..64cd84da1 100644 --- a/sandboxd/pool/liveness.go +++ b/sandboxd/pool/liveness.go @@ -194,10 +194,7 @@ func (m *Manager) recoverVMM(ctx context.Context, sb *types.Sandbox) { case found && vm.State == vmStateRunning: return } - if !sb.ExitRecorded { - sb.ExitRecorded = true - m.recordUsage(ctx, usage.Event{Event: "vmm_exit", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant}) - } + m.recordExit(ctx, sb) reason := coldBootBlocker(sb, found, volumes) if reason == "" && m.view.Load().vmmRestart == types.VMMRestartNone { reason = "vmm exited" @@ -260,10 +257,16 @@ func (m *Manager) restartLocked(ctx context.Context, sb *types.Sandbox) error { m.stopVM(ctx, sb.VMName) return fmt.Errorf("cold boot: %w", err) } + m.mu.Lock() + if m.claimed[sb.ID] == sb { + sb.ExitRecorded = false + m.recordUsage(ctx, usage.Event{Event: "vmm_restart", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant}) + } + m.mu.Unlock() // cocoon keeps a VM's run dir across a cold boot, so the journaled socket path still holds sock, err := m.probeReady(ctx, sb.VMName, sb.VsockSocket, coldProbeTimeout) if err != nil { - m.stopVM(ctx, sb.VMName) + m.stopRestarted(ctx, sb) return fmt.Errorf("probe after cold boot: %w", err) } m.mu.Lock() @@ -276,7 +279,7 @@ func (m *Manager) restartLocked(ctx context.Context, sb *types.Sandbox) error { } if err != nil { m.out.Disarm(sb.ID, false) - m.stopVM(ctx, sb.VMName) + m.stopRestarted(ctx, sb) return fmt.Errorf("restore egress and env after cold boot: %w", err) } if live, _ := m.commitIfLive(ctx, sb, func() { @@ -291,18 +294,29 @@ func (m *Manager) restartLocked(ctx context.Context, sb *types.Sandbox) error { if m.disarmIfReleased(sb) { return ErrUnknownSandbox } - sb.ExitRecorded = false sb.Touch() - m.recordUsage(ctx, usage.Event{Event: "vmm_restart", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant}) log.WithFunc("pool.restartLocked").Warnf(ctx, "restarted %s (%s) from its disk; guest memory is lost", sb.ID, sb.VMName) return nil } +func (m *Manager) stopRestarted(ctx context.Context, sb *types.Sandbox) { + m.stopVM(ctx, sb.VMName) + m.recordExit(ctx, sb) +} + +func (m *Manager) recordExit(ctx context.Context, sb *types.Sandbox) { + if !sb.ExitRecorded { + sb.ExitRecorded = true + m.recordUsage(ctx, usage.Event{Event: "vmm_exit", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant}) + } +} + // failLocked parks sb as failed with reason; the caller holds sb.Transition. func (m *Manager) failLocked(ctx context.Context, sb *types.Sandbox, reason string) { if live, _ := m.commitIfLive(ctx, sb, func() { sb.Failed = reason }); !live { return } + m.recordExit(ctx, sb) m.recordUsage(ctx, usage.Event{Event: "vmm_failed", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant, Reference: reason}) log.WithFunc("pool.failLocked").Warnf(ctx, "sandbox %s failed: %s", sb.ID, reason) } diff --git a/sandboxd/pool/liveness_test.go b/sandboxd/pool/liveness_test.go index aad5e33c4..4eb0fe6f3 100644 --- a/sandboxd/pool/liveness_test.go +++ b/sandboxd/pool/liveness_test.go @@ -306,6 +306,23 @@ func TestRestartThatCannotArmTheEgressDoorFailsAndSpendsTheBudget(t *testing.T) assertRestartFailsUntilBudget(t, m, eng, sb) } +func TestARestartedGuestIsBilledFromItsBoot(t *testing.T) { + eng := newFakeEngine() + m := newTestManager(t, eng) + sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Env: types.Env{"MODE": {Value: "prod"}}}) + if err != nil { + t.Fatalf("claim: %v", err) + } + killVMM(eng, sb.VMName) + eng.mu.Lock() + eng.guestEnvErr = errors.New("guest write refused") + eng.mu.Unlock() + m.recoverVMM(t.Context(), sb) + if got := usageEventsOf(t, m, sb.ID); got != "claim,vmm_exit,vmm_restart,vmm_exit" { + t.Errorf("events %q, want the booted guest billed from its start to its stop", got) + } +} + func TestDaemonExitEventRecoversWithoutAList(t *testing.T) { eng := newFakeEngine() eng.vmEvents = make(chan fakeVMEvent) @@ -649,8 +666,9 @@ func assertRestartFailsUntilBudget(t *testing.T, m *Manager, eng *fakeEngine, sb if stops != vmmRestartBudget { t.Errorf("stops %d, want every failed restart's VM stopped", stops) } - if events := usageEventsOf(t, m, sb.ID); strings.Contains(events, "vmm_restart") { - t.Errorf("events %s record a restart that never completed", events) + events := usageEventsOf(t, m, sb.ID) + if !strings.HasSuffix(events, "vmm_exit,vmm_failed") || strings.Count(events, "vmm_restart") != strings.Count(events, "vmm_exit")-1 { + t.Errorf("events %s, want each booted attempt billed up to its stop, then the failure", events) } } From 0487fcb5ec3b59496aea9586648c1352494a2cda Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:28:15 +0800 Subject: [PATCH 09/40] pool: stamp adopt and move_in where they are published Both events were stamped after the claims commit, so a release committing in between was journaled first and the late adopt or move_in reopened a span to the deadline. adopt takes its time and deadline under m.mu; move_in records inside the commit's publish. A moved guest that cannot start lets failLocked record its vmm_exit. --- sandboxd/pool/claim.go | 6 ------ sandboxd/pool/failover.go | 4 ++-- sandboxd/pool/move_in.go | 8 ++++---- sandboxd/pool/pool_test.go | 6 ++++++ 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index daabbfefe..0a121bc5d 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -679,12 +679,6 @@ func (m *Manager) commitIfLive(ctx context.Context, sb *types.Sandbox, mutate fu return true, nil } -func (m *Manager) deadlineOf(sb *types.Sandbox) time.Time { - m.mu.Lock() - defer m.mu.Unlock() - return sb.Deadline -} - // admitOptions validates the key and options and admits the claim's volumes, which the caller unreserves until finalize. func (m *Manager) admitOptions(ctx context.Context, key types.PoolKey, o ClaimOptions) ([]resolvedVolume, []types.Volume, *claimHold, error) { if err := m.validate(key); err != nil { diff --git a/sandboxd/pool/failover.go b/sandboxd/pool/failover.go index 18a56cffb..46c1df4be 100644 --- a/sandboxd/pool/failover.go +++ b/sandboxd/pool/failover.go @@ -70,13 +70,13 @@ func (m *Manager) adoptArchived(ctx context.Context, id, token string) (bool, er sb.Deadline = m.retentionDeadline(sb.Key) } m.addClaimLocked(sb) - js := m.store.set(sb) + js, adoptedAt, retention := m.store.set(sb), time.Now(), sb.Deadline m.mu.Unlock() if err := m.store.commit(js); err != nil { m.forgetClaim(ctx, sb) return false, fmt.Errorf("adopt %s: persist claims: %w", id, err) } - m.recordUsage(ctx, usage.Event{Event: "adopt", ID: id, Reference: rec.Checkpoint, Tenant: sb.Tenant, Deadline: m.deadlineOf(sb)}) + m.recordUsage(ctx, usage.Event{Time: adoptedAt, Event: "adopt", ID: id, Reference: rec.Checkpoint, Tenant: sb.Tenant, Deadline: retention}) log.WithFunc("pool.adoptArchived").Infof(ctx, "adopted archived sandbox %s from %s", id, rec.Owner) return true, nil } diff --git a/sandboxd/pool/move_in.go b/sandboxd/pool/move_in.go index 32098a111..a26aab64d 100644 --- a/sandboxd/pool/move_in.go +++ b/sandboxd/pool/move_in.go @@ -150,7 +150,10 @@ func (m *Manager) startMoveIn(ctx context.Context, sb *types.Sandbox, from strin if !cold && !m.startMovedGuest(ctx, sb, committed) { return } - live, err := m.commitIfLive(ctx, sb, func() { sb.MoveIn = false }) + live, err := m.commitIfLive(ctx, sb, func() { + sb.MoveIn = false + m.recordUsage(ctx, usage.Event{Event: "move_in", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant, Deadline: sb.Deadline}) + }) if !live { m.out.Disarm(sb.ID, true) return @@ -159,7 +162,6 @@ func (m *Manager) startMoveIn(ctx context.Context, sb *types.Sandbox, from strin logger.Warnf(ctx, "persist the start of moved claim %s: %v", sb.ID, err) } sb.Touch() - m.recordUsage(ctx, usage.Event{Event: "move_in", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant, Deadline: m.deadlineOf(sb)}) _ = os.RemoveAll(m.moveDir(sb.ID)) if committed != 0 && err == nil { m.markMoveStarted(ctx, sb.ID, committed) @@ -181,8 +183,6 @@ func (m *Manager) startMovedGuest(ctx context.Context, sb *types.Sandbox, commit } if err != nil { _, persistErr := m.commitIfLive(ctx, sb, func() { sb.MoveIn = false }) - sb.ExitRecorded = true - m.recordUsage(ctx, usage.Event{Event: "vmm_exit", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant}) m.failLocked(ctx, sb, reasonMoveStartFailed) _ = os.RemoveAll(m.moveDir(sb.ID)) if committed != 0 && persistErr == nil { diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 5abdb35f9..61879245a 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -2240,6 +2240,12 @@ func (f *fakeEngine) snapRemoved(name string) bool { return slices.Contains(f.snapRemoves, name) } +func (m *Manager) deadlineOf(sb *types.Sandbox) time.Time { + m.mu.Lock() + defer m.mu.Unlock() + return sb.Deadline +} + func (m *Manager) epochOf(sb *types.Sandbox) int64 { m.mu.Lock() defer m.mu.Unlock() From a23f8ff7d72640dbbc72bc82e36189181be0ebb6 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:29:38 +0800 Subject: [PATCH 10/40] pool: keep the bill and address of a dropped claim another node runs or can adopt Reconcile ended a claim whose VM was gone with a vm_gone reap and freed its overlay address even when a move journal said the other node runs it, and freed the address of a replicated claim whose registry row stays adoptable, so a later claim could answer for the same address. Such claims now keep both; purging the row frees its address. --- sandboxd/pool/reconcile.go | 5 +++++ sandboxd/pool/tun_test.go | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index be458b554..3274b5318 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -340,8 +340,13 @@ func (m *Manager) recordReconciled(ctx context.Context, resumed, dropped []*type m.recordUsage(ctx, usage.Event{Event: "wake", ID: sb.ID, VMName: sb.VMName, Tenant: sb.Tenant, Deadline: sb.Deadline}) } for _, sb := range dropped { + if sb.MoveIn || sb.MoveOut != nil { + continue + } if answered || !sb.HasReplica() { m.recordEnd(ctx, sb, "vm_gone") + } + if !sb.HasReplica() { m.releaseTun(ctx, sb.ID, sb.TunAddr) } } diff --git a/sandboxd/pool/tun_test.go b/sandboxd/pool/tun_test.go index 8d025b939..6d4c5cafb 100644 --- a/sandboxd/pool/tun_test.go +++ b/sandboxd/pool/tun_test.go @@ -309,6 +309,26 @@ func TestFlushClaimsPersistsTheLiveTunBytes(t *testing.T) { } } +func TestReconcileKeepsTheAddressAndBillOfAClaimAnotherNodeRunsOrCanAdopt(t *testing.T) { + addrs := &fakeAddrs{} + m := tunManager(t, newFakeEngine(), addrs, config.PoolSpec{PoolKey: testKey, Tun: true, Egress: anyPolicy}) + addr := netip.MustParseAddr("240.0.0.9") + dropped := []*types.Sandbox{ + {ID: "sb_out", VMName: "sbx-out", TunAddr: addr, MoveOut: &types.MoveOut{Epoch: 1}}, + {ID: "sb_in", VMName: "sbx-in", TunAddr: addr, MoveIn: true}, + {ID: "sb_rep", VMName: "sbx-rep", TunAddr: addr, ReplicaCk: "ck_00000000000000aa"}, + } + m.recordReconciled(t.Context(), nil, dropped, true) + for id, want := range map[string]string{"sb_out": "", "sb_in": "", "sb_rep": "reap"} { + if got := usageEventsOf(t, m, id); got != want { + t.Errorf("%s events %q, want %q", id, got, want) + } + } + if got := addrs.releasedList(); len(got) != 0 { + t.Errorf("released %v, want every address kept", got) + } +} + func TestATunFlowEndWritesOneAuditRecord(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true} From daf147aeaab7cfa52eec6e87b754e82137ab7a07 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:30:43 +0800 Subject: [PATCH 11/40] pool: leave a replicated claim's exited VMM alone while the node is fenced Peers may adopt a replicated claim once the fence trips, but the liveness pass still journaled vmm_exit and cold-booted it with vmm_restart, which the cross-node fold reads after the peer's adopt: the adopted claim billed as running compute. docs/cluster.md says a fenced node only keeps the guest running until it returns. --- sandboxd/pool/lease_test.go | 16 ++++++++++++++++ sandboxd/pool/liveness.go | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/sandboxd/pool/lease_test.go b/sandboxd/pool/lease_test.go index ff0c0b640..b1cbba6d8 100644 --- a/sandboxd/pool/lease_test.go +++ b/sandboxd/pool/lease_test.go @@ -351,6 +351,22 @@ func TestLeaseFencedNodeDoesNotAnswerAsOwnerOfReplicatedClaims(t *testing.T) { } } +func TestLeaseFencedNodeLeavesAReplicatedClaimsExitedVMMAlone(t *testing.T) { + c := newReplicaCluster(t) + eng := newFakeEngine() + a := c.nodeOn(t, eng, "a", t.TempDir(), keepAnHour) + a.liftFence() + sb := mustClaim(t, a, testKey) + replicateNow(t, a, sb) + a.tripFence() + before := usageEventsOf(t, a, sb.ID) + killVMM(eng, sb.VMName) + a.recoverVMM(t.Context(), sb) + if got := usageEventsOf(t, a, sb.ID); got != before { + t.Errorf("events %q after %q, want nothing journaled for a claim a peer may have adopted", got, before) + } +} + func TestLeaseLiftAfterTheWindowClosedFencesAgain(t *testing.T) { c := newReplicaCluster(t) a := c.node(t, "a", t.TempDir(), keepAnHour) diff --git a/sandboxd/pool/liveness.go b/sandboxd/pool/liveness.go index 64cd84da1..886fcaac7 100644 --- a/sandboxd/pool/liveness.go +++ b/sandboxd/pool/liveness.go @@ -178,7 +178,7 @@ func (m *Manager) recoverVMM(ctx context.Context, sb *types.Sandbox) { defer sb.Transition.Unlock() now := time.Now() m.mu.Lock() - live := m.claimed[sb.ID] == sb && m.vmSettledLocked(sb) && !m.leaseLapsed(sb, now) + live := m.claimed[sb.ID] == sb && m.vmSettledLocked(sb) && !m.leaseLapsed(sb, now) && !(sb.Replicated && m.isFenced()) volumes := m.hasVolumeHolds(sb) m.mu.Unlock() if !live { From e88a0c64b6696d03ba7140329e4169831b3d8437 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:31:56 +0800 Subject: [PATCH 12/40] pool: hold an adopted claim's Transition until its commit settles An adopt whose claims.json write failed forgot the claim without the Transition lock, while a concurrent wake of the just-published claim took and consumed its registry row and then found the claim gone, so the checkpoint was deleted and every later Lookup answered 404. The adopt now holds the new claim's Transition across the commit, and a wake whose claim left during the take stops before consuming the row. --- sandboxd/pool/failover.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/sandboxd/pool/failover.go b/sandboxd/pool/failover.go index 46c1df4be..504ddc934 100644 --- a/sandboxd/pool/failover.go +++ b/sandboxd/pool/failover.go @@ -65,6 +65,8 @@ func (m *Manager) adoptArchived(ctx context.Context, id, token string) (bool, er } sb.Token, sb.ArchiveCk, sb.ArchiveEpoch, sb.Deadline = token, rec.Checkpoint, epoch, rec.Deadline sb.SetEnv(sb.Env) + sb.Transition.Lock() + defer sb.Transition.Unlock() m.mu.Lock() if sb.Deadline.IsZero() { sb.Deadline = m.retentionDeadline(sb.Key) @@ -244,7 +246,9 @@ func (m *Manager) takeArchive(ctx context.Context, sb *types.Sandbox) (int64, er if err != nil { return 0, fmt.Errorf("wake %s: %w", sb.ID, err) } - _, _ = m.commitIfLive(ctx, sb, func() { sb.ArchiveEpoch = epoch }) + if live, _ := m.commitIfLive(ctx, sb, func() { sb.ArchiveEpoch = epoch }); !live { + return 0, ErrUnknownSandbox + } return epoch, nil } From 27f70ff838f764eaa14f18be135b59e58e915698 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:32:44 +0800 Subject: [PATCH 13/40] pool: check a claim's injects against the policy layer it will run under finalizeBatch set the claim's layer only after the inject check, so a pooled key whose pool has no egress resolved to the tenant class during the check: the claim answered 200 for an inject that no proxy would ever apply, while the same env through PATCH .../env answered 400. --- sandboxd/pool/claim.go | 12 ++++++++---- sandboxd/pool/env_test.go | 13 +++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index 0a121bc5d..708026ed9 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -574,6 +574,14 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t } sb.TouchAt(now) } + m.mu.Lock() + for _, sb := range sbs { + sb.Layer = types.LayerUnpooled + if _, pooled := m.activePool(sb.PolicyKey()); pooled { + sb.Layer = types.LayerPooled + } + } + m.mu.Unlock() err := m.checkInjects(m.view.Load(), sbs) if err == nil { err = m.deliverEnv(ctx, sbs, inherited) @@ -597,10 +605,6 @@ func (m *Manager) finalizeBatch(ctx context.Context, sbs []*types.Sandbox, ttl t return err } for _, sb := range sbs { - sb.Layer = types.LayerUnpooled - if _, pooled := m.activePool(sb.PolicyKey()); pooled { - sb.Layer = types.LayerPooled - } m.addClaimLocked(sb) } js := m.store.set(sbs...) diff --git a/sandboxd/pool/env_test.go b/sandboxd/pool/env_test.go index 424d61cb6..d5b0720e4 100644 --- a/sandboxd/pool/env_test.go +++ b/sandboxd/pool/env_test.go @@ -510,6 +510,19 @@ func TestAClaimWithAnUninterceptedInjectIsRefused(t *testing.T) { } } +func TestAClaimOnAPoolWithoutEgressRefusesAnInjectOnlyItsTenantClassIntercepts(t *testing.T) { + eng := newFakeEngine() + eng.sockRoot = outboundtest.SockRoot(t) + m := egressManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1}) + editEgress(t, m, func(cfg *config.Config) { + cfg.EgressClasses = []config.EgressClass{{Name: "desk", Egress: interceptPolicy()}} + }) + _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Tenant: "acme", EgressClass: "desk", Env: injectEnv("k", "api.github.com")}) + if !errors.Is(err, ErrBadEnv) { + t.Fatalf("claim: %v, want ErrBadEnv: a pooled claim never takes its class policy, so the inject could never apply", err) + } +} + func TestEachClaimInjectsItsOwnCredentialAndAHostOnlyChangeNeedsNoGuest(t *testing.T) { eng := newFakeEngine() eng.sockRoot = outboundtest.SockRoot(t) From e5932fef8235ff8e19238c1ede1090254da4065a Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:33:20 +0800 Subject: [PATCH 14/40] outbound: re-admit only internal addresses through egress_internal_allow destVerdict reported any address an allow entry covered as a re-admitted internal one, public addresses included, and the upstream path dials those direct. With the documented guard-off allow (0.0.0.0/0, ::/0) and egress_upstream, every connection left from the node's own address instead of the claim's upstream, and its audit named no upstream. --- sandboxd/outbound/dial.go | 8 ++++---- sandboxd/outbound/dial_test.go | 9 +++++++++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/sandboxd/outbound/dial.go b/sandboxd/outbound/dial.go index 00ad507f5..883041508 100644 --- a/sandboxd/outbound/dial.go +++ b/sandboxd/outbound/dial.go @@ -96,14 +96,14 @@ func newDialer(verdict VerdictFunc) *net.Dialer { // destVerdict reports whether ip is an internal address allow re-admits; any other internal address, self included, is an error. func destVerdict(allow []egress.InternalAllow, self []netip.Addr, ip netip.Addr, port uint16) (bool, error) { ip = canonicalAddr(ip) + if !internalAddr(self, ip) { + return false, nil + } // After the NAT64 unwrap and before the block, so a named prefix wins if slices.ContainsFunc(allow, func(a egress.InternalAllow) bool { return a.Admits(ip, port) }) { return true, nil } - if internalAddr(self, ip) { - return false, fmt.Errorf("%w %s", egress.ErrInternalAddr, ip) - } - return false, nil + return false, fmt.Errorf("%w %s", egress.ErrInternalAddr, ip) } func canonicalAddr(ip netip.Addr) netip.Addr { diff --git a/sandboxd/outbound/dial_test.go b/sandboxd/outbound/dial_test.go index 711e574dd..df8abf9c1 100644 --- a/sandboxd/outbound/dial_test.go +++ b/sandboxd/outbound/dial_test.go @@ -90,6 +90,15 @@ func TestEgressDialerWildcardAllowsEverything(t *testing.T) { } } +func TestAWildcardAllowReadmitsOnlyInternalAddresses(t *testing.T) { + verdict := verdictOf([]string{"0.0.0.0/0", "::/0"}) + for addr, want := range map[string]bool{"93.184.216.34": false, "10.8.7.149": true, "2606:4700::1111": false, "fd00:c0c0:38::5": true} { + if internal, err := verdict(netip.MustParseAddr(addr), 443); err != nil || internal != want { + t.Errorf("%s internal %v err %v, want %v: a public address keeps its upstream", addr, internal, err, want) + } + } +} + func TestEgressDialerScopesInternalAllowToPorts(t *testing.T) { d := newDialer(verdictOf([]string{"10.8.0.1/32:18090,9000", "fdc8::/16:443", "10.9.0.0/16"})) for name, tt := range map[string]struct { From fa54d294d9bdc83c36c500efba9db3def3131d9a Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:33:26 +0800 Subject: [PATCH 15/40] ci: run the tun address store's PostgreSQL tests The PostgreSQL step's package list left out tunnet, so tunnet/pg's tests skipped and its SKIP check never saw them. --- .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 47463cbf9..5e4c17792 100644 --- a/.github/workflows/sandboxd.yml +++ b/.github/workflows/sandboxd.yml @@ -77,7 +77,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/... ./moves/... | tee "$RUNNER_TEMP/pg-test.log" + GOWORK=off go test -race -count=1 -v ./tenants/... ./poolset/... ./archives/... ./usage/... ./moves/... ./tunnet/pg/... | 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 c6344139a3a884d2746406f6b8d64aa4545f4d07 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:35:11 +0800 Subject: [PATCH 16/40] pool: skip a wake whose caller left while it queued on the claim A relay or exec wake can wait on sb.Transition behind a hibernate. It then restored the guest under a detached ctx, recorded a wake and left it running and billed until the idle sweep or the deadline, with nobody connected. It now returns the ctx error once it holds the lock. --- sandboxd/pool/hibernate.go | 3 +++ sandboxd/pool/hibernate_test.go | 15 +++++++++++++++ 2 files changed, 18 insertions(+) diff --git a/sandboxd/pool/hibernate.go b/sandboxd/pool/hibernate.go index b66be743d..0a9c50f95 100644 --- a/sandboxd/pool/hibernate.go +++ b/sandboxd/pool/hibernate.go @@ -158,6 +158,9 @@ func (m *Manager) wakeOrRead(ctx context.Context, sb *types.Sandbox, readerOK bo } sb.Transition.Lock() defer sb.Transition.Unlock() + if err := ctx.Err(); err != nil { + return "", err + } if sb.ArchiveCk != "" { if readerOK && sb.ReaderSafe { return "", ErrSleeping diff --git a/sandboxd/pool/hibernate_test.go b/sandboxd/pool/hibernate_test.go index 4c4df3ad6..f206204ee 100644 --- a/sandboxd/pool/hibernate_test.go +++ b/sandboxd/pool/hibernate_test.go @@ -1,6 +1,7 @@ package pool import ( + "context" "errors" "os" "path/filepath" @@ -121,6 +122,20 @@ func TestAReleaseRacingAWakeBillsTheWakeFirst(t *testing.T) { } } +func TestAWakeWhoseCallerLeftWhileQueuedLeavesTheClaimAsleep(t *testing.T) { + m := newTestManager(t, newFakeEngine()) + sb := mustClaim(t, m, testKey) + hibernateClaim(t, m, sb) + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if _, err := m.wakeResolved(ctx, sb); !errors.Is(err, context.Canceled) { + t.Fatalf("wake: %v, want context.Canceled", err) + } + if got := usageEventsOf(t, m, sb.ID); got != "claim,hibernate" { + t.Errorf("events %q, want no wake for a caller that left", got) + } +} + func TestHibernateAmbiguousErrorTreatedAsDone(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) From d0de3de0690e86e47548e7ba7356c907a6e4a2d3 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:35:37 +0800 Subject: [PATCH 17/40] pool: relock egress taps at startup when the lock listing fails One failed LockedTaps call at boot quarantined every live egress-lane claim and removed its VM, though Relock is idempotent and settles each tap on its own. A failed listing now falls back to relocking each tap. --- sandboxd/pool/reconcile.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index 3274b5318..7bebc7770 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -291,8 +291,8 @@ func (m *Manager) resyncEgress(ctx context.Context, live map[string]types.VMReco quarantine = append(quarantine, sb) continue } - err := lockedErr - if err == nil && !locked[tap] { + var err error + if lockedErr != nil || !locked[tap] { err = m.out.Relock(ctx, sb, tap) } if err != nil { From 79f369e9c3fb9015ee7f679271d619ab0963c712 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:36:17 +0800 Subject: [PATCH 18/40] pool: keep a move's reservation as long as a capture may take The target dropped a reservation 2 minutes after admission, while the source's baseline capture first waits on the dump slots it shares with hibernates and saves and then writes all guest memory, so a large guest on a slow run dir always failed its first round with 404. The idle drop now matches the 15-minute dump timeout; a retried move still replaces its reservation at once. --- docs/cluster.md | 2 +- sandboxd/pool/move.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/cluster.md b/docs/cluster.md index 9fa5da1c9..ccf9a1f39 100644 --- a/docs/cluster.md +++ b/docs/cluster.md @@ -416,7 +416,7 @@ before the receive, and a target that started draining refuses the commit. A new replaces it and resets its staged files, so a move retried after a source crash does not wait out the idle drop; one from another source answers 409. A reservation with no round arriving and none finished or -commit asked for 2 minutes is dropped with its staged files. +commit asked for 15 minutes, the longest a capture may take, is dropped with its staged files. **Same paths.** Both nodes need the same path strings in cocoon's `root_dir` and `run_dir`. The requirement is on the string, not the real directory: a symlink diff --git a/sandboxd/pool/move.go b/sandboxd/pool/move.go index 7beb1efa8..bd44e3d76 100644 --- a/sandboxd/pool/move.go +++ b/sandboxd/pool/move.go @@ -18,7 +18,7 @@ const ( defaultMoveDowntime = 2 * time.Second movesDir = "moves" - moveReservationIdle = 2 * time.Minute + moveReservationIdle = 15 * time.Minute moveRetryInterval = 2 * time.Second moveDeltaLimit = 4 moveStaleAge = time.Hour From 357de7880a809836439dd69b875b26647c0a364b Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:37:50 +0800 Subject: [PATCH 19/40] tunnet: look a frame's exit up before charging or opening its flow forward charged the claim's quota and opened and billed a flow before it looked the exit up, so frames dropped as no_exit (a PUT /tun profile not in tun.exits, or a reload that removed a label in use) were billed as egress_bytes and consumed quota. --- sandboxd/tunnet/router.go | 10 +++++----- sandboxd/tunnet/router_test.go | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/sandboxd/tunnet/router.go b/sandboxd/tunnet/router.go index 822b78953..94be6f8ea 100644 --- a/sandboxd/tunnet/router.go +++ b/sandboxd/tunnet/router.go @@ -476,13 +476,17 @@ func (l *link) read(br *bufio.Reader) error { } } -// forward carries one guest frame out: parse, size, charge and policy, then rewrite, segment and send. +// forward carries one guest frame out: parse, exit, size, charge and policy, then rewrite, segment and send. func (l *link) forward(frame []byte) Drop { p, drop := Parse(frame) if drop != DropNone { return drop } o := l.r.conf.Load() + exit, ok := o.exit(l.a.c.Profile) + if !ok { + return DropNoExit + } segs, wire, ok := wireCost(frame, l.mtu) if !ok { return DropSize @@ -497,10 +501,6 @@ func (l *link) forward(frame []byte) Drop { if drop = f.outbound(p, wire, now); drop != DropNone { return drop } - exit, ok := o.exit(l.a.c.Profile) - if !ok { - return DropNoExit - } RewriteSrc(frame, l.a.c.Addr) if icmpError(p) { rewriteQuoted(frame, dstAt, GuestAddr, l.a.c.Addr) diff --git a/sandboxd/tunnet/router_test.go b/sandboxd/tunnet/router_test.go index 357b54141..a4a62734c 100644 --- a/sandboxd/tunnet/router_test.go +++ b/sandboxd/tunnet/router_test.go @@ -149,6 +149,25 @@ func TestRouterQuotaStopsTraffic(t *testing.T) { } } +func TestRouterChargesNothingForAFrameWithNoExit(t *testing.T) { + _, r := newRouter() + used := new(atomic.Int64) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Profile: "nowhere", Used: used, Allow: func(netip.Addr, uint16) bool { return true }}) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) + waitFor(t, func() bool { return r.Stats().Drops[DropNoExit] == 1 }) + r.mu.RLock() + a := r.claims[claimAddr] + r.mu.RUnlock() + a.flows.mu.RLock() + opened := len(a.flows.table) + a.flows.mu.RUnlock() + if used.Load() != 0 || opened != 0 { + t.Errorf("used %d bytes and opened %d flows for a frame that never left", used.Load(), opened) + } +} + func TestDeliverCoalescesInOrderSegmentsIntoOneGSOFrame(t *testing.T) { u, r := newRouter() r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) From 9b01bb10f10fbecae50b119d0319ca04a99dff5e Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:39:52 +0800 Subject: [PATCH 20/40] tunnet: stamp a packet's activity with the time its flow check already read #480 touched the claim through Sandbox.Touch, which reads the clock again on every packet in both directions; TouchAt takes the time the flow check read. --- sandboxd/outbound/tun.go | 2 +- sandboxd/tunnet/activity_test.go | 3 ++- sandboxd/tunnet/router.go | 9 +++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/sandboxd/outbound/tun.go b/sandboxd/outbound/tun.go index 7c6d132ec..2b9209de4 100644 --- a/sandboxd/outbound/tun.go +++ b/sandboxd/outbound/tun.go @@ -121,7 +121,7 @@ func (h *Host) armTun(ctx context.Context, sb *types.Sandbox, d *doors, v *View, }, Limits: v.tunLimits, Used: t.used, - Touch: sb.Touch, + Touch: sb.TouchAt, Record: func(r tunnet.FlowRecord) { if h.o.Flow != nil { h.o.Flow(evCtx, id, tenant, profile, r) diff --git a/sandboxd/tunnet/activity_test.go b/sandboxd/tunnet/activity_test.go index 149131046..b88faef28 100644 --- a/sandboxd/tunnet/activity_test.go +++ b/sandboxd/tunnet/activity_test.go @@ -3,6 +3,7 @@ package tunnet import ( "net/netip" "testing" + "time" ) func TestOnlyAdmittedTrafficTouchesClaim(t *testing.T) { @@ -12,7 +13,7 @@ func TestOnlyAdmittedTrafficTouchesClaim(t *testing.T) { r.Arm(Claim{ ID: "c1", Addr: claimAddr, Allow: func(_ netip.Addr, port uint16) bool { return port == 443 }, - Touch: func() { touches++ }, + Touch: func(time.Time) { touches++ }, }) a := r.claims[claimAddr] l := &link{r: r, a: a, mtu: 1450} diff --git a/sandboxd/tunnet/router.go b/sandboxd/tunnet/router.go index 94be6f8ea..7ac357daa 100644 --- a/sandboxd/tunnet/router.go +++ b/sandboxd/tunnet/router.go @@ -108,7 +108,7 @@ type Claim struct { // Used counts the claim's bytes both ways; the caller persists it. Used *atomic.Int64 Record RecordFunc - Touch func() + Touch func(time.Time) } // Stats counts what crossed each hop and why packets were dropped. @@ -342,11 +342,12 @@ func (r *Router) admit(frame []byte) (*link, Drop) { if l == nil { return nil, DropUnknownClaim } - if drop = a.flows.inbound(p, frame, len(frame)-VnetHdrLen, time.Now()); drop != DropNone { + now := time.Now() + if drop = a.flows.inbound(p, frame, len(frame)-VnetHdrLen, now); drop != DropNone { return nil, drop } if a.c.Touch != nil { - a.c.Touch() + a.c.Touch(now) } RewriteDst(frame, GuestAddr) if icmpError(p) { @@ -517,7 +518,7 @@ func (l *link) forward(frame []byte) Drop { return DropSend } if l.a.c.Touch != nil { - l.a.c.Touch() + l.a.c.Touch(now) } l.r.dgramsOut.Add(uint64(n)) //nolint:gosec // a count return DropNone From 3592600bd43f4b626d173622fd12133bbce1a1e4 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:39:52 +0800 Subject: [PATCH 21/40] test: hand the GRO tests their segments as one receive batch The fake underlay's Recv returned whenever its channel was momentarily empty, so the router flushed the coalescer between injected segments and TestDeliverCoalescesInOrderSegmentsIntoOneGSOFrame failed intermittently under -race; the no-TSO merge test passed vacuously the same way. A batch now arrives in one Recv. --- sandboxd/tunnet/router_test.go | 35 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/sandboxd/tunnet/router_test.go b/sandboxd/tunnet/router_test.go index a4a62734c..47c5ab3c3 100644 --- a/sandboxd/tunnet/router_test.go +++ b/sandboxd/tunnet/router_test.go @@ -178,10 +178,7 @@ func TestDeliverCoalescesInOrderSegmentsIntoOneGSOFrame(t *testing.T) { u.sent(t) bad := dataSegment(5000, 1000) bad[len(bad)-1] ^= 0xff - batch := [][]byte{dataSegment(1000, 1000), dataSegment(2000, 1000), dataSegment(3000, 1000), dataSegment(4000, 400), bad} - for _, d := range batch { - u.inject(t, d) - } + u.injectBatch(t, dataSegment(1000, 1000), dataSegment(2000, 1000), dataSegment(3000, 1000), dataSegment(4000, 400), bad) merged := readFrame(t, guest) ip := merged[VnetHdrLen:] if merged[1] != gsoTCPv4 || binary.LittleEndian.Uint16(merged[4:]) != 1000 || len(ip) != 40+3400 { @@ -337,8 +334,7 @@ func TestDeliverMergesOnlyForAGuestThatOfferedTSO(t *testing.T) { guestHello(t, guest) sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) u.sent(t) - u.inject(t, dataSegment(1000, 1000)) - u.inject(t, dataSegment(2000, 1000)) + u.injectBatch(t, dataSegment(1000, 1000), dataSegment(2000, 1000)) for range 2 { if f := readFrame(t, guest); f[1] != gsoNone || len(f) != VnetHdrLen+1040 { t.Fatalf("frame gso %d len %d, want each segment alone", f[1], len(f)) @@ -372,7 +368,7 @@ func TestRouterPointsAGuestICMPErrorAtTheClaimAddress(t *testing.T) { } func newRouter() (*fakeUnder, *Router) { - u := &fakeUnder{out: make(chan sent, 64), in: make(chan sent, 16)} + u := &fakeUnder{out: make(chan sent, 64), in: make(chan []sent, 16)} r := New(u, &Overlay{Subnet: subnet, VNI: 7, Port: 4789, MTU: 1450, Exits: map[string]Exit{"default": {VTEP: vtep, MAC: exitMAC}}}) go func() { _ = r.Run(context.Background()) }() return u, r @@ -471,7 +467,7 @@ type sent struct { type fakeUnder struct { out chan sent - in chan sent + in chan []sent batches atomic.Int64 } @@ -486,16 +482,10 @@ func (u *fakeUnder) Send(b []byte, seg int, dst netip.AddrPort, _ uint64) error } func (u *fakeUnder) Recv(each func(d []byte, from netip.AddrPort)) error { - s := <-u.in - each(s.d, s.to) - for { - select { - case s = <-u.in: - each(s.d, s.to) - default: - return nil - } + for _, s := range <-u.in { + each(s.d, s.to) } + return nil } func (u *fakeUnder) inject(t *testing.T, d []byte) { @@ -505,7 +495,16 @@ func (u *fakeUnder) inject(t *testing.T, d []byte) { func (u *fakeUnder) injectFrom(t *testing.T, d []byte, from netip.AddrPort) { t.Helper() - u.in <- sent{d: d, to: from} + u.in <- []sent{{d: d, to: from}} +} + +func (u *fakeUnder) injectBatch(t *testing.T, ds ...[]byte) { + t.Helper() + batch := make([]sent, len(ds)) + for i, d := range ds { + batch[i] = sent{d: d, to: netip.AddrPortFrom(vtep, 50000)} + } + u.in <- batch } func (u *fakeUnder) sent(t *testing.T) ([]byte, uint16, netip.AddrPort) { From 1ec82da11177fd73a9b848a015fb4f9086a46e46 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:41:55 +0800 Subject: [PATCH 22/40] pool: free a dropped claim's overlay address right after its drop commits Release, reap, rollback and quarantine freed the address only after the VM teardown, which can take minutes. A crash in between left the claim gone from claims.json with no release on record, so the restart treated the address as free with no reuse window and a new claim could inherit the old flows' state at the exit. --- sandboxd/pool/claim.go | 4 ++-- sandboxd/pool/reap.go | 2 +- sandboxd/pool/reconcile.go | 2 +- sandboxd/pool/tun_test.go | 18 ++++++++++++++++++ 4 files changed, 22 insertions(+), 4 deletions(-) diff --git a/sandboxd/pool/claim.go b/sandboxd/pool/claim.go index 708026ed9..f43b11f3e 100644 --- a/sandboxd/pool/claim.go +++ b/sandboxd/pool/claim.go @@ -337,6 +337,7 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand m.recordUsage(ctx, usage.Event{Event: "release", ID: id, VMName: vmName, Tenant: sb.Tenant}) // Cleanup must survive the caller hanging up; the claim is already dropped. ctx = context.WithoutCancel(ctx) + m.releaseTun(ctx, id, tunAddr) if ck != "" { m.purgeArchiveCk(ctx, id, ck, sb.Tenant) // archived: no local VM m.untrack(m.pendingCks, ck) @@ -357,7 +358,6 @@ func (m *Manager) releaseResolved(ctx context.Context, id string, sb *types.Sand } } m.out.Disarm(id, removed) - m.releaseTun(ctx, id, tunAddr) m.dropSnap(ctx, snap) return err } @@ -649,10 +649,10 @@ func (m *Manager) rollbackClaim(ctx context.Context, sbs []*types.Sandbox) { m.mu.Unlock() m.store.recommit(ctx, rb) for i, sb := range sbs { + m.releaseTun(ctx, sb.ID, sb.TunAddr) td := m.quiesceVolumes(ctx, sb) td.holds = holds[i] m.out.Disarm(sb.ID, m.removeOrRetry(ctx, sb.VMName, sb.ID, td)) - m.releaseTun(ctx, sb.ID, sb.TunAddr) } } diff --git a/sandboxd/pool/reap.go b/sandboxd/pool/reap.go index df30eab8d..9ce89de44 100644 --- a/sandboxd/pool/reap.go +++ b/sandboxd/pool/reap.go @@ -150,10 +150,10 @@ func (m *Manager) reapOnce(ctx context.Context) { case reapPause: logSweepResult(ctx, logger, m.pauseArchive(ctx, v.sb), "archived expired sandbox "+v.id, "archive expired sandbox "+v.id) default: + m.releaseTun(ctx, v.id, v.tun) td := m.quiesceVolumes(ctx, v.sb) td.holds = v.holds m.out.Disarm(v.id, m.removeOrRetry(ctx, v.vmName, v.id, td)) - m.releaseTun(ctx, v.id, v.tun) m.dropSnap(ctx, v.snap) m.counters.reaps.Add(1) m.recordUsage(ctx, usage.Event{Event: reapEvent, ID: v.id, VMName: v.vmName, Tenant: v.tenant, Deadline: v.deadline}) diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index 7bebc7770..516e93f8d 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -330,8 +330,8 @@ func (m *Manager) quarantineClaim(ctx context.Context, sb *types.Sandbox) bool { td := m.quiesceVolumes(ctx, sb) gone := m.removeOrRetry(ctx, sb.VMName, sb.ID, td) m.forgetClaim(ctx, sb) - m.recordEnd(ctx, sb, "quarantined") m.releaseTun(ctx, sb.ID, sb.TunAddr) + m.recordEnd(ctx, sb, "quarantined") return gone } diff --git a/sandboxd/pool/tun_test.go b/sandboxd/pool/tun_test.go index 6d4c5cafb..88b4b1fab 100644 --- a/sandboxd/pool/tun_test.go +++ b/sandboxd/pool/tun_test.go @@ -329,6 +329,24 @@ func TestReconcileKeepsTheAddressAndBillOfAClaimAnotherNodeRunsOrCanAdopt(t *tes } } +func TestAReleaseFreesTheOverlayAddressBeforeTheSlowTeardown(t *testing.T) { + addrs := &fakeAddrs{} + eng := newFakeEngine() + m := tunManager(t, eng, addrs, config.PoolSpec{PoolKey: testKey, Tun: true, Egress: anyPolicy}) + sb := mustClaim(t, m, testKey) + stall := make(chan struct{}) + eng.mu.Lock() + eng.removeStall = stall + eng.mu.Unlock() + released := make(chan error, 1) + go func() { released <- m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) }() + waitFor(t, func() bool { return len(addrs.releasedList()) == 1 }) + close(stall) + if err := <-released; err != nil { + t.Fatalf("release: %v", err) + } +} + func TestATunFlowEndWritesOneAuditRecord(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true} From 62e596cb512eea0c784552e37ddc4eda4dd7b682 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:43:49 +0800 Subject: [PATCH 23/40] pool: retry an overlay address release the address store refused A release that failed while the shared meta store was down was only logged, and nothing swept the table, so each failure held one address for good. A refused release is now kept and retried on the hourly store sweep. --- sandboxd/pool/pool.go | 2 ++ sandboxd/pool/tun.go | 23 ++++++++++++++++++++++- sandboxd/pool/tun_test.go | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 24b1f5da1..6329ce8b4 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -365,6 +365,7 @@ type Manager struct { claimed map[string]*types.Sandbox claimedVMs map[string]*types.Sandbox pendingRemovals map[string]pendingRemoval + tunReleases map[string]netip.Addr // moving holds this node's outgoing moves, incoming its reservations for moves to it; neither enters claimed. moving map[string]*outMove incoming map[string]*inMove @@ -555,6 +556,7 @@ func (m *Manager) Run(ctx context.Context) { if m.moves != nil { go m.sweepStaleMoves(ctx, time.Now()) } + go m.retryTunReleases(ctx) } } } diff --git a/sandboxd/pool/tun.go b/sandboxd/pool/tun.go index 59bb4a503..e1eec84b8 100644 --- a/sandboxd/pool/tun.go +++ b/sandboxd/pool/tun.go @@ -133,7 +133,28 @@ func (m *Manager) releaseTun(ctx context.Context, id string, addr netip.Addr) { return } if err := m.tunAddrs.Release(context.WithoutCancel(ctx), id, addr); err != nil { - log.WithFunc("pool.releaseTun").Warnf(ctx, "release overlay address %s of %s: %v", addr, id, err) + log.WithFunc("pool.releaseTun").Warnf(ctx, "release overlay address %s of %s, retrying hourly: %v", addr, id, err) + m.mu.Lock() + if m.tunReleases == nil { + m.tunReleases = map[string]netip.Addr{} + } + m.tunReleases[id] = addr + m.mu.Unlock() + } +} + +func (m *Manager) retryTunReleases(ctx context.Context) { + m.mu.Lock() + pending := maps.Clone(m.tunReleases) + m.mu.Unlock() + for id, addr := range pending { + if err := m.tunAddrs.Release(ctx, id, addr); err != nil { + log.WithFunc("pool.retryTunReleases").Warnf(ctx, "release overlay address %s of %s: %v", addr, id, err) + return + } + m.mu.Lock() + delete(m.tunReleases, id) + m.mu.Unlock() } } diff --git a/sandboxd/pool/tun_test.go b/sandboxd/pool/tun_test.go index 88b4b1fab..3fdaf2b74 100644 --- a/sandboxd/pool/tun_test.go +++ b/sandboxd/pool/tun_test.go @@ -347,6 +347,28 @@ func TestAReleaseFreesTheOverlayAddressBeforeTheSlowTeardown(t *testing.T) { } } +func TestAReleaseTheAddressStoreRefusedIsRetried(t *testing.T) { + addrs := &fakeAddrs{} + m := tunManager(t, newFakeEngine(), addrs, config.PoolSpec{PoolKey: testKey, Tun: true, Egress: anyPolicy}) + sb := mustClaim(t, m, testKey) + addrs.mu.Lock() + addrs.err = errors.New("meta store down") + addrs.mu.Unlock() + if err := m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Fatalf("release: %v", err) + } + if got := addrs.releasedList(); len(got) != 0 { + t.Fatalf("released %v while the store was down", got) + } + addrs.mu.Lock() + addrs.err = nil + addrs.mu.Unlock() + m.retryTunReleases(t.Context()) + if got := addrs.releasedList(); len(got) != 1 { + t.Errorf("released %v after the store came back, want the refused release retried", got) + } +} + func TestATunFlowEndWritesOneAuditRecord(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true} @@ -482,6 +504,9 @@ func (f *fakeAddrs) Assign(_ context.Context, id string) (netip.Addr, error) { func (f *fakeAddrs) Release(_ context.Context, id string, addr netip.Addr) error { f.mu.Lock() defer f.mu.Unlock() + if f.err != nil { + return f.err + } delete(f.held, id) f.released = append(f.released, fmt.Sprintf("%s %s", id, addr)) return nil From fb4edb772e5fd5b092b5bacb01f7fbb66341d8bf Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:47:03 +0800 Subject: [PATCH 24/40] scripts: say that each e2e script's cleanup removes every sbx-* VM on the cocoon root Only archive-e2e.sh said so; run beside a serving sandboxd on the same root, the others wipe its claims. The release workflow comment no longer claims only dist-extra/ is gitignored. --- .github/workflows/release.yml | 3 +-- scripts/credentials-e2e.sh | 2 +- scripts/egress-e2e.sh | 2 +- scripts/intercept-e2e.sh | 2 +- scripts/port-e2e.sh | 2 +- scripts/sandboxd-e2e.sh | 2 +- scripts/socks-e2e.sh | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 41f719ed7..90327f68b 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,8 +42,7 @@ jobs: - name: Build silkd (musl static, arm64) # Emulated cargo build via the carrier Dockerfile — byte-consistent # with the image artifact; slow but release-rare. - # Intermediates live outside the worktree: goreleaser refuses a dirty - # git state, and only dist-extra/ is gitignored. + # Intermediates live outside the worktree: goreleaser refuses a dirty git state. run: | docker buildx build --platform linux/arm64 -o type=local,dest=/tmp/silkd-arm64 silkd mkdir -p /tmp/silkd-dist && cp /tmp/silkd-arm64/silkd-static /tmp/silkd-dist/silkd diff --git a/scripts/credentials-e2e.sh b/scripts/credentials-e2e.sh index c24465a58..8b971d383 100755 --- a/scripts/credentials-e2e.sh +++ b/scripts/credentials-e2e.sh @@ -5,7 +5,7 @@ # survives a sandboxd restart; a pool whose only rule is intercept "inject" # intercepts only the claims that hold a credential for a host and fills query # and body placeholders. Runs a dedicated sandboxd on $ADDR. -# Needs outbound to $ECHO and a $TEMPLATE whose silkd binds the egress relay. +# Needs outbound to $ECHO and a $TEMPLATE whose silkd binds the egress relay; cleanup removes every sbx-* VM on the cocoon root. set -euo pipefail TEMPLATE=${TEMPLATE:-rt:24.04} diff --git a/scripts/egress-e2e.sh b/scripts/egress-e2e.sh index c5c5dea97..c975b7337 100755 --- a/scripts/egress-e2e.sh +++ b/scripts/egress-e2e.sh @@ -3,7 +3,7 @@ # through the host proxy with a host-injected credential it never holds, direct # egress is blocked (no NIC on the none lane, an nft lock on the egress lane), # and a disallowed host is denied. NET=none|egress selects the lane. -# Runs a dedicated sandboxd on port 7779 (7777=e2e, 7778=archive). +# Runs a dedicated sandboxd on port 7779 (7777=e2e, 7778=archive); cleanup removes every sbx-* VM on the cocoon root. set -euo pipefail TEMPLATE=${TEMPLATE:-rt:24.04} diff --git a/scripts/intercept-e2e.sh b/scripts/intercept-e2e.sh index 01b6bbdd7..15af3e2d0 100755 --- a/scripts/intercept-e2e.sh +++ b/scripts/intercept-e2e.sh @@ -4,7 +4,7 @@ # through the proxy where the leaf — signed by this node's intermediate, chaining # to the cluster root — is trusted and the credential is injected into the HTTPS # request; direct egress is blocked. Runs a dedicated sandboxd on port 7779. -# Needs outbound to $ECHO and a $TEMPLATE whose silkd binds the egress relay. +# Needs outbound to $ECHO and a $TEMPLATE whose silkd binds the egress relay; cleanup removes every sbx-* VM on the cocoon root. set -euo pipefail TEMPLATE=${TEMPLATE:-rt:24.04} diff --git a/scripts/port-e2e.sh b/scripts/port-e2e.sh index 983d399d2..58cd1dc3c 100755 --- a/scripts/port-e2e.sh +++ b/scripts/port-e2e.sh @@ -2,7 +2,7 @@ # Bare-metal proof for sandboxd's guest-port relay, the path an edge proxy takes # into a sandbox with no NIC: GET /v1/sandboxes/{id}/ports/{port}. # Run as root on a node with cocoon; K names a kit holding bin/sandboxd, -# bin/portsmoke, bin/guestserver, bin/jq and a cocoon on PATH. +# bin/portsmoke, bin/guestserver, bin/jq and a cocoon on PATH; cleanup removes every sbx-* VM on the cocoon root. set -uo pipefail K=${K:?kit dir} ADDR=${ADDR:-127.0.0.1:7990} diff --git a/scripts/sandboxd-e2e.sh b/scripts/sandboxd-e2e.sh index 2a4ae02b6..00b0f5e0a 100755 --- a/scripts/sandboxd-e2e.sh +++ b/scripts/sandboxd-e2e.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash # Bare-metal e2e for sandboxd v0: warm pool, claim tiers, reap, reconcile. -# Run on a node with cocoon and a silkd-baked template image (see os-image/). +# Needs a silkd-baked template (os-image/) and a cocoon root no other sandboxd serves; cleanup removes every sbx-* VM. set -euo pipefail TEMPLATE=${TEMPLATE:-rt:24.04} diff --git a/scripts/socks-e2e.sh b/scripts/socks-e2e.sh index 907620445..e70888527 100755 --- a/scripts/socks-e2e.sh +++ b/scripts/socks-e2e.sh @@ -4,7 +4,7 @@ # unlisted port are refused there, IMAPS rides the tunnel on its listed port, # the HTTP door refuses unlisted ports on CONNECT and forward requests, and a # pool without a policy or without socks5 leaves the host door unwired. Runs a -# dedicated sandboxd on port 7780 (7779=egress). +# dedicated sandboxd on port 7780 (7779=egress); cleanup removes every sbx-* VM on the cocoon root. set -euo pipefail TEMPLATE=${TEMPLATE:-rt:24.04} From 412dc41b0e6a6b9aa3b33a6d946db69c9f8f56aa Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:49:19 +0800 Subject: [PATCH 25/40] review: silkd lsp keeps one stdio pair, pty states its real drain WHY, test helpers lose restating docs The pty comment said the pump hangs without the drop; since the exit drain got its timeout, held slave dups only make it run to that timeout. --- silkd/src/lsp.rs | 14 ++++++-------- silkd/src/pty.rs | 2 +- silkd/tests/tun_e2e.rs | 2 -- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/silkd/src/lsp.rs b/silkd/src/lsp.rs index 3f305d841..73579d7d2 100644 --- a/silkd/src/lsp.rs +++ b/silkd/src/lsp.rs @@ -63,15 +63,14 @@ impl Broker { Ok(c) => c, Err(e) => return proto::err_frame(w, &e, "spawn language server").await, }; - let Some((stdin, stdout)) = child.stdin.take().zip(child.stdout.take()) else { + let Some(stdio) = child.stdout.take().zip(child.stdin.take()) else { let _ = child.start_kill(); return proto::error_frame(w, ErrorKind::Internal, "language server stdio not piped") .await; }; let server = Server { child, - stdin: Some(stdin), - stdout: Some(stdout), + stdio: Some(stdio), }; let id = format!("lsp-{}", NEXT_ID.fetch_add(1, Ordering::Relaxed)); @@ -106,7 +105,7 @@ impl Broker { ) -> std::io::Result<()> { let taken = sysutil::lock(&self.inner) .get_mut(server_id) - .map(|s| s.stdout.take().zip(s.stdin.take())); + .map(|s| s.stdio.take()); let (stdout, stdin) = match taken { None => return proto::error_frame(w, ErrorKind::NotFound, "no such lsp server").await, Some(None) => { @@ -158,7 +157,7 @@ impl Broker { let removed = { let mut table = sysutil::lock(&self.inner); match table.get(server_id) { - Some(server) if server.stdout.is_some() => table.remove(server_id), + Some(server) if server.stdio.is_some() => table.remove(server_id), _ => None, } }; @@ -168,11 +167,10 @@ impl Broker { } } -/// One running language server; a present `stdout` means no connection has attached. +/// One running language server; a present `stdio` means no connection has attached. struct Server { child: Child, - stdin: Option, - stdout: Option, + stdio: Option<(ChildStdout, ChildStdin)>, } async fn kill(mut server: Server) { diff --git a/silkd/src/pty.rs b/silkd/src/pty.rs index ab8b226e6..feca62efc 100644 --- a/silkd/src/pty.rs +++ b/silkd/src/pty.rs @@ -68,7 +68,7 @@ pub async fn open( Ok(c) => c, Err(e) => return proto::err_frame(out, &e, "spawn shell").await, }; - // drop cmd so its slave dups close, else the master never sees the shell's exit and the pump hangs. + // close silkd's slave dups, else the master never hangs up and the exit drain runs to its timeout. drop(cmd); let pid = child.id().unwrap_or(0); diff --git a/silkd/tests/tun_e2e.rs b/silkd/tests/tun_e2e.rs index 0502e6a8b..b1296a216 100644 --- a/silkd/tests/tun_e2e.rs +++ b/silkd/tests/tun_e2e.rs @@ -553,7 +553,6 @@ async fn serve_flood(l: TcpListener) { } } -/// Copies frames one way, rewriting the IPv4 address at header offset `at` from `from` to `to`. fn forward( r: UnixStream, mut w: UnixStream, @@ -610,7 +609,6 @@ fn frame_buffered(buf: &[u8]) -> bool { tun::next_frame(buf).ok().flatten().is_some() } -/// Swaps an IPv4 address at header offset `at`, patching the IP checksum and the L4 checksum (partial or full). fn rewrite(frame: &mut [u8], at: usize, from: [u8; 4], to: [u8; 4]) { let partial = frame[0] & NEEDS_CSUM != 0; let csum_at = usize::from(u16::from_le_bytes([frame[6], frame[7]])) From 98beeb5ca01becc1e467fe0a95ae59bdf3af6d5c Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:56:22 +0800 Subject: [PATCH 26/40] review: SDK godocs to one line, frame.go layout, Python forwarder and private helper Multi-line godocs across sdk/go and protocol/wire fold to one line each (a stale Promote parenthetical goes); frame.go's unexported types move below the exported ones; Python drops the _post_json forwarder and makes require_field private. --- e2e/cmd/rpcbench/main.go | 1 - mcp/server.go | 1 - protocol/wire/frame.go | 43 ++++++++++------------ sdk/go/checkpoint.go | 22 ++--------- sdk/go/drain_test.go | 10 ++--- sdk/go/find.go | 10 ++--- sdk/go/git.go | 10 ++--- sdk/go/keepalive.go | 5 +-- sdk/go/lsp.go | 13 ++----- sdk/go/options.go | 12 ++---- sdk/go/pools.go | 8 +--- sdk/go/port.go | 27 +++----------- sdk/go/proc.go | 20 +++------- sdk/go/sandbox.go | 36 ++++-------------- sdk/go/template.go | 13 ++----- sdk/go/watch.go | 8 +--- sdk/openai/cocoonsandbox_openai/adapter.py | 2 +- sdk/python/cocoonsandbox/checkpoint.py | 4 +- sdk/python/cocoonsandbox/client.py | 17 +++------ sdk/python/cocoonsandbox/errors.py | 3 +- sdk/python/cocoonsandbox/sandbox.py | 23 +++++------- sdk/python/cocoonsandbox/template.py | 2 +- sdk/python/e2e.py | 1 - sdk/python/pyproject.toml | 1 - sdk/python/tests/test_upgrade.py | 21 +++-------- 25 files changed, 93 insertions(+), 220 deletions(-) diff --git a/e2e/cmd/rpcbench/main.go b/e2e/cmd/rpcbench/main.go index ccd8e6671..ab5b2f3cd 100644 --- a/e2e/cmd/rpcbench/main.go +++ b/e2e/cmd/rpcbench/main.go @@ -154,7 +154,6 @@ func statRPC(conn net.Conn) error { return nil } -// 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://") { return nil, nil diff --git a/mcp/server.go b/mcp/server.go index 7c2264d43..ef90058bd 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -198,7 +198,6 @@ 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 { diff --git a/protocol/wire/frame.go b/protocol/wire/frame.go index 052b4744a..53fb3e561 100644 --- a/protocol/wire/frame.go +++ b/protocol/wire/frame.go @@ -137,24 +137,6 @@ type Request interface{ Op() string } // Response is a server→client frame; RespType is its wire tag. type Response interface{ RespType() string } -type respDecoder func([]byte) (Response, error) - -type bulkResponse struct { - head []byte - mk func([]byte) Response -} - -// reqPtr and respPtr turn a mistyped decoder table entry into a compile error. -type reqPtr[T any] interface { - *T - Request -} - -type respPtr[T any] interface { - *T - Response -} - // Exec starts a process; with Session set it runs inside that persistent shell instead. type Exec struct { Argv []string `json:"argv"` @@ -345,17 +327,14 @@ type PtyResize struct { func (PtyResize) Op() string { return "pty_resize" } -// PortForward relays a guest TCP port over this connection: Ready once -// connected, then Data both ways (DataEnd half-closes the guest socket); -// the guest server closing ends the stream with Done. +// PortForward relays a guest TCP port: Ready, then Data both ways (DataEnd half-closes the guest socket), Done when the guest server closes. type PortForward struct { Port uint16 `json:"port"` } func (PortForward) Op() string { return "port_forward" } -// GitClone clones a repo; network-lane only. Auth is a token passed as an -// in-memory Authorization header, never written to guest disk. +// GitClone clones a repo; network-lane only; Auth rides an in-memory Authorization header, never guest disk. type GitClone struct { URL string `json:"url"` Path string `json:"path"` @@ -649,6 +628,24 @@ type GitBranches struct { func (GitBranches) RespType() string { return "git_branches" } +type respDecoder func([]byte) (Response, error) + +type bulkResponse struct { + head []byte + mk func([]byte) Response +} + +// reqPtr and respPtr turn a mistyped decoder table entry into a compile error. +type reqPtr[T any] interface { + *T + Request +} + +type respPtr[T any] interface { + *T + Response +} + // EncodeRequest renders {"v":1,"op":...,fields} without a trailing newline. func EncodeRequest(r Request) ([]byte, error) { return encodeTagged(requestHead+r.Op()+`"`, r) diff --git a/sdk/go/checkpoint.go b/sdk/go/checkpoint.go index 877f942f3..e49669a70 100644 --- a/sdk/go/checkpoint.go +++ b/sdk/go/checkpoint.go @@ -37,9 +37,6 @@ type checkpointListResponse struct { } // Checkpoint is a captured sandbox state bound to the node that holds it. -// New claims fresh sandboxes branched from the captured moment, any number -// of times; the source sandbox is unaffected and can keep being -// checkpointed, so successive captures form a tree. type Checkpoint struct { ID string Name string @@ -50,11 +47,7 @@ type Checkpoint struct { addr string } -// New claims a sandbox cloned from the checkpoint on the checkpoint's node. -// The snapshot pins the key axes; WithTimeout, WithMetadata, -// WithArchiveOnExpire and WithEnv apply. A node that no longer holds the -// checkpoint redirects to a probed owner, which New follows; if the last -// candidate fails transiently, New falls back to the origin once to heal there. +// New claims a sandbox branched from the checkpoint, following an owner redirect. func (ck *Checkpoint) New(ctx context.Context, opts ...Option) (*Sandbox, error) { var claim claimRequest for _, opt := range opts { @@ -77,12 +70,7 @@ func (ck *Checkpoint) New(ctx context.Context, opts ...Option) (*Sandbox, error) return ck.c.handleFrom(addr, cr), nil } -// Delete removes the checkpoint from its node and asks every peer that node -// currently sees to drop any replica a heal pulled — best-effort eventual -// cleanup, not a fleet-wide revocation. A peer that misses that broadcast -// (offline, partitioned, or joined later) keeps serving branches from its -// replica until the node's checkpoint_ttl_hours ages it out; a node later run -// with healing off and that TTL back at 0 keeps the replica until a delete. +// Delete removes the checkpoint from its node; peer replicas are dropped best-effort. func (ck *Checkpoint) Delete(ctx context.Context) error { return ck.c.doNoContent(ctx, http.MethodDelete, ck.addr, "/v1/checkpoints/"+ck.ID, nil, ck.c.apiToken, "delete checkpoint") } @@ -104,8 +92,7 @@ func (s *Sandbox) Checkpoint(ctx context.Context, name string) (*Checkpoint, err return checkpointHandle(s.c, s.owner, cr.Checkpoint), nil } -// Checkpoints lists the CONNECTED node's checkpoints, newest first; the -// returned handles are bound to that node. +// Checkpoints lists the connected node's checkpoints, newest first, as handles bound to it. func (c *Client) Checkpoints(ctx context.Context) ([]*Checkpoint, error) { lr, err := c.doJSON[checkpointListResponse](ctx, http.MethodGet, c.addr, "/v1/checkpoints", nil, c.apiToken, "list checkpoints") if err != nil { @@ -118,8 +105,7 @@ func (c *Client) Checkpoints(ctx context.Context) ([]*Checkpoint, error) { return ckpts, nil } -// Checkpoint returns a handle for a known checkpoint id, bound to the entry -// node — no listing round-trip; an unknown id surfaces as 404 at claim time. +// Checkpoint returns an entry-node handle for a known id; an unknown id surfaces as 404 at claim. func (c *Client) Checkpoint(id string) *Checkpoint { return checkpointHandle(c, c.addr, checkpointRecord{ID: id}) } diff --git a/sdk/go/drain_test.go b/sdk/go/drain_test.go index 1b0959c3f..1515088ac 100644 --- a/sdk/go/drain_test.go +++ b/sdk/go/drain_test.go @@ -4,6 +4,7 @@ import ( "encoding/json/v2" "net/http" "net/http/httptest" + "slices" "testing" ) @@ -39,12 +40,7 @@ func TestDrainCordonVerbs(t *testing.T) { } } want := []string{"POST /v1/drain", "DELETE /v1/drain", "POST /v1/cordon", "DELETE /v1/cordon"} - if len(calls) != len(want) { - t.Fatalf("calls = %v, want %v", calls, want) - } - for i := range want { - if calls[i] != want[i] { - t.Errorf("call %d = %s, want %s", i, calls[i], want[i]) - } + if !slices.Equal(calls, want) { + t.Errorf("calls = %v, want %v", calls, want) } } diff --git a/sdk/go/find.go b/sdk/go/find.go index 7517d43f6..a14362695 100644 --- a/sdk/go/find.go +++ b/sdk/go/find.go @@ -7,21 +7,17 @@ import ( "github.com/cocoonstack/sandbox/protocol/wire" ) -// Find returns the lines under path matching pattern (a regular expression); -// glob, when non-empty, narrows the walk to file names matching it (`*` and -// `?` wildcards). +// Find returns the lines under path matching the regular expression pattern; a non-empty glob narrows the file names. func (s *Sandbox) Find(ctx context.Context, path, pattern, glob string) ([]wire.Match, error) { return s.collectRPC[wire.Match](ctx, &wire.FsFind{Path: path, Pattern: pattern, Glob: glob}) } -// FindSeq streams the matches Find would collect; breaking out of the loop -// ends the walk in the guest, so the caller bounds the result. +// FindSeq streams the matches Find would collect; breaking out of the loop ends the guest walk. func (s *Sandbox) FindSeq(ctx context.Context, path, pattern, glob string) iter.Seq2[wire.Match, error] { return s.streamRPC[wire.Match](ctx, &wire.FsFind{Path: path, Pattern: pattern, Glob: glob}) } -// Replace rewrites pattern (a regular expression) to replacement in each of -// files, returning one result per file with its replacement count. +// Replace rewrites the regular expression pattern in each of files, returning one result per file. func (s *Sandbox) Replace(ctx context.Context, files []string, pattern, replacement string) ([]wire.Replaced, error) { return s.collectRPC[wire.Replaced](ctx, &wire.FsReplace{Files: files, Pattern: pattern, Replacement: replacement}) } diff --git a/sdk/go/git.go b/sdk/go/git.go index 92992d01d..167357fdd 100644 --- a/sdk/go/git.go +++ b/sdk/go/git.go @@ -6,9 +6,7 @@ import ( "github.com/cocoonstack/sandbox/protocol/wire" ) -// GitClone clones url into path in the sandbox; depth > 0 makes a shallow -// clone. Auth, when non-empty, is a token sent as an in-memory Authorization -// header. Needs the egress lane. +// GitClone clones url into path, shallow when depth > 0; auth is an in-memory header token; needs the egress lane. func (s *Sandbox) GitClone(ctx context.Context, url, path, branch string, depth uint32, auth string) error { return s.doneRPC(ctx, &wire.GitClone{URL: url, Path: path, Branch: branch, Depth: depth, Auth: auth}) } @@ -18,8 +16,7 @@ func (s *Sandbox) GitStatus(ctx context.Context, path string) (*wire.GitStatusRe return s.oneShotRPC[wire.GitStatusResult](ctx, &wire.GitStatus{Path: path}) } -// GitCommit stages nothing (call GitAdd first); it commits the index with -// message and author ("Name ") and returns the new commit hash. +// GitCommit commits the index with author "Name " and returns the new hash; it stages nothing. func (s *Sandbox) GitCommit(ctx context.Context, path, message, author string) (string, error) { c, err := s.oneShotRPC[wire.GitCommitResult](ctx, &wire.GitCommit{Path: path, Message: message, Author: author}) if err != nil { @@ -33,8 +30,7 @@ func (s *Sandbox) GitAdd(ctx context.Context, path string, files ...string) erro return s.doneRPC(ctx, &wire.GitAdd{Path: path, Files: files}) } -// GitPush pushes the current branch. Auth as in GitClone. Needs the egress -// lane; on the no-network lane it fails with a typed error pointing at Push. +// GitPush pushes the current branch; on the no-network lane it fails with a typed error pointing at Push. func (s *Sandbox) GitPush(ctx context.Context, path, auth string) error { return s.doneRPC(ctx, &wire.GitPush{Path: path, Auth: auth}) } diff --git a/sdk/go/keepalive.go b/sdk/go/keepalive.go index 85a81cdde..8b73be550 100644 --- a/sdk/go/keepalive.go +++ b/sdk/go/keepalive.go @@ -148,10 +148,7 @@ func (l *lease) close() { _ = 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. +// WithKeepAlive bounds how long a handle keeps an idle relay connection (default 30s; 0 dials per call); an open one holds the idle-hibernate clock. func WithKeepAlive(idle time.Duration) ClientOption { return func(c *Client) { c.keepAlive = idle } } diff --git a/sdk/go/lsp.go b/sdk/go/lsp.go index 35f44642b..a4e3d29f5 100644 --- a/sdk/go/lsp.go +++ b/sdk/go/lsp.go @@ -7,19 +7,14 @@ import ( "github.com/cocoonstack/sandbox/protocol/wire" ) -// Lsp is a language server running in the sandbox, spoken to over the relay. -// silkd is a broker: it spawns the flavor image's server and pipes JSON-RPC -// bytes; the caller frames and correlates (agents already speak LSP). +// Lsp is a language server in the sandbox; silkd pipes JSON-RPC bytes and the caller frames them. type Lsp struct { ServerID string s *Sandbox } -// Request opens the JSON-RPC byte stream to the language server: the returned -// PortConn writes go to the server's stdin, reads come from its stdout. A -// server serves one Request for its lifetime — closing the stream ends the -// session and reaps the server (start a new one to work again). +// Request opens the server's one JSON-RPC byte stream; closing it reaps the server. func (l *Lsp) Request(ctx context.Context) (*PortConn, error) { return l.s.openStream(ctx, &wire.LspRequest{ServerID: l.ServerID}) } @@ -29,9 +24,7 @@ func (l *Lsp) Stop(ctx context.Context) error { return l.s.doneRPC(ctx, &wire.LspStop{ServerID: l.ServerID}) } -// StartLsp spawns the language server the flavor image provides for language -// (rooted at root), returning a handle. On the base image, which ships no -// language servers, this fails with silkd's typed not_found. +// StartLsp spawns the flavor image's language server rooted at root; an image without one fails with not_found. func (s *Sandbox) StartLsp(ctx context.Context, language, root string) (*Lsp, error) { started, err := s.oneShotRPC[wire.LspStarted](ctx, &wire.LspStart{Language: language, Root: root}) if err != nil { diff --git a/sdk/go/options.go b/sdk/go/options.go index 89d2b8d6d..e30475499 100644 --- a/sdk/go/options.go +++ b/sdk/go/options.go @@ -22,8 +22,7 @@ const ( // NetShape selects the sandbox network lane. type NetShape string -// Size is a T-shirt resource tier; free-form CPU/memory would fragment the -// node's warm pools. +// Size is a T-shirt resource tier; free-form CPU/memory would fragment the node's warm pools. type Size string // Volume requests one catalog entry at an optional guest mount path and mode. @@ -75,23 +74,20 @@ func WithSize(s Size) Option { return func(r *claimRequest) { r.Size = string(s) } } -// WithVolumes requests catalog volumes for a claim; each entry defaults to -// read-only, set Volume.Mode to "rw" for a writable mount. +// WithVolumes requests catalog volumes for a claim; each entry defaults to read-only. func WithVolumes(volumes ...Volume) Option { volumes = slices.Clone(volumes) return func(r *claimRequest) { r.Volumes = volumes } } -// WithTimeout bounds the sandbox's lifetime: the owning node reaps it after -// d (rounded up to seconds) even if the client vanishes. +// WithTimeout makes the owning node reap the sandbox after d, rounded up to seconds. func WithTimeout(d time.Duration) Option { return func(r *claimRequest) { r.TTLSeconds = ttlSeconds(d) } } -// WithClaimRef records an opaque caller reference on the claim (the aggregated -// apiserver passes its k8s "/"), echoed by Client.Sandboxes. +// WithClaimRef records an opaque caller reference on the claim, echoed by Client.Sandboxes. func WithClaimRef(ref string) Option { return func(r *claimRequest) { r.ClaimRef = ref } } diff --git a/sdk/go/pools.go b/sdk/go/pools.go index 97099aeee..396a1e9c9 100644 --- a/sdk/go/pools.go +++ b/sdk/go/pools.go @@ -22,16 +22,12 @@ type poolUpdate struct { Pools []PoolSpec `json:"pools"` } -// SetPools replaces the entry node's desired warm pools (PUT /v1/pools): omitted -// pools drain and lose their golden once idle. Requires the operator token. +// SetPools replaces the entry node's desired warm pools with the operator token; omitted pools drain. func (c *Client) SetPools(ctx context.Context, pools []PoolSpec) (*NodeInfo, error) { return c.setPoolsAt(ctx, c.addr, pools) } -// SetPoolsCluster applies pools to the entry node and every peer, returning a -// 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). +// SetPoolsCluster applies pools to every node and returns per-node results; an error means only the entry node was reached. func (c *Client) SetPoolsCluster(ctx context.Context, pools []PoolSpec) ([]NodeResult, error) { return c.eachNode(ctx, func(addr string) (*NodeInfo, error) { return c.setPoolsAt(ctx, addr, pools) }) } diff --git a/sdk/go/port.go b/sdk/go/port.go index 885835abc..62980bb46 100644 --- a/sdk/go/port.go +++ b/sdk/go/port.go @@ -26,10 +26,7 @@ type previewResponse struct { var _ net.Conn = (*PortConn)(nil) -// PortConn is a net.Conn to a TCP port inside the sandbox, relayed over the -// silkd protocol (works on the no-network lane — the vsock relay is its only -// transport). Read returns io.EOF when the guest server closes; CloseWrite -// half-closes the guest socket. +// PortConn is a net.Conn to a guest TCP port over the silkd relay; Read returns io.EOF when the guest server closes. type PortConn struct { conn *silkd.Conn stop func() @@ -40,14 +37,12 @@ type PortConn struct { func (p *PortConn) Read(b []byte) (int, error) { return p.out.Read(b) } -// Write chunks b so no frame (with its base64 and envelope overhead) can -// exceed the protocol cap — one oversized frame would kill the relay. +// Write chunks b because one frame over the protocol cap kills the relay. func (p *PortConn) Write(b []byte) (int, error) { return sendChunks(p.conn, wire.PortWriteChunk, func(chunk []byte) wire.Request { return &wire.Data{Data: chunk} }, b) } -// CloseWrite half-closes the guest socket (the server sees EOF) while reads -// keep draining its response. +// CloseWrite half-closes the guest socket while reads keep draining its response. func (p *PortConn) CloseWrite() error { return p.conn.Send(wire.DataEnd{}) } @@ -61,7 +56,6 @@ func (p *PortConn) Close() error { return nil } -// LocalAddr implements net.Conn; the relay has no meaningful socket address. func (p *PortConn) LocalAddr() net.Addr { return portAddr("sandbox-relay") } func (p *PortConn) RemoteAddr() net.Addr { return portAddr("sandbox-guest") } @@ -69,10 +63,8 @@ func (p *PortConn) RemoteAddr() net.Addr { return portAddr("sandbox-guest") } // SetDeadline implements net.Conn; deadlines are unsupported, so bound the DialPort ctx instead. func (p *PortConn) SetDeadline(time.Time) error { return errors.ErrUnsupported } -// SetReadDeadline implements net.Conn; unsupported, see SetDeadline. func (p *PortConn) SetReadDeadline(time.Time) error { return errors.ErrUnsupported } -// SetWriteDeadline implements net.Conn; unsupported, see SetDeadline. func (p *PortConn) SetWriteDeadline(time.Time) error { return errors.ErrUnsupported } // drain relays guest bytes into the pipe; a relay dropped before its terminal frame ends in drainData's truncation error, not EOF. @@ -83,16 +75,12 @@ func (p *PortConn) drain(ctx context.Context, pw *io.PipeWriter) { })) } -// DialPort connects to 127.0.0.1:port inside the sandbox. The ctx governs -// the connection's lifetime: canceling it (or Close) tears the relay down. -// A port nobody listens on fails here with silkd's not_found error. +// DialPort connects to 127.0.0.1:port inside the sandbox for the lifetime of ctx; a closed port fails with not_found. func (s *Sandbox) DialPort(ctx context.Context, port uint16) (*PortConn, error) { return s.openStream(ctx, &wire.PortForward{Port: port}) } -// PreviewURL mints a shareable URL that serves the sandbox's guest HTTP -// port from a plain browser, valid for ttl (the node clamps it to the -// claim's lease). Requires the node to have preview configured. +// PreviewURL mints a browser URL for a guest HTTP port, valid for ttl clamped to the lease. func (s *Sandbox) PreviewURL(ctx context.Context, port uint16, ttl time.Duration) (string, error) { body, err := encodeBody("preview", previewRequest{Token: s.token, Port: port, TTLSeconds: ttlSeconds(ttl)}) if err != nil { @@ -105,10 +93,7 @@ func (s *Sandbox) PreviewURL(ctx context.Context, port uint16, ttl time.Duration return pr.URL, nil } -// ProxyPort listens on localAddr (e.g. "127.0.0.1:0") and pipes every -// accepted connection to the sandbox port, so unmodified local tools reach -// the guest server. Closing the listener stops new connections; canceling -// ctx tears down the live ones. +// ProxyPort pipes every connection accepted on localAddr to the sandbox port; canceling ctx tears down the live ones. func (s *Sandbox) ProxyPort(ctx context.Context, localAddr string, port uint16) (net.Listener, error) { l, err := net.Listen("tcp", localAddr) if err != nil { diff --git a/sdk/go/proc.go b/sdk/go/proc.go index 7bbca2d6a..8f4c6963b 100644 --- a/sdk/go/proc.go +++ b/sdk/go/proc.go @@ -8,10 +8,7 @@ import ( "github.com/cocoonstack/sandbox/protocol/wire" ) -// Spawn starts cmd detached: it returns the guest pid as soon as the -// process starts, and the process keeps running with a bounded output ring -// readable later via Logs or Attach. cmd's Stdin/Stdout/Stderr are ignored; -// cmd.Session must be empty — the session exec path cannot detach. +// Spawn starts cmd detached and returns its guest pid; stdio is ignored and Session must be empty. func (s *Sandbox) Spawn(ctx context.Context, cmd Cmd) (uint32, error) { if len(cmd.Argv) == 0 { return 0, fmt.Errorf("empty argv") @@ -27,8 +24,7 @@ func (s *Sandbox) Spawn(ctx context.Context, cmd Cmd) (uint32, error) { return started.PID, nil } -// Ps lists the guest's tracked processes — execs, spawns, and ptys — with -// state and exit codes. +// Ps lists the guest's tracked processes with state and exit codes. func (s *Sandbox) Ps(ctx context.Context) ([]wire.ProcInfo, error) { procs, err := s.oneShotRPC[wire.Procs](ctx, wire.Ps{}) if err != nil { @@ -37,9 +33,7 @@ func (s *Sandbox) Ps(ctx context.Context) ([]wire.ProcInfo, error) { return procs.Procs, nil } -// Kill signals a tracked process; sig 0 sends SIGKILL. Killing one that -// already exited is a no-op success — its OS pid may be recycled, so silkd -// never signals a reaped child. +// Kill signals the process group a tracked pid leads; sig 0 sends SIGKILL, and an exited process is a no-op success. func (s *Sandbox) Kill(ctx context.Context, pid uint32, sig int32) error { req := &wire.Kill{PID: pid} if sig != 0 { @@ -48,16 +42,12 @@ func (s *Sandbox) Kill(ctx context.Context, pid uint32, sig int32) error { return s.doneRPC(ctx, req) } -// Logs replays a tracked process's ring-buffered output into the writers -// (nil discards). exited reports whether the process has ended; code is its -// exit code when it has. +// Logs replays a tracked process's ring-buffered output; code is valid only when exited. func (s *Sandbox) Logs(ctx context.Context, pid uint32, stdout, stderr io.Writer) (code int32, exited bool, err error) { return s.drainProc(ctx, &wire.Logs{PID: pid}, stdout, stderr) } -// Attach replays the buffered output, then follows live output until the -// process exits, returning its exit code. exited is false only when the -// proc table dropped the process mid-attach (reap race). +// Attach replays the buffered output, then follows live output until the process exits. func (s *Sandbox) Attach(ctx context.Context, pid uint32, stdout, stderr io.Writer) (code int32, exited bool, err error) { return s.drainProc(ctx, &wire.Attach{PID: pid}, stdout, stderr) } diff --git a/sdk/go/sandbox.go b/sdk/go/sandbox.go index 706c8d583..f4e2c8afd 100644 --- a/sdk/go/sandbox.go +++ b/sdk/go/sandbox.go @@ -28,11 +28,7 @@ type Cmd struct { User string Session string // when set, runs inside that persistent shell session - // Stdin is consumed until EOF or until the command exits. A blocking - // reader (e.g. os.Stdin) whose command exits first keeps its pump - // goroutine parked in Read until the next bytes arrive — do not share - // one reader across Runs, the stale pump would swallow them. nil closes - // the child's stdin immediately. + // Stdin is read until EOF or exit; a reader shared across Runs loses bytes to a stale pump, and nil closes the child's stdin. Stdin io.Reader Stdout io.Writer // nil discards Stderr io.Writer // nil discards @@ -60,16 +56,13 @@ type Sandbox struct { Volumes []Volume // VolumesWant is the persisted desired set, which may differ after a failed convergence. VolumesWant []Volume - // TemplateDigest is the content identity of the promoted-template export - // this sandbox was cloned from; empty for any other source. + // TemplateDigest is the content identity of the promoted-template export this sandbox was cloned from; empty otherwise. TemplateDigest string - // FromCheckpoint names the checkpoint this sandbox branched from; empty - // for pool and template claims. + // FromCheckpoint names the checkpoint this sandbox branched from; empty otherwise. FromCheckpoint string - // NetRoute is how the guest reaches the network at claim: "relay" through - // the host proxy, "direct" over its own NIC, or "none". + // NetRoute is the guest's network path at claim, as the node reports it. NetRoute string c *Client @@ -149,12 +142,7 @@ func (s *Sandbox) Run(ctx context.Context, cmd Cmd) (int, error) { return int(code), nil } -// Fork clones the sandbox into count children — memory, disk, and guest -// state (sessions, processes, tmpfs) duplicate at the fork point, and each -// child gets a fresh machine identity and its own lease. ttl bounds every -// child's lifetime; zero means the server default. All-or-nothing: on error -// no child survived. Forking a hibernated sandbox reuses its memory image -// without waking it. +// Fork clones the sandbox into count children with their own leases; on error no child survives. func (s *Sandbox) Fork(ctx context.Context, count int, ttl time.Duration) ([]*Sandbox, error) { body, err := encodeBody("fork", forkRequest{Token: s.token, Count: count, TTLSeconds: ttlSeconds(ttl)}) if err != nil { @@ -171,14 +159,7 @@ func (s *Sandbox) Fork(ctx context.Context, count int, ttl time.Duration) ([]*Sa return children, nil } -// Promote publishes the sandbox's current state as a template on its owning -// node: later claims for it (with this sandbox's network lane and size) -// clone from it, provision-on-demand. Re-promoting to the same name replaces -// the template. Like Fork, a hibernated sandbox is promoted from its memory -// image without waking. Templates are node-local — the returned handle is -// bound to the owning node. Delete and New without volumes reach that node; -// New with volumes may follow one placement redirect (name-based Client calls -// only see the connected node's templates). +// Promote publishes the sandbox's current state as a template; the returned handle is bound to the owning node. func (s *Sandbox) Promote(ctx context.Context, template string) (*Template, error) { body, err := encodeBody("promote", promoteRequest{Token: s.token, Template: template}) if err != nil { @@ -194,10 +175,7 @@ func (s *Sandbox) Promote(ctx context.Context, template string) (*Template, erro }, nil } -// Hibernate atomically snapshots the sandbox and stops its VM, freeing its -// memory; the next call that reaches the guest wakes it transparently with -// sessions, processes, and memory state intact. The TTL keeps running: at its -// deadline a hibernated sandbox is destroyed, or archived under WithArchiveOnExpire. +// Hibernate snapshots the sandbox and stops its VM; the next guest call wakes it, and the TTL keeps running. func (s *Sandbox) Hibernate(ctx context.Context) error { s.pool.drain() return s.c.doNoContent(ctx, http.MethodPost, s.owner, "/v1/sandboxes/"+s.ID+"/hibernate", nil, s.token, "hibernate") diff --git a/sdk/go/template.go b/sdk/go/template.go index c7f45c7bd..30fb7aafb 100644 --- a/sdk/go/template.go +++ b/sdk/go/template.go @@ -5,9 +5,7 @@ import ( "net/url" ) -// Template is a promoted template bound to the node that holds it. Delete and -// New without volumes dial the owner directly, so they are usable the instant -// Promote returns; a volume claim may follow one placement redirect. +// Template is a promoted template bound to the node that holds it. type Template struct { Name string ContentDigest string @@ -18,9 +16,7 @@ type Template struct { size string } -// New claims a sandbox cloned from the template. Options may set the TTL or -// request volumes; the key axes (network lane, size) are the template's own -// and cannot be overridden. +// New claims a sandbox cloned from the template; its key axes cannot be overridden. func (t *Template) New(ctx context.Context, opts ...Option) (*Sandbox, error) { claim := claimRequest{Template: t.Name} for _, opt := range opts { @@ -43,10 +39,7 @@ func (t *Template) New(ctx context.Context, opts ...Option) (*Sandbox, error) { return t.c.handleFrom(addr, cr), nil } -// Delete removes the template from its node. The handle is owner-bound, so -// the delete carries no_redirect: the owner answers for itself (404 once the -// template is gone there) and gossip about same-name templates elsewhere is -// never consulted. +// Delete removes the template from its owning node, never a same-name template elsewhere. func (t *Template) Delete(ctx context.Context) error { u := url.Values{ templateQueryParam: {t.Name}, diff --git a/sdk/go/watch.go b/sdk/go/watch.go index 3c5fac601..cc732977a 100644 --- a/sdk/go/watch.go +++ b/sdk/go/watch.go @@ -8,9 +8,7 @@ import ( "github.com/cocoonstack/sandbox/sdk/go/silkd" ) -// Watcher is an open filesystem watch. Events yields events until the watch -// ends — by Close, ctx cancellation, or a watcher error on the sandbox side — -// after which Err reports the terminal error (nil after a clean Close). +// Watcher is an open filesystem watch; Err reports why it ended. type Watcher struct { events chan wire.Event stop func() @@ -77,9 +75,7 @@ func (w *Watcher) setErr(err error) { w.err = err } -// Watch streams filesystem events under path (recursively when set). It -// returns once the sandbox acknowledges the watch is armed, so events caused -// after Watch returns are guaranteed captured. +// Watch streams filesystem events under path; events caused after it returns are guaranteed captured. func (s *Sandbox) Watch(ctx context.Context, path string, recursive bool) (*Watcher, error) { conn, l, err := s.call(ctx, &wire.FsWatch{Path: path, Recursive: recursive}) if err != nil { diff --git a/sdk/openai/cocoonsandbox_openai/adapter.py b/sdk/openai/cocoonsandbox_openai/adapter.py index 817e96186..0a2587e98 100644 --- a/sdk/openai/cocoonsandbox_openai/adapter.py +++ b/sdk/openai/cocoonsandbox_openai/adapter.py @@ -169,7 +169,7 @@ async def delete(self, session: SandboxSession) -> SandboxSession: if not isinstance(inner, CocoonSandboxSession): raise TypeError("CocoonSandboxClient.delete expects a CocoonSandboxSession") await inner._shutdown_backend() - await asyncio.to_thread(inner._sandbox().close) # 404-safe in the SDK + await asyncio.to_thread(inner._sandbox().close) return session async def resume(self, state: SandboxSessionState) -> SandboxSession: diff --git a/sdk/python/cocoonsandbox/checkpoint.py b/sdk/python/cocoonsandbox/checkpoint.py index f76dba987..2d443c3cb 100644 --- a/sdk/python/cocoonsandbox/checkpoint.py +++ b/sdk/python/cocoonsandbox/checkpoint.py @@ -4,7 +4,7 @@ from typing import TYPE_CHECKING, Any -from .errors import require_field +from .errors import _require_field if TYPE_CHECKING: from .client import Client @@ -17,7 +17,7 @@ class Checkpoint: def __init__(self, client: Client, addr: str, rec: dict[str, Any]) -> None: self._client = client self._addr = addr - self.id = require_field(rec, "id", "checkpoint") + self.id = _require_field(rec, "id", "checkpoint") self.name = rec.get("name", "") self.sandbox_id = rec.get("sandbox_id", "") self.created_at = rec.get("created_at", "") diff --git a/sdk/python/cocoonsandbox/client.py b/sdk/python/cocoonsandbox/client.py index f3f97ba05..24532184e 100644 --- a/sdk/python/cocoonsandbox/client.py +++ b/sdk/python/cocoonsandbox/client.py @@ -15,7 +15,7 @@ from .checkpoint import Checkpoint from .conn import Conn, dial_agent, is_quiet, remaining_timeout from .endpoint import _endpoint_url -from .errors import APIError, SandboxTimeout, _error_message, require_field +from .errors import APIError, SandboxTimeout, _error_message, _require_field from .sandbox import Sandbox from .template import _claim_body, _template_query from .volumes import Volume @@ -38,7 +38,7 @@ def __init__( ssl_context: ssl.SSLContext | None = None, keep_alive: float = 30.0, ) -> None: - """keep_alive parks idle control and relay connections, which hold the sandbox's idle clock; 0 dials per call. + """keep_alive parks idle connections, 0 dials per call; a parked relay one holds the sandbox's idle clock. ssl_context is set to offer ALPN http/1.1, which the relay upgrade needs; pass a dedicated context. """ @@ -154,7 +154,7 @@ def _claim_from( *, deadline: float | None = None, ) -> Sandbox: - reply = self._post_json(addr, path, claim, verb, deadline=deadline) + reply = self._request(addr, "POST", path, claim, verb, deadline=deadline) redirect = reply.get("redirect") or [] if not redirect: return self._handle_from(addr, reply, verb) @@ -163,7 +163,7 @@ def _claim_from( claim["require_promoted"] = True def post(peer: str) -> dict[str, Any]: - return self._post_json(peer, path, claim, verb, deadline=deadline) + return self._request(peer, "POST", path, claim, verb, deadline=deadline) owner, reply = _redirect_fallback(addr, redirect, post, verb) return self._handle_from(owner, reply, verb) @@ -182,8 +182,8 @@ def _peers(self) -> list[str]: def _handle_from(self, dialed: str, reply: dict[str, Any], verb: str = "claim") -> Sandbox: return Sandbox( client=self, - id=require_field(reply, "id", verb), - token=require_field(reply, "token", verb), + id=_require_field(reply, "id", verb), + token=_require_field(reply, "token", verb), owner=reply.get("owner_addr") or dialed, deadline=reply.get("deadline", ""), from_checkpoint=reply.get("from_checkpoint", ""), @@ -191,11 +191,6 @@ def _handle_from(self, dialed: str, reply: dict[str, Any], verb: str = "claim") volumes=reply.get("volumes") or [], ) - def _post_json( - self, addr: str, path: str, body: dict[str, Any], verb: str, *, deadline: float | None = None - ) -> dict[str, Any]: - return self._request(addr, "POST", path, body, verb, deadline=deadline) - def _request( self, addr: str, diff --git a/sdk/python/cocoonsandbox/errors.py b/sdk/python/cocoonsandbox/errors.py index 52b26e073..e649b3f7b 100644 --- a/sdk/python/cocoonsandbox/errors.py +++ b/sdk/python/cocoonsandbox/errors.py @@ -59,8 +59,7 @@ class ProtocolError(SandboxError): """The peer broke the frame protocol (unexpected frame, oversized line).""" -def require_field(reply: dict[str, Any], key: str, verb: str) -> Any: - """The named member of a control-plane reply, or an APIError naming the verb that got a truncated one.""" +def _require_field(reply: dict[str, Any], key: str, verb: str) -> Any: if key not in reply: raise APIError(verb, 0, f"reply without {key}") return reply[key] diff --git a/sdk/python/cocoonsandbox/sandbox.py b/sdk/python/cocoonsandbox/sandbox.py index f1a0a142e..cc34f6145 100644 --- a/sdk/python/cocoonsandbox/sandbox.py +++ b/sdk/python/cocoonsandbox/sandbox.py @@ -20,7 +20,7 @@ SandboxTimeout, SilkdError, VolumeError, - require_field, + _require_field, ) from .frames import BULK_CHUNK, STREAM_CHUNK from .template import Template @@ -329,7 +329,7 @@ def fork(self, count: int, ttl_seconds: int = 0) -> list[Sandbox]: body = {"token": self.token, "count": count} if ttl_seconds: body["ttl_seconds"] = ttl_seconds - reply = self._client._post_json(self.owner, f"/v1/sandboxes/{self.id}/fork", body, "fork") + reply = self._client._request(self.owner, "POST", f"/v1/sandboxes/{self.id}/fork", body, "fork") return [self._client._handle_from(self.owner, child, "fork") for child in reply.get("children") or []] def hibernate(self) -> None: @@ -345,7 +345,7 @@ def renew(self, ttl_seconds: int = 0) -> str: reply = self._client._request( self.owner, "POST", f"/v1/sandboxes/{self.id}/renew", body, "renew", bearer=self.token ) - self.deadline = cast(str, require_field(reply, "deadline", "renew")) + self.deadline = cast(str, _require_field(reply, "deadline", "renew")) return self.deadline def checkpoint(self, name: str = "") -> Checkpoint: @@ -353,15 +353,14 @@ def checkpoint(self, name: str = "") -> Checkpoint: body = {"token": self.token} if name: body["name"] = name - reply = self._client._post_json(self.owner, f"/v1/sandboxes/{self.id}/checkpoint", body, "checkpoint") - return Checkpoint(self._client, self.owner, require_field(reply, "checkpoint", "checkpoint")) + reply = self._client._request(self.owner, "POST", f"/v1/sandboxes/{self.id}/checkpoint", body, "checkpoint") + return Checkpoint(self._client, self.owner, _require_field(reply, "checkpoint", "checkpoint")) def promote(self, template: str) -> Template: """Publishes a claimable template and returns an owner-bound handle.""" - reply = self._client._post_json( - self.owner, f"/v1/sandboxes/{self.id}/promote", {"token": self.token, "template": template}, "promote" - ) - key = require_field(reply, "key", "promote") + body = {"token": self.token, "template": template} + reply = self._client._request(self.owner, "POST", f"/v1/sandboxes/{self.id}/promote", body, "promote") + key = _require_field(reply, "key", "promote") return Template( self._client, self.owner, @@ -399,8 +398,8 @@ def preview_url(self, port: int, ttl_seconds: int = 0) -> str: body = {"token": self.token, "port": port} if ttl_seconds: body["ttl_seconds"] = ttl_seconds - reply = self._client._post_json(self.owner, f"/v1/sandboxes/{self.id}/preview", body, "preview") - return cast(str, require_field(reply, "url", "preview")) + reply = self._client._request(self.owner, "POST", f"/v1/sandboxes/{self.id}/preview", body, "preview") + return cast(str, _require_field(reply, "url", "preview")) def dial_port(self, port: int) -> PortConn: """Opens a byte stream to 127.0.0.1:port inside the guest.""" @@ -467,7 +466,6 @@ 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.""" conn = self._pool.take() if conn is None: conn = self._dial(deadline) @@ -484,7 +482,6 @@ def _connect(self, op: str, deadline: float | None = None, /, **fields: object) @contextlib.contextmanager def _lease(self, conn: Conn) -> Iterator[Conn]: - """Runs one RPC on conn; a terminal frame parks it and anything else drops it.""" try: yield conn except SilkdError: diff --git a/sdk/python/cocoonsandbox/template.py b/sdk/python/cocoonsandbox/template.py index 16f4f2134..983cff3ee 100644 --- a/sdk/python/cocoonsandbox/template.py +++ b/sdk/python/cocoonsandbox/template.py @@ -36,7 +36,7 @@ def new( if volumes: return self._client._claim_from(self._addr, claim, deadline=deadline) claim["no_redirect"] = True - reply = self._client._post_json(self._addr, "/v1/claim", claim, "claim", deadline=deadline) + reply = self._client._request(self._addr, "POST", "/v1/claim", claim, "claim", deadline=deadline) return self._client._handle_from(self._addr, reply) def delete(self) -> None: diff --git a/sdk/python/e2e.py b/sdk/python/e2e.py index 8f18b8b07..53ee44301 100644 --- a/sdk/python/e2e.py +++ b/sdk/python/e2e.py @@ -173,7 +173,6 @@ def _port(): raise print(f" {name:<18} ok ({int((time.time() - start) * 1000)}ms)") finally: - # never mask a step failure with a close error. with contextlib.suppress(Exception): sb.close() print("PY-E2E PASS") diff --git a/sdk/python/pyproject.toml b/sdk/python/pyproject.toml index 6ca902216..10e279959 100644 --- a/sdk/python/pyproject.toml +++ b/sdk/python/pyproject.toml @@ -29,4 +29,3 @@ target-version = "py39" [tool.ruff.lint] select = ["E", "F", "W", "I", "UP", "B", "SIM"] -ignore = ["UP045"] # keep Optional-style unions out; X | None needs 3.10 at runtime without future imports diff --git a/sdk/python/tests/test_upgrade.py b/sdk/python/tests/test_upgrade.py index da95eed64..4ac25de7f 100644 --- a/sdk/python/tests/test_upgrade.py +++ b/sdk/python/tests/test_upgrade.py @@ -16,6 +16,9 @@ "fs_mkdir": {"type": "done"}, } REJECT = b"HTTP/1.1 404 Not Found\r\nContent-Type: text/plain\r\nContent-Length: 15\r\n\r\nunknown sandbox" +REJECT_JSON = ( + b'HTTP/1.1 404 Not Found\r\nContent-Type: application/json\r\nContent-Length: 27\r\n\r\n{"error":"unknown sandbox"}' +) class EarlyFrameAgent: @@ -59,8 +62,9 @@ def test_the_first_frame_rides_ahead_of_the_101(keep_alive): assert agent.early == ["fs_mkdir"], agent.early -def test_rejected_upgrade_is_an_api_error(): - agent = EarlyFrameAgent(REJECT) +@pytest.mark.parametrize("reply", [REJECT, REJECT_JSON], ids=["text body", "json error body"]) +def test_rejected_upgrade_is_an_api_error(reply): + agent = EarlyFrameAgent(reply) sb = sandbox_at(agent.addr, timeout=5, keep_alive=0) try: with pytest.raises(APIError) as refused: @@ -71,19 +75,6 @@ def test_rejected_upgrade_is_an_api_error(): assert agent.early == ["fs_mkdir"], agent.early -def test_rejected_upgrade_reports_the_servers_error_text(): - body = b'{"error":"unknown sandbox"}' - head = b"HTTP/1.1 404 Not Found\r\nContent-Type: application/json\r\nContent-Length: %d\r\n\r\n" % len(body) - agent = EarlyFrameAgent(head + body) - sb = sandbox_at(agent.addr, timeout=5, keep_alive=0) - try: - with pytest.raises(APIError) as refused: - sb.mkdir("/w") - finally: - agent.stop() - assert (refused.value.status, refused.value.message) == (404, "unknown sandbox"), refused.value - - def test_run_timeout_cuts_an_unanswered_upgrade(black_hole): sb = sandbox_at(black_hole, timeout=30, keep_alive=0) started = time.monotonic() From 4282424caacdf87851c80bf755996e924869d52c Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:56:22 +0800 Subject: [PATCH 27/40] sdk/python: drop the trailing ': ' from ExitError when stderr is empty, as the Go SDK does --- sdk/python/cocoonsandbox/errors.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sdk/python/cocoonsandbox/errors.py b/sdk/python/cocoonsandbox/errors.py index e649b3f7b..ef50fdfd3 100644 --- a/sdk/python/cocoonsandbox/errors.py +++ b/sdk/python/cocoonsandbox/errors.py @@ -49,7 +49,8 @@ class ExitError(SandboxError): """A command exited non-zero; carries the code, stderr, and the stdout produced before it failed.""" def __init__(self, code: int, stderr: str, stdout: str = "") -> None: - super().__init__(f"exit status {code}: {stderr.strip()}") + detail = stderr.strip() + super().__init__(f"exit status {code}: {detail}" if detail else f"exit status {code}") self.code = code self.stderr = stderr self.stdout = stdout From 080eafd4d50f1de06a521ba8a3efed039ec1f478 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:58:20 +0800 Subject: [PATCH 28/40] review: pool comments, a tunPlan for commitTun, parseCheckpoint reuse, vocabulary types ahead of volumeCatalog Stale and restating comments go (the liveness one is contradicted: the tap survives a VMM exit); a failed golden snapshot rm that blocks the next build logs at Warn; reconcile reuses parseCheckpoint; planTun returns one tunPlan so commitTun's signature fits a line; volumeHolders and catalogVolume move ahead of volumeCatalog. --- sandboxd/pool/capture.go | 5 ++-- sandboxd/pool/golden.go | 2 +- sandboxd/pool/hibernate.go | 2 +- sandboxd/pool/liveness.go | 1 - sandboxd/pool/pool.go | 3 +-- sandboxd/pool/reconcile.go | 5 ++-- sandboxd/pool/tun.go | 46 ++++++++++++++++++--------------- sandboxd/pool/volume.go | 9 +------ sandboxd/pool/volume_catalog.go | 28 ++++++++++++-------- 9 files changed, 50 insertions(+), 51 deletions(-) diff --git a/sandboxd/pool/capture.go b/sandboxd/pool/capture.go index fd635aa3b..ad3c4a096 100644 --- a/sandboxd/pool/capture.go +++ b/sandboxd/pool/capture.go @@ -13,7 +13,7 @@ import ( // sourceSnap picks the snapshot to export a claimed sandbox from, with its cleanup; the caller holds sb.Transition. func (m *Manager) sourceSnap(ctx context.Context, sb *types.Sandbox) (string, func(), error) { - // archive() clears VMName under the same lock, so this is the check that cannot race it + // The archive clears VMName under the same lock, so this check cannot race it if sb.ArchiveCk != "" { return "", nil, ErrArchived } @@ -57,7 +57,6 @@ func (m *Manager) captureAllowed(sb *types.Sandbox) error { return nil } -// exportSource captures a claimed sandbox into exportDir and returns its snapshot, VM name and guest-env presence. func (m *Manager) exportSource(ctx context.Context, sb *types.Sandbox, exportDir, from string) (snap, vmName string, guestEnv bool, err error) { if lockErr := m.lockTransition(sb); lockErr != nil { return "", "", false, lockErr @@ -91,7 +90,7 @@ func (m *Manager) exportSource(ctx context.Context, sb *types.Sandbox, exportDir return snap, vmName, guestEnv, m.eng.SnapshotExport(ctx, snap, exportDir) } -// trimForCapture trims a live sandbox's copy-on-write disk when its pool asks for it; a failed trim only leaves the capture larger. +// A failed trim only leaves the capture larger. func (m *Manager) trimForCapture(ctx context.Context, sb *types.Sandbox) { if !m.view.Load().poolTrims[sb.PolicyKey()] || sb.HibernateSnap != "" || sb.ArchiveCk != "" { return diff --git a/sandboxd/pool/golden.go b/sandboxd/pool/golden.go index a74ca64c9..ff4bd34e0 100644 --- a/sandboxd/pool/golden.go +++ b/sandboxd/pool/golden.go @@ -35,7 +35,7 @@ func (m *Manager) buildGolden(ctx context.Context, p *pool) { err := m.buildGoldenSteps(ctx, v, p.key, name, snap, final, imageID) if rmErr := m.eng.SnapshotRemove(ctx, snap); rmErr != nil && err == nil { - logger.Debugf(ctx, "drop golden snapshot %s: %v", snap, rmErr) + logger.Warnf(ctx, "drop golden snapshot %s: %v", snap, rmErr) } m.destroy(ctx, name) diff --git a/sandboxd/pool/hibernate.go b/sandboxd/pool/hibernate.go index 0a9c50f95..72a7a668d 100644 --- a/sandboxd/pool/hibernate.go +++ b/sandboxd/pool/hibernate.go @@ -62,7 +62,7 @@ func (m *Manager) agentSocket(ctx context.Context, id, token string, readerOK bo // Caller holds sb.Transition. func (m *Manager) hibernateLocked(ctx context.Context, sb *types.Sandbox) error { if sb.ArchiveCk != "" { - return nil // already off the node + return nil } if _, err := m.fenceWatch(sb); err != nil { return err diff --git a/sandboxd/pool/liveness.go b/sandboxd/pool/liveness.go index 886fcaac7..0efde3fc0 100644 --- a/sandboxd/pool/liveness.go +++ b/sandboxd/pool/liveness.go @@ -343,7 +343,6 @@ func coldBootBlocker(sb *types.Sandbox, found, volumes bool) string { case !found: return "vm record gone" case sb.Key.Net == types.NetEgress: - // The netdev nft chain dies with the tap, so the guest would boot with an unlocked NIC return "vmm exited; an egress-lane sandbox cannot cold-boot" case volumes: return "vmm exited; volume mounts do not survive a cold boot" diff --git a/sandboxd/pool/pool.go b/sandboxd/pool/pool.go index 6329ce8b4..8f6c9fe33 100644 --- a/sandboxd/pool/pool.go +++ b/sandboxd/pool/pool.go @@ -353,8 +353,7 @@ type Manager struct { // notifyTemplates is set before serving starts and fires after a promote or template delete. notifyTemplates func() - out *outbound.Host - // tunRouter and tunAddrs are nil when the node has no tun block. + out *outbound.Host tunRouter *tunnet.Router tunAddrs tunnet.Addresses diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index 516e93f8d..81fb79ad4 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -2,7 +2,6 @@ package pool import ( "context" - "encoding/json/v2" "fmt" "os" "path/filepath" @@ -382,8 +381,8 @@ func (m *Manager) reclaimOrphanArchiveCks(ctx context.Context, claims map[string var orphans []types.Checkpoint var ids []string for _, rec := range metas { - var ckpt types.Checkpoint - if json.Unmarshal(rec.Meta, &ckpt) != nil || !ckpt.Archive { + ckpt, err := parseCheckpoint(rec.Meta) + if err != nil || !ckpt.Archive { continue } orig, mine := claims[ckpt.SandboxID] diff --git a/sandboxd/pool/tun.go b/sandboxd/pool/tun.go index e1eec84b8..6906690af 100644 --- a/sandboxd/pool/tun.go +++ b/sandboxd/pool/tun.go @@ -32,6 +32,11 @@ type tunState struct { addr netip.Addr } +type tunPlan struct { + prev, next tunState + assign bool +} + // SetTun updates a claim's tun state without waking a hibernated or archived claim. func (m *Manager) SetTun(ctx context.Context, id string, enabled *bool, profile *string) error { sb, ok := m.byID(id) @@ -60,27 +65,27 @@ func (m *Manager) TunStats() (tunnet.Stats, bool) { func (m *Manager) setTun(ctx context.Context, sb *types.Sandbox, locked bool, enabled *bool, profile *string) error { id := sb.ID - prev, next, assign, err := m.planTun(sb, locked, enabled, profile) + plan, err := m.planTun(sb, locked, enabled, profile) if err != nil { return err } - if assign { - if next.addr, err = m.tunAddrs.Assign(ctx, id); err != nil { + if plan.assign { + if plan.next.addr, err = m.tunAddrs.Assign(ctx, id); err != nil { return fmt.Errorf("set tun %s: %w", id, err) } } - if next == prev { + if plan.next == plan.prev { if m.tunRowed(sb) { return m.rewriteArchiveRow(ctx, sb, locked) } return nil } - sock, rowed, err := m.commitTun(ctx, sb, locked, prev, next, assign) + sock, rowed, err := m.commitTun(ctx, sb, locked, plan) if err != nil { return err } switch { - case next.off: + case plan.next.off: m.out.DisarmTun(ctx, id, sock) case sock != "": m.out.RearmTun(ctx, sb) @@ -198,46 +203,45 @@ func (m *Manager) tunRowed(sb *types.Sandbox) bool { return m.rowFollows(sb) || sb.ArchivePending != "" } -func (m *Manager) planTun(sb *types.Sandbox, locked bool, enabled *bool, profile *string) (prev, next tunState, assign bool, err error) { +func (m *Manager) planTun(sb *types.Sandbox, locked bool, enabled *bool, profile *string) (tunPlan, error) { m.mu.Lock() defer m.mu.Unlock() switch { case m.claimed[sb.ID] != sb: - return prev, next, false, ErrUnknownSandbox + return tunPlan{}, ErrUnknownSandbox case m.pausedForMoveLocked(sb): - return prev, next, false, ErrEvacuating + return tunPlan{}, ErrEvacuating } - prev = tunState{off: sb.TunOff, profile: sb.TunProfile, addr: sb.TunAddr} - next = prev + prev := tunState{off: sb.TunOff, profile: sb.TunProfile, addr: sb.TunAddr} + next := prev if enabled != nil { next.off = !*enabled } if profile != nil { next.profile = *profile } - // only a disable runs beside a transition, which reads the rest of the tun state + // Only a disable runs beside a transition, which reads the rest of the tun state if !locked && (!next.off || next.profile != prev.profile) { - return prev, next, false, ErrPaused + return tunPlan{}, ErrPaused } - assign = !next.off && !prev.addr.IsValid() && m.tunAddrs != nil && m.view.Load().out.Tun(sb.PolicyKey()) - return prev, next, assign, nil + assign := !next.off && !prev.addr.IsValid() && m.tunAddrs != nil && m.view.Load().out.Tun(sb.PolicyKey()) + return tunPlan{prev: prev, next: next, assign: assign}, nil } // commitTun persists next on sb and returns the running guest's socket, empty when it is not running or a capture holds it. -func (m *Manager) commitTun( - ctx context.Context, sb *types.Sandbox, locked bool, prev, next tunState, assign bool, -) (sock string, rowed bool, err error) { +func (m *Manager) commitTun(ctx context.Context, sb *types.Sandbox, locked bool, plan tunPlan) (sock string, rowed bool, err error) { + prev, next := plan.prev, plan.next m.mu.Lock() if m.claimed[sb.ID] != sb { m.mu.Unlock() - if assign { + if plan.assign { m.releaseTun(ctx, sb.ID, next.addr) } return "", false, ErrUnknownSandbox } if (tunState{off: sb.TunOff, profile: sb.TunProfile, addr: sb.TunAddr}) != prev { m.mu.Unlock() - if assign { + if plan.assign { m.releaseTun(ctx, sb.ID, next.addr) } return "", false, errTunRaced @@ -261,7 +265,7 @@ func (m *Manager) commitTun( } m.mu.Unlock() m.store.recommit(ctx, rb) - if undone && assign { + if undone && plan.assign { m.releaseTun(ctx, sb.ID, next.addr) } return "", false, fmt.Errorf("set tun %s: persist claims: %w", sb.ID, err) diff --git a/sandboxd/pool/volume.go b/sandboxd/pool/volume.go index e0073b4f3..7819f789a 100644 --- a/sandboxd/pool/volume.go +++ b/sandboxd/pool/volume.go @@ -20,12 +20,6 @@ import ( // Sidecar, not data_dir: the marker travels with the image and survives a data_dir wipe const volumeDirtySuffix = ".dirty" -// volumeHolders is one name's live admission state: a writer excludes every other claim. -type volumeHolders struct { - writers int - readers int -} - type resolvedVolume struct { entry *catalogVolume disk engine.VolumeSpec @@ -186,7 +180,6 @@ func (m *Manager) applyVolumes(ctx context.Context, sb *types.Sandbox, volumes [ // applyVolume keeps one volume's steps strictly ordered; siblings overlap freely. func (m *Manager) applyVolume(ctx context.Context, sb *types.Sandbox, volume resolvedVolume, live bool) error { - // Attach-only mounts nothing, so there is no umount to verify and no marker. attachOnly := volume.applied.AttachOnly // Write-ahead: the marker must be durable before any guest write can be. if volume.disk.RW && !attachOnly { @@ -226,7 +219,7 @@ func (m *Manager) quiesceVolumes(ctx context.Context, sb *types.Sandbox) volumeT stuck := false for _, volume := range slices.Backward(sb.Volumes) { if !needsUnmount(volume) { - continue // attach-only: no mount to bring down, VM removal closes the device + continue } unmountCtx, cancel := context.WithTimeout(ctx, engine.VolumeMutationTimeout) clean, err := m.eng.UnmountVolume(unmountCtx, sb.VsockSocket, volume.Mount) diff --git a/sandboxd/pool/volume_catalog.go b/sandboxd/pool/volume_catalog.go index a01d59b90..a5248eac1 100644 --- a/sandboxd/pool/volume_catalog.go +++ b/sandboxd/pool/volume_catalog.go @@ -21,6 +21,23 @@ import ( const runtimeVolumesFile = "runtime-volumes.json" +// volumeHolders is one name's live admission state: a writer excludes every other claim. +type volumeHolders struct { + writers int + readers int +} + +type catalogVolume struct { + runtime bool + disk engine.VolumeSpec + tenants []string + writable bool +} + +func (v *catalogVolume) allowed(tenant string) bool { + return tenant == "" || len(v.tenants) == 0 || slices.Contains(v.tenants, tenant) +} + type volumeCatalog struct { entries map[string]*catalogVolume roots []string @@ -104,17 +121,6 @@ func (c *volumeCatalog) checkResolvedLocked(volumes []resolvedVolume) error { return nil } -type catalogVolume struct { - runtime bool - disk engine.VolumeSpec - tenants []string - writable bool -} - -func (v *catalogVolume) allowed(tenant string) bool { - return tenant == "" || len(v.tenants) == 0 || slices.Contains(v.tenants, tenant) -} - // RegisterVolume durably binds a root-managed name to a backing path. func (m *Manager) RegisterVolume(ctx context.Context, name string, spec types.VolumeRegistration) error { volume, err := m.catalog.normalize(name, spec) From 8622a8285b6985900d36e52024aafd841e89665f Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 12:58:20 +0800 Subject: [PATCH 29/40] test: pool setup through mustManager and restartReconciled, wg.Go waits, mustClaim reuse --- sandboxd/pool/archive_test.go | 6 +- sandboxd/pool/checkpoint_cleanup_test.go | 5 +- sandboxd/pool/disk_test.go | 6 +- sandboxd/pool/drain_test.go | 10 ++- sandboxd/pool/failover_test.go | 88 +++++++----------------- sandboxd/pool/fork_test.go | 5 +- sandboxd/pool/liveness_test.go | 10 +-- sandboxd/pool/memory_test.go | 5 +- sandboxd/pool/pool_test.go | 30 ++++---- sandboxd/pool/reader_test.go | 6 +- sandboxd/pool/replica_test.go | 35 ++-------- sandboxd/pool/telemetry_test.go | 15 +--- sandboxd/pool/tenants_test.go | 15 +--- sandboxd/pool/tun_test.go | 9 +-- sandboxd/pool/volume_attach_test.go | 67 +++++++----------- sandboxd/pool/volume_capture_test.go | 5 +- sandboxd/pool/volume_catalog_test.go | 5 +- sandboxd/pool/volume_mutation_test.go | 5 +- sandboxd/pool/volume_test.go | 11 +-- 19 files changed, 96 insertions(+), 242 deletions(-) diff --git a/sandboxd/pool/archive_test.go b/sandboxd/pool/archive_test.go index 5ad1cef23..d964708d0 100644 --- a/sandboxd/pool/archive_test.go +++ b/sandboxd/pool/archive_test.go @@ -137,11 +137,7 @@ func TestArchiveCkHiddenAcrossNodes(t *testing.T) { DataDir: t.TempDir(), CheckpointDir: shared, Pools: []config.PoolSpec{archivePool(3600)}, } - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("manager: %v", err) - } - return m + return mustManager(t, cfg, newFakeEngine()) } mA, mB := mkMgr(), mkMgr() sb := mustClaim(t, mA, testKey) diff --git a/sandboxd/pool/checkpoint_cleanup_test.go b/sandboxd/pool/checkpoint_cleanup_test.go index 20327c23f..de2c3e9b7 100644 --- a/sandboxd/pool/checkpoint_cleanup_test.go +++ b/sandboxd/pool/checkpoint_cleanup_test.go @@ -109,10 +109,7 @@ func TestCheckpointCleanupWaitsForTheClaimWrite(t *testing.T) { } restoreWrites() if restart { - m = c.restart(t, m) - if err := m.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + m = c.restartReconciled(t, m) var ok bool if sb, ok = m.byID(sb.ID); !ok { t.Fatal("restart dropped the claim") diff --git a/sandboxd/pool/disk_test.go b/sandboxd/pool/disk_test.go index 8d64b1723..c2d462210 100644 --- a/sandboxd/pool/disk_test.go +++ b/sandboxd/pool/disk_test.go @@ -89,9 +89,5 @@ func TestAFailedStatfsKeepsTheDiskLowState(t *testing.T) { func diskReserveManager(t *testing.T, free string) *Manager { t.Helper() cfg := &config.Config{DataDir: t.TempDir(), DiskReserve: &config.DiskReserve{Free: free}, Pools: []config.PoolSpec{{PoolKey: testKey}}} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } - return m + return mustManager(t, cfg, newFakeEngine()) } diff --git a/sandboxd/pool/drain_test.go b/sandboxd/pool/drain_test.go index c91060a28..c26286cfd 100644 --- a/sandboxd/pool/drain_test.go +++ b/sandboxd/pool/drain_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "slices" + "sync" "testing" "testing/synctest" "time" @@ -263,16 +264,13 @@ func TestAnUndrainDuringAnEvacuationsDrainStopsIt(t *testing.T) { return len(infos) == 1 && infos[0].Warm == 1 }) release := holdMaint(a) - done := make(chan struct{}) - go func() { - defer close(done) - a.Evacuate(t.Context()) - }() + var wg sync.WaitGroup + wg.Go(func() { a.Evacuate(t.Context()) }) synctest.Wait() a.Undrain(t.Context()) release() - <-done + wg.Wait() synctest.Wait() assertEvacuation(t, a, 0, 0) diff --git a/sandboxd/pool/failover_test.go b/sandboxd/pool/failover_test.go index d58f572f0..2284c2957 100644 --- a/sandboxd/pool/failover_test.go +++ b/sandboxd/pool/failover_test.go @@ -504,10 +504,7 @@ func TestFailoverRestartDropsAClaimWhoseInterruptedArchiveWasAdopted(t *testing. t.Fatalf("put: %v", err) } - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); held { t.Error("the restarted node kept a claim whose archive another node adopted") } @@ -701,18 +698,12 @@ func TestFailoverSweepRunsOnTheHourlyStoreTicker(t *testing.T) { synctest.Test(t, func(t *testing.T) { c := newFailoverCluster(t) cfg := &config.Config{DataDir: t.TempDir(), CheckpointDir: c.ckDir} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) counted := &countingRegistry{Registry: c.reg} m.archives, m.nodeID = counted, "b" ctx, cancel := context.WithCancel(t.Context()) - done := make(chan struct{}) - go func() { - defer close(done) - m.Run(ctx) - }() + var wg sync.WaitGroup + wg.Go(func() { m.Run(ctx) }) time.Sleep(time.Hour - time.Minute) synctest.Wait() if n := counted.sweepable.Load(); n != 0 { @@ -724,7 +715,7 @@ func TestFailoverSweepRunsOnTheHourlyStoreTicker(t *testing.T) { t.Errorf("%d sweeps after the hourly tick, want 1", n) } cancel() - <-done + wg.Wait() }) } @@ -811,10 +802,7 @@ func TestFailoverRestartAfterAConsumeWithoutCommitKeepsTheClaim(t *testing.T) { t.Fatalf("flush: %v", err) } - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) held, _ := restarted.byID(sb.ID) if rec, ok, _ := c.reg.Get(t.Context(), sb.ID); !ok || rec.Owner != "a" || rec.Consumed || held == nil || held.ArchiveEpoch != rec.Epoch { t.Fatalf("row %+v present %v, held %v, want the tombstone re-armed at the claim's epoch", rec, ok, held != nil) @@ -840,10 +828,7 @@ func TestFailoverAnArchiveWokenElsewhereIsDropped(t *testing.T) { } c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); held { t.Error("the restart kept a claim whose checkpoint an adopter consumed") } @@ -892,10 +877,7 @@ func TestFailoverRestartClearsTheMarkerOfAnAdoptedClaim(t *testing.T) { mustAdopt(t, b, sb, "adopt") c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if !ckExists(t, b, ck) || archiveCkMarked(restarted, ck) { t.Error("the restart deleted the adopter's checkpoint or kept its marker") } @@ -965,10 +947,7 @@ func TestFailoverRestartInsideAnAdoptersWakeDropsTheClaim(t *testing.T) { } c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); held { t.Error("the restarted node kept a claim the adopter woke") } @@ -998,10 +977,7 @@ func TestFailoverRestartAfterAnAdopterReleasedDropsTheClaim(t *testing.T) { } c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); held { t.Error("the restarted node kept a claim the adopter released") } @@ -1119,10 +1095,7 @@ func TestFailoverRestartLeavesItsOwnTombstoneToTheSweep(t *testing.T) { } before, _, _ := c.reg.Get(t.Context(), sb.ID) - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); !held { t.Error("the restart dropped a claim whose wake committed") } @@ -1404,10 +1377,7 @@ func TestFailoverRestartAfterAConsumeWhileAPeerSweptKeepsTheClaim(t *testing.T) b.sweepOrphanArchives(t.Context()) c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); !held { t.Fatal("the restart dropped a claim whose only copy a peer swept") } @@ -1598,10 +1568,7 @@ func TestFailoverRestartDropsAClaimWhoseInterruptedArchiveWasConsumedElsewhere(t } c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); held { t.Error("the restarted node kept a claim another node adopted and woke") } @@ -1617,10 +1584,7 @@ func TestFailoverRestartKeepsAClaimWhoseArchivePutNeverCommitted(t *testing.T) { hibernateClaim(t, a, sb) ck := interruptArchive(t, a, sb, false) - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) held, ok := restarted.byID(sb.ID) if !ok || held.ArchivePending != "" { t.Fatalf("claim held %v, want it kept with its intent cleared", ok) @@ -1661,10 +1625,7 @@ func TestFailoverRestartAfterAnAbandonClearedItsIntentReclaimsTheExport(t *testi t.Fatal("setup: clear intent") } - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); !held || ckExists(t, restarted, ck) { t.Error("the restart dropped the claim or kept the export an abandon left") } @@ -1705,10 +1666,7 @@ func TestFailoverAnOwnerReturningWithItsWakeCommittedPurgesItsTombstone(t *testi b.sweepOrphanArchives(t.Context()) c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) restarted.sweepOrphanArchives(t.Context()) if held, ok := restarted.byID(sb.ID); !ok || held.ArchiveCk != "" { t.Fatal("the returning owner lost the claim its wake committed") @@ -1756,10 +1714,7 @@ func TestFailoverRestartUndoesItsOwnInterruptedArchiveAndItsIntent(t *testing.T) hibernateClaim(t, a, sb) ck := interruptArchive(t, a, sb, true) - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) held, ok := restarted.byID(sb.ID) if !ok || held.ArchivePending != "" { t.Fatalf("claim held %v, want it kept with its intent cleared", ok) @@ -1965,6 +1920,15 @@ func (c *failoverCluster) restart(t *testing.T, old *Manager) *Manager { return c.nodeOn(t, fakeOf(old), old.nodeID, old.dataDir, keepAnHour) } +func (c *failoverCluster) restartReconciled(t *testing.T, old *Manager) *Manager { + t.Helper() + restarted := c.restart(t, old) + if err := restarted.Reconcile(t.Context()); err != nil { + t.Fatalf("reconcile: %v", err) + } + return restarted +} + func (c *failoverCluster) nodeOn(t *testing.T, eng *fakeEngine, name, dataDir string, deleteAfter int) *Manager { t.Helper() spec := archivePool(deleteAfter) diff --git a/sandboxd/pool/fork_test.go b/sandboxd/pool/fork_test.go index 038c16c89..802d38343 100644 --- a/sandboxd/pool/fork_test.go +++ b/sandboxd/pool/fork_test.go @@ -165,10 +165,7 @@ func TestForkWithAWrongTokenIsAnUnknownSandbox(t *testing.T) { func TestForkAllOrNothing(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - parent, err := claimAny(t.Context(), m, testKey, ClaimOptions{}) - if err != nil { - t.Fatalf("Claim: %v", err) - } + parent := mustClaim(t, m, testKey) eng.cloneFailNth = 2 if _, err := m.Fork(t.Context(), parent.ID, Cred{Token: parent.Token}, 3, 0, "", ""); err == nil { diff --git a/sandboxd/pool/liveness_test.go b/sandboxd/pool/liveness_test.go index 4eb0fe6f3..5ce7b0129 100644 --- a/sandboxd/pool/liveness_test.go +++ b/sandboxd/pool/liveness_test.go @@ -209,10 +209,7 @@ func TestWakeOfAFailedClaimRearmsEgressAfterARestart(t *testing.T) { DataDir: t.TempDir(), Bridges: []string{"sbxbr0"}, EgressCA: writeTestEgressCA(t), VMMRestart: types.VMMRestartNone, Pools: []config.PoolSpec{{PoolKey: testKey, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "example.com"}}}}}, } - first, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("manager: %v", err) - } + first := mustManager(t, cfg, eng) sb := mustClaim(t, first, testKey) door := engine.EgressSocketPath(sb.VsockSocket) killVMM(eng, sb.VMName) @@ -598,10 +595,7 @@ func TestReconcileKeepsRunningClaimsTheHostLost(t *testing.T) { } seedClaims(t, dataDir, claims) cfg := &config.Config{DataDir: dataDir, VMMRestart: tt.policy} - m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } + m := mustManager(t, cfg, eng) if err := m.Reconcile(t.Context()); err != nil { t.Fatalf("Reconcile: %v", err) diff --git a/sandboxd/pool/memory_test.go b/sandboxd/pool/memory_test.go index 1266b5d9c..a27560154 100644 --- a/sandboxd/pool/memory_test.go +++ b/sandboxd/pool/memory_test.go @@ -179,10 +179,7 @@ func TestMemorySignalsParseTheHostProc(t *testing.T) { func memoryManager(t *testing.T, limit config.MemoryLimit) *Manager { t.Helper() cfg := &config.Config{DataDir: t.TempDir(), MemoryLimit: &limit, Pools: []config.PoolSpec{{PoolKey: testKey}}} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) m.procDir = t.TempDir() if err := os.Mkdir(filepath.Join(m.procDir, "pressure"), 0o750); err != nil { t.Fatalf("setup proc dir: %v", err) diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 61879245a..bc3ea3a05 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -114,9 +114,7 @@ func TestClaimUnpooledKeyColdBoots(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) - if _, err := claimAny(t.Context(), m, testKey, ClaimOptions{}); err != nil { - t.Fatalf("Claim: %v", err) - } + mustClaim(t, m, testKey) if len(eng.colds) != 1 || len(eng.clones) != 0 { t.Fatalf("got clones=%v colds=%v, want one cold boot", eng.clones, eng.colds) } @@ -762,10 +760,7 @@ func TestDirectClaimSkipsProbeGate(t *testing.T) { func TestRefillBudgetFollowsConfig(t *testing.T) { cfg := &config.Config{DataDir: t.TempDir(), RefillConcurrency: 2} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("NewManager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) if got := cap(m.refillSem); got != 2 { t.Errorf("budget %d, want the configured 2", got) } @@ -897,10 +892,7 @@ func TestProbeReadyWaitsForVsock(t *testing.T) { eng.vsockLateN = 2 m := newTestManager(t, eng) - sb, err := claimAny(t.Context(), m, testKey, ClaimOptions{}) - if err != nil { - t.Fatalf("Claim: %v", err) - } + sb := mustClaim(t, m, testKey) if sb.VsockSocket == "" { t.Error("claim returned an empty vsock socket") } @@ -2388,13 +2380,19 @@ func buildGoldenAt(t *testing.T, m *Manager, key types.PoolKey, imageID string) return final } -func newTestManagerAt(t testing.TB, eng *fakeEngine, dataDir string, pools ...config.PoolSpec) *Manager { +func mustManager(t testing.TB, cfg *config.Config, eng Engine) *Manager { t.Helper() - cfg := &config.Config{DataDir: dataDir, Pools: pools} m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) if err != nil { t.Fatalf("setup manager: %v", err) } + return m +} + +func newTestManagerAt(t testing.TB, eng *fakeEngine, dataDir string, pools ...config.PoolSpec) *Manager { + t.Helper() + cfg := &config.Config{DataDir: dataDir, Pools: pools} + m := mustManager(t, cfg, eng) t.Cleanup(func() { assertRunningCount(t, m) }) t.Cleanup(func() { settleGoldenBuilds(m) }) t.Cleanup(func() { settleCaptures(m) }) @@ -2739,11 +2737,7 @@ func volumeHoldersOf(m *Manager, name string) volumeHolders { func newVolumeManagerAt(t *testing.T, eng *fakeEngine, dataDir string, volumes []config.VolumeSpec) *Manager { t.Helper() cfg := &config.Config{DataDir: dataDir, Volumes: volumes} - m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } - return m + return mustManager(t, cfg, eng) } func liveCatalog(t *testing.T) []config.VolumeSpec { diff --git a/sandboxd/pool/reader_test.go b/sandboxd/pool/reader_test.go index ddd03a47d..00c8e6b76 100644 --- a/sandboxd/pool/reader_test.go +++ b/sandboxd/pool/reader_test.go @@ -824,11 +824,7 @@ func (s *fetchCountingStore) files() []string { func newReaderManager(t *testing.T, eng *fakeEngine, extra ...config.PoolSpec) *Manager { t.Helper() cfg := &config.Config{DataDir: t.TempDir(), Pools: append([]config.PoolSpec{{PoolKey: testKey, Warm: 1, Reader: true}}, extra...)} - m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } - return m + return mustManager(t, cfg, eng) } func readerManagerIf(t *testing.T, eng *fakeEngine, on bool) *Manager { diff --git a/sandboxd/pool/replica_test.go b/sandboxd/pool/replica_test.go index 8f43ba9a6..8f19098e8 100644 --- a/sandboxd/pool/replica_test.go +++ b/sandboxd/pool/replica_test.go @@ -197,10 +197,7 @@ func TestReplicaRestartSettlesALostPutReply(t *testing.T) { t.Fatalf("flush: %v", err) } - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) held, ok := restarted.byID(sb.ID) if !ok || restarted.replicaOf(held) != pending || !ckExists(t, restarted, pending) { t.Fatalf("claim held %v, replica %q, ck kept %v, want the row's checkpoint %s adopted", ok, restarted.replicaOf(held), ckExists(t, restarted, pending), pending) @@ -803,10 +800,7 @@ func TestReplicaRestartKeepsItsRowAndStaysFencedUntilALeaseRenews(t *testing.T) } before, _, _ := c.reg.Get(t.Context(), sb.ID) - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) held, ok := restarted.byID(sb.ID) if !ok { t.Fatal("the restart dropped a replicated claim whose row is its own") @@ -839,10 +833,7 @@ func TestReplicaRestartAdoptsTheRowsCheckpointOfAnInterruptedRound(t *testing.T) } second := interrupted.ID - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) held, ok := restarted.byID(sb.ID) if !ok { t.Fatal("the restart dropped the claim") @@ -922,10 +913,7 @@ func TestReconcileWithTheRegistryDownRecordsOnlyTheEndNoOtherNodeCanHold(t *test delete(eng.vms, replicated.VMName) delete(eng.vms, plain.VMName) c.reg.SetDown(true) - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) c.reg.SetDown(false) if got, want := usageEventsOf(t, restarted, replicated.ID), "claim"; got != want { t.Errorf("events of the replicated claim %s, want %s", got, want) @@ -949,10 +937,7 @@ func TestReplicaRestartDropsAClaimAdoptedElsewhere(t *testing.T) { mustAdopt(t, b, sb, "adopt") c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); held { t.Error("the restarted node kept a claim another node adopted") } @@ -977,10 +962,7 @@ func TestReplicaRestartDropsAClaimTheAdopterWokeAndFinished(t *testing.T) { } c.renew(t, "a") - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) if _, held := restarted.byID(sb.ID); held { t.Error("the restarted node kept a claim whose adopter woke it and consumed the row") } @@ -1008,10 +990,7 @@ func TestReplicaRestartKeepsAClaimWhoseUnconfirmedRowIsGone(t *testing.T) { t.Fatalf("commit: %v", err) } - restarted := c.restart(t, a) - if err := restarted.Reconcile(t.Context()); err != nil { - t.Fatalf("reconcile: %v", err) - } + restarted := c.restartReconciled(t, a) held, ok := restarted.byID(sb.ID) if !ok || restarted.replicaOf(held) != ck || !ckExists(t, restarted, ck) { t.Errorf("claim held %v, replica %q, ck kept %v, want the claim and its replica kept", ok, restarted.replicaOf(held), ckExists(t, restarted, ck)) diff --git a/sandboxd/pool/telemetry_test.go b/sandboxd/pool/telemetry_test.go index 7b50d328a..8c2f19d5f 100644 --- a/sandboxd/pool/telemetry_test.go +++ b/sandboxd/pool/telemetry_test.go @@ -496,10 +496,7 @@ func TestGuestPortDialsWriteAuditEvents(t *testing.T) { t.Run(tt.name, func(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true, Pools: []config.PoolSpec{}} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) sb := mustClaim(t, m, testKey) if dialErr := tt.dial(m, sb); dialErr == nil { t.Fatal("fake engine dial unexpectedly succeeded") @@ -609,10 +606,7 @@ func TestEgressUsageEventNamesTheInjectedCredentials(t *testing.T) { func TestEgressAuditNamesTenantAndEndsARelayedFrameCannot(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true, Pools: []config.PoolSpec{}} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) m.recordEgress(t.Context(), "sb_v", "acme", egress.Event{ Method: http.MethodConnect, Host: "example.com", Port: 443, Decision: egress.DecisionAllow, Local: netip.MustParseAddrPort("198.51.100.7:40123"), Remote: netip.MustParseAddrPort("93.184.216.34:443"), @@ -643,10 +637,7 @@ func TestEgressAuditNamesTenantAndEndsARelayedFrameCannot(t *testing.T) { func TestARelayedFrameCannotForgeOrDropItsAuditLine(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true, Pools: []config.PoolSpec{}} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) frames := []string{ `{"op":"exec","argv":["id"],"decision":1,"src_addr":"x","secret":"gh","upstream":"up:3128","method":"GET","added":["t"]}`, `{"op":"exec","argv":["id"],"path":1}`, diff --git a/sandboxd/pool/tenants_test.go b/sandboxd/pool/tenants_test.go index 6f51a041e..0c943ce43 100644 --- a/sandboxd/pool/tenants_test.go +++ b/sandboxd/pool/tenants_test.go @@ -229,10 +229,7 @@ func TestTenantChangesAreValidatedAndAudited(t *testing.T) { } cfg = &config.Config{DataDir: t.TempDir()} - open, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup open manager: %v", err) - } + open := mustManager(t, cfg, newFakeEngine()) if err := open.PutTenant(t.Context(), tenants.TenantSpec{Name: "acme", Token: "t"}); !errors.Is(err, ErrBadTenant) { t.Errorf("tenant on a node without api_token: %v, want ErrBadTenant", err) } @@ -316,10 +313,7 @@ func TestARuntimeTenantTakesItsEgressClassWithoutARestart(t *testing.T) { t.Error("a tenant without a class gained egress") } - restarted, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("restart manager: %v", err) - } + restarted := mustManager(t, cfg, newFakeEngine()) if r, _ := restarted.tenants.Peek("u-1"); r.EgressClass != "desk" { t.Errorf("after a restart u-1 has class %q, want desk from tenants.json", r.EgressClass) } @@ -421,10 +415,7 @@ func listTenants(t *testing.T, m *Manager) []TenantInfo { func tenantManager(t *testing.T, dir string, specs ...tenants.TenantSpec) *Manager { t.Helper() cfg := &config.Config{DataDir: dir, APIToken: "root"} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) putTenants(t, m, specs...) return m } diff --git a/sandboxd/pool/tun_test.go b/sandboxd/pool/tun_test.go index 3fdaf2b74..e94139939 100644 --- a/sandboxd/pool/tun_test.go +++ b/sandboxd/pool/tun_test.go @@ -193,14 +193,14 @@ func TestAProfileChangeADisableOvertookDoesNotReEnable(t *testing.T) { eng, addrs := newFakeEngine(), &fakeAddrs{} m := tunManager(t, eng, addrs, config.PoolSpec{PoolKey: testKey, Tun: true, Egress: anyPolicy}) sb := mustClaim(t, m, testKey) - prev, next, assign, err := m.planTun(sb, true, nil, new("res")) + plan, err := m.planTun(sb, true, nil, new("res")) if err != nil { t.Fatalf("plan: %v", err) } m.mu.Lock() sb.TunOff = true m.mu.Unlock() - if _, _, err = m.commitTun(t.Context(), sb, true, prev, next, assign); !errors.Is(err, errTunRaced) { + if _, _, err = m.commitTun(t.Context(), sb, true, plan); !errors.Is(err, errTunRaced) { t.Fatalf("commit over a newer disable: %v, want errTunRaced", err) } if st := m.tunState(sb); st.On || st.Profile != "" { @@ -372,10 +372,7 @@ func TestAReleaseTheAddressStoreRefusedIsRetried(t *testing.T) { func TestATunFlowEndWritesOneAuditRecord(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true} - m, err := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) start := time.Date(2026, 11, 5, 12, 0, 0, 0, time.UTC) m.recordTunFlow(t.Context(), "sb_t", "acme", "", tunnet.FlowRecord{ Proto: tunnet.ProtoUDP, Dst: netip.MustParseAddr("93.184.216.34"), Port: 443, Allowed: true, diff --git a/sandboxd/pool/volume_attach_test.go b/sandboxd/pool/volume_attach_test.go index 140a0b81e..76575706c 100644 --- a/sandboxd/pool/volume_attach_test.go +++ b/sandboxd/pool/volume_attach_test.go @@ -126,27 +126,7 @@ func TestClaimProvisionAttachFailureKeepsHoldsUntilRemoval(t *testing.T) { if err == nil || !strings.Contains(err.Error(), `attach volume "broken"`) { t.Fatalf("ClaimProvision: %v, want the failing volume's attach error", err) } - if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{writers: 1}) { - t.Errorf("scratch registry after failed removal=%+v, want writer retained", holders) - } - if holders := volumeHoldersOf(m, "broken"); holders != (volumeHolders{readers: 1}) { - t.Errorf("broken registry after failed removal=%+v, want reader retained", holders) - } - if _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: requested[:1]}); !errors.Is(err, ErrVolumeBusy) { - t.Errorf("claim against surviving VM: %v, want ErrVolumeBusy", err) - } - - eng.removeErrFor = "" - m.retryRemovals(t.Context()).Wait() - - if !eng.removed(vmName) { - t.Errorf("removes=%v, want %s drained", eng.removedNames(), vmName) - } - for _, name := range []string{"scratch", "broken"} { - if holders := volumeHoldersOf(m, name); holders != (volumeHolders{}) { - t.Errorf("registry for %s after retry=%+v, want empty", name, holders) - } - } + assertHoldsUntilRetry(t, m, eng, vmName, requested[:1]) } func TestClaimWarmAttachFailureKeepsHoldsUntilRemoval(t *testing.T) { @@ -172,27 +152,7 @@ func TestClaimWarmAttachFailureKeepsHoldsUntilRemoval(t *testing.T) { if err == nil || !strings.Contains(err.Error(), `attach volume "broken"`) { t.Fatalf("ClaimWarm: %v, want the failing volume's attach error", err) } - if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{writers: 1}) { - t.Errorf("scratch registry after failed removal=%+v, want writer retained", holders) - } - if holders := volumeHoldersOf(m, "broken"); holders != (volumeHolders{readers: 1}) { - t.Errorf("broken registry after failed removal=%+v, want reader retained", holders) - } - if _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: requested[:1]}); !errors.Is(err, ErrVolumeBusy) { - t.Errorf("claim against surviving warm VM: %v, want ErrVolumeBusy", err) - } - - eng.removeErrFor = "" - m.retryRemovals(t.Context()).Wait() - - if !eng.removed(warm.VMName) { - t.Errorf("removes=%v, want %s drained", eng.removedNames(), warm.VMName) - } - for _, name := range []string{"scratch", "broken"} { - if holders := volumeHoldersOf(m, name); holders != (volumeHolders{}) { - t.Errorf("registry for %s after retry=%+v, want empty", name, holders) - } - } + assertHoldsUntilRetry(t, m, eng, warm.VMName, requested[:1]) } func TestFinalizeQuotaFailureKeepsHoldsUntilRemoval(t *testing.T) { @@ -392,3 +352,26 @@ func soleVM(eng *fakeEngine, except ...string) (name string, count int) { } return name, count } + +func assertHoldsUntilRetry(t *testing.T, m *Manager, eng *fakeEngine, vmName string, writer []types.Volume) { + t.Helper() + if holders := volumeHoldersOf(m, "scratch"); holders != (volumeHolders{writers: 1}) { + t.Errorf("scratch registry after failed removal=%+v, want writer retained", holders) + } + if holders := volumeHoldersOf(m, "broken"); holders != (volumeHolders{readers: 1}) { + t.Errorf("broken registry after failed removal=%+v, want reader retained", holders) + } + if _, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: writer}); !errors.Is(err, ErrVolumeBusy) { + t.Errorf("claim against surviving VM %s: %v, want ErrVolumeBusy", vmName, err) + } + eng.removeErrFor = "" + m.retryRemovals(t.Context()).Wait() + if !eng.removed(vmName) { + t.Errorf("removes=%v, want %s drained", eng.removedNames(), vmName) + } + for _, name := range []string{"scratch", "broken"} { + if holders := volumeHoldersOf(m, name); holders != (volumeHolders{}) { + t.Errorf("registry for %s after retry=%+v, want empty", name, holders) + } + } +} diff --git a/sandboxd/pool/volume_capture_test.go b/sandboxd/pool/volume_capture_test.go index 5110e17e7..b78adf91b 100644 --- a/sandboxd/pool/volume_capture_test.go +++ b/sandboxd/pool/volume_capture_test.go @@ -37,10 +37,7 @@ func TestReconcileRetainsVolumeCaptureGateWithoutCatalog(t *testing.T) { DataDir: dataDir, Volumes: []config.VolumeSpec{{Name: "dataset", Path: volumePath, DirectIO: "off"}}, } - m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } + m := mustManager(t, cfg, eng) sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: []types.Volume{{Name: "dataset", Mount: "/datasets"}}}) if err != nil { t.Fatalf("ClaimProvision: %v", err) diff --git a/sandboxd/pool/volume_catalog_test.go b/sandboxd/pool/volume_catalog_test.go index 8f9714e9d..b3f3a9d75 100644 --- a/sandboxd/pool/volume_catalog_test.go +++ b/sandboxd/pool/volume_catalog_test.go @@ -85,10 +85,7 @@ func TestRuntimeCatalogPersistsNormalizedSemantics(t *testing.T) { func TestRuntimeCatalogChangesAreAudited(t *testing.T) { cfg := &config.Config{DataDir: t.TempDir(), AuditLog: true} - m, initErr := NewManager(t.Context(), cfg, newFakeEngine(), testSecrets(t), testBackends(t, cfg)) - if initErr != nil { - t.Fatalf("NewManager: %v", initErr) - } + m := mustManager(t, cfg, newFakeEngine()) spec := types.VolumeRegistration{Path: "/images/data"} restore := blockCatalogRename(t, m) if err := m.RegisterVolume(t.Context(), "data", spec); !errors.Is(err, ErrVolumeCatalogUnavailable) { diff --git a/sandboxd/pool/volume_mutation_test.go b/sandboxd/pool/volume_mutation_test.go index cdfbd2ca7..9bee11c28 100644 --- a/sandboxd/pool/volume_mutation_test.go +++ b/sandboxd/pool/volume_mutation_test.go @@ -976,10 +976,7 @@ func (e *stalledDetachEngine) DetachVolume(ctx context.Context, _, _, _, _ strin func newTenantVolumeManager(t *testing.T, eng *fakeEngine, catalog []config.VolumeSpec, volumes ...types.Volume) (*Manager, *types.Sandbox) { t.Helper() cfg := &config.Config{DataDir: t.TempDir(), APIToken: "root", Volumes: catalog} - m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("NewManager: %v", err) - } + m := mustManager(t, cfg, eng) putTenants(t, m, tenants.TenantSpec{Name: "acme", Token: "acme-token"}) sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Tenant: "acme", Volumes: volumes}) if err != nil { diff --git a/sandboxd/pool/volume_test.go b/sandboxd/pool/volume_test.go index 4a054ca5c..4d6474876 100644 --- a/sandboxd/pool/volume_test.go +++ b/sandboxd/pool/volume_test.go @@ -336,10 +336,7 @@ func TestPooledKeyOutranksPromotedTemplate(t *testing.T) { Pools: []config.PoolSpec{{PoolKey: key, Warm: 1}}, Volumes: catalog, } - m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("restart with the key pooled: %v", err) - } + m := mustManager(t, cfg, eng) m.pools[key].goldenDir = "/goldens/pooled" if m.HasPromotedTemplate(t.Context(), key, "") { @@ -590,11 +587,7 @@ func newVolumePoolManager(t *testing.T, eng *fakeEngine, dataDir string, volumes Pools: []config.PoolSpec{{PoolKey: testKey, Warm: 1}}, Volumes: volumes, } - m, err := NewManager(t.Context(), cfg, eng, testSecrets(t), testBackends(t, cfg)) - if err != nil { - t.Fatalf("setup manager: %v", err) - } - return m + return mustManager(t, cfg, eng) } func writeVolumeImage(t testing.TB, name, content string) string { From 18a63040b0a7c3638545f98c8e15914b3bda7168 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 13:00:49 +0800 Subject: [PATCH 30/40] pool: leave a received move to its resolve at restart, and sweep a dropped one's staging Reconcile armed a MoveIn claim's doors and tun before its move record resolved, so after an aborted move both nodes announced the overlay address until dropMoveIn; the resolve arms it (a running guest on start, a stopped one through the cold restart). The staging sweep kept the guest-memory files of a MoveIn claim that the restart dropped because it checked the loaded set instead of the adopted one. --- sandboxd/pool/move_test.go | 17 +++++++++++++++++ sandboxd/pool/reconcile.go | 4 ++-- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/sandboxd/pool/move_test.go b/sandboxd/pool/move_test.go index 35083f890..0936dd33f 100644 --- a/sandboxd/pool/move_test.go +++ b/sandboxd/pool/move_test.go @@ -652,6 +652,23 @@ func TestMoveRestartAfterTheConsumeKeepsTheClaim(t *testing.T) { } } +func TestRestartSweepsTheStagingOfAMovedInClaimItDrops(t *testing.T) { + dataDir := t.TempDir() + sb := &types.Sandbox{ID: "sb_in", VMName: "sbx-in", Key: testKey, Token: "tok", Deadline: time.Now().Add(time.Hour), MoveIn: true} + seedClaims(t, dataDir, map[string]*types.Sandbox{sb.ID: sb}) + staging := filepath.Join(dataDir, movesDir, sb.ID) + if err := os.MkdirAll(staging, 0o750); err != nil { + t.Fatalf("setup: %v", err) + } + m := newTestManagerAt(t, newFakeEngine(), dataDir) + if err := m.Reconcile(t.Context()); err != nil { + t.Fatalf("reconcile: %v", err) + } + if _, err := os.Stat(staging); !errors.Is(err, os.ErrNotExist) { + t.Errorf("staging of a dropped moved-in claim: %v, want it removed", err) + } +} + func TestMoveHoldsOffCapturesReleasesAndEvacuationWhileItRuns(t *testing.T) { for name, archived := range map[string]bool{"archives": true, "no archives": false} { t.Run(name, func(t *testing.T) { testMoveHoldsOffCallers(t, archived) }) diff --git a/sandboxd/pool/reconcile.go b/sandboxd/pool/reconcile.go index 81fb79ad4..f58598e36 100644 --- a/sandboxd/pool/reconcile.go +++ b/sandboxd/pool/reconcile.go @@ -110,7 +110,7 @@ func (m *Manager) Reconcile(ctx context.Context) error { // A crash mid-export leaves a fork-* staging dir nothing in this life reuses _ = utils.RemoveDirEntries(m.dataDir, func(name string) bool { return strings.HasPrefix(name, "fork-") }) _ = 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 }) + _ = utils.RemoveDirEntries(filepath.Join(m.dataDir, movesDir), func(id string) bool { return m.claimed[id] == nil || !m.claimed[id].MoveIn }) if answered { m.reclaimOrphanArchiveCks(ctx, claims) @@ -276,7 +276,7 @@ func (m *Manager) resyncEgress(ctx context.Context, live map[string]types.VMReco for _, sb := range m.claimed { sb.TouchAt(now) // A hibernated or archived claim has no guest to serve; its wake arms the door - if sb.HibernateSnap != "" || sb.ArchiveCk != "" || sb.Failed != "" { + if sb.HibernateSnap != "" || sb.ArchiveCk != "" || sb.Failed != "" || sb.MoveIn { continue } if m.out.LocksNIC(sb.Key) { From 6f2aed74b067903af88c7c4c465cef9f470d0f81 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 13:03:17 +0800 Subject: [PATCH 31/40] store/dir: fsync a template's digest file before its meta.json commits The digest went through a plain write and rename while meta.json was fsynced, so a power loss right after a publish could leave a template whose digest reads empty and whose digest-guarded delete or labels call answers 409. --- sandboxd/store/dir/dir.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/sandboxd/store/dir/dir.go b/sandboxd/store/dir/dir.go index 49e14c32e..14f9f7f07 100644 --- a/sandboxd/store/dir/dir.go +++ b/sandboxd/store/dir/dir.go @@ -354,16 +354,12 @@ func removeAgedEntry(final string, entry fs.DirEntry) error { func writeGenerationDigest(final string, meta []byte, digest string) error { path := filepath.Join(final, digestName(meta)) - tmp := path + ".tmp" if digest == "" { - return errors.Join(os.RemoveAll(path), os.RemoveAll(tmp)) //nolint:gosec // hash-derived paths under our root + return os.RemoveAll(path) //nolint:gosec // hash-derived path under our root } - if err := os.WriteFile(tmp, []byte(digest), 0o600); err != nil { //nolint:gosec // hash-derived path under our root + if err := utils.WriteFileSync(path, []byte(digest), 0o600); err != nil { return fmt.Errorf("write digest: %w", err) } - if err := os.Rename(tmp, path); err != nil { //nolint:gosec // hash-derived paths under our root - return fmt.Errorf("commit digest: %w", err) - } return nil } From 826acc69e9eb6c5524ee2f0257e4cd5143a88ed9 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 13:03:29 +0800 Subject: [PATCH 32/40] outbound: read a tun door's guest socket under the owner's lock on reload Reloaded read sb.VsockSocket without the pool lock that a cold restart or a move writes it under; a torn string read is a crash. The socket now comes through TunState, which already reads the claim's tun fields under that lock. --- sandboxd/outbound/outbound.go | 2 +- sandboxd/outbound/tun.go | 3 ++- sandboxd/pool/tun.go | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sandboxd/outbound/outbound.go b/sandboxd/outbound/outbound.go index d6db469d1..9ed720d3a 100644 --- a/sandboxd/outbound/outbound.go +++ b/sandboxd/outbound/outbound.go @@ -259,7 +259,7 @@ func (h *Host) Reloaded() { } for _, t := range off { h.dropTun(t) - h.tunDown(t.sb.ID, t.sb.VsockSocket) + h.tunDown(t.sb.ID, h.tunState(t.sb).Sock) } } diff --git a/sandboxd/outbound/tun.go b/sandboxd/outbound/tun.go index 2b9209de4..d34d93b74 100644 --- a/sandboxd/outbound/tun.go +++ b/sandboxd/outbound/tun.go @@ -28,6 +28,7 @@ type TunState struct { Addr netip.Addr Profile string Bytes int64 + Sock string } type tunDoor struct { @@ -190,7 +191,7 @@ func (h *Host) serveTun(ctx context.Context, t *tunDoor) { func (h *Host) tunState(sb *types.Sandbox) TunState { if h.o.TunState == nil { - return TunState{On: !sb.TunOff, Addr: sb.TunAddr, Profile: sb.TunProfile, Bytes: sb.TunBytes} + return TunState{On: !sb.TunOff, Addr: sb.TunAddr, Profile: sb.TunProfile, Bytes: sb.TunBytes, Sock: sb.VsockSocket} } return h.o.TunState(sb) } diff --git a/sandboxd/pool/tun.go b/sandboxd/pool/tun.go index 6906690af..2dad76a71 100644 --- a/sandboxd/pool/tun.go +++ b/sandboxd/pool/tun.go @@ -194,7 +194,7 @@ func (m *Manager) foldTunBytes() { func (m *Manager) tunState(sb *types.Sandbox) outbound.TunState { m.mu.Lock() defer m.mu.Unlock() - return outbound.TunState{On: !sb.TunOff, Addr: sb.TunAddr, Profile: sb.TunProfile, Bytes: sb.TunBytes} + return outbound.TunState{On: !sb.TunOff, Addr: sb.TunAddr, Profile: sb.TunProfile, Bytes: sb.TunBytes, Sock: sb.VsockSocket} } func (m *Manager) tunRowed(sb *types.Sandbox) bool { From 0c20db2de4cdc3646a2db99e9a27cd542f4ee2cc Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 13:03:37 +0800 Subject: [PATCH 33/40] cut: drop Host.ResolvedName and the DNS resolver's name map Nothing outside its own test read it, and no audit record carries a name, so every kept A/AAAA answer paid a mutex and a map write for nothing. --- sandboxd/outbound/dns.go | 42 ++-------------------------- sandboxd/outbound/dns_test.go | 52 +++++++++++------------------------ sandboxd/outbound/outbound.go | 15 ++-------- 3 files changed, 21 insertions(+), 88 deletions(-) diff --git a/sandboxd/outbound/dns.go b/sandboxd/outbound/dns.go index ad58eb882..c07f4c169 100644 --- a/sandboxd/outbound/dns.go +++ b/sandboxd/outbound/dns.go @@ -37,7 +37,6 @@ const ( dnsQueryBurst = 200 dnsRefusalRate = 1 dnsRefusalBurst = 10 - dnsNamesMax = 4096 dnsMsgMax = 65535 dnsHeaderLen = 12 dnsQueryMax = 4096 @@ -60,11 +59,6 @@ type DNSEvent struct { Decision string } -type resolvedName struct { - name string - expires time.Time -} - type nameserverList struct { servers []string read time.Time @@ -78,9 +72,6 @@ type resolver struct { queries *rate.Limiter refusals *rate.Limiter inflight chan struct{} - - mu sync.Mutex - names map[netip.Addr]resolvedName } func newResolver(ctx context.Context, h *Host, record func(DNSEvent)) *resolver { @@ -93,7 +84,6 @@ func newResolver(ctx context.Context, h *Host, record func(DNSEvent)) *resolver queries: rate.NewLimiter(dnsQueryRate, dnsQueryBurst), refusals: rate.NewLimiter(dnsRefusalRate, dnsRefusalBurst), inflight: make(chan struct{}, dnsInflight), - names: map[netip.Addr]resolvedName{}, } } @@ -164,7 +154,7 @@ func (r *resolver) answer(query, buf []byte, datagram bool) ([]byte, bool) { resp, err := r.exchange(query, buf, datagram) var drop bool if err == nil { - ev.Answers, drop, err = r.scan(resp, hdr.ID, ev.Name) + ev.Answers, drop, err = r.scan(resp, hdr.ID) } switch { case err != nil: @@ -184,8 +174,8 @@ func (r *resolver) answer(query, buf []byte, datagram bool) ([]byte, bool) { return out, true } -// scan walks resp without allocating: it counts the answers kept, reports whether an address record must go, and remembers the kept answer addresses. -func (r *resolver) scan(resp []byte, id uint16, name string) (int, bool, error) { +// scan walks resp without allocating: it counts the answers kept and reports whether an address record must go. +func (r *resolver) scan(resp []byte, id uint16) (int, bool, error) { var p dnsmessage.Parser hdr, err := p.Start(resp) if err != nil { @@ -198,7 +188,6 @@ func (r *resolver) scan(resp []byte, id uint16, name string) (int, bool, error) return 0, false, err } self, allow := *r.h.self.Load(), r.h.o.View().internalAllow - now := time.Now() answers, drop := 0, false for section := range 3 { for { @@ -218,9 +207,6 @@ func (r *resolver) scan(resp []byte, id uint16, name string) (int, bool, error) drop = true case section == 0: answers++ - if ip.IsValid() { - r.remember(ip, name, now.Add(time.Duration(rh.TTL)*time.Second)) - } } } } @@ -244,28 +230,6 @@ func (r *resolver) strip(resp []byte) ([]byte, error) { return msg.Pack() } -func (r *resolver) remember(ip netip.Addr, name string, expires time.Time) { - r.mu.Lock() - defer r.mu.Unlock() - if _, ok := r.names[ip]; !ok && len(r.names) >= dnsNamesMax { - for addr := range r.names { - delete(r.names, addr) - break - } - } - r.names[ip] = resolvedName{name: name, expires: expires} -} - -func (r *resolver) name(ip netip.Addr) (string, bool) { - r.mu.Lock() - defer r.mu.Unlock() - rn, ok := r.names[canonicalAddr(ip)] - if !ok || time.Now().After(rn.expires) { - return "", false - } - return rn.name, true -} - func (r *resolver) exchange(query, buf []byte, datagram bool) ([]byte, error) { var errs error var last []byte diff --git a/sandboxd/outbound/dns_test.go b/sandboxd/outbound/dns_test.go index fa3ceb57e..6446c032d 100644 --- a/sandboxd/outbound/dns_test.go +++ b/sandboxd/outbound/dns_test.go @@ -86,7 +86,7 @@ func TestDNSDoorDropsInternalAnswersAndRemembersTheRest(t *testing.T) { {"allowed.test.", []string{"10.9.1.1"}, DNSAllow}, {"public.test.", []string{"93.184.216.35", "2606:2800:220:1::1"}, DNSAllow}, } { - msg := queryDoor(t, sb, tt.name, dnsmessage.TypeA) + msg := queryDoor(t, sb, tt.name) if got := addresses(msg); !slices.Equal(got, tt.want) { t.Errorf("%s answers %v, want %v", tt.name, got, tt.want) } @@ -98,12 +98,6 @@ func TestDNSDoorDropsInternalAnswersAndRemembersTheRest(t *testing.T) { t.Errorf("%s event %+v, want %d answers and %s", tt.name, ev, len(tt.want), tt.decision) } } - if name, ok := h.ResolvedName(sb.ID, netip.MustParseAddr("93.184.216.34")); !ok || name != "mixed.test" { - t.Errorf("name of 93.184.216.34 = %q, %v, want mixed.test", name, ok) - } - if _, ok := h.ResolvedName(sb.ID, netip.MustParseAddr("10.0.0.5")); ok { - t.Error("a dropped internal address has a name") - } } func TestDNSDoorPassesAnUnfilteredAnswerByteForByte(t *testing.T) { @@ -132,7 +126,7 @@ func TestDNSDoorDropsAnInternalGlueRecord(t *testing.T) { o.Nameservers = func() []string { return []string{upstream.udp} } o.RecordDNS = events.record }) - msg := queryDoor(t, armDNS(t, h), "glue.test.", dnsmessage.TypeA) + msg := queryDoor(t, armDNS(t, h), "glue.test.") if got := addresses(msg); !slices.Equal(got, []string{"93.184.216.37"}) { t.Errorf("answers %v, want the public address", got) } @@ -151,19 +145,12 @@ func TestDNSDoorDisarmEndsAnOpenConnection(t *testing.T) { }) sb := armDNS(t, h) conn := dialDNS(t, sb) - if got := addresses(exchangeDoor(t, conn, "public.test.", dnsmessage.TypeA)); len(got) != 2 { + if got := addresses(exchangeDoor(t, conn, "public.test.")); len(got) != 2 { t.Fatalf("armed door answers %v, want two addresses", got) } h.Disarm(sb.ID, false) _ = conn.SetDeadline(time.Now().Add(5 * time.Second)) - query, err := (&dnsmessage.Message{ - ID: 1, - Questions: []dnsmessage.Question{{Name: dnsmessage.MustNewName("public.test."), Type: dnsmessage.TypeA, Class: dnsmessage.ClassINET}}, - }).Pack() - if err != nil { - t.Fatal(err) - } - _ = writeDNSMsg(conn, query) + _ = writeDNSMsg(conn, packQuery(t, dnsmessage.Header{ID: 1}, "public.test.")) if answer, err := readDNSMsg(conn, dnsMsgMax); err == nil { t.Fatalf("a disarmed door answered %d bytes on a connection opened while armed", len(answer)) } @@ -180,10 +167,10 @@ func TestDNSDoorsHandATruncatedAnswerToEachTransport(t *testing.T) { t.Fatalf("dial datagram door: %v", err) } t.Cleanup(func() { _ = gram.Close() }) - if msg := exchangeDoor(t, gram, "big.test.", dnsmessage.TypeA); !msg.Truncated || len(msg.Answers) != 0 { + if msg := exchangeDoor(t, gram, "big.test."); !msg.Truncated || len(msg.Answers) != 0 { t.Errorf("datagram door answered truncated=%v with %d answers, want the upstream's truncated reply", msg.Truncated, len(msg.Answers)) } - if msg := queryDoor(t, sb, "big.test.", dnsmessage.TypeA); msg.Truncated || len(addresses(msg)) != bigAnswers { + if msg := queryDoor(t, sb, "big.test."); msg.Truncated || len(addresses(msg)) != bigAnswers { t.Errorf("stream door answered truncated=%v with %d addresses, want %d over TCP", msg.Truncated, len(addresses(msg)), bigAnswers) } } @@ -218,7 +205,7 @@ func TestADNSDialBeforeTheClaimForcesARebind(t *testing.T) { if answer, err := readDNSMsg(early, dnsMsgMax); err == nil { t.Fatalf("a query queued before the claim got %d bytes under the claim", len(answer)) } - if got := addresses(queryDoor(t, sb, "public.test.", dnsmessage.TypeA)); len(got) != 2 { + if got := addresses(queryDoor(t, sb, "public.test.")); len(got) != 2 { t.Errorf("the rebound door answers %v, want two addresses", got) } if n := events.count(DNSAllow); n != 1 { @@ -232,13 +219,13 @@ func TestDNSDoorMovesPastANameserverThatAnswersServfail(t *testing.T) { h := newHost(t, &fakeEngine{}, poolConfig(testKey, dnsPolicy), func(o *Options) { o.Nameservers = func() []string { return []string{failing, upstream.udp} } }) - if got := addresses(queryDoor(t, armDNS(t, h), "public.test.", dnsmessage.TypeA)); len(got) != 2 { + if got := addresses(queryDoor(t, armDNS(t, h), "public.test.")); len(got) != 2 { t.Errorf("answers %v, want the second nameserver's two addresses", got) } allFailing := newHost(t, &fakeEngine{}, poolConfig(testKey, dnsPolicy), func(o *Options) { o.Nameservers = func() []string { return []string{failing, failing} } }) - if msg := queryDoor(t, armDNS(t, allFailing), "public.test.", dnsmessage.TypeA); msg.RCode != dnsmessage.RCodeServerFailure { + if msg := queryDoor(t, armDNS(t, allFailing), "public.test."); msg.RCode != dnsmessage.RCodeServerFailure { t.Errorf("rcode %v with every nameserver failing, want their SERVFAIL", msg.RCode) } } @@ -255,7 +242,7 @@ func TestDNSDoorAnswersServfailWhenNoNameserverAnswers(t *testing.T) { o.Nameservers = func() []string { return []string{addr} } o.RecordDNS = events.record }) - msg := queryDoor(t, armDNS(t, h), "public.test.", dnsmessage.TypeA) + msg := queryDoor(t, armDNS(t, h), "public.test.") if msg.RCode != dnsmessage.RCodeServerFailure || events.last(t).Decision != DNSFailed { t.Errorf("rcode %v decision %q, want SERVFAIL and failed", msg.RCode, events.last(t).Decision) } @@ -277,7 +264,7 @@ func TestDNSDoorRefusesAQueryFloodPastItsBucket(t *testing.T) { conn := dialDNS(t, sb) refused := 0 for range dnsQueryBurst + 100 { - if exchangeDoor(t, conn, "public.test.", dnsmessage.TypeA).RCode == dnsmessage.RCodeRefused { + if exchangeDoor(t, conn, "public.test.").RCode == dnsmessage.RCodeRefused { refused++ } } @@ -326,7 +313,7 @@ func TestDNSDoorDropsAndCountsMalformedQueries(t *testing.T) { } }) } - if got := addresses(queryDoor(t, sb, "public.test.", dnsmessage.TypeA)); len(got) != 2 { + if got := addresses(queryDoor(t, sb, "public.test.")); len(got) != 2 { t.Errorf("door after the malformed queries answers %v, want two addresses", got) } } @@ -543,21 +530,14 @@ func dialDNS(t *testing.T, sb *types.Sandbox) net.Conn { return conn } -func queryDoor(t *testing.T, sb *types.Sandbox, name string, qtype dnsmessage.Type) dnsmessage.Message { +func queryDoor(t *testing.T, sb *types.Sandbox, name string) dnsmessage.Message { t.Helper() - return exchangeDoor(t, dialDNS(t, sb), name, qtype) + return exchangeDoor(t, dialDNS(t, sb), name) } -func exchangeDoor(t *testing.T, conn net.Conn, name string, qtype dnsmessage.Type) dnsmessage.Message { +func exchangeDoor(t *testing.T, conn net.Conn, name string) dnsmessage.Message { t.Helper() - query, err := (&dnsmessage.Message{ - ID: 0x5a5a, RecursionDesired: true, - Questions: []dnsmessage.Question{{Name: dnsmessage.MustNewName(name), Type: qtype, Class: dnsmessage.ClassINET}}, - }).Pack() - if err != nil { - t.Fatal(err) - } - if err = writeDNSMsg(conn, query); err != nil { + if err := writeDNSMsg(conn, packQuery(t, dnsmessage.Header{ID: 0x5a5a, RecursionDesired: true}, name)); err != nil { t.Fatalf("write query: %v", err) } raw, err := readDNSMsg(conn, dnsMsgMax) diff --git a/sandboxd/outbound/outbound.go b/sandboxd/outbound/outbound.go index 9ed720d3a..8443a79f3 100644 --- a/sandboxd/outbound/outbound.go +++ b/sandboxd/outbound/outbound.go @@ -51,10 +51,10 @@ type Options struct { RecordEnds bool NoRxSplice bool // Router carries the advanced network mode; nil when the node has no tun block. - Router *tunnet.Router - // Flow records one ended tun flow; TunBytes takes a disarmed claim's tun byte count back; TunState reads the claim's tun fields under the owner's lock. + Router *tunnet.Router Flow func(ctx context.Context, id, tenant, profile string, r tunnet.FlowRecord) TunBytes func(sb *types.Sandbox, used int64) + // TunState reads the claim's tun fields under the owner's lock. TunState func(sb *types.Sandbox) TunState // Dial, Verdict, Lock, Unlock, Sweep and Nameservers default to the internal-address guard, netfilter and /etc/resolv.conf; tests swap them. Dial egress.DialFunc @@ -231,17 +231,6 @@ func (h *Host) DNSMalformed() uint64 { return h.dnsMalformed.Load() } -// ResolvedName is the name sandbox id's guest last resolved to ip, while that answer's TTL holds. -func (h *Host) ResolvedName(id string, ip netip.Addr) (string, bool) { - h.mu.Lock() - d := h.doors[id] - h.mu.Unlock() - if d == nil || d.resolver == nil { - return "", false - } - return d.resolver.name(ip) -} - // Reloaded follows a view swap: live proxies drop their pooled connections, and claims of a pool whose tun switch went off lose their packet path. func (h *Host) Reloaded() { v := h.o.View() From 44b4814fea96d296346237fabe522a40e51f6ea8 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 13:03:37 +0800 Subject: [PATCH 34/40] review: sandboxd comments, layout and one constant Stale and restating comments go or are corrected (Engine.Version no longer names a master- form cocoon never prints, the outbound Resolve godoc, the address log's compaction bound); own-line comments start uppercase; vocabulary types move ahead of their users in server/move.go, tunnet/gro.go and tunnet/udp; the relay's 101 reply builds on upgradeProto; Local's one-caller compact is inlined. --- sandboxd/backends.go | 3 +-- sandboxd/config/reload.go | 1 - sandboxd/engine/engine.go | 2 +- sandboxd/engine/migrate.go | 2 +- sandboxd/outbound/view.go | 2 +- sandboxd/server/move.go | 31 +++++++++++++++---------------- sandboxd/server/relay.go | 2 +- sandboxd/store/peer/broadcast.go | 2 +- sandboxd/store/s3/s3.go | 1 - sandboxd/tunnet/addrs.go | 8 ++------ sandboxd/tunnet/gro.go | 14 +++++++------- sandboxd/tunnet/udp/udp_linux.go | 14 +++++++------- 12 files changed, 37 insertions(+), 45 deletions(-) diff --git a/sandboxd/backends.go b/sandboxd/backends.go index 3d4a32c3a..5f6057722 100644 --- a/sandboxd/backends.go +++ b/sandboxd/backends.go @@ -66,7 +66,6 @@ func openBackends(ctx context.Context, cfg *config.Config) (pool.Backends, runFu return b, runTun, nil } -// openTun returns run, which receives VXLAN until its context ends. func openTun(ctx context.Context, cfg *config.Config, b *pool.Backends) (runFunc, error) { t := cfg.Tun under, err := udp.Listen(t.ListenAddr(), t.Senders) @@ -87,7 +86,7 @@ func openTun(ctx context.Context, cfg *config.Config, b *pool.Backends) (runFunc router := tunnet.New(under, t.Overlay()) b.TunRouter, b.TunAddrs = router, addrs return func(ctx context.Context) { - // closing the underlay is what ends the blocked receive + // Closing the underlay is what ends the blocked receive. context.AfterFunc(ctx, under.Close) log.WithFunc("main.openTun").Error(ctx, router.Run(ctx), "tun router stopped; the advanced network mode is down") }, nil diff --git a/sandboxd/config/reload.go b/sandboxd/config/reload.go index 9e2dc276e..2b424ea93 100644 --- a/sandboxd/config/reload.go +++ b/sandboxd/config/reload.go @@ -85,7 +85,6 @@ func diffPools(cur, next []PoolSpec) (changed, ignored, restart []string) { return changed, ignored, restart } -// diffTun reloads the exits, MTU, exit port and limits; the socket, the subnet, the VNI and the allocator need a restart, as does adding or dropping the block. func diffTun(cur, next *TunConfig) (changed, restart []string) { switch { case reflect.DeepEqual(cur, next): diff --git a/sandboxd/engine/engine.go b/sandboxd/engine/engine.go index 9a65f3d14..95873c923 100644 --- a/sandboxd/engine/engine.go +++ b/sandboxd/engine/engine.go @@ -113,7 +113,7 @@ func (e *Engine) Close() { e.running.Wait() } -// Version reports cocoon's version string: a "vX.Y.Z" release or a "master-" dev build. +// Version reports cocoon's version string. func (e *Engine) Version(ctx context.Context) (string, error) { if v := e.version.Load(); v != nil { return *v, nil diff --git a/sandboxd/engine/migrate.go b/sandboxd/engine/migrate.go index e87b502e0..10ce8c176 100644 --- a/sandboxd/engine/migrate.go +++ b/sandboxd/engine/migrate.go @@ -43,7 +43,7 @@ func (e *Engine) MigrateCapture(ctx context.Context, vmName, series string, fina func (e *Engine) MigrateReceive(ctx context.Context, fromDir, name string, key types.PoolKey) (types.VMRecord, error) { restore := e.restoreArgs() if e.restoreMode == types.RestoreOnDemand { - // cocoon's hypervisor rejects ondemand for a series with deltas, so its default applies + // Cocoon's hypervisor rejects ondemand for a series with deltas, so its default applies restore = nil } args := append([]string{"vm", "migrate", "receive", "--from-dir", fromDir}, e.cloneFlags(name, key, restore)...) diff --git a/sandboxd/outbound/view.go b/sandboxd/outbound/view.go index 45df86f72..fc4c6ede9 100644 --- a/sandboxd/outbound/view.go +++ b/sandboxd/outbound/view.go @@ -100,7 +100,7 @@ func (v *View) HasClass(name string) bool { return ok } -// Resolve is sb's effective policy, pool ∩ tenant: root has no tenant layer, an unpooled key no pool one, a NoEgress claim none at all; pooled runs only when the key has no pool policy. +// Resolve is sb's effective policy, pool ∩ tenant: root has no tenant layer, an unpooled key no pool one, a NoEgress claim none at all; a pooled claim whose key lost its pool policy gets none. func (v *View) Resolve(sb *types.Sandbox) (egress.PolicyEvaluator, bool) { if sb.NoEgress { return nil, false diff --git a/sandboxd/server/move.go b/sandboxd/server/move.go index 371350ea3..c60c19f0c 100644 --- a/sandboxd/server/move.go +++ b/sandboxd/server/move.go @@ -20,6 +20,21 @@ import ( const moveCallTimeout = 10 * time.Minute +type moveAdmitRequest struct { + Tenant string `json:"tenant,omitempty"` + Key types.PoolKey `json:"key"` + From string `json:"from"` +} + +type moveAdmitResponse struct { + Node string `json:"node"` +} + +type moveCommitRequest struct { + Claim jsontext.Value `json:"claim"` + Epoch int64 `json:"epoch"` +} + var _ pool.MovePeer = (*MoveClient)(nil) // MoveClient reaches a move's target on its control port with the root token. @@ -49,7 +64,6 @@ func (c *MoveClient) Drop(ctx context.Context, addr, id string) error { return c.call(ctx, http.MethodDelete, addr, movePath(id, ""), nil, nil) } -// Maps the target's status codes onto pool errors. func (c *MoveClient) call(ctx context.Context, method, addr, path string, in, out any) error { ctx, cancel := context.WithTimeout(ctx, moveCallTimeout) defer cancel() @@ -99,21 +113,6 @@ type MoveResponse struct { DowntimeMS int64 `json:"downtime_ms"` } -type moveAdmitRequest struct { - Tenant string `json:"tenant,omitempty"` - Key types.PoolKey `json:"key"` - From string `json:"from"` -} - -type moveAdmitResponse struct { - Node string `json:"node"` -} - -type moveCommitRequest struct { - Claim jsontext.Value `json:"claim"` - Epoch int64 `json:"epoch"` -} - func (s *Server) handleMove(w http.ResponseWriter, r *http.Request) { req, ok := decodeBodyStrict[MoveRequest](w, r) if !ok { diff --git a/sandboxd/server/relay.go b/sandboxd/server/relay.go index e46a68046..7193e3118 100644 --- a/sandboxd/server/relay.go +++ b/sandboxd/server/relay.go @@ -28,7 +28,7 @@ const ( ) var ( - switchingProtocols = []byte("HTTP/1.1 101 Switching Protocols\r\nUpgrade: silkd\r\nConnection: Upgrade\r\n\r\n") + switchingProtocols = []byte("HTTP/1.1 101 Switching Protocols\r\nUpgrade: " + upgradeProto + "\r\nConnection: Upgrade\r\n\r\n") newline = []byte{'\n'} // silkd delimits input with terminal frames, so a TCP half-close carries no meaning here. diff --git a/sandboxd/store/peer/broadcast.go b/sandboxd/store/peer/broadcast.go index 11ba0c6df..e177a5445 100644 --- a/sandboxd/store/peer/broadcast.go +++ b/sandboxd/store/peer/broadcast.go @@ -27,7 +27,7 @@ func (b *Broadcaster) Delete(ctx context.Context, id string) { var wg sync.WaitGroup for _, addr := range addrs { wg.Go(func() { - // a 404 means the peer never healed a copy + // A 404 means the peer never healed a copy. if err := peerCall(ctx, http.MethodDelete, checkpointURL(addr, id, "?no_forward=1"), b.Token, nil, peerCallTimeout, http.StatusNoContent, http.StatusNotFound); err != nil { logger.Warnf(ctx, "broadcast delete %s to %s: %v", id, addr, err) } diff --git a/sandboxd/store/s3/s3.go b/sandboxd/store/s3/s3.go index 6d25341af..a7d1fcbb5 100644 --- a/sandboxd/store/s3/s3.go +++ b/sandboxd/store/s3/s3.go @@ -434,7 +434,6 @@ func (s *Store) prepare(ctx context.Context, staging, id, lineage string, digest }, nil } -// populate downloads one cache generation and installs it atomically. func (s *Store) populate(ctx context.Context, id string, meta []byte, gen string) error { if _, err := os.Stat(gen); err == nil { return nil // another flight installed it between stat and Do diff --git a/sandboxd/tunnet/addrs.go b/sandboxd/tunnet/addrs.go index 9aa1d7348..38bd075f9 100644 --- a/sandboxd/tunnet/addrs.go +++ b/sandboxd/tunnet/addrs.go @@ -21,7 +21,7 @@ import ( const ( freedFile = "tun-freed.log" - // compactAfter is how far the log may grow past twice its holds before it is rewritten with the live ones. + // compactAfter is how far the log may grow past twice its in-window releases before it is rewritten with them. compactAfter = 1024 ) @@ -72,7 +72,7 @@ func NewLocal(dir string, subnet netip.Prefix, reuse time.Duration) (*Local, err l.freed[addr] = time.Unix(at, 0) } } - if err = l.compact(); err != nil { + if err = utils.WriteFileSync(l.path, l.snapshot(), 0o600); err != nil { return nil, err } return l, nil @@ -151,10 +151,6 @@ func (l *Local) Release(_ context.Context, claimID string, addr netip.Addr) erro return nil } -func (l *Local) compact() error { - return utils.WriteFileSync(l.path, l.snapshot(), 0o600) -} - func (l *Local) snapshot() []byte { var b []byte for addr, at := range l.freed { diff --git a/sandboxd/tunnet/gro.go b/sandboxd/tunnet/gro.go index 5e5c8f76c..5cc0921a3 100644 --- a/sandboxd/tunnet/gro.go +++ b/sandboxd/tunnet/gro.go @@ -11,6 +11,13 @@ const ( ipTTL = 8 ) +type tcpSegment struct { + seq uint32 + hdr int + payload int + push bool +} + // coalescer merges in-order TCP segments of one flow into a GSO frame for the guest, as kernel GRO does; only the receive goroutine uses it. type coalescer struct { l *link @@ -105,13 +112,6 @@ func (c *coalescer) finish(frame []byte) { binary.LittleEndian.PutUint16(frame[8:], 16) } -type tcpSegment struct { - seq uint32 - hdr int - payload int - push bool -} - // segmentOf accepts a data segment with only ACK or ACK+PSH set and a valid checksum; GRO never vouches for a checksum it did not check. func segmentOf(frame []byte) (tcpSegment, bool) { ip := frame[VnetHdrLen:] diff --git a/sandboxd/tunnet/udp/udp_linux.go b/sandboxd/tunnet/udp/udp_linux.go index 8c45bcae8..bab2e80fe 100644 --- a/sandboxd/tunnet/udp/udp_linux.go +++ b/sandboxd/tunnet/udp/udp_linux.go @@ -22,6 +22,13 @@ const ( var errGSORefused = unix.EIO +// mmsghdr is the kernel's struct mmsghdr. +type mmsghdr struct { + hdr unix.Msghdr + n uint32 + _ [4]byte +} + // receiver reads up to recvBatch datagrams per recvmmsg into buffers it owns, without allocating. type receiver struct { rc syscall.RawConn @@ -82,13 +89,6 @@ func (r *receiver) recvmmsg(fd uintptr) bool { } } -// mmsghdr is the kernel's struct mmsghdr. -type mmsghdr struct { - hdr unix.Msghdr - n uint32 - _ [4]byte -} - // segmentCmsg fills b with a control message asking the kernel to cut one send into seg-byte datagrams. func segmentCmsg(b *[cmsgCap]byte, seg int) []byte { h := (*unix.Cmsghdr)(unsafe.Pointer(&b[0])) //nolint:gosec // b holds a whole Cmsghdr From 2a38d5f01e98935f4ae3186daa865537bb8ae300 Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 13:03:37 +0800 Subject: [PATCH 35/40] test: sandboxd table and helper reuse, synctest for the address log, no redundant build tags --- sandboxd/config/config_test.go | 32 +++++++----------- sandboxd/egress/policy_test.go | 10 ------ sandboxd/engine/engine_test.go | 8 ++--- sandboxd/outbound/outbound_test.go | 2 +- sandboxd/server/server_test.go | 2 +- sandboxd/tunnet/addrs_test.go | 45 ++++++++++++++------------ sandboxd/tunnet/router_test.go | 42 ++++++++++-------------- sandboxd/tunnet/segment_test.go | 3 +- sandboxd/tunnet/udp/exit_linux_test.go | 2 -- sandboxd/tunnet/udp/udp_linux_test.go | 2 -- 10 files changed, 58 insertions(+), 90 deletions(-) diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index aed30dc6e..07284879a 100644 --- a/sandboxd/config/config_test.go +++ b/sandboxd/config/config_test.go @@ -419,21 +419,18 @@ func TestLoadAcceptsEgressPolicy(t *testing.T) { } } -func TestLoadAcceptsUnguardedCNINetwork(t *testing.T) { - path := writeConfig(t, `{"networks":["cni"],"pools":[{"template":"rt:24.04","net":"egress","size":"small"}]}`) - if _, err := Load(path); err != nil { - t.Fatalf("Load: %v", err) - } -} - -func TestLoadAcceptsNoneLanePolicyOnCNI(t *testing.T) { - for _, body := range []string{ - `{"networks":["cni"],"pools":[{"template":"rt:24.04","net":"none","size":"small","egress":{"allow":[{"host":"x"}]}}]}`, - `{"networks":["cni"],"pools":[{"template":"rt:24.04","net":"egress","size":"small"},{"template":"rt:24.04","net":"none","size":"medium","egress":{"allow":[{"host":"x"}]}}]}`, +func TestLoadAcceptsValidNetworkShapes(t *testing.T) { + for name, body := range map[string]string{ + "unguarded cni network": `{"networks":["cni"],"pools":[{"template":"rt:24.04","net":"egress","size":"small"}]}`, + "none-lane policy on cni": `{"networks":["cni"],"pools":[{"template":"rt:24.04","net":"none","size":"small","egress":{"allow":[{"host":"x"}]}}]}`, + "none-lane policy beside cni egress": `{"networks":["cni"],"pools":[{"template":"rt:24.04","net":"egress","size":"small"},{"template":"rt:24.04","net":"none","size":"medium","egress":{"allow":[{"host":"x"}]}}]}`, + "guarded egress on a bridges list": `{"bridges":["sbx0","sbx1"],"pools":[{"template":"rt:24.04","net":"egress","size":"small","egress":{"allow":[{"host":"x"}]}}]}`, } { - if _, err := Load(writeConfig(t, body)); err != nil { - t.Errorf("Load rejected a none-lane policy on CNI: %v", err) - } + t.Run(name, func(t *testing.T) { + if _, err := Load(writeConfig(t, body)); err != nil { + t.Errorf("Load: %v", err) + } + }) } } @@ -523,13 +520,6 @@ func TestLoadRejectsUnusableNetworkLists(t *testing.T) { } } -func TestLoadAcceptsGuardedEgressOnABridgesList(t *testing.T) { - path := writeConfig(t, `{"bridges":["sbx0","sbx1"],"pools":[{"template":"rt:24.04","net":"egress","size":"small","egress":{"allow":[{"host":"x"}]}}]}`) - if _, err := Load(path); err != nil { - t.Fatalf("guarded egress on a bridges list must load (taps stay in the root netns): %v", err) - } -} - func TestLoadAcceptsAShardedNetworkList(t *testing.T) { path := writeConfig(t, `{"networks":["cocoon-sbx0","cocoon-sbx1","cocoon-sbx2","cocoon-sbx3"],"pools":[]}`) cfg := mustLoad(t, path) diff --git a/sandboxd/egress/policy_test.go b/sandboxd/egress/policy_test.go index f53d9274e..1395e4b18 100644 --- a/sandboxd/egress/policy_test.go +++ b/sandboxd/egress/policy_test.go @@ -155,16 +155,6 @@ func TestInterceptsHostCoversPatterns(t *testing.T) { } } -func TestWildcardApexIsNotMatched(t *testing.T) { - policy := Policy{Allow: []Rule{{Host: "*.example.com"}}} - if _, d := policy.Eval("example.com", "GET", 443); d != DecisionDeny { - t.Error("apex example.com matched *.example.com, want deny") - } - if _, d := policy.Eval("a.example.com", "GET", 443); d != DecisionAllow { - t.Error("a.example.com did not match *.example.com, want allow") - } -} - func TestEvalFlowMatchesOnlyBareWildcardRules(t *testing.T) { p := Policy{Allow: []Rule{ {Host: "api.example.com", Ports: []uint16{443}}, diff --git a/sandboxd/engine/engine_test.go b/sandboxd/engine/engine_test.go index 4aeff2647..a9b39d713 100644 --- a/sandboxd/engine/engine_test.go +++ b/sandboxd/engine/engine_test.go @@ -221,16 +221,12 @@ func TestACanceledCommandGetsSIGTERMSoCocoonCanResumeTheVM(t *testing.T) { trace := filepath.Join(t.TempDir(), "trace") t.Setenv("COCOON_TEST_TRACE", trace) fakeCocoon(t, "#!/bin/sh\ntrap 'echo term >> \"$COCOON_TEST_TRACE\"; kill $!; exit 3' TERM\n"+ - "sleep 30 >/dev/null 2>&1 &\necho ready >> \"$COCOON_TEST_TRACE\"\nwait\n") + "sleep 30 >/dev/null 2>&1 &\n: > \"$COCOON_TEST_TRACE.ready\"\nwait\n") ctx, cancel := context.WithCancel(t.Context()) defer cancel() done := make(chan error, 1) go func() { done <- newTestEngine().Hibernate(ctx, "vm", "snap") }() - for deadline := time.Now().Add(5 * time.Second); time.Now().Before(deadline); time.Sleep(10 * time.Millisecond) { - if data, _ := os.ReadFile(trace); strings.Contains(string(data), "ready") { - break - } - } + waitFile(t, trace+".ready") cancel() if err := <-done; err == nil { t.Fatal("a canceled hibernate reported success") diff --git a/sandboxd/outbound/outbound_test.go b/sandboxd/outbound/outbound_test.go index b87b095b8..f34ca7e21 100644 --- a/sandboxd/outbound/outbound_test.go +++ b/sandboxd/outbound/outbound_test.go @@ -135,7 +135,7 @@ func TestArmEgressLocksEgressLaneWithoutPolicy(t *testing.T) { } } -func TestLockFallsBackToALookupForPreTapClaims(t *testing.T) { +func TestLockFallsBackToALookupForAClaimWithoutARecordedTap(t *testing.T) { eng := &fakeEngine{taps: map[string]string{"sbx-old": "tap-fake1"}} var taps outboundtest.TapLog h := newHost(t, eng, poolConfig(egKey, egPolicy), func(o *Options) { diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index b629cb317..0f530825d 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -828,7 +828,7 @@ func TestPutPoolsUpdatesTargets(t *testing.T) { } } -func TestReleaseAndHibernateMapEachOutcomeToItsStatus(t *testing.T) { +func TestReleaseHibernateAndWakeMapEachOutcomeToItsStatus(t *testing.T) { verbs := []struct { name string hook func(f *fakeManager, h sandboxVerbFunc) diff --git a/sandboxd/tunnet/addrs_test.go b/sandboxd/tunnet/addrs_test.go index b169ed749..fc576475b 100644 --- a/sandboxd/tunnet/addrs_test.go +++ b/sandboxd/tunnet/addrs_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "testing" + "testing/synctest" "time" ) @@ -74,29 +75,31 @@ func TestLocalSkipsHeldAddressesAndHoldsFreedOnesForTheReuseWindow(t *testing.T) } func TestLocalCompactsTheLogToTheLiveHolds(t *testing.T) { - dir := t.TempDir() - l, err := NewLocal(dir, netip.MustParsePrefix("10.1.0.0/16"), 20*time.Millisecond) - if err != nil { - t.Fatal(err) - } - churn := func(from, to int) { - for i := from; i < to; i++ { - id := fmt.Sprint(i) - if relErr := l.Release(t.Context(), id, assign(t, l, id)); relErr != nil { - t.Fatalf("release %d: %v", i, relErr) + synctest.Test(t, func(t *testing.T) { + dir := t.TempDir() + l, err := NewLocal(dir, netip.MustParsePrefix("10.1.0.0/16"), 20*time.Millisecond) + if err != nil { + t.Fatal(err) + } + churn := func(from, to int) { + for i := from; i < to; i++ { + id := fmt.Sprint(i) + if relErr := l.Release(t.Context(), id, assign(t, l, id)); relErr != nil { + t.Fatalf("release %d: %v", i, relErr) + } } } - } - churn(0, compactAfter-24) - time.Sleep(30 * time.Millisecond) - churn(compactAfter-24, compactAfter+100) - raw, err := os.ReadFile(filepath.Join(dir, freedFile)) - if err != nil { - t.Fatal(err) - } - if n := bytes.Count(raw, []byte("\n")); n > 200 { - t.Errorf("log holds %d lines after its expired holds were compacted away, want at most 200", n) - } + churn(0, compactAfter-24) + time.Sleep(30 * time.Millisecond) + churn(compactAfter-24, compactAfter+100) + raw, err := os.ReadFile(filepath.Join(dir, freedFile)) + if err != nil { + t.Fatal(err) + } + if n := bytes.Count(raw, []byte("\n")); n > 200 { + t.Errorf("log holds %d lines after its expired holds were compacted away, want at most 200", n) + } + }) } func TestLocalRefusesWhenTheSubnetIsFull(t *testing.T) { diff --git a/sandboxd/tunnet/router_test.go b/sandboxd/tunnet/router_test.go index 47c5ab3c3..76a1566bf 100644 --- a/sandboxd/tunnet/router_test.go +++ b/sandboxd/tunnet/router_test.go @@ -200,8 +200,7 @@ func TestRouterChargesSegmentsAndRefusesOversizedPackets(t *testing.T) { r.Arm(Claim{ID: "c1", Addr: claimAddr, Limits: Limits{PacketsPerSecond: 10}, Allow: func(netip.Addr, uint16) bool { return true }}) u.sent(t) go func() { - for s := range u.out { - _ = s + for range u.out { } }() guest := serve(t, r, claimAddr) @@ -244,17 +243,7 @@ func TestRouterHandsAnICMPErrorBackWithTheQuotedGuestAddress(t *testing.T) { guestHello(t, guest) sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) inner, _, _ := u.sent(t) - icmp := make([]byte, 20+8+28) - icmp[0], icmp[8], icmp[9] = 0x45, 64, ProtoICMP - binary.BigEndian.PutUint16(icmp[2:], uint16(len(icmp))) - s, d := remote.As4(), claimAddr.As4() - copy(icmp[srcAt:], s[:]) - copy(icmp[dstAt:], d[:]) - binary.BigEndian.PutUint16(icmp[10:], ^fold(onesSum(icmp[:20], 0))) - icmp[20], icmp[21] = icmpUnreachable, 4 - copy(icmp[28:], inner[:28]) - binary.BigEndian.PutUint16(icmp[22:], ^fold(onesSum(icmp[20:], 0))) - u.inject(t, append(appendHeader(nil, 7, MAC(claimAddr), exitMAC, etherIPv4), icmp...)) + u.inject(t, append(appendHeader(nil, 7, MAC(claimAddr), exitMAC, etherIPv4), unreachable(remote, claimAddr, 4, inner[:28])...)) got := readFrame(t, guest) ip := got[VnetHdrLen:] if netip.AddrFrom4([4]byte(ip[28+srcAt:])) != GuestAddr || fold(onesSum(ip[20:], 0)) != 0xffff || fold(onesSum(ip[28:48], 0)) != 0xffff || !validIP(got) { @@ -349,18 +338,7 @@ func TestRouterPointsAGuestICMPErrorAtTheClaimAddress(t *testing.T) { guest := serve(t, r, claimAddr) guestHello(t, guest) quoted := frame(ProtoUDP, remote, GuestAddr, false)[VnetHdrLen:][:28] - icmp := make([]byte, VnetHdrLen+20+8+28) - ip := icmp[VnetHdrLen:] - ip[0], ip[8], ip[9] = 0x45, 64, ProtoICMP - binary.BigEndian.PutUint16(ip[2:], uint16(len(ip))) - s, d := GuestAddr.As4(), remote.As4() - copy(ip[srcAt:], s[:]) - copy(ip[dstAt:], d[:]) - binary.BigEndian.PutUint16(ip[10:], ^fold(onesSum(ip[:20], 0))) - ip[20], ip[21] = icmpUnreachable, 3 - copy(ip[28:], quoted) - binary.BigEndian.PutUint16(ip[22:], ^fold(onesSum(ip[20:], 0))) - sendFrame(t, guest, icmp) + sendFrame(t, guest, append(make([]byte, VnetHdrLen), unreachable(GuestAddr, remote, 3, quoted)...)) inner, _, _ := u.sent(t) if got := netip.AddrFrom4([4]byte(inner[28+dstAt:])); got != claimAddr || fold(onesSum(inner[20:], 0)) != 0xffff { t.Errorf("quoted destination %v, icmp checksum %#x; want the claim address and a valid checksum", got, fold(onesSum(inner[20:], 0))) @@ -450,6 +428,20 @@ func dataSegment(seq uint32, payload int) []byte { return append(appendHeader(nil, 7, MAC(claimAddr), exitMAC, etherIPv4), ip...) } +func unreachable(src, dst netip.Addr, code byte, quoted []byte) []byte { + ip := make([]byte, 20+8+28) + ip[0], ip[8], ip[9] = 0x45, 64, ProtoICMP + binary.BigEndian.PutUint16(ip[2:], uint16(len(ip))) + s, d := src.As4(), dst.As4() + copy(ip[srcAt:], s[:]) + copy(ip[dstAt:], d[:]) + binary.BigEndian.PutUint16(ip[10:], ^fold(onesSum(ip[:20], 0))) + ip[20], ip[21] = icmpUnreachable, code + copy(ip[28:], quoted) + binary.BigEndian.PutUint16(ip[22:], ^fold(onesSum(ip[20:], 0))) + return ip +} + func waitFor(t *testing.T, ok func() bool) { t.Helper() for deadline := time.Now().Add(2 * time.Second); time.Now().Before(deadline); time.Sleep(5 * time.Millisecond) { diff --git a/sandboxd/tunnet/segment_test.go b/sandboxd/tunnet/segment_test.go index 0f231909d..bbc95bdfe 100644 --- a/sandboxd/tunnet/segment_test.go +++ b/sandboxd/tunnet/segment_test.go @@ -3,6 +3,7 @@ package tunnet import ( "bytes" "encoding/binary" + "slices" "testing" ) @@ -10,7 +11,7 @@ func TestSegmentSplitsTCPAtGSOSizeWithValidHeaders(t *testing.T) { f := gsoFrame(ProtoTCP, 5000, 1400) ip := f[VnetHdrLen:] ip[20+13] = tcpFin | tcpPsh | tcpCwr - payload := append([]byte(nil), ip[40:]...) + payload := slices.Clone(ip[40:]) got := segments(f) if len(got) != 4 { t.Fatalf("segments = %d, want 4", len(got)) diff --git a/sandboxd/tunnet/udp/exit_linux_test.go b/sandboxd/tunnet/udp/exit_linux_test.go index 89e35b720..674051b55 100644 --- a/sandboxd/tunnet/udp/exit_linux_test.go +++ b/sandboxd/tunnet/udp/exit_linux_test.go @@ -1,5 +1,3 @@ -//go:build linux - package udp_test import ( diff --git a/sandboxd/tunnet/udp/udp_linux_test.go b/sandboxd/tunnet/udp/udp_linux_test.go index c1938a32d..c48aee670 100644 --- a/sandboxd/tunnet/udp/udp_linux_test.go +++ b/sandboxd/tunnet/udp/udp_linux_test.go @@ -1,5 +1,3 @@ -//go:build linux - package udp import ( From 1f876d1f834cab025a337bca5230e8b1f770755b Mon Sep 17 00:00:00 2001 From: CMGS Date: Fri, 6 Nov 2026 13:04:42 +0800 Subject: [PATCH 36/40] docs: match the code after the audit fixes, cut release-scoped upgrade notes and duplicates Drift the readers found is fixed against the code (each change cites its line in the round's ledger): tun traffic resets the idle clock, downtime_ms includes the dump-slot wait, a disarm only stages the tun quota counter, a promote of a claim whose tun is off answers 409, the tun pool flag is config-owned, and the shared tun address table gets its DDL. Upgrade notes for releases older than any running fleet, the duplicated /v1/info payload and stale version history go; silkd's README lists the DNS relay, tun verbs and IMDS. --- docs/advanced-network.md | 21 ++++--- docs/benchmarks.md | 3 +- docs/cluster.md | 34 ++++------- docs/deploy.md | 118 +++++++++++++++---------------------- docs/e2b.md | 3 +- docs/egress.md | 10 +--- docs/performance.md | 4 +- docs/sandboxd-api.md | 32 ++++------ docs/sdk-python.md | 28 +++++++-- docs/sdk.md | 45 ++++---------- docs/security.md | 13 ++-- docs/silkd.md | 13 ++-- docs/volume-controller.md | 4 +- mcp/README.md | 5 +- os-image/android/README.md | 5 +- protocol/README.md | 4 +- sdk/go/README.md | 8 ++- silkd/README.md | 14 +++-- 18 files changed, 168 insertions(+), 196 deletions(-) diff --git a/docs/advanced-network.md b/docs/advanced-network.md index 0202352a7..f23a12585 100644 --- a/docs/advanced-network.md +++ b/docs/advanced-network.md @@ -25,8 +25,8 @@ This page is the contract between silkd and the host. The host side design is Both modes run at once. silkd keeps the proxy variables in every exec, so a client that honours them still reaches `127.0.0.1:3128` over loopback; all other traffic takes the default route through `tun0`. Loopback never enters -`tun0`, so the guest DNS stub of #472 is unaffected; the tun path does not -intercept DNS. +`tun0`, so the [guest DNS relay](egress.md#name-resolution-dns) on +`127.0.0.153` is unaffected; the tun path does not intercept DNS. ### Turning it on and off @@ -81,7 +81,8 @@ Offload bits are the kernel's `TUN_F_*` values: `0x01` CSUM, `0x02` TSO4, `0x04` TSO6, `0x08` TSO_ECN, `0x20` USO4, `0x40` USO6. Each side probes its kernel (USO needs Linux 6.2) and then enables, with `TUNSETOFFLOAD`, only the offloads both hellos carry; segmentation offloads are dropped when CSUM is not -common. A side never builds a frame whose GSO type the peer did not offer; +common, and USO is enabled only as the USO4 and USO6 pair. The host offers +USO4 alone, so a guest never enables USO. A side never builds a frame whose GSO type the peer did not offer; packets its kernel queued before a reconnect keep their old type, and a receiver may drop one it cannot take. @@ -117,7 +118,6 @@ lost; TCP retransmits. sandboxd needs no TUN, no kernel routes and no NAT rules on the node. It carries each claim's packets over VXLAN, on an ordinary UDP socket, to an **exit**: plain Linux configuration on the same host or on a gateway machine. -The design is [#471](https://github.com/cocoonstack/sandbox/issues/471). ``` guest -> tun0 -> silkd --vsock 2051--> sandboxd --VXLAN--> exit (vxlan0, forward, NAT) -> uplink @@ -129,7 +129,7 @@ guest -> tun0 -> silkd --vsock 2051--> sandboxd --VXLAN--> exit (vxlan0, forward disk write on the claim path; each release is appended to `/tun-freed.log` so its reuse window outlives a restart. A node with a `mesh` must have `meta_store.cell` and allocates it in the cell's - table, one round trip per claim, and stays off (proxy mode only) while the + [table](deploy.md#shared-tun-address-table), one round trip per claim, and stays off (proxy mode only) while the meta store is unreachable. A freed address is reused after `tun.reuse_after_seconds`, so the subnet's size over that window bounds the sustained claim rate: a `/8` with the 5-day default carries about 38 new tun @@ -180,13 +180,16 @@ guest -> tun0 -> silkd --vsock 2051--> sandboxd --VXLAN--> exit (vxlan0, forward usage event with its IP bytes both ways, the destination `ip:port` as its reference, and `upstream` `tun/