Skip to content

Latest commit

 

History

History
144 lines (127 loc) · 9.22 KB

cleanup.md

File metadata and controls

144 lines (127 loc) · 9.22 KB

Cleanup priorities (review of changes since origin/main)

The recent commits add tailnet TCP fallback forwarding, repair fork/undo history, rework session teardown, and make real e2e tests faster with keepers, pooled SSH transports, cached BusyBox installation, VMX-once preparation and selective btrfs cleanup. The fixes are useful, but the lifecycle boundary matters: one VMX can contain multiple frames; one frame can have multiple live sessions. A transport is not a session, and a session is not the lifetime of its VM.

P0 — correctness / concurrent lifecycle

  1. [DONE] Make VMX exit and release transitions atomic (cmd/thundersnapd/main.go, vmxSessionManager). The exit-monitor goroutine unconditionally deletes its key when an old VM exits, even if getOrCreateVMX has already replaced that entry. releaseVMX also calls blocking session.Close() under the global manager mutex. Adopt an identity-checked, stopping/stopped transition like the control-server manager: only remove the matching instance, wait for same-key shutdown without locking out unrelated VMXs, and ensure each acquisition releases the instance it acquired, not whichever VM later occupies its key. Test simultaneous last-release, VM exit/panic, and new sessions (including sessions in two different frames in one VMX).

  2. [DONE] Fix the process-wide working-directory change in control-server startup (cmd/thundersnapd/main.go, startControlServer). It calls os.Chdir to shorten a Unix socket address while other session goroutines can use relative paths; os.Chdir(cwd) is also unchecked. Bind through a directory fd or a stable short path without changing the daemon's cwd. Cover concurrent control-server creation in different frames and failure paths.

  3. [DONE] Harden VM vsock handler replacement and connection shutdown (thundersnap/vm.go). SetControlHandler writes controlHandler on VMX reuse while handleVsockConnection reads it without synchronization. Use an immutable shared handler or an atomic/locked replacement. Track in-flight vsock requests when closing a VM: listener closure alone does not wait for the per-connection goroutines. Add a race-enabled test with concurrent requests and reused VMX sessions.

  4. [DONE] Audit frame deletion and control-plane session ownership (cmd/thundersnapd/main.go, session.go, containerns/containerns.go). A refcounted control server can serve multiple sessions in the same frame; deleting or undoing a frame must account for all of them, and tearing down one frame must not destroy the VMX that still serves others. In particular, handleEnter constructs a target path from an unvalidated UUID/relative string and prepares it before acquiring its control-server ref. Restrict it to a validated frame ID under the caller's namespace, reject traversal and missing/deleted frames, and test authorization and concurrent enter/delete. Prefer typed frame/session handles to implicit path-keyed ownership. Progress: /enter now validates canonical frame UUIDs under the caller's namespace, rejects absent targets, and acquires the target control-server ref before preparation. Delete now reserves frame ownership against new sessions, rejects active sessions, and closes its control server before removing data. Race/behavior tests pass; authorization and enter/delete end-to-end coverage remain useful follow-ups.

P1 — resource bounds / operational recovery

  1. [DONE] Bound control and forward connections as well as their sessions. The tailnet forwarding cap counts active accepted connections but uses a load-then-increment check (cmd/thundersnapd/main.go), so concurrent arrivals can exceed the configured maximum. Reserve a slot atomically (or use a semaphore) and release it on every dial/proxy exit. Test simultaneous arrivals, failed backend dials and half-closes. Apply connection deadlines and request-size limits to control-socket/vsock request parsing; the vsock response writer buffers its whole response in memory (thundersnap/vm.go). Progress: atomic forwarding reservations; 30s control-handshake/HTTP-read deadlines; 1 MiB request limits; bounded 8 MiB vsock response buffering.

  2. [~] Recover orphaned snapshots and stale sessions after abnormal exits. As already tracked in TODO.md, .tmp btrfs snapshots and sidecars survive a daemon crash; clean them on startup without touching active jobs. Also investigate the reported ts go -c frontend-kill wedge and provide a non-destructive stop/reap operation. First reproduce against current teardown code: a client stdin EOF and a disconnected transport have different meanings; do not kill a live session merely because stdin closed. Add crash/restart and disconnect tests for both host and VMX paths. Progress: startup cleans orphan .tmp captures; EOF behavior is implemented. make test, current make e2e, and make not_e2e pass. Frontend-disconnect recovery and a non-destructive stop/reap command remain.

  3. [~] Complete non-interactive ts go stdin and image hygiene (existing TODO.md items). An explicit empty stdin frame for -c prevents stdin readers from hanging; expired docker-image accounts can make su prompt indefinitely. Fix both, verify actual command exit and frame reusability, and separately assess pipe-mode Setsid isolation. Preserve interactive stdin and PTY behavior. Progress: stdin EOF and account-aging fixes are implemented; the e2e EOF assertion is present. Full make e2e passes; direct expired-account end-to-end coverage remains useful.

  4. [~] Define temporary-file ownership and retention. TODO.md documents unbounded /tmp files and unwanted snapshot indexing. Decide whether temp data belongs to a command, frame namespace, or persisted snapshot; do not clear /tmp when an individual session ends while another session in that frame may still be using it. Bound log retention and exclude temporary content from content-addressable publication before opting into tmpfs. Progress: /tmp remains frame-owned and is not cleared per session; TSM indexing excludes it and MCP logs older than 30 days are pruned at new-job allocation. tmpfs and broader temporary-file ownership policy remain open.

P2 — test quality and maintenance

  1. [~] Replace implicit e2e keeper heuristics with explicit frame lifecycle (e2e/ssh_test.go). strings.Contains(cmd, "delete") both closes unrelated keepers and misses destructive operations named differently; immediately calling ensureKeeper can recreate a keeper before the destructive command. Key warm sessions by resolved frame identity rather than SSH user spelling; close the target keeper and wait for teardown before deletion. Pool only SSH transports across commands, continue opening one SSH channel/vshd session per command, and leave interactive sessions caller-owned. Test two frames sharing a VMX, multiple sessions on one frame, deletion and reconnect.

  2. [~] Broaden real-e2e assertions without reintroducing per-command overhead. Completed: /home//work owner checks, actual ts undo -c at the fork point, corrupt snap-listing, and 1–3-component snap progress tests. The full make e2e passes. Functional setuid execution, nested cgroup/namespace coverage, real tailnet fallback integration, and remaining VM negatives are still open. The forward test (e2e/forward_test.go) exercises the production loopback proxy through a test listener, but not tsnet's real fallback dispatch or privileged-port refusal; keep unit coverage for those and add an isolated tailnet integration check when practical. Assert the connection cap under concurrency. Add the already-tracked /home//work ownership, actual ts undo -c at the fork point, corrupt snap-listing, functional setuid, nested cgroup/namespace, and remaining deep VM negatives from TODO.md. Drive frame writes through SSH/SFTP rather than host-side fixture edits.

  3. [DONE] Make e2e cleanup resilient and keep its performance gain (e2e/e2e_test.go). Inode 256 selection avoids subprocesses for ordinary directories, but cleanup currently stops at the first error, potentially leaving later subvolumes that testing.T cannot remove. Collect errors while attempting all deepest-first deletions; keep failures visible (never skip e2e prerequisites). Avoid extra SSH handshakes or simulated shells when adding test coverage. Keep make e2e comfortably below its two-minute timeout rather than relying on a borderline pass.

  4. [~] Finish smaller protocol and packaging follow-ups from TODO.md, TODO-mcp.md, TODO.vsock.md, and TODO.e2e-on-nix.md. SFTP atime/mtime and 1–3-component snap progress parsing are complete. MCP policy and coverage, consolidation of internal control on the intended Unix-socket path, and remaining VM/Nix e2e prerequisites are still open. Prioritize these after lifecycle races and unbounded-resource paths.

For implementation, keep fixes separate by subsystem and run make test, make e2e, then make not_e2e after each change. Do not turn missing e2e prerequisites into skips.