diff --git a/.github/workflows/silkd.yml b/.github/workflows/silkd.yml index be5575cce..75640c1c7 100644 --- a/.github/workflows/silkd.yml +++ b/.github/workflows/silkd.yml @@ -36,4 +36,4 @@ jobs: - name: root-only tests working-directory: silkd - run: sudo -E env "PATH=$PATH" "HOME=$HOME" cargo test --locked --test exec_e2e --test git_e2e --test fs_e2e --test find_e2e --test tun_e2e -- --ignored --skip throughput + run: sudo -E env "PATH=$PATH" "HOME=$HOME" cargo test --locked --test exec_e2e --test git_e2e --test fs_e2e --test find_e2e --test tun_e2e -- --ignored diff --git a/e2e/internal/harness/harness_test.go b/e2e/internal/harness/harness_test.go index 29d3292d0..980a0c205 100644 --- a/e2e/internal/harness/harness_test.go +++ b/e2e/internal/harness/harness_test.go @@ -10,6 +10,7 @@ import ( "net/http" "strings" "testing" + "testing/synctest" "time" ) @@ -53,29 +54,31 @@ func TestPortRelayPreservesGreetingAndHalfClose(t *testing.T) { } } -func TestPoll(t *testing.T) { - errTry := errors.New("not yet") +func TestPollRetriesUntilSuccessThenReturnsLastErrorOrContextError(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + errTry := errors.New("not yet") - calls := 0 - if err := Poll(t.Context(), time.Second, time.Millisecond, func() error { - calls++ - if calls < 3 { - return errTry + calls := 0 + if err := Poll(t.Context(), time.Second, time.Millisecond, func() error { + calls++ + if calls < 3 { + return errTry + } + return nil + }); err != nil || calls != 3 { + t.Errorf("success: err %v calls %d, want nil and 3", err, calls) } - return nil - }); err != nil || calls != 3 { - t.Errorf("success: err %v calls %d, want nil and 3", err, calls) - } - if err := Poll(t.Context(), 5*time.Millisecond, time.Millisecond, func() error { return errTry }); !errors.Is(err, errTry) { - t.Errorf("exhausted: err %v, want the last try error", err) - } + if err := Poll(t.Context(), 5*time.Millisecond, time.Millisecond, func() error { return errTry }); !errors.Is(err, errTry) { + t.Errorf("exhausted: err %v, want the last try error", err) + } - ctx, cancel := context.WithCancel(t.Context()) - cancel() - if err := Poll(ctx, time.Minute, time.Minute, func() error { return errTry }); !errors.Is(err, context.Canceled) { - t.Errorf("canceled: err %v, want context.Canceled", err) - } + ctx, cancel := context.WithCancel(t.Context()) + cancel() + if err := Poll(ctx, time.Minute, time.Minute, func() error { return errTry }); !errors.Is(err, context.Canceled) { + t.Errorf("canceled: err %v, want context.Canceled", err) + } + }) } func serveGuestFirstHalfClose(listener net.Listener) error { diff --git a/e2e/volume_mutation_test.go b/e2e/volume_mutation_test.go index eb31e207d..a958b5c1e 100644 --- a/e2e/volume_mutation_test.go +++ b/e2e/volume_mutation_test.go @@ -27,6 +27,8 @@ func TestLiveVolumesEndToEnd(t *testing.T) { } if err := sb.Hibernate(t.Context()); err == nil { t.Fatal("captured an attached volume") + } else if refusal, ok := errors.AsType[*sandbox.APIError](err); !ok || refusal.Status != http.StatusConflict { + t.Fatalf("attached capture refusal: %v", err) } for range 2 { got, err := sb.SetVolumes(t.Context(), []sandbox.Volume{}) diff --git a/e2e/wire_parity_test.go b/e2e/wire_parity_test.go index 727a7e551..1a8c5a7b8 100644 --- a/e2e/wire_parity_test.go +++ b/e2e/wire_parity_test.go @@ -92,6 +92,13 @@ func TestSDKStructsMatchServerWireShape(t *testing.T) { t.Errorf("%s: field %q is in the SDK but not on the server", tt.sdk, name) } } + for _, name := range tt.allow { + _, onServer := want[name] + _, inSDK := got[name] + if !onServer && !inSDK { + t.Errorf("%s: allowlisted field %q is on neither side", tt.sdk, name) + } + } }) } } diff --git a/mcp/server_test.go b/mcp/server_test.go index 8febe938d..eb9afac0b 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -27,7 +27,7 @@ var infoFrame = func() string { return string(frame) + "\n" }() -func TestServeSpeaksMCP(t *testing.T) { +func TestServeAnswersEveryRequestAndSkipsNotifications(t *testing.T) { srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) bool { if !strings.HasSuffix(r.URL.Path, "/checkpoint") { return false @@ -177,13 +177,13 @@ func TestToolRepliesReplaceInvalidUTF8(t *testing.T) { } func TestCloseBoxesWaitsForEveryReleaseOnce(t *testing.T) { - var releases atomic.Int32 - var arrived sync.WaitGroup - arrived.Add(3) + var requests, releases atomic.Int32 + arrived := make(chan struct{}, 8) gate := make(chan struct{}) node := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if strings.HasSuffix(r.URL.Path, "/release") { - arrived.Done() + requests.Add(1) + arrived <- struct{}{} <-gate releases.Add(1) } @@ -211,9 +211,14 @@ func TestCloseBoxesWaitsForEveryReleaseOnce(t *testing.T) { } <-called <-called - arrived.Wait() + for range 3 { + <-arrived + } close(gate) wg.Wait() + if got := requests.Load(); got != 3 { + t.Errorf("release requests = %d, want 3 (one per box across both callers)", got) + } } func TestExecRejectsAnEmptyCommand(t *testing.T) { diff --git a/sandboxd/archives/archivestest/archivestest.go b/sandboxd/archives/archivestest/archivestest.go index 21fa63370..f9fad3868 100644 --- a/sandboxd/archives/archivestest/archivestest.go +++ b/sandboxd/archives/archivestest/archivestest.go @@ -15,6 +15,7 @@ import ( const ( racers = 8 + liveLease = 30 * time.Second shortLease = 300 * time.Millisecond leaseWait = 10 * time.Second ) @@ -518,7 +519,7 @@ func testReplace(t *testing.T, r archives.Registry) { func testTakeLease(t *testing.T, r archives.Registry) { ctx := t.Context() first := mustPut(t, r, record("sb_1", "a", time.Time{})) - if err := r.Renew(ctx, "a", shortLease); err != nil { + if err := r.Renew(ctx, "a", liveLease); err != nil { t.Fatalf("renew: %v", err) } if _, err := r.Take(ctx, "sb_1", first, "b"); !errors.Is(err, archives.ErrLost) { @@ -531,6 +532,9 @@ func testTakeLease(t *testing.T, r archives.Registry) { if err != nil || own <= first { t.Fatalf("the owner's own take with a live lease: %d %v, want a new epoch", own, err) } + if err = r.Renew(ctx, "a", shortLease); err != nil { + t.Fatalf("renew short: %v", err) + } eventually(t, "the owner's lease to expire", func() bool { _, err = r.Take(ctx, "sb_1", own, "b") return err == nil @@ -597,13 +601,16 @@ func testSweepableLease(t *testing.T, r archives.Registry) { past := time.Now().Add(-time.Minute) mustPut(t, r, record("sb_live", "a", past)) mustPut(t, r, record("sb_dead", "z", past)) - if err := r.Renew(ctx, "a", shortLease); err != nil { + if err := r.Renew(ctx, "a", liveLease); err != nil { t.Fatalf("renew: %v", err) } got, err := r.Sweepable(ctx, time.Now(), "me") if err != nil || len(got) != 1 || got[0].ID != "sb_dead" { t.Fatalf("sweepable while a's lease is live: %+v %v, want sb_dead only", got, err) } + if err = r.Renew(ctx, "a", shortLease); err != nil { + t.Fatalf("renew short: %v", err) + } eventually(t, "a's lease to expire", func() bool { got, err = r.Sweepable(ctx, time.Now(), "me") return err == nil && len(got) == 2 diff --git a/sandboxd/ca_test.go b/sandboxd/ca_test.go index ebf17dd30..31d511f41 100644 --- a/sandboxd/ca_test.go +++ b/sandboxd/ca_test.go @@ -14,6 +14,9 @@ func TestWriteCAFilesRefusesOverwrite(t *testing.T) { if err := writeCAFiles(dir, "root", []byte("cert2"), []byte("key2"), false); err == nil { t.Error("second write without -force overwrote an existing key") } + if got, err := os.ReadFile(filepath.Join(dir, "root.key")); err != nil || string(got) != "key" { + t.Errorf("root.key after the refused write = %q, %v; want the original key", got, err) + } if err := writeCAFiles(dir, "root", []byte("cert2"), []byte("key2"), true); err != nil { t.Errorf("-force write: %v", err) } diff --git a/sandboxd/config/client_endpoint_test.go b/sandboxd/config/client_endpoint_test.go index c407949c8..107216f10 100644 --- a/sandboxd/config/client_endpoint_test.go +++ b/sandboxd/config/client_endpoint_test.go @@ -2,7 +2,7 @@ package config import "testing" -func TestClientAdvertise(t *testing.T) { +func TestClientAdvertiseAcceptsAbsoluteHTTPOriginsOnly(t *testing.T) { for _, addr := range []string{"", "https://node.example", "https://node.example:8443/", "http://[::1]:7777"} { t.Run(addr, func(t *testing.T) { cfg := Config{ClientAdvertise: addr} diff --git a/sandboxd/config/config_test.go b/sandboxd/config/config_test.go index 63701f42b..aa16a8176 100644 --- a/sandboxd/config/config_test.go +++ b/sandboxd/config/config_test.go @@ -610,6 +610,13 @@ func TestLoadAcceptsAShardedNetworkList(t *testing.T) { } } +func TestPoolSpecRejectsEmptyWarmupArgument(t *testing.T) { + spec := PoolSpec{Template: "rt:24.04", Net: types.NetNone, Size: types.SizeSmall, Warm: 1, Warmup: []string{"node", ""}} + if err := spec.ValidateLimits(); err == nil || !strings.Contains(err.Error(), "warmup") { + t.Errorf("ValidateLimits error = %v, want an empty-argument rejection", err) + } +} + func writeConfig(t *testing.T, body string) string { t.Helper() path := filepath.Join(t.TempDir(), "config.json") diff --git a/sandboxd/egress/ca_test.go b/sandboxd/egress/ca_test.go index 687f9c1ee..266af5a9b 100644 --- a/sandboxd/egress/ca_test.go +++ b/sandboxd/egress/ca_test.go @@ -137,7 +137,7 @@ func TestLoadCARejectsNonCertBlockInRoot(t *testing.T) { } } -func TestFingerprintIsRootAndStable(t *testing.T) { +func TestFingerprintTracksOnlyTheRoot(t *testing.T) { rootCert, rootKey := testRoot(t, "root") c1, k1 := testIntermediate(t, rootCert, rootKey, "n1") c2, k2 := testIntermediate(t, rootCert, rootKey, "n2") diff --git a/sandboxd/egress/limit_test.go b/sandboxd/egress/limit_test.go index 2e72161ab..0838fc2fa 100644 --- a/sandboxd/egress/limit_test.go +++ b/sandboxd/egress/limit_test.go @@ -7,11 +7,14 @@ import ( "net" "net/http" "net/http/httptest" + "runtime" "sync" "sync/atomic" "testing" "time" + "golang.org/x/time/rate" + "github.com/cocoonstack/sandbox/sandboxd/types" ) @@ -43,7 +46,7 @@ func TestALimitCapsOneDirectionOfATunnel(t *testing.T) { if got.took < 700*time.Millisecond { t.Errorf("upload of two seconds' worth took %v under a limit with a one-second burst", got.took) } - if down > 500*time.Millisecond { + if down > 5*time.Second { t.Errorf("download took %v with only tx limited", down) } }) @@ -51,25 +54,23 @@ func TestALimitCapsOneDirectionOfATunnel(t *testing.T) { } func TestChangingTheLimitMovesAnOpenTunnel(t *testing.T) { - sink, received := sinkServer(t) - p, _, conn := openConnect(t, sink) - go writeUntilClosed(conn) - waitBytes(t, received, 1<<20) + addr, s := sinkServer(t) + p, _, conn := openConnect(t, addr) + if _, err := conn.Write(make([]byte, 1<<20)); err != nil { + t.Fatalf("write: %v", err) + } + waitBytes(t, s, 1<<20) p.SetLimit(types.EgressLimit{TxBytesPerSecond: testRate / 2}) - time.Sleep(500 * time.Millisecond) - before := received.Load() - time.Sleep(time.Second) - if moved := received.Load() - before; moved < testRate/4 || moved > testRate { - t.Errorf("moved %d bytes in the second after the limit dropped to %d/s", moved, testRate/2) + start := time.Now() + go writeUntilClosed(conn) + waitBytes(t, s, 1<<20+limitBurst+testRate/2) + if took := time.Since(start); took < 700*time.Millisecond { + t.Errorf("a burst and a second's worth past it took %v after the limit dropped to %d/s", took, testRate/2) } p.SetLimit(types.EgressLimit{}) - before = received.Load() - time.Sleep(300 * time.Millisecond) - if moved := received.Load() - before; moved < 4<<20 { - t.Errorf("moved %d bytes in 300ms after the limit was lifted", moved) - } + waitBytes(t, s, s.received.Load()+4<<20) } func TestChangingOneDirectionKeepsTheOtherBucket(t *testing.T) { @@ -92,15 +93,15 @@ func TestChangingOneDirectionKeepsTheOtherBucket(t *testing.T) { } func TestCloseReleasesAReadWaitingForTokens(t *testing.T) { - sink, received := sinkServer(t) + addr, s := sinkServer(t) rows := make(chan int64, 4) - p, _, conn := openConnect(t, sink, func(p *Proxy) { + p, _, conn := openConnect(t, addr, func(p *Proxy) { p.OnTransfer(func(_ Event, sent, _ int64) { rows <- sent }) }) p.SetLimit(types.EgressLimit{TxBytesPerSecond: 64}) go writeUntilClosed(conn) - waitBytes(t, received, limitBurst) - time.Sleep(100 * time.Millisecond) + waitBytes(t, s, limitBurst) + waitDebt(t, p.limits.Load().tx) start := time.Now() p.Close() @@ -287,6 +288,11 @@ type bulk struct { uploads chan upload } +type sink struct { + received atomic.Int64 + reads chan struct{} +} + func openConnect(t *testing.T, target string, edits ...proxyEdit) (*Proxy, io.Reader, net.Conn) { t.Helper() p := New(Policy{Allow: []Rule{{Host: "bulk.internal"}}}, nil, nil, fixedDial(target), nil, nil) @@ -382,14 +388,14 @@ func bulkServer(t *testing.T, down int) *bulk { return b } -func sinkServer(t *testing.T) (string, *atomic.Int64) { +func sinkServer(t *testing.T) (string, *sink) { t.Helper() ln, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { t.Fatalf("listen sink: %v", err) } t.Cleanup(func() { _ = ln.Close() }) - received := new(atomic.Int64) + s := &sink{reads: make(chan struct{}, 1)} go func() { conn, err := ln.Accept() if err != nil { @@ -399,22 +405,38 @@ func sinkServer(t *testing.T) (string, *atomic.Int64) { buf := make([]byte, 64<<10) for { n, err := conn.Read(buf) - received.Add(int64(n)) + s.received.Add(int64(n)) + select { + case s.reads <- struct{}{}: + default: + } if err != nil { return } } }() - return ln.Addr().String(), received + return ln.Addr().String(), s +} + +func waitBytes(t *testing.T, s *sink, want int64) { + t.Helper() + deadline := time.After(5 * time.Second) + for s.received.Load() < want { + select { + case <-s.reads: + case <-deadline: + t.Fatalf("sink got %d bytes, want %d", s.received.Load(), want) + } + } } -func waitBytes(t *testing.T, received *atomic.Int64, want int64) { +func waitDebt(t *testing.T, lim *rate.Limiter) { t.Helper() deadline := time.Now().Add(5 * time.Second) - for received.Load() < want { + for lim.Tokens() >= 0 { if time.Now().After(deadline) { - t.Fatalf("sink got %d bytes, want %d", received.Load(), want) + t.Fatalf("the bucket kept %v tokens", lim.Tokens()) } - time.Sleep(10 * time.Millisecond) + runtime.Gosched() } } diff --git a/sandboxd/egress/proxy_test.go b/sandboxd/egress/proxy_test.go index f0c86f26b..623b86662 100644 --- a/sandboxd/egress/proxy_test.go +++ b/sandboxd/egress/proxy_test.go @@ -11,6 +11,7 @@ import ( "net/http/httptest" "net/netip" "net/url" + "os" "strings" "sync/atomic" "testing" @@ -365,6 +366,7 @@ func TestCloseEndsSplicedTunnel(t *testing.T) { conn := dialConnect(t, front.Listener.Addr().String(), "echo.internal:443") defer func() { _ = conn.Close() }() + _ = conn.SetReadDeadline(time.Now().Add(5 * time.Second)) br := bufio.NewReader(conn) if status := readStatus(t, br); status != "HTTP/1.1 200 Connection Established" { t.Fatalf("CONNECT status = %q, want 200 Connection Established", status) @@ -380,8 +382,8 @@ func TestCloseEndsSplicedTunnel(t *testing.T) { p.Close() _, _ = io.WriteString(conn, "ping") - if _, err := io.ReadFull(br, got); err == nil { - t.Error("established spliced tunnel survived proxy Close") + if _, err := io.ReadFull(br, got); err == nil || errors.Is(err, os.ErrDeadlineExceeded) { + t.Errorf("established spliced tunnel survived proxy Close: %v", err) } } diff --git a/sandboxd/egress/socks_test.go b/sandboxd/egress/socks_test.go index 2d5116a3f..1f43da124 100644 --- a/sandboxd/egress/socks_test.go +++ b/sandboxd/egress/socks_test.go @@ -108,7 +108,7 @@ func TestSocksRefusesWhatItCannotServe(t *testing.T) { select { case ev := <-events: t.Errorf("refused handshake recorded %+v", ev) - default: + case <-time.After(100 * time.Millisecond): } }) } @@ -151,8 +151,9 @@ func TestCloseEndsSocksTunnelAndHandshake(t *testing.T) { p.Close() _, _ = io.WriteString(tunnel, "ping") - if _, err := io.ReadFull(tunnel, make([]byte, 4)); err == nil { - t.Error("established SOCKS tunnel survived proxy Close") + _ = tunnel.SetReadDeadline(time.Now().Add(5 * time.Second)) + if _, err := io.ReadFull(tunnel, make([]byte, 4)); err == nil || errors.Is(err, os.ErrDeadlineExceeded) { + t.Errorf("established SOCKS tunnel survived proxy Close: %v", err) } _ = pending.SetReadDeadline(time.Now().Add(2 * time.Second)) if _, err := pending.Read(make([]byte, 1)); err == nil || errors.Is(err, os.ErrDeadlineExceeded) { diff --git a/sandboxd/egress/splice_linux_test.go b/sandboxd/egress/splice_linux_test.go index d32f73084..5ea7c6974 100644 --- a/sandboxd/egress/splice_linux_test.go +++ b/sandboxd/egress/splice_linux_test.go @@ -8,6 +8,7 @@ import ( "net" "path/filepath" "runtime" + "runtime/metrics" "sync" "testing" "time" @@ -55,7 +56,7 @@ func TestSpliceCopiesThroughADoorConnInTheKernel(t *testing.T) { defer func() { _ = c.Close() }() buf := make([]byte, 64<<10) buffered <- struct{}{} - for n := 0; n < total; n += len(buf) { + for range total / len(buf) { _, _ = c.Write(buf) } _ = c.(*net.UnixConn).CloseWrite() @@ -83,18 +84,16 @@ func TestSpliceCopiesThroughADoorConnInTheKernel(t *testing.T) { tr := p.begin(Event{}, door, upstream) <-buffered <-buffered - runtime.GC() - var before, after runtime.MemStats - runtime.ReadMemStats(&before) + before := bufferAllocs() p.splice(tr) - runtime.ReadMemStats(&after) + allocs := bufferAllocs() - before <-guestDone up, down := tr.sent.Load(), tr.received.Load() if up != total || down != total { t.Fatalf("moved %d up and %d down, want %d each", up, down, total) } - if alloc := after.TotalAlloc - before.TotalAlloc; !tt.noSplice && alloc >= 32<<10 { - t.Errorf("splice allocated %d bytes: a door conn fell back to buffered copies", alloc) + if !tt.noSplice && allocs > 0 { + t.Errorf("splice made %d buffer-sized allocations: a door conn fell back to buffered copies", allocs) } }) } @@ -204,7 +203,21 @@ func pump(c net.Conn, total int, buffered chan<- struct{}) { return } } - for n := 0; n < total; n += len(buf) { + for range total / len(buf) { _, _ = c.Write(buf) } } + +func bufferAllocs() uint64 { + runtime.GC() + s := []metrics.Sample{{Name: "/gc/heap/allocs-by-size:bytes"}} + metrics.Read(s) + h := s[0].Value.Float64Histogram() + var n uint64 + for i, c := range h.Counts { + if h.Buckets[i] >= 16<<10 { + n += c + } + } + return n +} diff --git a/sandboxd/egress/upstream_test.go b/sandboxd/egress/upstream_test.go index dd3a3ddd5..f92398448 100644 --- a/sandboxd/egress/upstream_test.go +++ b/sandboxd/egress/upstream_test.go @@ -247,6 +247,16 @@ func TestTransferCountsTunnelAndRequestBytes(t *testing.T) { p := New(Policy{Allow: []Rule{{Host: "*"}}}, nil, nil, router, nil, nil) t.Cleanup(p.Close) p.OnTransfer(func(_ Event, sent, received int64) { got <- transfer{sent, received} }) + next := func() transfer { + t.Helper() + select { + case tr := <-got: + return tr + case <-time.After(5 * time.Second): + t.Fatal("no transfer row arrived") + return transfer{} + } + } srv := serveFront(t, p) conn := dialConnect(t, srv.Listener.Addr().String(), "tunnel.example:7") @@ -262,7 +272,7 @@ func TestTransferCountsTunnelAndRequestBytes(t *testing.T) { _ = conn.(*net.TCPConn).CloseWrite() _, _ = io.Copy(io.Discard, br) _ = conn.Close() - if tr := <-got; tr != (transfer{5, 5}) { + if tr := next(); tr != (transfer{5, 5}) { t.Errorf("tunnel transfer %+v, want 5 sent and 5 received", tr) } @@ -272,7 +282,7 @@ func TestTransferCountsTunnelAndRequestBytes(t *testing.T) { } _, _ = io.Copy(io.Discard, resp.Body) _ = resp.Body.Close() - if tr := <-got; tr != (transfer{11, 7}) { + if tr := next(); tr != (transfer{11, 7}) { t.Errorf("request transfer %+v, want 11 sent and 7 received", tr) } resp, err = proxyClient(t, srv.URL).Post(origin.URL+"/fail", "text/plain", strings.NewReader("eleven char")) @@ -283,7 +293,7 @@ func TestTransferCountsTunnelAndRequestBytes(t *testing.T) { if resp.StatusCode != http.StatusBadGateway { t.Errorf("failed post: %d, want 502", resp.StatusCode) } - if tr := <-got; tr != (transfer{11, 0}) { + if tr := next(); tr != (transfer{11, 0}) { t.Errorf("failed request transfer %+v, want 11 sent and 0 received", tr) } } diff --git a/sandboxd/engine/cloneargs_test.go b/sandboxd/engine/cloneargs_test.go index 24a790b35..6d477e60c 100644 --- a/sandboxd/engine/cloneargs_test.go +++ b/sandboxd/engine/cloneargs_test.go @@ -8,7 +8,7 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/types" ) -func TestCloneArgsRestoreMode(t *testing.T) { +func TestCloneAndRestoreArgsCarryTheRestoreModeOnlyWhenSet(t *testing.T) { noneKey := types.PoolKey{Template: "rt:24.04", Net: types.NetNone, Size: types.SizeMedium} egressKey := types.PoolKey{Template: "rt:24.04", Net: types.NetEgress, Size: types.SizeMedium} cases := []struct { diff --git a/sandboxd/engine/engine_test.go b/sandboxd/engine/engine_test.go index 8a81707b9..af62c9712 100644 --- a/sandboxd/engine/engine_test.go +++ b/sandboxd/engine/engine_test.go @@ -44,7 +44,7 @@ func TestDialSilkdConsumesOnlyHandshake(t *testing.T) { } } -func TestDialSilkdRejectedHandshake(t *testing.T) { +func TestDialSilkdSurfacesARejectedHandshake(t *testing.T) { path := sockPath(t) listenMuxer(t, path, "ERR no guest listener\n") @@ -105,7 +105,7 @@ func TestProbeRetriesPastFailures(t *testing.T) { } } -func TestDialGuestPortCtxCancel(t *testing.T) { +func TestDialGuestPortTimesOutWhenTheGuestPortNeverReplies(t *testing.T) { path := sockPath(t) ln, err := net.Listen("unix", path) if err != nil { @@ -342,13 +342,9 @@ func TestADialOfAGuestThatNeverAnswersFails(t *testing.T) { t.Fatalf("setup listener: %v", err) } t.Cleanup(func() { _ = ln.Close() }) - start := time.Now() if _, err := newTestEngine().DialSilkd(t.Context(), path); !errors.Is(err, context.DeadlineExceeded) { t.Errorf("dial: %v, want a deadline error", err) } - if elapsed := time.Since(start); elapsed > 2*connectTimeout { - t.Errorf("dial took %v, want about %v", elapsed, connectTimeout) - } } func TestMemoryDumpsRunAtMostEightAtOnce(t *testing.T) { diff --git a/sandboxd/engine/volume_live_test.go b/sandboxd/engine/volume_live_test.go index 4c0ea1726..d54c89291 100644 --- a/sandboxd/engine/volume_live_test.go +++ b/sandboxd/engine/volume_live_test.go @@ -1,6 +1,7 @@ package engine import ( + "cmp" "errors" "os" "path/filepath" @@ -26,9 +27,6 @@ func TestLiveVolumeConvergesMountAndDetach(t *testing.T) { } { t.Run(tt.name, func(t *testing.T) { sock, fake, spec := newLiveFake(t, "", "") - if err := os.Remove(os.Getenv("COCOON_TEST_ATTACHED")); err != nil { - t.Fatal(err) - } fake.set(func(f *fakeSilkd) { f.realpath, f.noRealpath = tt.realpath, tt.noRealpath f.execHook = func(argv []string) { @@ -151,7 +149,7 @@ func TestLiveDetachKeepsBusyAndUnobservableDevices(t *testing.T) { fake.execDrop = true case "guest error": fake.serial["/proc/self/mountinfo"] = []byte(rootMountInfo + "2 1 8:32 / /data ro - ext4 /dev/vdc ro\n") - fake.execErr = "guest connection failed /private/runtime" + fake.execErr = "guest connection failed" case "probe error": fake.readErr["/proc/self/mountinfo"] = "read failed" case "stacked mount": @@ -170,7 +168,7 @@ func TestLiveDetachKeepsBusyAndUnobservableDevices(t *testing.T) { if isConflict != conflictExpected { t.Fatalf("conflict=%v error=%v", conflictExpected, err) } - if conflictExpected && (!strings.Contains(conflict.Error(), strconv.Quote(wantPath)) || strings.Contains(conflict.Error(), "/private")) { + if conflictExpected && !strings.Contains(conflict.Error(), strconv.Quote(wantPath)) { t.Fatalf("unsafe or missing guest path %s: %v", wantPath, conflict) } if mode == "busy" && !strings.HasPrefix(conflict.Error(), "cannot unmount") { @@ -187,7 +185,7 @@ func TestLiveDetachKeepsBusyAndUnobservableDevices(t *testing.T) { func TestLiveDetachMissingGuestStillChecksHost(t *testing.T) { for _, hostErr := range []string{"", "vm not running", "backend unavailable"} { - t.Run(hostErr, func(t *testing.T) { + t.Run(cmp.Or(hostErr, "host ok"), func(t *testing.T) { sock := sockPath(t) serveFakeSilkd(t, sock) spec := liveDiskCLI(t, hostErr) @@ -318,22 +316,11 @@ func newLiveFake(t *testing.T, detachErr, mountinfo string) (string, *fakeSilkd, func liveDiskCLI(t *testing.T, detachErr string) VolumeSpec { t.Helper() - dir := t.TempDir() - image := filepath.Join(dir, "volume.img") - if err := os.WriteFile(image, nil, 0o600); err != nil { - t.Fatal(err) - } t.Setenv("COCOON_TEST_DETACH_ERROR", detachErr) - attached := filepath.Join(dir, "attached") - if err := os.WriteFile(attached, nil, 0o600); err != nil { - t.Fatal(err) - } - t.Setenv("COCOON_TEST_ATTACHED", attached) - t.Setenv("COCOON_TEST_ATTACH_ARGS", filepath.Join(dir, "args")) + t.Setenv("COCOON_TEST_ATTACH_ARGS", filepath.Join(t.TempDir(), "args")) fakeCocoon(t, `#!/bin/sh if [ "$1 $2 $3" = "vm disk attach" ]; then printf '%s\n' "$*" >> "$COCOON_TEST_ATTACH_ARGS" - touch "$COCOON_TEST_ATTACHED" exit 0 fi if [ "$1 $2 $3" = "vm disk detach" ]; then @@ -342,7 +329,7 @@ if [ "$1 $2 $3" = "vm disk detach" ]; then fi exit 2 `) - return VolumeSpec{Name: "imagenet", Path: image} + return VolumeSpec{Name: "imagenet", Path: "/srv/volumes/imagenet.img"} } func guestChanges(calls [][]string) [][]string { diff --git a/sandboxd/engine/volume_test.go b/sandboxd/engine/volume_test.go index ee40b25d2..804847bce 100644 --- a/sandboxd/engine/volume_test.go +++ b/sandboxd/engine/volume_test.go @@ -320,7 +320,7 @@ func TestMountVolumeNotReadyUntilDeadlineReportsTheTimeout(t *testing.T) { ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) defer cancel() err := newTestEngine().MountVolume(ctx, path, "imagenet", "/datasets/training", false) - if !errors.Is(err, context.DeadlineExceeded) || !strings.Contains(err.Error(), "mount volume imagenet") { + if !errors.Is(err, context.DeadlineExceeded) { t.Fatalf("got %v, want the mount deadline", err) } } @@ -373,14 +373,10 @@ func TestMountVolumeDevNodeNeverAppearsTimesOut(t *testing.T) { fake := serveFakeSilkd(t, path) configureVolumeDevices(fake) fake.set(func(f *fakeSilkd) { delete(f.stat, "/dev/vdc") }) - start := time.Now() err := newTestEngine().MountVolume(t.Context(), path, "imagenet", "/datasets/training", false) if !errors.Is(err, context.DeadlineExceeded) || !strings.Contains(err.Error(), "wait for volume device imagenet") { t.Errorf("got %v, want a bounded wait-for-device timeout", err) } - if elapsed := time.Since(start); elapsed >= volumeSetupTimeout { - t.Errorf("the probe took %v, want it to end before the %v setup budget", elapsed, volumeSetupTimeout) - } } func TestSilkdListAndReadAreBounded(t *testing.T) { diff --git a/sandboxd/main_test.go b/sandboxd/main_test.go index 633aa5611..57b04d394 100644 --- a/sandboxd/main_test.go +++ b/sandboxd/main_test.go @@ -1,6 +1,7 @@ package main import ( + "context" "errors" "net" "net/http" @@ -32,12 +33,31 @@ func TestServePreviewExitsWhenTheListenerCannotBind(t *testing.T) { } func TestServePreviewReturnsAfterShutdown(t *testing.T) { - srv := &http.Server{Addr: "127.0.0.1:0"} + serving := make(chan net.Addr, 1) + srv := &http.Server{Addr: "127.0.0.1:0", BaseContext: func(ln net.Listener) context.Context { + serving <- ln.Addr() + return t.Context() + }} done := make(chan struct{}) go func() { defer close(done) servePreview(t.Context(), srv) }() + var addr net.Addr + select { + case addr = <-serving: + case <-time.After(5 * time.Second): + t.Fatal("servePreview never started serving") + } + req, err := http.NewRequestWithContext(t.Context(), http.MethodGet, "http://"+addr.String()+"/", nil) + if err != nil { + t.Fatalf("build request: %v", err) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("request the running preview server: %v", err) + } + _ = resp.Body.Close() if err := srv.Shutdown(t.Context()); err != nil { t.Fatalf("shutdown: %v", err) } diff --git a/sandboxd/mesh/mesh_test.go b/sandboxd/mesh/mesh_test.go index 8511e2bbb..e292ca4d7 100644 --- a/sandboxd/mesh/mesh_test.go +++ b/sandboxd/mesh/mesh_test.go @@ -35,10 +35,13 @@ func TestMergeNeverOverwritesSelf(t *testing.T) { mergeStates(t, m, []NodeState{{NodeID: "a", Addr: "evil:9999", Epoch: 999, Pools: map[string]int{"k": 0}}}) - for _, st := range m.Members() { - if st.NodeID == "a" && (st.Addr != "a:7777" || st.Pools["k"] != 3) { - t.Errorf("self overwritten: %+v", st) - } + members := m.Members() + i := slices.IndexFunc(members, func(st NodeState) bool { return st.NodeID == "a" }) + if i < 0 { + t.Fatalf("self missing from members %+v", members) + } + if st := members[i]; st.Addr != "a:7777" || st.Pools["k"] != 3 { + t.Errorf("self overwritten: %+v", st) } } @@ -148,7 +151,7 @@ func TestAPeerRefusingClaimsIsNoRedirectTarget(t *testing.T) { } } -func TestCandidatesPowerOfTwo(t *testing.T) { +func TestCandidatesPicksTwoDistinctWarmPeers(t *testing.T) { m := newTestMesh(t, "a") mergeStates(t, m, []NodeState{ {NodeID: "b", Addr: "b:7777", Epoch: 1, Pools: map[string]int{"k": 1}}, diff --git a/sandboxd/outbound/claim_test.go b/sandboxd/outbound/claim_test.go index 98eb7fa27..c4f8a556f 100644 --- a/sandboxd/outbound/claim_test.go +++ b/sandboxd/outbound/claim_test.go @@ -71,10 +71,10 @@ func TestClaimsOnOnePoolLeaveThroughTheirOwnUpstreams(t *testing.T) { } pingPong(t, conn) } - if got := up1.targets(); len(got) != 1 || got[0] != origin { + if got := up1.targets(); !slices.Equal(got, []string{origin}) { t.Errorf("upstream 1 tunnels %v, want only the first claim's %s", got, origin) } - if got := up2.targets(); len(got) != 1 || got[0] != origin { + if got := up2.targets(); !slices.Equal(got, []string{origin}) { t.Errorf("upstream 2 tunnels %v, want only the second claim's %s", got, origin) } if direct.Load() != 1 { @@ -102,7 +102,7 @@ func TestATenantClaimUnderAClassDialsItsOwnUpstreamLikeARootClaim(t *testing.T) t.Fatalf("dial through the claim's own upstream: %v", err) } pingPong(t, conn) - if got := own.targets(); len(got) != 1 || got[0] != origin || classDefault.accepted.Load() != 0 { + if got := own.targets(); !slices.Equal(got, []string{origin}) || classDefault.accepted.Load() != 0 { t.Errorf("own upstream tunnels %v, class default saw %d: the claim value must win over the class default", got, classDefault.accepted.Load()) } r = newClaim(h, tenantClaim("socks5://192.0.2.1:1080"), nil, nil) diff --git a/sandboxd/outbound/dial.go b/sandboxd/outbound/dial.go index 883041508..4eac7f52d 100644 --- a/sandboxd/outbound/dial.go +++ b/sandboxd/outbound/dial.go @@ -58,30 +58,37 @@ func (h *Host) upstreamTarget(ctx context.Context, addr string) (netip.AddrPort, if err != nil { return netip.AddrPort{}, false, err } - var target netip.AddrPort - for _, ip := range ips { - ap := netip.AddrPortFrom(ip.Unmap(), uint16(port)) - internal, err := h.o.Verdict(ap.Addr(), ap.Port()) + for i, ip := range ips { + ips[i] = ip.Unmap() + internal, err := h.o.Verdict(ips[i], uint16(port)) if err != nil { return netip.AddrPort{}, false, err } if internal { - return ap, true, nil - } - if !target.IsValid() || target.Addr().Is6() && ap.Addr().Is4() { - target = ap + return netip.AddrPortFrom(ips[i], uint16(port)), true, nil } } + target := tunnelAddr(ips) if !target.IsValid() { return netip.AddrPort{}, false, fmt.Errorf("egress: %s resolves to no address", host) } - return target, false, nil + return netip.AddrPortFrom(target, uint16(port)), false, nil } func (h *Host) verdict(ip netip.Addr, port uint16) (bool, error) { return destVerdict(h.o.View().internalAllow, *h.self.Load(), ip, port) } +func tunnelAddr(ips []netip.Addr) netip.Addr { + if i := slices.IndexFunc(ips, netip.Addr.Is4); i >= 0 { + return ips[i] + } + if len(ips) == 0 { + return netip.Addr{} + } + return ips[0] +} + func newDialer(verdict VerdictFunc) *net.Dialer { return &net.Dialer{Control: func(_, address string, _ syscall.RawConn) error { ap, err := netip.ParseAddrPort(address) diff --git a/sandboxd/outbound/dial_test.go b/sandboxd/outbound/dial_test.go index 3bca90661..515d151f8 100644 --- a/sandboxd/outbound/dial_test.go +++ b/sandboxd/outbound/dial_test.go @@ -145,12 +145,22 @@ func TestTheHostGuardFollowsARefresh(t *testing.T) { } func TestTheTunnelTargetPrefersIPv4(t *testing.T) { - h := newHost(t, &fakeEngine{}, &config.Config{}, func(o *Options) { - o.Verdict = func(netip.Addr, uint16) (bool, error) { return false, nil } - }) - target, internal, err := h.upstreamTarget(t.Context(), "localhost:80") - if err != nil || internal || target != netip.MustParseAddrPort("127.0.0.1:80") { - t.Errorf("target %v internal %v err %v, want 127.0.0.1:80 over ::1", target, internal, err) + for name, tt := range map[string]struct { + ips []string + want string + }{ + "IPv4 after IPv6": {[]string{"::1", "127.0.0.1"}, "127.0.0.1"}, + "IPv6 only": {[]string{"::1"}, "::1"}, + } { + t.Run(name, func(t *testing.T) { + ips := make([]netip.Addr, 0, len(tt.ips)) + for _, ip := range tt.ips { + ips = append(ips, netip.MustParseAddr(ip)) + } + if got := tunnelAddr(ips); got != netip.MustParseAddr(tt.want) { + t.Errorf("tunnelAddr(%v) = %v, want %s", tt.ips, got, tt.want) + } + }) } } diff --git a/sandboxd/outbound/outbound_test.go b/sandboxd/outbound/outbound_test.go index f1c6db604..008e2859e 100644 --- a/sandboxd/outbound/outbound_test.go +++ b/sandboxd/outbound/outbound_test.go @@ -129,10 +129,15 @@ func TestArmEgressLocksEgressLaneWithoutPolicy(t *testing.T) { {PoolKey: testKey, Egress: &egress.Policy{Allow: []egress.Rule{{Host: "a.test"}}}}, {PoolKey: egKey}, }} - h := newHost(t, &fakeEngine{}, cfg, func(o *Options) { o.LockNIC = true }) - sb := &types.Sandbox{ID: "sb_eg_np", Key: egKey, VMName: "sbx-np-1"} - if armErr := h.Arm(t.Context(), sb); armErr == nil { - t.Fatal("policyless egress-lane claim must still lock the NIC, not skip it") + var taps outboundtest.TapLog + h := newHost(t, &fakeEngine{}, cfg, func(o *Options) { + o.LockNIC = true + o.Lock, o.Unlock = taps.Lock, taps.Unlock + }) + sb := &types.Sandbox{ID: "sb_eg_np", Key: egKey, VMName: "sbx-np-1", TAP: "tap-np"} + lockErr := h.Arm(t.Context(), sb) + if got := taps.Locked(); lockErr != nil || !slices.Equal(got, []string{"tap-np"}) { + t.Fatalf("policyless egress-lane claim locked %v (err %v), want tap-np", got, lockErr) } } @@ -310,13 +315,14 @@ func TestArmAppliesTheClaimsLimitAndSetEgressLimitMovesIt(t *testing.T) { return time.Since(start) } - if took := fetch(); took < 700*time.Millisecond { - t.Errorf("download under the armed limit took %v", took) + throttled := fetch() + if throttled < 700*time.Millisecond { + t.Errorf("download under the armed limit took %v", throttled) } limit.Store(&types.EgressLimit{}) h.SetEgressLimit(sb) - if took := fetch(); took > 300*time.Millisecond { - t.Errorf("download after the limit was lifted took %v", took) + if took := fetch(); took >= throttled/2 { + t.Errorf("download after the limit was lifted took %v, want under half the throttled %v", took, throttled) } limit.Store(&types.EgressLimit{RxBytesPerSecond: rate}) h.SetEgressLimit(sb) @@ -383,7 +389,6 @@ func TestAnArmNeverAppliesAnOlderCapThanASetterRead(t *testing.T) { } } -// newHost serves cfg's egress with the gh secret set to s3cr3t; its NIC locks succeed unless edit swaps them. func newHost(t *testing.T, eng *fakeEngine, cfg *config.Config, edit func(*Options)) *Host { t.Helper() v := NewView(cfg, testSecrets(t)) diff --git a/sandboxd/pool/archive_test.go b/sandboxd/pool/archive_test.go index 069971025..f3cfa5fe6 100644 --- a/sandboxd/pool/archive_test.go +++ b/sandboxd/pool/archive_test.go @@ -696,7 +696,14 @@ func TestIdleOnceSkipsArchived(t *testing.T) { hibBefore := eng.hibernateCount() backdate(m, sb, time.Hour) - sweepIdle(t, m) + sb.Transition.Lock() + m.idleOnce(t.Context()) + picked := m.sweeps.idle.Load() + sb.Transition.Unlock() + waitFor(t, func() bool { return !m.sweeps.idle.Load() }) + if picked { + t.Error("idle sweep picked an archived claim") + } if got := eng.hibernateCount(); got != hibBefore { t.Errorf("idle sweep hibernated an archived claim: %d->%d", hibBefore, got) } diff --git a/sandboxd/pool/checkpoint_test.go b/sandboxd/pool/checkpoint_test.go index e0b7461f8..22f6a8063 100644 --- a/sandboxd/pool/checkpoint_test.go +++ b/sandboxd/pool/checkpoint_test.go @@ -133,7 +133,7 @@ func TestCheckpointBranchTakesItsOwnMetadata(t *testing.T) { } } -func TestCheckpointRejectsABadTokenNameOrUnknownID(t *testing.T) { +func TestCheckpointRejectsABadTokenOrName(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) src := mustClaim(t, m, testKey) @@ -144,14 +144,6 @@ func TestCheckpointRejectsABadTokenNameOrUnknownID(t *testing.T) { if _, err := m.Checkpoint(t.Context(), src.ID, Cred{Token: src.Token}, "bad name", ""); !errors.Is(err, ErrBadName) { t.Errorf("bad name: %v, want ErrBadName", err) } - for _, id := range []string{"../../etc", "ck_zz", "", "ck_0011223344556677x"} { - if _, err := m.ClaimCheckpoint(t.Context(), id, ClaimOptions{TTL: time.Hour}); !errors.Is(err, ErrUnknownCheckpoint) { - t.Errorf("claim %q: %v, want ErrUnknownCheckpoint", id, err) - } - if err := m.DeleteCheckpoint(t.Context(), id, "", DeleteFleet); !errors.Is(err, ErrUnknownCheckpoint) { - t.Errorf("delete %q: %v, want ErrUnknownCheckpoint", id, err) - } - } } func TestReconcileSweepsCheckpointStaging(t *testing.T) { @@ -200,30 +192,32 @@ func TestRetentionSweepsExpiredCheckpoints(t *testing.T) { } func TestRunRefillsWhileTheRetentionSweepBlocks(t *testing.T) { - eng := newFakeEngine() - m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1}) - m.pools[testKey].goldenDir = "/goldens/x" - editView(m, func(v *configView) { v.ckptTTL = time.Hour }) - stall := &stallingMetasStore{Store: m.ckpts, entered: make(chan struct{}, 1), release: make(chan struct{})} - m.ckpts = stall + synctest.Test(t, func(t *testing.T) { + eng := newFakeEngine() + m := newTestManager(t, eng, config.PoolSpec{PoolKey: testKey, Warm: 1}) + m.pools[testKey].goldenDir = "/goldens/x" + editView(m, func(v *configView) { v.ckptTTL = time.Hour }) + stall := &stallingMetasStore{Store: m.ckpts, entered: make(chan struct{}, 1), release: make(chan struct{})} + m.ckpts = stall - ctx, cancel := context.WithCancel(t.Context()) - done := make(chan struct{}) - go func() { - m.Run(ctx) - close(done) - }() - defer func() { - close(stall.release) - cancel() - <-done - }() - <-stall.entered + ctx, cancel := context.WithCancel(t.Context()) + done := make(chan struct{}) + go func() { + m.Run(ctx) + close(done) + }() + defer func() { + close(stall.release) + cancel() + <-done + }() + <-stall.entered - waitFor(t, func() bool { return warmLen(m, testKey) > 0 }) - if got := eng.cloneCount(); got != 1 { - t.Errorf("clones=%d, want one warm VM spawned while the store listing is stuck", got) - } + waitFor(t, func() bool { return warmLen(m, testKey) > 0 }) + if got := eng.cloneCount(); got != 1 { + t.Errorf("clones=%d, want one warm VM spawned while the store listing is stuck", got) + } + }) } func TestRunRefillsWhileTheGenerationSweepBlocks(t *testing.T) { @@ -326,7 +320,7 @@ func TestRejectedCheckpointOpsLeakNoRecLock(t *testing.T) { m := newTestManager(t, newFakeEngine()) base := lockCount(m) - for _, id := range []string{"../../etc", "ck_zz", "", "ck_00112233445566ff", "ck_deadbeefdeadbeef"} { + for _, id := range []string{"../../etc", "ck_zz", "", "ck_0011223344556677x", "ck_00112233445566ff", "ck_deadbeefdeadbeef"} { if _, err := m.ClaimCheckpoint(t.Context(), id, ClaimOptions{TTL: time.Hour}); !errors.Is(err, ErrUnknownCheckpoint) { t.Errorf("claim %q: %v, want ErrUnknownCheckpoint", id, err) } diff --git a/sandboxd/pool/claims_test.go b/sandboxd/pool/claims_test.go index b8fdd9141..6650d485e 100644 --- a/sandboxd/pool/claims_test.go +++ b/sandboxd/pool/claims_test.go @@ -1,6 +1,7 @@ package pool import ( + "bytes" "errors" "io" "io/fs" @@ -8,8 +9,9 @@ import ( "os" "path/filepath" "reflect" - "runtime" + "runtime/pprof" "slices" + "strconv" "strings" "sync" "syscall" @@ -25,15 +27,15 @@ func TestRecommitKeepsOneRetrierWhileWritesFail(t *testing.T) { dir := filepath.Join(t.TempDir(), "absent") m.store.path = filepath.Join(dir, "claims.json") - before := runtime.NumGoroutine() + before := recommitGoroutines(t) for range 50 { m.mu.Lock() snap := m.store.set(sb) m.mu.Unlock() m.store.recommit(t.Context(), snap) } - if grown := runtime.NumGoroutine() - before; grown > 2 { - t.Fatalf("%d goroutines for 50 failed persists, want one retrier", grown) + if n := recommitGoroutines(t) - before; n > 1 || !m.store.recommitting.Load() { + t.Fatalf("%d new retriers for 50 failed persists, want one", n) } if err := os.MkdirAll(dir, 0o750); err != nil { @@ -237,3 +239,26 @@ func TestClaimStoreConcurrentCommits(t *testing.T) { t.Fatalf("after concurrent commits: %+v, %v", got, err) } } + +func recommitGoroutines(t *testing.T) int { + t.Helper() + var profile bytes.Buffer + if err := pprof.Lookup("goroutine").WriteTo(&profile, 1); err != nil { + t.Fatalf("goroutine profile: %v", err) + } + n := 0 + for record := range strings.SplitSeq(profile.String(), "\n\n") { + if !strings.Contains(record, "(*claimStore).recommit.func1") { + continue + } + for line := range strings.SplitSeq(record, "\n") { + if count, _, ok := strings.Cut(line, " @ "); ok { + if c, err := strconv.Atoi(count); err == nil { + n += c + break + } + } + } + } + return n +} diff --git a/sandboxd/pool/disk_test.go b/sandboxd/pool/disk_test.go index 166c20cc8..c4c284e97 100644 --- a/sandboxd/pool/disk_test.go +++ b/sandboxd/pool/disk_test.go @@ -11,7 +11,7 @@ import ( ) func TestDiskUnderReserveRefusesClaimsAndWakesUntilItRecovers(t *testing.T) { - m := diskReserveManager(t, "1000P") + m := diskReserveManager(t) sb := mustClaim(t, m, testKey) hibernateClaim(t, m, sb) @@ -36,7 +36,7 @@ func TestDiskUnderReserveRefusesClaimsAndWakesUntilItRecovers(t *testing.T) { } func TestDiskUnderReserveAdvertisesNoWarm(t *testing.T) { - m := diskReserveManager(t, "1000P") + m := diskReserveManager(t) p := m.pools[testKey] p.warm = append(p.warm, &types.Sandbox{VMName: "sbx-warm", Key: testKey, VsockSocket: "/vsock/warm"}) if n := m.WarmCounts()[p.hash]; n != 1 { @@ -49,7 +49,7 @@ func TestDiskUnderReserveAdvertisesNoWarm(t *testing.T) { } func TestDiskUnderReserveHoldsHibernatesAndCaptures(t *testing.T) { - m := diskReserveManager(t, "1000P") + m := diskReserveManager(t) sb := mustClaim(t, m, testKey) cred := Cred{Token: sb.Token} m.checkDisk(t.Context()) @@ -74,7 +74,7 @@ func TestDiskUnderReserveHoldsHibernatesAndCaptures(t *testing.T) { } func TestAFailedStatfsKeepsTheDiskLowState(t *testing.T) { - m := diskReserveManager(t, "1000P") + m := diskReserveManager(t) m.checkDisk(t.Context()) setDiskReserve(t, m, &config.DiskReserve{Free: "1K", Dirs: []string{filepath.Join(m.dataDir, "missing")}}) if _, err := claimAny(t.Context(), m, testKey, ClaimOptions{TTL: time.Hour}); !errors.Is(err, ErrQuota) { @@ -89,8 +89,8 @@ func setDiskReserve(t *testing.T, m *Manager, reserve *config.DiskReserve) { m.checkDisk(t.Context()) } -func diskReserveManager(t *testing.T, free string) *Manager { +func diskReserveManager(t *testing.T) *Manager { t.Helper() - cfg := &config.Config{DataDir: t.TempDir(), DiskReserve: &config.DiskReserve{Free: free}, Pools: []config.PoolSpec{{PoolKey: testKey}}} + cfg := &config.Config{DataDir: t.TempDir(), DiskReserve: &config.DiskReserve{Free: "1000P"}, Pools: []config.PoolSpec{{PoolKey: testKey}}} return mustManager(t, cfg, newFakeEngine()) } diff --git a/sandboxd/pool/drain_test.go b/sandboxd/pool/drain_test.go index f272b631e..f1fbb2dfe 100644 --- a/sandboxd/pool/drain_test.go +++ b/sandboxd/pool/drain_test.go @@ -336,6 +336,9 @@ func TestAQueuedFailedClaimStillRestarts(t *testing.T) { if err := a.Wake(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { t.Errorf("wake of a queued failed claim: %v", err) } + if got, _ := a.Sandbox(sb.ID); got.Restarts != 1 || got.Failed != "" { + t.Errorf("summary %+v, want the wake to cold-boot the failed claim", got) + } a.Undrain(t.Context()) release() @@ -343,7 +346,7 @@ func TestAQueuedFailedClaimStillRestarts(t *testing.T) { }) } -func TestEvacuateRequeuesAClaimAWakeUnarchived(t *testing.T) { +func TestHandOffOfAClaimAWakeUnarchivedAnswersBusy(t *testing.T) { synctest.Test(t, func(t *testing.T) { c := newFailoverCluster(t) a := c.node(t, "a", t.TempDir(), keepAnHour) diff --git a/sandboxd/pool/egress_test.go b/sandboxd/pool/egress_test.go index d08b8bd34..146d36942 100644 --- a/sandboxd/pool/egress_test.go +++ b/sandboxd/pool/egress_test.go @@ -8,6 +8,7 @@ import ( "path/filepath" "slices" "strings" + "sync" "testing" "time" @@ -224,6 +225,8 @@ func TestArmFailureEndsTheClaimEventsBeforeTheTeardown(t *testing.T) { eng := newFakeEngine() m := egressManager(t, eng, config.PoolSpec{PoolKey: egKey, Egress: egPolicy}) stall := make(chan struct{}) + unstall := sync.OnceFunc(func() { close(stall) }) + t.Cleanup(unstall) eng.mu.Lock() eng.removeStall = stall eng.mu.Unlock() @@ -234,7 +237,7 @@ func TestArmFailureEndsTheClaimEventsBeforeTheTeardown(t *testing.T) { raw, _ := os.ReadFile(filepath.Join(m.dataDir, usage.JournalFile)) return strings.Count(string(raw), `"ev":"release"`) == len(sbs) }) - close(stall) + unstall() if err := <-done; err == nil { t.Fatal("finalizeBatch must fail when a batch member cannot arm") } diff --git a/sandboxd/pool/failover_test.go b/sandboxd/pool/failover_test.go index 3d55dfa93..d4eef8ada 100644 --- a/sandboxd/pool/failover_test.go +++ b/sandboxd/pool/failover_test.go @@ -874,47 +874,6 @@ func TestFailoverRestartClearsTheMarkerOfAnAdoptedClaim(t *testing.T) { } } -func TestFailoverARepeatedAdoptionOnOneNodeKeepsOneClaim(t *testing.T) { - c := newFailoverCluster(t) - a, b := c.pair(t) - for _, m := range []*Manager{a, b} { - if err := m.PutTenant(t.Context(), tenants.TenantSpec{Name: "acme", Token: "tok-acme"}); err != nil { - t.Fatalf("put tenant: %v", err) - } - } - sb, err := claimAny(t.Context(), a, testKey, ClaimOptions{Tenant: "acme"}) - if err != nil { - t.Fatalf("claim: %v", err) - } - mustArchive(t, a, sb) - c.expire("a") - - var wg sync.WaitGroup - for range 2 { - wg.Go(func() { - if _, err := b.AdoptArchived(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { - t.Errorf("racing adopt: %v", err) - } - }) - } - wg.Wait() - mustAdopt(t, b, sb, "repeated adopt") - if n := b.TenantClaims()["acme"]; n != 1 { - t.Errorf("tenant acme counts %d claims on b, want 1", n) - } - held, _ := b.byID(sb.ID) - rec, _, _ := c.reg.Get(t.Context(), sb.ID) - b.mu.Lock() - epoch := held.ArchiveEpoch - b.mu.Unlock() - if rec.Owner != "b" || epoch != rec.Epoch { - t.Errorf("row %s@%d, local epoch %d, want the claim at the row's epoch", rec.Owner, rec.Epoch, epoch) - } - if _, _, _, err := b.WakeAgentSocket(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { - t.Errorf("wake after repeated adoptions: %v", err) - } -} - func TestFailoverRestartInsideAnAdoptersWakeDropsTheClaim(t *testing.T) { c := newFailoverCluster(t) a, b := c.pair(t) @@ -1471,8 +1430,16 @@ func TestFailoverConcurrentAdoptionsOnOneNodeAllAnswerForIt(t *testing.T) { if n := b.TenantClaims()["acme"]; n != 1 { t.Errorf("tenant acme counts %d claims on b, want 1", n) } - if rec, _, _ := c.reg.Get(t.Context(), sb.ID); rec.Owner != "b" { - t.Errorf("row owner %s, want b", rec.Owner) + held, _ := b.byID(sb.ID) + rec, _, _ := c.reg.Get(t.Context(), sb.ID) + b.mu.Lock() + epoch := held.ArchiveEpoch + b.mu.Unlock() + if rec.Owner != "b" || epoch != rec.Epoch { + t.Errorf("row %s@%d, local epoch %d, want the claim on b at the row's epoch", rec.Owner, rec.Epoch, epoch) + } + if _, _, _, err := b.WakeAgentSocket(t.Context(), sb.ID, Cred{Token: sb.Token}); err != nil { + t.Errorf("wake after concurrent adoptions: %v", err) } } @@ -1489,10 +1456,11 @@ func TestFailoverACoalescedAdoptionStillNeedsTheToken(t *testing.T) { ok, _ := b.AdoptArchived(t.Context(), sb.ID, Cred{Token: "wrong"}) wrong <- ok }() + synctest.Wait() select { case ok := <-wrong: wrong <- ok - case <-time.After(200 * time.Millisecond): + default: } }} mustAdopt(t, b, sb, "adopt") diff --git a/sandboxd/pool/hibernate_test.go b/sandboxd/pool/hibernate_test.go index d663decf5..d357875e6 100644 --- a/sandboxd/pool/hibernate_test.go +++ b/sandboxd/pool/hibernate_test.go @@ -220,6 +220,9 @@ func TestWakeFailureStopsTheVMAndRetries(t *testing.T) { if eng.snapRemoved(snap) { t.Error("the failure dropped the snapshot needed to retry") } + if _, g := m.Info(); g.Hibernated != 1 { + t.Errorf("hibernated count %d, want 1 (wake failed, snapshot kept)", g.Hibernated) + } if got := journaled(t, m); got[sb.ID] == nil || got[sb.ID].HibernateSnap != snap { t.Fatalf("journal %+v, want HibernateSnap=%q kept for the retry", got[sb.ID], snap) } @@ -443,24 +446,6 @@ func TestWakeDropsSnapshotOffReturnPath(t *testing.T) { }) } -func TestWakeFailureKeepsHibernated(t *testing.T) { - eng := newFakeEngine() - m := newTestManager(t, eng) - sb := mustClaim(t, m, testKey) - hibernateClaim(t, m, sb) - - eng.restoreErr = errors.New("restore boom") - if _, _, _, err := m.WakeAgentSocket(t.Context(), sb.ID, Cred{Token: sb.Token}); err == nil { - t.Fatal("WakeAgentSocket succeeded despite restore failure") - } - if _, g := m.Info(); g.Hibernated != 1 { - t.Errorf("hibernated count %d, want 1 (wake failed, snapshot kept)", g.Hibernated) - } - if len(eng.snapRemoves) != 0 { - t.Errorf("snapRemoves=%v, want none after failed wake", eng.snapRemoves) - } -} - func TestReleaseHibernatedDropsSnapshot(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) diff --git a/sandboxd/pool/liveness_test.go b/sandboxd/pool/liveness_test.go index 7cd5ab310..08798754b 100644 --- a/sandboxd/pool/liveness_test.go +++ b/sandboxd/pool/liveness_test.go @@ -131,7 +131,7 @@ func TestLivenessRestartFailureRetriesThenFails(t *testing.T) { killVMM(eng, sb.VMName) eng.startErr = errors.New("boot failed") - assertRestartFailsUntilBudget(t, m, eng, sb) + assertRestartFailsUntilBudget(t, m, eng, sb, false) if got, want := usageEventsOf(t, m, sb.ID), "claim,vmm_exit,vmm_failed"; got != want { t.Errorf("events %s, want one exit across the attempts: %s", got, want) } @@ -282,7 +282,7 @@ func TestRestartThatCannotDeliverTheEnvFailsAndSpendsTheBudget(t *testing.T) { eng.guestEnvErr = errors.New("guest write refused") eng.mu.Unlock() - assertRestartFailsUntilBudget(t, m, eng, sb) + assertRestartFailsUntilBudget(t, m, eng, sb, true) } func TestRestartThatCannotArmTheEgressDoorFailsAndSpendsTheBudget(t *testing.T) { @@ -299,7 +299,7 @@ func TestRestartThatCannotArmTheEgressDoorFailsAndSpendsTheBudget(t *testing.T) } t.Cleanup(func() { _ = os.Chmod(eng.sockRoot, 0o700) }) - assertRestartFailsUntilBudget(t, m, eng, sb) + assertRestartFailsUntilBudget(t, m, eng, sb, true) } func TestARestartedGuestIsBilledFromItsBoot(t *testing.T) { @@ -577,14 +577,15 @@ func TestExpiredFailedClaimIsDestroyedNotArchived(t *testing.T) { func TestReconcileKeepsRunningClaimsTheHostLost(t *testing.T) { for _, tt := range []struct { + name string policy types.VMMRestart wantFailed string wantRestarts int }{ - {"", "", 1}, - {types.VMMRestartNone, "vmm exited", 0}, + {"default", "", "", 1}, + {"none", types.VMMRestartNone, "vmm exited", 0}, } { - t.Run(string(tt.policy), func(t *testing.T) { + t.Run(tt.name, func(t *testing.T) { eng := newFakeEngine() eng.vms["sbx-lost-1"] = "/vsock/lost" eng.stopped["sbx-lost-1"] = true @@ -634,14 +635,12 @@ func TestAVMEventFindsAWokenArchivedClaimByItsNewVM(t *testing.T) { if got := m.watchedClaims(newVM); len(got) != 1 || got[0].sb != sb { t.Errorf("an event for the woken VM %s found %v, want the claim", newVM, got) } - if newVM != oldVM { - if got := m.watchedClaims(oldVM); len(got) != 0 { - t.Errorf("an event for the old VM %s still finds %v", oldVM, got) - } + if got := m.watchedClaims(oldVM); len(got) != 0 { + t.Errorf("an event for the old VM %s still finds %v", oldVM, got) } } -func assertRestartFailsUntilBudget(t *testing.T, m *Manager, eng *fakeEngine, sb *types.Sandbox) { +func assertRestartFailsUntilBudget(t *testing.T, m *Manager, eng *fakeEngine, sb *types.Sandbox, booted bool) { t.Helper() for attempt := range vmmRestartBudget { m.recoverVMM(t.Context(), sb) @@ -663,6 +662,9 @@ func assertRestartFailsUntilBudget(t *testing.T, m *Manager, eng *fakeEngine, sb 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) } + if booted && strings.Count(events, "vmm_restart") != vmmRestartBudget { + t.Errorf("events %s, want each of the %d booted attempts billed", events, vmmRestartBudget) + } } func runLiveness(t *testing.T, m *Manager) { diff --git a/sandboxd/pool/memory_test.go b/sandboxd/pool/memory_test.go index 6f6e082b0..7e1a5e85d 100644 --- a/sandboxd/pool/memory_test.go +++ b/sandboxd/pool/memory_test.go @@ -140,6 +140,7 @@ func TestMemoryOverLimitParksRefill(t *testing.T) { if n := refilling(); n == 0 { t.Error("no refill under the limit") } + waitPoolSettled(t, m, testKey) } func TestAnUnreadableSignalDoesNotHoldTheNodeLow(t *testing.T) { @@ -215,9 +216,9 @@ func TestAnArchivedWakeOnASqueezedOwnerMovesToAPeerWithRoom(t *testing.T) { func TestAnArchivedWakeNoPeerAdoptsRunsOnTheSqueezedOwner(t *testing.T) { c := newFailoverCluster(t) a, b := c.pair(t) - sb, capped := mustClaim(t, a, testKey), mustClaim(t, a, testKey) + sb, second := mustClaim(t, a, testKey), mustClaim(t, a, testKey) mustArchive(t, a, sb) - mustArchive(t, a, capped) + mustArchive(t, a, second) a.memoryLow.Store(true) b.memoryLow.Store(true) if err := wakeClaim(t, a, sb); err != nil { @@ -230,7 +231,7 @@ func TestAnArchivedWakeNoPeerAdoptsRunsOnTheSqueezedOwner(t *testing.T) { t.Errorf("owner counters memory %d fallbacks %d hand-offs %d, want 1 1 0", got.WakesRefusedMemory, got.WakeMemoryFallbacks, got.WakeHandoffs) } editView(a, func(v *configView) { v.maxRunning = 1 }) - if err := wakeClaim(t, a, capped); !errors.Is(err, ErrQuota) || !strings.Contains(err.Error(), "running claims") { + if err := wakeClaim(t, a, second); !errors.Is(err, ErrQuota) || !strings.Contains(err.Error(), "running claims") { t.Errorf("fallback wake at max_running: %v, want the running cap to refuse it", err) } } diff --git a/sandboxd/pool/move_test.go b/sandboxd/pool/move_test.go index 7ce8c87ac..fea71f0d3 100644 --- a/sandboxd/pool/move_test.go +++ b/sandboxd/pool/move_test.go @@ -139,15 +139,15 @@ func TestMoveRefusedByTheTargetLeavesTheClaimRunningOnTheSource(t *testing.T) { func TestMoveFailingAtAnyStepKeepsOneOwner(t *testing.T) { for name, tt := range map[string]struct { budget time.Duration - fail func(mc *moveCluster) + fail func(t *testing.T, mc *moveCluster) want error }{ - "admission": {fail: func(mc *moveCluster) { mc.p.admitErr = ErrMoveTarget }, want: ErrMoveTarget}, - "move record": {fail: func(mc *moveCluster) { mc.moveReg.SetDown(true) }, want: ErrMoveStoreDown}, - "baseline": {fail: func(mc *moveCluster) { fakeOf(mc.s).captureErr = errors.New("no reflink") }, want: ErrNotMovable}, - "round transfer": {fail: func(mc *moveCluster) { mc.p.roundHook = failRound(2) }, want: ErrMoveTarget}, + "admission": {fail: func(_ *testing.T, mc *moveCluster) { mc.p.admitErr = ErrMoveTarget }, want: ErrMoveTarget}, + "move record": {fail: func(_ *testing.T, mc *moveCluster) { mc.moveReg.SetDown(true) }, want: ErrMoveStoreDown}, + "baseline": {fail: func(_ *testing.T, mc *moveCluster) { fakeOf(mc.s).captureErr = errors.New("no reflink") }, want: ErrNotMovable}, + "round transfer": {fail: func(_ *testing.T, mc *moveCluster) { mc.p.roundHook = failRound(2) }, want: ErrMoveTarget}, "downtime budget": {budget: time.Millisecond, want: ErrMoveDowntime}, - "delta capture": {fail: func(mc *moveCluster) { + "delta capture": {fail: func(_ *testing.T, mc *moveCluster) { mc.p.roundHook = func(round int) error { if round == 1 { fakeOf(mc.s).captureErr = errors.New("vmm gone") @@ -155,11 +155,13 @@ func TestMoveFailingAtAnyStepKeepsOneOwner(t *testing.T) { return nil } }, want: ErrNotMovable}, - "final capture": {fail: func(mc *moveCluster) { fakeOf(mc.s).finalCaptureErr = errors.New("pause failed") }, want: ErrNotMovable}, - "final transfer": {fail: func(mc *moveCluster) { mc.p.roundHook = failRound(3) }, want: ErrMoveTarget}, - "receive": {fail: func(mc *moveCluster) { fakeOf(mc.t).receiveErr = errors.New("restore failed") }}, - "target memory": {fail: func(mc *moveCluster) { mc.p.commitHook = func(int) error { mc.t.memoryLow.Store(true); return nil } }, want: ErrQuota}, - "source aborted first": {fail: func(mc *moveCluster) { + "final capture": {fail: func(_ *testing.T, mc *moveCluster) { fakeOf(mc.s).finalCaptureErr = errors.New("pause failed") }, want: ErrNotMovable}, + "final transfer": {fail: func(_ *testing.T, mc *moveCluster) { mc.p.roundHook = failRound(3) }, want: ErrMoveTarget}, + "receive": {fail: func(_ *testing.T, mc *moveCluster) { fakeOf(mc.t).receiveErr = errors.New("restore failed") }}, + "target memory": {fail: func(_ *testing.T, mc *moveCluster) { + mc.p.commitHook = func(int) error { mc.t.memoryLow.Store(true); return nil } + }, want: ErrQuota}, + "source aborted first": {fail: func(t *testing.T, mc *moveCluster) { mc.p.commitHook = func(int) error { abortRecord(t, mc); return nil } }, want: ErrNotMovable}, } { @@ -167,7 +169,7 @@ func TestMoveFailingAtAnyStepKeepsOneOwner(t *testing.T) { mc := newMoveCluster(t) sb := claimTenant(t, mc.s, "acme") if tt.fail != nil { - tt.fail(mc) + tt.fail(t, mc) } _, err := mc.s.Move(t.Context(), sb.ID, "b", cmp.Or(tt.budget, time.Minute)) if err == nil || tt.want != nil && !errors.Is(err, tt.want) { @@ -211,6 +213,8 @@ func TestMoveWhoseCommitAnswerIsLostResolvesThroughTheRecord(t *testing.T) { func TestMoveKeepsThePausedGuestUntilItsAbortLands(t *testing.T) { mc := newMoveCluster(t) sb := claimTenant(t, mc.s, "acme") + reads := &countingMoves{Registry: mc.s.moves} + mc.s.moves = reads mc.p.commitHook = func(int) error { mc.moveReg.SetDown(true) return ErrMoveTarget @@ -218,13 +222,16 @@ func TestMoveKeepsThePausedGuestUntilItsAbortLands(t *testing.T) { if _, err := mc.s.Move(t.Context(), sb.ID, "b", time.Minute); !errors.Is(err, ErrMoveStoreDown) { t.Fatalf("move with the registry down after the final round: %v, want ErrMoveStoreDown", err) } + failed := reads.failedGets.Load() if !pausedOn(mc.s, sb.VMName) { t.Fatal("the source resumed its guest before the abort landed") } if _, _, _, err := mc.s.WakeAgentSocket(t.Context(), sb.ID, Cred{Token: sb.Token}); !errors.Is(err, ErrEvacuating) { t.Errorf("dial during the unresolved final round: %v, want ErrEvacuating", err) } - time.Sleep(2 * moveRetryInterval) + waitFor(t, func() bool { return reads.failedGets.Load() > failed }) + failed = reads.failedGets.Load() + waitFor(t, func() bool { return reads.failedGets.Load() > failed }) if !pausedOn(mc.s, sb.VMName) { t.Fatal("the source resumed its guest while the registry was down") } @@ -617,7 +624,7 @@ func TestMoveRestartAfterTheConsumeKeepsTheClaim(t *testing.T) { reg := movestest.NewMem() c.moves = reg ctx := t.Context() - epoch, _ := reg.Begin(ctx, "sb_moved", "a", "b") + epoch := seedMoveRecord(t, reg, "sb_moved", "a", "b", moves.Pending) rowEpoch, err := c.reg.Put(ctx, archives.Record{ID: "sb_moved", TokenSHA256: tenants.TokenSHA256("tok"), Owner: "a", Checkpoint: "ck_00000000000000aa", Claim: []byte("{}")}) if err != nil { t.Fatalf("seed row: %v", err) @@ -1700,6 +1707,19 @@ func (r *lossyMoves) Get(ctx context.Context, id string) (moves.Record, bool, er func (r *lossyMoves) setDown(down bool) { r.down.Store(down) } +type countingMoves struct { + moves.Registry + failedGets atomic.Int32 +} + +func (r *countingMoves) Get(ctx context.Context, id string) (moves.Record, bool, error) { + rec, ok, err := r.Registry.Get(ctx, id) + if err != nil { + r.failedGets.Add(1) + } + return rec, ok, err +} + func seedMoveRecord(t *testing.T, reg moves.Registry, id, from, to string, state moves.State) int64 { t.Helper() epoch, err := reg.Begin(t.Context(), id, from, to) diff --git a/sandboxd/pool/pool_test.go b/sandboxd/pool/pool_test.go index 8853fc668..1965fd5e5 100644 --- a/sandboxd/pool/pool_test.go +++ b/sandboxd/pool/pool_test.go @@ -241,13 +241,15 @@ func TestReleaseRecordsItsUsageEventBeforeTheTeardown(t *testing.T) { m := newTestManager(t, eng) sb := mustClaim(t, m, testKey) stall := make(chan struct{}) + unstall := sync.OnceFunc(func() { close(stall) }) + t.Cleanup(unstall) eng.mu.Lock() eng.removeStall = stall eng.mu.Unlock() done := make(chan error, 1) go func() { done <- m.Release(t.Context(), sb.ID, Cred{Token: sb.Token}) }() waitFor(t, func() bool { return strings.HasSuffix(usageEventsOf(t, m, sb.ID), "release") }) - close(stall) + unstall() if err := <-done; err != nil { t.Fatalf("release: %v", err) } @@ -783,18 +785,20 @@ func TestRefillCanceledWhileWaitingForProbeCleansUp(t *testing.T) { } func TestDirectClaimSkipsProbeGate(t *testing.T) { - m := newTestManager(t, newFakeEngine()) - for range cap(m.probeSem) { - m.probeSem <- struct{}{} - } - defer func() { + synctest.Test(t, func(t *testing.T) { + m := newTestManager(t, newFakeEngine()) for range cap(m.probeSem) { - <-m.probeSem + m.probeSem <- struct{}{} } - }() - if _, err := claimAny(t.Context(), m, testKey, ClaimOptions{}); err != nil { - t.Fatalf("direct claim waited on probe gate: %v", err) - } + defer func() { + for range cap(m.probeSem) { + <-m.probeSem + } + }() + if _, err := claimAny(t.Context(), m, testKey, ClaimOptions{}); err != nil { + t.Fatalf("direct claim waited on probe gate: %v", err) + } + }) } func TestRefillBudgetFollowsConfig(t *testing.T) { @@ -996,16 +1000,13 @@ func TestClaimedAtIsTheFirstGrant(t *testing.T) { } } -func TestTheByIDReadCarriesTheTokenAndTheIndexDoesNot(t *testing.T) { +func TestTheByIDReadCarriesTheToken(t *testing.T) { m := newTestManager(t, newFakeEngine()) sb := mustClaim(t, m, testKey) if got, ok := m.Sandbox(sb.ID); !ok || got.Token != sb.Token { t.Errorf("Sandbox(%s) token %q, want the claim's %q", sb.ID, got.Token, sb.Token) } - if list := m.Sandboxes("", "", nil); len(list) != 1 || list[0].Token != "" { - t.Errorf("index %+v carries a token", list) - } } func TestRenewClampsAndDefaults(t *testing.T) { diff --git a/sandboxd/pool/promote_test.go b/sandboxd/pool/promote_test.go index ab6ae4efc..e4d4977b3 100644 --- a/sandboxd/pool/promote_test.go +++ b/sandboxd/pool/promote_test.go @@ -8,8 +8,8 @@ import ( "os" "path/filepath" "slices" + "sync" "testing" - "testing/synctest" "time" "github.com/cocoonstack/sandbox/sandboxd/config" @@ -262,7 +262,7 @@ func TestResolveGoldenSkipsPromotedEgressTemplate(t *testing.T) { } } -func TestTemplateTenantScopedDelete(t *testing.T) { +func TestATenantDeletesOnlyItsOwnTemplate(t *testing.T) { eng := newFakeEngine() m := newTestManager(t, eng) parent := claimTenant(t, m, "acme") @@ -538,40 +538,37 @@ func TestRejectedLabelWritesLeakNoRecLock(t *testing.T) { } func TestClaimFromATemplateRunsBesideItsRepromote(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - m := newTestManager(t, newFakeEngine()) - parent := mustClaim(t, m, testKey) - key, _ := mustPromote(t, m, parent, "tpl:x", "") - slow := &slowPrepareStore{Store: m.tpls, entered: make(chan struct{}), release: make(chan struct{})} - m.tpls = slow - repromoted := make(chan error, 1) - go func() { - _, _, promoteErr := m.Promote(t.Context(), parent.ID, Cred{Token: parent.Token}, "tpl:x", "") - repromoted <- promoteErr - }() - <-slow.entered - - ctx, cancel := context.WithTimeout(t.Context(), 100*time.Millisecond) - defer cancel() - claimed := make(chan error, 1) - go func() { - _, claimErr := claimAny(ctx, m, key, ClaimOptions{}) - claimed <- claimErr - }() - synctest.Wait() - select { - case err := <-claimed: - if err != nil { - t.Errorf("claim during a re-promote: %v, want a clone of the committed template", err) - } - default: - t.Error("a claim with a 100 ms context was still blocked during a re-promote") - } - close(slow.release) - if err := <-repromoted; err != nil { - t.Fatalf("re-promote: %v", err) + m := newTestManager(t, newFakeEngine()) + parent := mustClaim(t, m, testKey) + key, _ := mustPromote(t, m, parent, "tpl:x", "") + slow := &slowPrepareStore{Store: m.tpls, entered: make(chan struct{}), release: make(chan struct{})} + m.tpls = slow + release := sync.OnceFunc(func() { close(slow.release) }) + t.Cleanup(release) + repromoted := make(chan error, 1) + go func() { + _, _, promoteErr := m.Promote(t.Context(), parent.ID, Cred{Token: parent.Token}, "tpl:x", "") + repromoted <- promoteErr + }() + <-slow.entered + + claimed := make(chan error, 1) + go func() { + _, claimErr := claimAny(t.Context(), m, key, ClaimOptions{}) + claimed <- claimErr + }() + select { + case err := <-claimed: + if err != nil { + t.Errorf("claim during a re-promote: %v, want a clone of the committed template", err) } - }) + case <-time.After(time.Second): + t.Error("a claim was still blocked a second into a re-promote") + } + release() + if err := <-repromoted; err != nil { + t.Fatalf("re-promote: %v", err) + } } type slowPrepareStore struct { diff --git a/sandboxd/pool/reader_test.go b/sandboxd/pool/reader_test.go index 2eb328da5..00e7748c9 100644 --- a/sandboxd/pool/reader_test.go +++ b/sandboxd/pool/reader_test.go @@ -85,41 +85,53 @@ func TestReaderObeysTheNodeLeaseFence(t *testing.T) { } func TestReaderConnectionEndsWhenTheFenceTrips(t *testing.T) { - eng := newFakeEngine() - eng.readerPeers = make(chan net.Conn, 2) - m := newReaderManager(t, eng) - m.liftFence() - replicated, plain := mustClaim(t, m, testKey), mustClaim(t, m, testKey) - replicated.Replicated = true - hibernateClaim(t, m, replicated) - hibernateClaim(t, m, plain) - seedReaderVMs(m, testKey, "sbx-reader-1", "sbx-reader-2") - - peers := map[string]net.Conn{} - for name, sb := range map[string]*types.Sandbox{"replicated": replicated, "plain": plain} { - conn, _, err := m.DialReader(t.Context(), sb.ID, sb.Token) - if err != nil { - t.Fatalf("DialReader of the %s claim: %v", name, err) + synctest.Test(t, func(t *testing.T) { + eng := newFakeEngine() + eng.readerPeers = make(chan net.Conn, 2) + m := newReaderManager(t, eng) + m.liftFence() + replicated, plain := mustClaim(t, m, testKey), mustClaim(t, m, testKey) + replicated.Replicated = true + hibernateClaim(t, m, replicated) + hibernateClaim(t, m, plain) + seedReaderVMs(m, testKey, "sbx-reader-1", "sbx-reader-2") + + reads := map[string]chan error{} + for name, sb := range map[string]*types.Sandbox{"replicated": replicated, "plain": plain} { + conn, _, err := m.DialReader(t.Context(), sb.ID, sb.Token) + if err != nil { + t.Fatalf("DialReader of the %s claim: %v", name, err) + } + defer func() { _ = conn.Close() }() + peer, read := <-eng.readerPeers, make(chan error, 1) + go func() { + _, readErr := peer.Read(make([]byte, 1)) + read <- readErr + }() + reads[name] = read + } + ended := func(read chan error) bool { + select { + case <-read: + return true + default: + return false + } + } + synctest.Wait() + if ended(reads["replicated"]) || ended(reads["plain"]) { + t.Fatal("a reader connection ended before the fence tripped") } - defer func() { _ = conn.Close() }() - peers[name] = <-eng.readerPeers - } - ended := func(peer net.Conn, wait time.Duration) bool { - _ = peer.SetReadDeadline(time.Now().Add(wait)) - _, err := peer.Read(make([]byte, 1)) - return !errors.Is(err, os.ErrDeadlineExceeded) - } - if ended(peers["replicated"], 100*time.Millisecond) || ended(peers["plain"], 100*time.Millisecond) { - t.Fatal("a reader connection ended before the fence tripped") - } - m.trip(t.Context()) - if !ended(peers["replicated"], 3*time.Second) { - t.Error("the replicated claim's reader connection survived the fence") - } - if ended(peers["plain"], 100*time.Millisecond) { - t.Error("the fence ended an unreplicated claim's reader connection") - } + m.trip(t.Context()) + synctest.Wait() + if !ended(reads["replicated"]) { + t.Error("the replicated claim's reader connection survived the fence") + } + if ended(reads["plain"]) { + t.Error("the fence ended an unreplicated claim's reader connection") + } + }) } func TestReaderTakesItsVMFromTheClaimsOwnPool(t *testing.T) { @@ -158,8 +170,8 @@ func TestReaderServesOnlyPoolsThatAskForIt(t *testing.T) { t.Fatalf("WakeAgentSocketOrRead = %q, %v, want a wake", sock, err) } defer done() - if len(eng.restores) != 1 || len(eng.attachedCopies) != 0 || len(m.pools[otherKey].warm) != 1 { - t.Errorf("restores=%v attached=%v warm=%d, want a wake with no reader taken", eng.restores, eng.attachedCopies, len(m.pools[otherKey].warm)) + if len(eng.restores) != 1 || sb.HibernateSnap != "" || len(eng.attachedCopies) != 0 || len(m.pools[otherKey].warm) != 1 { + t.Errorf("restores=%v snapshot=%q attached=%v warm=%d, want a wake with no reader taken", eng.restores, sb.HibernateSnap, eng.attachedCopies, len(m.pools[otherKey].warm)) } } @@ -465,19 +477,16 @@ func TestReaderRefusesWhatAWakeRefuses(t *testing.T) { spec := archivePool(0) spec.Reader = true cfg := &config.Config{DataDir: t.TempDir(), APIToken: "root", Pools: []config.PoolSpec{spec}} - 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) putTenants(t, m, tenants.TenantSpec{Name: "acme", Token: "acme-tok"}) sb := claimTenant(t, m, "acme") mustArchive(t, m, sb) seedReaderVMs(m, testKey, "sbx-reader-1") - if err = m.DeleteTenant(t.Context(), "acme"); err != nil { + if err := m.DeleteTenant(t.Context(), "acme"); err != nil { t.Fatalf("DeleteTenant: %v", err) } - if _, _, err = m.DialReader(t.Context(), sb.ID, sb.Token); !errors.Is(err, ErrNoReader) || !errors.Is(err, ErrTenantRemoved) { + if _, _, err := m.DialReader(t.Context(), sb.ID, sb.Token); !errors.Is(err, ErrNoReader) || !errors.Is(err, ErrTenantRemoved) { t.Errorf("DialReader of a removed tenant's archive = %v, want ErrNoReader wrapping ErrTenantRemoved", err) } if len(eng.startedReaders) != 0 || len(m.pools[testKey].warm) != 1 { @@ -760,25 +769,6 @@ func TestWakeClearsReaderSafe(t *testing.T) { } } -func TestReaderOffRefusesADialAndWakesTheClaim(t *testing.T) { - eng := newFakeEngine() - m := newTestManager(t, eng) - sb := mustClaim(t, m, testKey) - hibernateClaim(t, m, sb) - - if _, _, err := m.DialReader(t.Context(), sb.ID, sb.Token); !errors.Is(err, ErrNoReader) { - t.Errorf("DialReader with the reader off = %v, want ErrNoReader", err) - } - sock, _, done, err := m.WakeAgentSocketOrRead(t.Context(), sb.ID, Cred{Token: sb.Token}) - if err != nil || sock == "" { - t.Fatalf("WakeAgentSocketOrRead with the reader off = %q, %v, want a wake", sock, err) - } - defer done() - if len(eng.restores) != 1 || sb.HibernateSnap != "" { - t.Errorf("restores=%v snapshot=%q, want the claim woken", eng.restores, sb.HibernateSnap) - } -} - func TestReaderOnLeavesRunningClaimsAlone(t *testing.T) { eng := newFakeEngine() m := newReaderManager(t, eng) diff --git a/sandboxd/pool/reload_test.go b/sandboxd/pool/reload_test.go index c17a7c77a..41d8203a0 100644 --- a/sandboxd/pool/reload_test.go +++ b/sandboxd/pool/reload_test.go @@ -262,13 +262,10 @@ func TestAReloadDropsTheLiveProxiesPooledConnections(t *testing.T) { func TestReloadIsAudited(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("setup manager: %v", err) - } + m := mustManager(t, cfg, newFakeEngine()) next := *m.cfg next.EgressInternalAllow = []string{"10.8.0.0/16"} - if _, err = m.ReloadConfig(t.Context(), &next); err != nil { + if _, err := m.ReloadConfig(t.Context(), &next); err != nil { t.Fatalf("reload: %v", err) } raw, err := os.ReadFile(filepath.Join(dir, "audit.jsonl")) @@ -295,9 +292,9 @@ func TestAnInterruptedReloadStillDestroysTheRetiredWarmVMs(t *testing.T) { waitFor(t, func() bool { return refilledTo(t, m, testKey, 2) }) m.mu.Lock() retired := []string{m.pools[testKey].warm[0].VMName, m.pools[testKey].warm[1].VMName} - m.mu.Unlock() m.refillSem = make(chan struct{}, 1) m.refillSem <- struct{}{} + m.mu.Unlock() ctx, cancel := context.WithCancel(t.Context()) cancel() next := *m.cfg diff --git a/sandboxd/pool/stale_claim_test.go b/sandboxd/pool/stale_claim_test.go index ae5ea3edf..1c93730cd 100644 --- a/sandboxd/pool/stale_claim_test.go +++ b/sandboxd/pool/stale_claim_test.go @@ -3,6 +3,7 @@ package pool import ( "errors" "fmt" + "sync" "testing" "github.com/cocoonstack/sandbox/sandboxd/types" @@ -15,6 +16,8 @@ func TestStaleClaimOperationsPreserveTheCurrentClaim(t *testing.T) { rollback := func(op func(*testing.T, *Manager, *types.Sandbox) error) func(*testing.T, *Manager, *types.Sandbox, func()) { return func(t *testing.T, m *Manager, sb *types.Sandbox, stale func()) { m.store.writeMu.Lock() + unlock := sync.OnceFunc(m.store.writeMu.Unlock) + t.Cleanup(unlock) done := make(chan error, 1) go func() { done <- op(t, m, sb) }() waitFor(t, func() bool { @@ -24,7 +27,7 @@ func TestStaleClaimOperationsPreserveTheCurrentClaim(t *testing.T) { }) stale() restore := failClaimWrites(t, m.store) - m.store.writeMu.Unlock() + unlock() if err := <-done; err == nil { t.Fatal("intent write succeeded despite filesystem failure") } diff --git a/sandboxd/pool/telemetry_test.go b/sandboxd/pool/telemetry_test.go index c5c2b993a..317aaa36c 100644 --- a/sandboxd/pool/telemetry_test.go +++ b/sandboxd/pool/telemetry_test.go @@ -560,7 +560,7 @@ func TestEgressUsageEventNamesTheInjectedCredentials(t *testing.T) { } } -func TestEgressAuditNamesTenantAndEndsARelayedFrameCannot(t *testing.T) { +func TestEgressAuditNamesTheTenantAndAddressesARelayedFrameCannotSet(t *testing.T) { dir := t.TempDir() cfg := &config.Config{DataDir: dir, AuditLog: true, Pools: []config.PoolSpec{}} m := mustManager(t, cfg, newFakeEngine()) diff --git a/sandboxd/pool/tenants_test.go b/sandboxd/pool/tenants_test.go index 1d8e5dbc6..baea8da2c 100644 --- a/sandboxd/pool/tenants_test.go +++ b/sandboxd/pool/tenants_test.go @@ -104,9 +104,7 @@ func TestRemovedTenantRunsToItsDeadlineButCannotExtend(t *testing.T) { if _, err := m.Renew(t.Context(), sb.ID, Cred{Operator: true}, time.Hour, ""); err != nil { t.Errorf("renew by the operator: %v", err) } - m.mu.Lock() - sb.Deadline = time.Now().Add(-time.Second) - m.mu.Unlock() + expire(m, sb) m.reapOnce(t.Context()) m.mu.Lock() _, live := m.claimed[sb.ID] diff --git a/sandboxd/pool/tun_test.go b/sandboxd/pool/tun_test.go index a343d8f88..85c786cb0 100644 --- a/sandboxd/pool/tun_test.go +++ b/sandboxd/pool/tun_test.go @@ -232,7 +232,7 @@ func TestAWakeIntoAPoolWithoutTunTellsTheGuestTunDown(t *testing.T) { } } -func TestADisableTheWakeMissedStillTurnsTheGuestOff(t *testing.T) { +func TestAHelloAfterADisableAnswersOff(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) @@ -455,10 +455,12 @@ func TestAnAdoptionConfirmsTheClaimsAddressBeforeTakingItsRow(t *testing.T) { addrs.lost = []string{sb.ID} } b.tunAddrs = addrs + var atTake []string + b.archives = &hookRegistry{Registry: b.archives, beforeTake: func() { atTake = addrs.confirmedList() }} mustAdopt(t, b, sb, "adopt") - if !slices.Equal(addrs.confirmedList(), []string{sb.ID}) { - t.Errorf("taken=%v: confirmed %v, want the adopted claim", taken, addrs.confirmedList()) + if !slices.Equal(atTake, []string{sb.ID}) { + t.Errorf("taken=%v: confirmed %v when the row was taken, want the adopted claim", taken, atTake) } adopted, _ := b.byID(sb.ID) if got := adopted.TunAddr.IsValid(); got == taken { diff --git a/sandboxd/pool/upstream_test.go b/sandboxd/pool/upstream_test.go index dc5440b3d..8c0c235d5 100644 --- a/sandboxd/pool/upstream_test.go +++ b/sandboxd/pool/upstream_test.go @@ -6,12 +6,11 @@ import ( "time" "github.com/cocoonstack/sandbox/sandboxd/config" - "github.com/cocoonstack/sandbox/sandboxd/egress" "github.com/cocoonstack/sandbox/sandboxd/types" ) func TestUpstreamEnvIsAdmittedHostOnlyAndAllowed(t *testing.T) { - m := upstreamManager(t, "127.0.0.1") + m := upstreamManager(t) for name, tt := range map[string]struct { v types.EnvVar ok bool @@ -35,7 +34,7 @@ func TestUpstreamEnvIsAdmittedHostOnlyAndAllowed(t *testing.T) { } func TestCheckpointClaimAdmitsTheUpstreamEnvLikeAnyClaim(t *testing.T) { - m := upstreamManager(t, "127.0.0.1") + m := upstreamManager(t) src := mustClaim(t, m, testKey) ckpt, err := m.Checkpoint(t.Context(), src.ID, Cred{Token: src.Token}, "", "") if err != nil { @@ -53,7 +52,7 @@ func TestCheckpointClaimAdmitsTheUpstreamEnvLikeAnyClaim(t *testing.T) { } func TestPatchSwitchesTheUpstreamForNewConnections(t *testing.T) { - m := upstreamManager(t, "127.0.0.1") + m := upstreamManager(t) sb := mustClaim(t, m, testKey) if err := m.PatchEnv(t.Context(), sb.ID, types.EnvPatch{"EGRESS_UPSTREAM": {Value: "http://127.0.0.1:3001", Guest: hostOnly}}, ""); err != nil { t.Fatalf("patch: %v", err) @@ -79,14 +78,11 @@ func TestPatchSwitchesTheUpstreamForNewConnections(t *testing.T) { } } -func upstreamManager(t *testing.T, allow ...string) *Manager { +func upstreamManager(t *testing.T) *Manager { t.Helper() m := newTestManager(t, newFakeEngine()) - if _, err := egress.ParseUpstreamAllow(allow); err != nil { - t.Fatalf("allow: %v", err) - } editEgress(t, m, func(cfg *config.Config) { - cfg.EgressUpstream = &config.EgressUpstreamConfig{ClaimEnv: "EGRESS_UPSTREAM", Allow: allow} + cfg.EgressUpstream = &config.EgressUpstreamConfig{ClaimEnv: "EGRESS_UPSTREAM", Allow: []string{"127.0.0.1"}} }) return m } diff --git a/sandboxd/pool/volume_attach_test.go b/sandboxd/pool/volume_attach_test.go index 76575706c..4fa7a2237 100644 --- a/sandboxd/pool/volume_attach_test.go +++ b/sandboxd/pool/volume_attach_test.go @@ -5,6 +5,7 @@ import ( "slices" "strings" "testing" + "testing/synctest" "github.com/cocoonstack/sandbox/sandboxd/config" "github.com/cocoonstack/sandbox/sandboxd/types" @@ -58,36 +59,38 @@ func TestAttachOnlyClaimAttachesWithoutMounting(t *testing.T) { } func TestAttachOnlyClaimAttachesEveryVolumeConcurrently(t *testing.T) { - dataset := writeVolumeImage(t, "dataset.img", "dataset") - scratch := writeVolumeImage(t, "scratch.img", "scratch") - cache := writeVolumeImage(t, "cache.img", "cache") - eng := newFakeEngine() - m := newVolumeManager(t, eng, []config.VolumeSpec{ - {Name: "dataset", Path: dataset}, - {Name: "scratch", Path: scratch, Writable: true}, - {Name: "cache", Path: cache, Writable: true}, - }) - requested := []types.Volume{ - {Name: "scratch", Mode: types.VolumeModeRW, AttachOnly: true}, - {Name: "dataset", AttachOnly: true}, - {Name: "cache", Mode: types.VolumeModeRW, AttachOnly: true}, - } - rendezvousAttaches(eng, len(requested)) + synctest.Test(t, func(t *testing.T) { + dataset := writeVolumeImage(t, "dataset.img", "dataset") + scratch := writeVolumeImage(t, "scratch.img", "scratch") + cache := writeVolumeImage(t, "cache.img", "cache") + eng := newFakeEngine() + m := newVolumeManager(t, eng, []config.VolumeSpec{ + {Name: "dataset", Path: dataset}, + {Name: "scratch", Path: scratch, Writable: true}, + {Name: "cache", Path: cache, Writable: true}, + }) + requested := []types.Volume{ + {Name: "scratch", Mode: types.VolumeModeRW, AttachOnly: true}, + {Name: "dataset", AttachOnly: true}, + {Name: "cache", Mode: types.VolumeModeRW, AttachOnly: true}, + } + rendezvousAttaches(eng, len(requested)) - sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: requested}) - if err != nil { - t.Fatalf("ClaimProvision: %v", err) - } - if !slices.Equal(sb.Volumes, requested) { - t.Errorf("volumes=%v, want the request order %v", sb.Volumes, requested) - } - wantOps := []string{"attach:cache", "attach:dataset", "attach:scratch", "probe", "provision"} - if ops := slices.Sorted(slices.Values(eng.volumeOpsLog())); !slices.Equal(ops, wantOps) { - t.Errorf("volume ops=%v, want exactly %v", eng.volumeOpsLog(), wantOps) - } - if volumeDirty(scratch) || volumeDirty(cache) { - t.Error("attach-only writers marked their images dirty") - } + sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: requested}) + if err != nil { + t.Fatalf("ClaimProvision: %v", err) + } + if !slices.Equal(sb.Volumes, requested) { + t.Errorf("volumes=%v, want the request order %v", sb.Volumes, requested) + } + wantOps := []string{"attach:cache", "attach:dataset", "attach:scratch", "probe", "provision"} + if ops := slices.Sorted(slices.Values(eng.volumeOpsLog())); !slices.Equal(ops, wantOps) { + t.Errorf("volume ops=%v, want exactly %v", eng.volumeOpsLog(), wantOps) + } + if volumeDirty(scratch) || volumeDirty(cache) { + t.Error("attach-only writers marked their images dirty") + } + }) } func TestClaimProvisionAttachFailureKeepsHoldsUntilRemoval(t *testing.T) { @@ -364,7 +367,9 @@ func assertHoldsUntilRetry(t *testing.T, m *Manager, eng *fakeEngine, vmName str 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.mu.Lock() eng.removeErrFor = "" + eng.mu.Unlock() m.retryRemovals(t.Context()).Wait() if !eng.removed(vmName) { t.Errorf("removes=%v, want %s drained", eng.removedNames(), vmName) diff --git a/sandboxd/pool/volume_capture_test.go b/sandboxd/pool/volume_capture_test.go index fbf281aaf..6d4a009e8 100644 --- a/sandboxd/pool/volume_capture_test.go +++ b/sandboxd/pool/volume_capture_test.go @@ -11,24 +11,6 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/types" ) -func TestVolumeClaimRunningWakeUnchanged(t *testing.T) { - eng := newFakeEngine() - m := newTestManager(t, eng) - sb := mustClaim(t, m, testKey) - sb.Volumes = []types.Volume{{Name: "dataset", Mount: "/datasets"}} - - sock, _, _, err := m.WakeAgentSocket(t.Context(), sb.ID, Cred{Token: sb.Token}) - if err != nil { - t.Fatalf("WakeAgentSocket: %v", err) - } - if sock != sb.VsockSocket { - t.Errorf("socket %q, want %q", sock, sb.VsockSocket) - } - if len(eng.restores) != 0 { - t.Errorf("running wake restored %v, want no restore", eng.restores) - } -} - func TestReconcileRetainsVolumeCaptureGateWithoutCatalog(t *testing.T) { eng := newFakeEngine() dataDir := t.TempDir() @@ -71,7 +53,7 @@ func TestReconcileRetainsVolumeCaptureGateWithoutCatalog(t *testing.T) { } }) } - if got, ok := m2.claimed[sb.ID]; !ok || !slices.Equal(got.Volumes, []types.Volume{{Name: "dataset", Mount: "/datasets"}}) { + if got, ok := m2.byID(sb.ID); !ok || !slices.Equal(got.Volumes, []types.Volume{{Name: "dataset", Mount: "/datasets"}}) { t.Errorf("reconciled claim=%+v, want persisted volume", got) } if len(eng.hibernates) != 0 || len(eng.snapSaves) != 0 || len(eng.exports) != 0 { @@ -87,7 +69,14 @@ func TestIdleOnceSkipsVolumeClaim(t *testing.T) { sb.Volumes = []types.Volume{{Name: "dataset", Mount: "/datasets"}} backdate(m, sb, 2*time.Second) - sweepIdle(t, m) + sb.Transition.Lock() + m.idleOnce(t.Context()) + picked := m.sweeps.idle.Load() + sb.Transition.Unlock() + waitFor(t, func() bool { return !m.sweeps.idle.Load() }) + if picked { + t.Error("idle sweep picked a volume claim") + } if len(eng.hibernates) != 0 || hibernated(m) != 0 { t.Errorf("idle sweep hibernated volume claim: engine=%v count=%d", eng.hibernates, hibernated(m)) } diff --git a/sandboxd/pool/volume_mutation_test.go b/sandboxd/pool/volume_mutation_test.go index 9145152e7..73e1fcfeb 100644 --- a/sandboxd/pool/volume_mutation_test.go +++ b/sandboxd/pool/volume_mutation_test.go @@ -18,7 +18,7 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/types" ) -func TestLiveVolumeFourModesAndMixedRoundtrip(t *testing.T) { +func TestLiveVolumeRoundtripLeavesDevicesAndRestartRecoversModes(t *testing.T) { eng := newFakeEngine() catalog := []config.VolumeSpec{} request := []types.Volume{{Name: "mounted"}, {Name: "writer", Mode: types.VolumeModeRW}, {Name: "raw", AttachOnly: true}, {Name: "raw-writer", Mode: types.VolumeModeRW, AttachOnly: true}} @@ -80,7 +80,7 @@ func TestLiveVolumeFourModesAndMixedRoundtrip(t *testing.T) { } } -func TestLiveVolumesCommitAndRetry(t *testing.T) { +func TestARepeatedVolumeTargetTakesOneHold(t *testing.T) { eng := newFakeEngine() m := newVolumeManager(t, eng, liveCatalog(t)) sb := mustClaim(t, m, testKey) @@ -452,7 +452,7 @@ func TestLiveVolumeCancelHiddenByTheEngineStillReadsAsCanceled(t *testing.T) { } } -func TestLiveVolumeAdmissionAndState(t *testing.T) { +func TestLiveVolumeAttachRefusesAClaimThatCannotTakeIt(t *testing.T) { for _, state := range []string{"paused", "failed", "archived", "archive policy", "dirty", "writer"} { t.Run(state, func(t *testing.T) { eng := newFakeEngine() diff --git a/sandboxd/pool/volume_test.go b/sandboxd/pool/volume_test.go index 4d6474876..30c788fd5 100644 --- a/sandboxd/pool/volume_test.go +++ b/sandboxd/pool/volume_test.go @@ -9,6 +9,7 @@ import ( "strings" "sync" "testing" + "testing/synctest" "github.com/cocoonstack/sandbox/sandboxd/config" "github.com/cocoonstack/sandbox/sandboxd/engine" @@ -73,45 +74,47 @@ func TestClaimProvisionAppliesVolumesInRequestOrder(t *testing.T) { } func TestClaimProvisionBringsMixedVolumesUpConcurrently(t *testing.T) { - dataset := writeVolumeImage(t, "dataset.img", "dataset") - weights := writeVolumeImage(t, "weights.img", "weights") - scratch := writeVolumeImage(t, "scratch.img", "scratch") - cache := writeVolumeImage(t, "cache.img", "cache") - eng := newFakeEngine() - m := newVolumeManager(t, eng, []config.VolumeSpec{ - {Name: "dataset", Path: dataset}, - {Name: "weights", Path: weights}, - {Name: "scratch", Path: scratch, Writable: true}, - {Name: "cache", Path: cache, Writable: true}, - }) - requested := []types.Volume{ - {Name: "weights", Mount: "/models"}, - {Name: "scratch", Mode: types.VolumeModeRW}, - {Name: "dataset"}, - {Name: "cache", Mount: "/cache", Mode: types.VolumeModeRW}, - } - wantApplied := []types.Volume{ - {Name: "weights", Mount: "/models"}, - {Name: "scratch", Mount: "/volumes/scratch", Mode: types.VolumeModeRW}, - {Name: "dataset", Mount: "/volumes/dataset"}, - {Name: "cache", Mount: "/cache", Mode: types.VolumeModeRW}, - } - rendezvousAttaches(eng, len(requested)) + synctest.Test(t, func(t *testing.T) { + dataset := writeVolumeImage(t, "dataset.img", "dataset") + weights := writeVolumeImage(t, "weights.img", "weights") + scratch := writeVolumeImage(t, "scratch.img", "scratch") + cache := writeVolumeImage(t, "cache.img", "cache") + eng := newFakeEngine() + m := newVolumeManager(t, eng, []config.VolumeSpec{ + {Name: "dataset", Path: dataset}, + {Name: "weights", Path: weights}, + {Name: "scratch", Path: scratch, Writable: true}, + {Name: "cache", Path: cache, Writable: true}, + }) + requested := []types.Volume{ + {Name: "weights", Mount: "/models"}, + {Name: "scratch", Mode: types.VolumeModeRW}, + {Name: "dataset"}, + {Name: "cache", Mount: "/cache", Mode: types.VolumeModeRW}, + } + wantApplied := []types.Volume{ + {Name: "weights", Mount: "/models"}, + {Name: "scratch", Mount: "/volumes/scratch", Mode: types.VolumeModeRW}, + {Name: "dataset", Mount: "/volumes/dataset"}, + {Name: "cache", Mount: "/cache", Mode: types.VolumeModeRW}, + } + rendezvousAttaches(eng, len(requested)) - sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: requested}) - if err != nil { - t.Fatalf("ClaimProvision: %v", err) - } - if !slices.Equal(sb.Volumes, wantApplied) { - t.Errorf("volumes=%v, want the request order %v", sb.Volumes, wantApplied) - } - assertVolumeBringUp(t, eng, wantApplied) - if !volumeDirty(scratch) || !volumeDirty(cache) { - t.Error("live writable claim left an image unmarked") - } - if volumeDirty(dataset) || volumeDirty(weights) { - t.Error("read-only volume of a mixed claim was marked dirty") - } + sb, err := m.ClaimProvision(t.Context(), testKey, ClaimOptions{Volumes: requested}) + if err != nil { + t.Fatalf("ClaimProvision: %v", err) + } + if !slices.Equal(sb.Volumes, wantApplied) { + t.Errorf("volumes=%v, want the request order %v", sb.Volumes, wantApplied) + } + assertVolumeBringUp(t, eng, wantApplied) + if !volumeDirty(scratch) || !volumeDirty(cache) { + t.Error("live writable claim left an image unmarked") + } + if volumeDirty(dataset) || volumeDirty(weights) { + t.Error("read-only volume of a mixed claim was marked dirty") + } + }) } func TestClaimProvisionOneVolumeAttachFailureFailsWholeClaim(t *testing.T) { diff --git a/sandboxd/pool/warmup_test.go b/sandboxd/pool/warmup_test.go index bcc7decc9..b0f2d6a05 100644 --- a/sandboxd/pool/warmup_test.go +++ b/sandboxd/pool/warmup_test.go @@ -5,7 +5,6 @@ import ( "os" "path/filepath" "slices" - "strings" "testing" "testing/synctest" @@ -94,13 +93,6 @@ func TestAdoptGoldenRequiresMatchingWarmup(t *testing.T) { } } -func TestPoolSpecRejectsEmptyWarmupArgument(t *testing.T) { - spec := config.PoolSpec{PoolKey: testKey, Warm: 1, Warmup: []string{"node", ""}} - if err := spec.ValidateLimits(); err == nil || !strings.Contains(err.Error(), "warmup") { - t.Errorf("ValidateLimits error = %v, want an empty-argument rejection", err) - } -} - func TestRefillWarmsEveryClone(t *testing.T) { synctest.Test(t, func(t *testing.T) { eng := newFakeEngine() diff --git a/sandboxd/poolset/pg/pg_test.go b/sandboxd/poolset/pg/pg_test.go index baa61e280..29315435d 100644 --- a/sandboxd/poolset/pg/pg_test.go +++ b/sandboxd/poolset/pg/pg_test.go @@ -4,6 +4,7 @@ import ( "context" "errors" "strings" + "sync" "testing" "time" @@ -50,12 +51,9 @@ func TestWatchFiresOnConnectAndOnItsCellsWrites(t *testing.T) { a, b, other := openIn(t, dsn, cell), openIn(t, dsn, cell), openIn(t, dsn, cell+"-other") ctx, cancel := context.WithCancel(t.Context()) fired := make(chan struct{}, 8) - done := make(chan struct{}) - go func() { - defer close(done) - a.Watch(ctx, func() { fired <- struct{}{} }) - }() - defer func() { cancel(); <-done }() + var wg sync.WaitGroup + wg.Go(func() { a.Watch(ctx, func() { fired <- struct{}{} }) }) + defer func() { cancel(); wg.Wait() }() waitFire(t, fired, "the listener connect") if _, err := other.Store(t.Context(), poolset.Set{}); err != nil { t.Fatalf("store on the other cell: %v", err) diff --git a/sandboxd/server/client_endpoint_test.go b/sandboxd/server/client_endpoint_test.go index c329b5ec0..48e09d648 100644 --- a/sandboxd/server/client_endpoint_test.go +++ b/sandboxd/server/client_endpoint_test.go @@ -11,7 +11,7 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/types" ) -func TestClientAddressesInResponses(t *testing.T) { +func TestResponsesNamePeersByTheirClientAddress(t *testing.T) { for _, tt := range []struct{ method, path, body, field string }{ {http.MethodPost, "/v1/claim", `{"template":"rt"}`, "redirect"}, {http.MethodPost, "/v1/checkpoints/ck_1/claim", `{}`, "redirect"}, @@ -39,7 +39,7 @@ func TestClientAddressesInResponses(t *testing.T) { } } -func TestClientOwner(t *testing.T) { +func TestOwnerAddrIsTheAdvertisedClientURL(t *testing.T) { s := New("", "https://self.example", &fakeManager{}, &fakeDialer{}, nil, nil, nil, nil) if got := s.claimResponse(&types.Sandbox{ID: "sb_1"}).OwnerAddr; got != "https://self.example" { t.Fatalf("owner = %q", got) diff --git a/sandboxd/server/exec_test.go b/sandboxd/server/exec_test.go index 457288642..930b9cdea 100644 --- a/sandboxd/server/exec_test.go +++ b/sandboxd/server/exec_test.go @@ -94,7 +94,7 @@ func TestExecAuditsTheFrameWithoutItsDelimiter(t *testing.T) { t.Fatalf("status %d, want 200: %s", status, body) } want, _ := wire.EncodeRequest(wire.Exec{Argv: []string{"true"}}) - if got := <-audited; !bytes.Equal(got, want) { + if got := await(t, audited, "the exec was never audited"); !bytes.Equal(got, want) { t.Errorf("audited %q, want the bare frame %q", got, want) } } @@ -215,7 +215,7 @@ func TestExecTimeoutStartsAfterWake(t *testing.T) { if status, body := postExec(t, ts, `{"argv":["true"],"timeout_seconds":1}`); status != http.StatusOK { t.Fatalf("status %d, want 200: %s", status, body) } - if <-wakeHadCommandDeadline { + if await(t, wakeHadCommandDeadline, "the exec never woke the sandbox") { t.Error("command timeout was active during wake") } } @@ -314,7 +314,7 @@ func TestExecTakesTheRootToken(t *testing.T) { } return } - if got := <-woke; got != tt.wantCred { + if got := await(t, woke, "the exec never woke the sandbox"); got != tt.wantCred { t.Errorf("woke with %+v, want %+v", got, tt.wantCred) } if wantAudit := code == http.StatusOK; (len(audited) == 1) != wantAudit { diff --git a/sandboxd/server/move_test.go b/sandboxd/server/move_test.go index 1a5ef35e0..4ede3bbbe 100644 --- a/sandboxd/server/move_test.go +++ b/sandboxd/server/move_test.go @@ -40,7 +40,6 @@ func TestMoveRouteResolvesThePeerAndMapsTheOutcome(t *testing.T) { }{ {`{"to":"10.0.0.9:7777"}`, nil, http.StatusBadRequest}, {`{"to":"10.0.0.2:7777","max_downtime_ms":-1}`, nil, http.StatusBadRequest}, - {`{"to":"10.0.0.2:7777","max_downtime_ms":400}`, nil, http.StatusBadRequest}, {`{"to":"10.0.0.2:7777","max_downtime_ms":401}`, nil, http.StatusOK}, {`{"to":"10.0.0.2:7777"}`, pool.ErrUnknownSandbox, http.StatusNotFound}, {`{"to":"10.0.0.2:7777"}`, fmt.Errorf("%w: it has volumes", pool.ErrNotMovable), http.StatusConflict}, diff --git a/sandboxd/server/port_test.go b/sandboxd/server/port_test.go index 694d262cd..d17a04e33 100644 --- a/sandboxd/server/port_test.go +++ b/sandboxd/server/port_test.go @@ -7,6 +7,7 @@ import ( "net" "net/http" "net/http/httptest" + "slices" "testing" "testing/iotest" "testing/synctest" @@ -21,7 +22,7 @@ const portRequest = "GET /v1/sandboxes/sb_1/ports/49983 HTTP/1.1\r\n" + "Upgrade: tcp\r\n" + "Connection: Upgrade\r\n\r\n" -func TestPortRelayRoundTrip(t *testing.T) { +func TestPortRelayEchoesBytesBothWaysAfterTheUpgrade(t *testing.T) { ts, _ := newPortServer(t, func(c net.Conn) { defer c.Close() _, _ = io.Copy(c, c) @@ -124,7 +125,7 @@ func TestPortRelayTakesTheRootTokenAsTheOperator(t *testing.T) { t.Errorf("%s: status %d, want 409 for a paused sandbox", token, code) } } - if len(got) != 2 || got[0] != "" || got[1] != "sandbox-token" { + if !slices.Equal(got, []string{"", "sandbox-token"}) { t.Errorf("manager saw tokens %q, want the operator credential then the sandbox token", got) } } diff --git a/sandboxd/server/preview_test.go b/sandboxd/server/preview_test.go index b01c1b536..2229fd29f 100644 --- a/sandboxd/server/preview_test.go +++ b/sandboxd/server/preview_test.go @@ -15,7 +15,7 @@ import ( "github.com/cocoonstack/sandbox/sandboxd/pool" ) -func TestPreviewTokenRoundTrip(t *testing.T) { +func TestPreviewTokenVerifiesOnlyUntamperedUnderItsSecret(t *testing.T) { ps := NewPreviewServer("secret", "https://preview.example.com", "node:7777", &fakePreviewMgr{}) token := mintToken(ps, "sb_1", 8080, time.Hour) diff --git a/sandboxd/server/relay_test.go b/sandboxd/server/relay_test.go index e3d3fde78..53054b846 100644 --- a/sandboxd/server/relay_test.go +++ b/sandboxd/server/relay_test.go @@ -57,7 +57,7 @@ func TestCloseRelaysRefusesLateRelays(t *testing.T) { } } -func TestRelayRoundTrip(t *testing.T) { +func TestRelayDeliversEveryGuestFrameThenEndsAtExit(t *testing.T) { frames := []string{ `{"type":"started","pid":7}` + "\n", `{"type":"stdout","data":"NDIK"}` + "\n", diff --git a/sandboxd/server/server_test.go b/sandboxd/server/server_test.go index ff7616536..8c0b0505f 100644 --- a/sandboxd/server/server_test.go +++ b/sandboxd/server/server_test.go @@ -88,7 +88,7 @@ func TestClaimCarriesAnEgressOptOut(t *testing.T) { } } -func TestClaimErrorMapping(t *testing.T) { +func TestClaimMapsEachManagerErrorToItsStatus(t *testing.T) { tests := []struct { name string body string @@ -309,7 +309,7 @@ func TestEnvVerbsForRootOrTheTenant(t *testing.T) { } } -func TestEnvPatchVerb(t *testing.T) { +func TestEnvPatchMergesForRootOrTheTenantAndRefusesBadBodies(t *testing.T) { mgr := &fakeManager{} ts := newTenantTestServer(t, "root", []tenants.TenantSpec{{Name: "acme", Token: "acme-tok"}}, mgr, nil) call := func(t *testing.T, token, body string) int { @@ -368,7 +368,7 @@ func TestClaimRefusesAnAmbiguousHostOnlyFlag(t *testing.T) { } } -func TestAPITokenGuard(t *testing.T) { +func TestTheAPITokenGatesEveryRouteButHealthz(t *testing.T) { ts := newTestServer(t, "sekret", &fakeManager{}, nil) tests := []struct { @@ -513,7 +513,7 @@ func TestSandboxesNarrowToAClaimRef(t *testing.T) { } } -func TestTenantAuthMatrix(t *testing.T) { +func TestEachTokenReachesTheManagerAsItsTenantOrIsRefused(t *testing.T) { mgr := &fakeManager{tenantClaims: map[string]int{"acme": 2}} specs := []tenants.TenantSpec{{Name: "acme", Token: "acme-tok"}, {Name: "beta", Token: "beta-tok"}} ts := newTenantTestServer(t, "sekret", specs, mgr, nil) @@ -546,12 +546,6 @@ func TestTenantAuthMatrix(t *testing.T) { {"root lists all volumes", http.MethodGet, "/v1/volumes", "sekret", http.StatusOK, ""}, {"tenant lists own sandboxes", http.MethodGet, "/v1/sandboxes", "acme-tok", http.StatusOK, "acme"}, {"root lists all sandboxes", http.MethodGet, "/v1/sandboxes", "sekret", http.StatusOK, ""}, - {"tenant forbidden on info", http.MethodGet, "/v1/info", "acme-tok", http.StatusForbidden, ""}, - {"tenant forbidden on metrics", http.MethodGet, "/metrics", "acme-tok", http.StatusForbidden, ""}, - {"tenant forbidden on pools", http.MethodPut, "/v1/pools", "acme-tok", http.StatusForbidden, ""}, - {"tenant forbidden on checkpoint blob", http.MethodGet, "/v1/checkpoints/ck_00000000000000aa/blob", "acme-tok", http.StatusForbidden, ""}, - {"tenant forbidden on cordon", http.MethodPost, "/v1/cordon", "acme-tok", http.StatusForbidden, ""}, - {"tenant forbidden on uncordon", http.MethodDelete, "/v1/cordon", "acme-tok", http.StatusForbidden, ""}, {"wrong token on info stays 401", http.MethodGet, "/v1/info", "nope", http.StatusUnauthorized, ""}, {"root reads info", http.MethodGet, "/v1/info", "sekret", http.StatusOK, ""}, } { @@ -571,7 +565,7 @@ func TestTenantAuthMatrix(t *testing.T) { } } -func TestMetricsTenantGauge(t *testing.T) { +func TestMetricsReportEachTenantsClaims(t *testing.T) { mgr := &fakeManager{tenantClaims: map[string]int{"acme": 2, "beta": 0}} ts := newTestServer(t, "", mgr, nil) @@ -662,7 +656,7 @@ func TestPutPoolsRejectsUnknownFields(t *testing.T) { } } -func TestInstanceMetadataVerb(t *testing.T) { +func TestInstanceMetadataStoresARootDocumentVerbatimAndMapsFailures(t *testing.T) { var got []byte var fail error mgr := &fakeManager{setInstanceMetadata: func(id string, doc []byte) error { @@ -673,12 +667,12 @@ func TestInstanceMetadataVerb(t *testing.T) { return fail }} ts := newTenantTestServer(t, "sekret", []tenants.TenantSpec{{Name: "acme", Token: "acme-tok"}}, mgr, nil) - put := func(token, id, body string) int { + put := func(t *testing.T, token, id, body string) int { t.Helper() return statusOf(t, http.MethodPut, ts.URL+"/v1/sandboxes/"+id+"/instance-metadata", token, body) } valid := `{"region": "local", "tags": ["a", "b"]}` - if code := put("sekret", "sb_1", valid); code != http.StatusNoContent { + if code := put(t, "sekret", "sb_1", valid); code != http.StatusNoContent { t.Fatalf("status %d, want 204", code) } if string(got) != valid { @@ -696,20 +690,20 @@ func TestInstanceMetadataVerb(t *testing.T) { {"unknown sandbox", "sekret", "sb_2", valid, http.StatusNotFound}, } { t.Run(tt.name, func(t *testing.T) { - if code := put(tt.token, tt.id, tt.body); code != tt.want { + if code := put(t, tt.token, tt.id, tt.body); code != tt.want { t.Errorf("%s: status %d, want %d", tt.name, code, tt.want) } }) } for _, err := range []error{pool.ErrPaused, pool.ErrArchived, pool.ErrNoInstanceMetadata} { fail = err - if code := put("sekret", "sb_1", valid); code != http.StatusConflict { + if code := put(t, "sekret", "sb_1", valid); code != http.StatusConflict { t.Errorf("%v: status %d, want 409", err, code) } } } -func TestDrainEndpoints(t *testing.T) { +func TestDrainSetsAndClearsDraining(t *testing.T) { mgr := &fakeManager{} ts := newTestServer(t, "sekret", mgr, nil) for _, tt := range []struct { @@ -730,7 +724,7 @@ func TestDrainEndpoints(t *testing.T) { } } -func TestCordonEndpoints(t *testing.T) { +func TestCordonSetsAndClearsCordonedWithoutDraining(t *testing.T) { mgr := &fakeManager{} ts := newTestServer(t, "sekret", mgr, nil) for _, tt := range []struct { @@ -883,7 +877,7 @@ func TestReleaseHibernateAndWakeMapEachOutcomeToItsStatus(t *testing.T) { } } -func TestReleaseOperatorToken(t *testing.T) { +func TestReleaseIsAnOperatorReleaseOnlyForTheRootToken(t *testing.T) { const rootTok, sbTok = "sekret", "sb-secret" specs := []tenants.TenantSpec{{Name: "acme", Token: "acme-tok"}} tests := []struct { @@ -935,7 +929,7 @@ func TestReleaseOperatorToken(t *testing.T) { } } -func TestAgentErrorPaths(t *testing.T) { +func TestAgentMapsEachFailureToItsStatus(t *testing.T) { tests := []struct { name string auth string @@ -1024,7 +1018,7 @@ func TestAgentRelayTakesTheRootOnlyAsAClaimToken(t *testing.T) { t.Fatalf("do: %v", err) } defer resp.Body.Close() - if got := <-woke; resp.StatusCode != http.StatusNotFound || got != "root" { + if got := await(t, woke, "the agent relay never asked for the socket"); resp.StatusCode != http.StatusNotFound || got != "root" { t.Errorf("status %d woke with %q, want 404 with the root as a claim token", resp.StatusCode, got) } } @@ -1212,7 +1206,7 @@ func TestCheckpointCanBeTakenClaimedAndDeleted(t *testing.T) { } } -func TestDeleteCheckpointNoForwardQueryParam(t *testing.T) { +func TestDeleteCheckpointStaysLocalOnlyWithNoForward(t *testing.T) { mgr := &fakeManager{deleteCheckpoint: func(string) error { return nil }} ts := newTestServer(t, "api", mgr, &fakeDialer{}) @@ -1243,7 +1237,7 @@ func TestDeleteCheckpointForgetsProbeCache(t *testing.T) { ts := newProberTestServer(t, "api", mgr, prober) resp := doReq(t, http.MethodDelete, ts.URL+"/v1/checkpoints/ck_0011223344556677", "api", "") defer resp.Body.Close() - if len(prober.forgotten) != 1 || prober.forgotten[0] != "ck_0011223344556677" { + if !slices.Equal(prober.forgotten, []string{"ck_0011223344556677"}) { t.Errorf("forgotten = %v, want [ck_0011223344556677]", prober.forgotten) } }) @@ -1254,7 +1248,7 @@ func TestDeleteCheckpointForgetsProbeCache(t *testing.T) { ts := newProberTestServer(t, "api", mgr, prober) resp := doReq(t, http.MethodDelete, ts.URL+"/v1/checkpoints/ck_0011223344556677?no_forward=1", "api", "") defer resp.Body.Close() - if len(prober.forgotten) != 1 || prober.forgotten[0] != "ck_0011223344556677" { + if !slices.Equal(prober.forgotten, []string{"ck_0011223344556677"}) { t.Errorf("forgotten = %v, want [ck_0011223344556677] even though nothing was held locally", prober.forgotten) } }) @@ -1340,7 +1334,7 @@ func TestDeleteTemplateRedirectsToOwner(t *testing.T) { t.Fatalf("status %d, want 200 redirect", resp.StatusCode) } cr := decodeJSON[ClaimResponse](t, resp.Body) - if len(cr.Redirect) != 1 || cr.Redirect[0] != "node-b:7777" { + if !slices.Equal(cr.Redirect, []string{"node-b:7777"}) { t.Errorf("redirect %v, want [node-b:7777]", cr.Redirect) } @@ -1987,7 +1981,7 @@ func TestCheckpointClaimRedirectsToOwner(t *testing.T) { t.Fatalf("status = %d, want 200 carrying a redirect (the mesh redirect is a 200, not a 3xx)", resp.StatusCode) } got := decodeJSON[ClaimResponse](t, resp.Body) - if len(got.Redirect) != 1 || got.Redirect[0] != "owner-a:7777" { + if !slices.Equal(got.Redirect, []string{"owner-a:7777"}) { t.Errorf("redirect = %v, want [owner-a:7777]", got.Redirect) } if got.ID != "" { @@ -2203,7 +2197,7 @@ func TestCheckpointClaimProbesRealPeerAndRedirects(t *testing.T) { t.Fatalf("status = %d, want 200 carrying a redirect", resp.StatusCode) } got := decodeJSON[ClaimResponse](t, resp.Body) - if len(got.Redirect) != 1 || got.Redirect[0] != tsB.URL { + if !slices.Equal(got.Redirect, []string{tsB.URL}) { t.Errorf("redirect = %v, want [%s]", got.Redirect, tsB.URL) } } @@ -2348,17 +2342,17 @@ func TestTunVerbIsRootOnlyAndPassesOnlyTheNamedFields(t *testing.T) { return nil }} ts := newTenantTestServer(t, "sekret", []tenants.TenantSpec{{Name: "acme", Token: "acme-tok"}}, mgr, nil) - put := func(token, id, body string) int { + put := func(t *testing.T, token, id, body string) int { t.Helper() return statusOf(t, http.MethodPut, ts.URL+"/v1/sandboxes/"+id+"/tun", token, body) } - if code := put("sekret", "sb_1", `{"enabled": false}`); code != http.StatusNoContent { + if code := put(t, "sekret", "sb_1", `{"enabled": false}`); code != http.StatusNoContent { t.Fatalf("status %d, want 204", code) } if gotEnabled == nil || *gotEnabled || gotProfile != nil { t.Errorf("manager got enabled %v profile %v, want false and no profile", gotEnabled, gotProfile) } - if code := put("sekret", "sb_1", `{"profile": "res"}`); code != http.StatusNoContent || gotEnabled != nil || gotProfile == nil || *gotProfile != "res" { + if code := put(t, "sekret", "sb_1", `{"profile": "res"}`); code != http.StatusNoContent || gotEnabled != nil || gotProfile == nil || *gotProfile != "res" { t.Errorf("profile only: status %d, enabled %v profile %v", code, gotEnabled, gotProfile) } for _, tt := range []struct { @@ -2372,7 +2366,7 @@ func TestTunVerbIsRootOnlyAndPassesOnlyTheNamedFields(t *testing.T) { {"unknown sandbox", "sekret", "sb_2", `{"enabled": true}`, http.StatusNotFound}, } { t.Run(tt.name, func(t *testing.T) { - if code := put(tt.token, tt.id, tt.body); code != tt.want { + if code := put(t, tt.token, tt.id, tt.body); code != tt.want { t.Errorf("%s: status %d, want %d", tt.name, code, tt.want) } }) diff --git a/sandboxd/server/volume_mutation_test.go b/sandboxd/server/volume_mutation_test.go index cd7432feb..5b4403f4e 100644 --- a/sandboxd/server/volume_mutation_test.go +++ b/sandboxd/server/volume_mutation_test.go @@ -67,7 +67,7 @@ func TestLiveVolumeRoutesAuthorizeAndDecode(t *testing.T) { } } -func TestLiveVolumeRoutesErrors(t *testing.T) { +func TestLiveVolumeRoutesMapEachErrorAndKeepThePartialView(t *testing.T) { for _, tt := range []struct { name, token, body string err error diff --git a/sandboxd/store/dir/dir_test.go b/sandboxd/store/dir/dir_test.go index 42bf7eb5c..760171010 100644 --- a/sandboxd/store/dir/dir_test.go +++ b/sandboxd/store/dir/dir_test.go @@ -62,7 +62,9 @@ func TestPlainPublishReplacesDigestWithEmpty(t *testing.T) { root := t.TempDir() st := newStore(t, root) firstMeta := []byte(storetest.MetaJSON("digested")) - firstDigest := mustPublishDigested(t, st, testID, "digested") + if mustPublishDigested(t, st, testID, "digested") == "" { + t.Fatal("digested generation returned an empty digest") + } firstDigestPath := filepath.Join(root, testID, digestName(firstMeta)) mustPublish(t, st, testID, "plain") @@ -70,9 +72,6 @@ func TestPlainPublishReplacesDigestWithEmpty(t *testing.T) { if string(meta) != storetest.MetaJSON("plain") || digest != "" { t.Fatalf("plain replacement meta/digest = %q/%q, want plain/empty", meta, digest) } - if firstDigest == "" { - t.Fatal("digested generation returned an empty digest") - } assertPresentf(t, firstDigestPath, "superseded digest removed inside the grace") mustPublish(t, st, testID, "digested") if _, _, digest = mustFetch(t, st, testID); digest != "" { @@ -447,10 +446,8 @@ func TestSweepGenerationsRemovesOnlyAgedStaging(t *testing.T) { func TestSweepGenerationsToleratesConcurrentDelete(t *testing.T) { st := newStore(t, t.TempDir()) ctx := t.Context() - done := make(chan struct{}) var g errgroup.Group g.Go(func() error { - defer close(done) for range 500 { staging, err := st.Stage(testID) if err != nil { @@ -472,16 +469,12 @@ func TestSweepGenerationsToleratesConcurrentDelete(t *testing.T) { return nil }) g.Go(func() error { - for { - select { - case <-done: - return nil - default: - } + for range 500 { if err := st.SweepGenerations(ctx); err != nil { return fmt.Errorf("sweep: %w", err) } } + return nil }) if err := g.Wait(); err != nil { t.Fatal(err) diff --git a/sandboxd/store/peer/broadcast_test.go b/sandboxd/store/peer/broadcast_test.go index 88d0e89d1..3ff762e23 100644 --- a/sandboxd/store/peer/broadcast_test.go +++ b/sandboxd/store/peer/broadcast_test.go @@ -38,12 +38,10 @@ func TestBroadcastDeleteFansOutToEveryPeer(t *testing.T) { b := &Broadcaster{Peers: func() []string { return addrs }} b.Delete(t.Context(), testID) - got := 0 - for _, n := range hits { - got += int(n.Load()) - } - if got != 3 { - t.Errorf("peers hit %d times, want 3 (one per distinct peer)", got) + for i, n := range hits { + if got := n.Load(); got != 1 { + t.Errorf("peer %d hit %d times, want 1", i, got) + } } } diff --git a/sandboxd/store/peer/move_test.go b/sandboxd/store/peer/move_test.go index 26b6d335a..7f1aab965 100644 --- a/sandboxd/store/peer/move_test.go +++ b/sandboxd/store/peer/move_test.go @@ -223,7 +223,7 @@ func TestSendRoundFailsOnARefusedPutOrAMissingFile(t *testing.T) { func TestSendRoundFailsWhenATargetStopsReading(t *testing.T) { src := t.TempDir() - if err := os.WriteFile(filepath.Join(src, "memory-ranges"), bytes.Repeat([]byte("guest-memory-"), 4<<20), 0o600); err != nil { + if err := os.WriteFile(filepath.Join(src, "memory-ranges"), bytes.Repeat([]byte("guest-memory-"), 1<<20), 0o600); err != nil { t.Fatal(err) } stop := make(chan struct{}) diff --git a/sandboxd/store/peer/probe_test.go b/sandboxd/store/peer/probe_test.go index 687da6556..667f72b74 100644 --- a/sandboxd/store/peer/probe_test.go +++ b/sandboxd/store/peer/probe_test.go @@ -39,7 +39,7 @@ func TestOwnersReturnsPromptlyWithOneOwner(t *testing.T) { p := &HTTPProber{Peers: func() []string { return []string{hung.URL, ok.URL} }} start := time.Now() owners := p.Owners(t.Context(), testID) - if elapsed := time.Since(start); elapsed > probeGrace+500*time.Millisecond { + if elapsed := time.Since(start); elapsed >= probeTimeout-100*time.Millisecond { t.Errorf("Owners took %v with one owner, want ~grace, not the full probe timeout", elapsed) } if len(owners) != 1 || owners[0] != ok.URL { @@ -82,7 +82,7 @@ func TestOwnersCancelsStragglersOnceCapReached(t *testing.T) { start := time.Now() p := &HTTPProber{Peers: func() []string { return addrs }} owners := p.Owners(t.Context(), testID) - if elapsed := time.Since(start); elapsed > 500*time.Millisecond { + if elapsed := time.Since(start); elapsed >= probeTimeout-100*time.Millisecond { t.Errorf("Owners took %v, want fast: the cap was met without the straggler", elapsed) } if len(owners) != maxRedirectOwners { @@ -156,7 +156,7 @@ func TestOwnersCacheCollapsesRepeatedRedirects(t *testing.T) { } w.WriteHeader(http.StatusNotFound) })) - defer srv.Close() + t.Cleanup(srv.Close) addrs = append(addrs, srv.URL) } diff --git a/sandboxd/store/s3/s3_test.go b/sandboxd/store/s3/s3_test.go index 591412378..d57c51c44 100644 --- a/sandboxd/store/s3/s3_test.go +++ b/sandboxd/store/s3/s3_test.go @@ -151,7 +151,7 @@ func TestFetchOutlivesTheCallerThatStartedIt(t *testing.T) { _, _, _, err := st.Fetch(first, id) started <- err }() - <-fake.exportEntered + fake.waitExport(t) joined := make(chan error, 1) go func() { _, _, _, err := st.Fetch(t.Context(), id) @@ -748,7 +748,7 @@ func TestSweepGenerationsDropsACacheACanceledFetchInstalledAfterDelete(t *testin _, _, _, err := st.Fetch(ctx, id) fetched <- err }() - <-fake.exportEntered + fake.waitExport(t) cancel() if err := <-fetched; !errors.Is(err, context.Canceled) { t.Fatalf("Fetch = %v, want its cancellation", err) @@ -758,13 +758,13 @@ func TestSweepGenerationsDropsACacheACanceledFetchInstalledAfterDelete(t *testin } release() cached := filepath.Join(st.staging, cacheDir, id) - for deadline := time.Now().Add(5 * time.Second); ; time.Sleep(5 * time.Millisecond) { - if entries, _ := os.ReadDir(cached); len(entries) > 0 { - break - } - if time.Now().After(deadline) { - t.Fatal("the detached download never installed its cache") - } + select { + case <-st.fetches.DoChan(filepath.Join(cached, store.ExportGenHash(metaJSON(id))), func() (any, error) { return nil, nil }): + case <-time.After(5 * time.Second): + t.Fatal("the detached download never finished") + } + if entries, _ := os.ReadDir(cached); len(entries) == 0 { + t.Fatal("the detached download never installed its cache") } if err := st.SweepGenerations(t.Context()); err != nil { @@ -843,7 +843,7 @@ func TestFetchFileInstallsOnlyACompleteDownload(t *testing.T) { go func() { fetched <- st.FetchFile(t.Context(), id, "disk.img", dst) }() - <-fake.exportEntered + fake.waitExport(t) if _, err := os.Stat(dst); !errors.Is(err, os.ErrNotExist) { t.Errorf("a download in flight is already visible at the destination: %v", err) } @@ -1107,6 +1107,15 @@ func (f *fakeS3) resetRequests() { clear(f.requests) } +func (f *fakeS3) waitExport(t *testing.T) { + t.Helper() + select { + case <-f.exportEntered: + case <-time.After(5 * time.Second): + t.Fatal("the gated download never started") + } +} + func (f *fakeS3) handlePutObject(w http.ResponseWriter, r *http.Request, key string) { body, err := io.ReadAll(r.Body) if err != nil { diff --git a/sandboxd/tenants/pg/pg_test.go b/sandboxd/tenants/pg/pg_test.go index 4c65c1b76..fb821baf4 100644 --- a/sandboxd/tenants/pg/pg_test.go +++ b/sandboxd/tenants/pg/pg_test.go @@ -103,12 +103,13 @@ func TestListAndClassesFailFastOnAStalledDatabase(t *testing.T) { } { t.Run(name, func(t *testing.T) { t.Parallel() - ctx, cancel := context.WithTimeout(t.Context(), metastore.QueryTimeout+3*time.Second) + bound := metastore.QueryTimeout + 3*time.Second + ctx, cancel := context.WithTimeout(t.Context(), bound) defer cancel() start := time.Now() err := call(ctx) - if elapsed := time.Since(start); elapsed > metastore.QueryTimeout+time.Second { - t.Errorf("took %v, want under %v", elapsed, metastore.QueryTimeout+time.Second) + if elapsed := time.Since(start); elapsed >= bound { + t.Errorf("took %v, want under the caller's %v", elapsed, bound) } if !errors.Is(err, tenants.ErrUnavailable) { t.Errorf("err = %v, want ErrUnavailable", err) diff --git a/sandboxd/tunnet/bench_test.go b/sandboxd/tunnet/bench_test.go index 3f304348c..7303277b1 100644 --- a/sandboxd/tunnet/bench_test.go +++ b/sandboxd/tunnet/bench_test.go @@ -6,7 +6,7 @@ import ( ) func BenchmarkForwardGSO(b *testing.B) { - l := benchLink() + l := benchLink(b) f := gsoFrame(ProtoTCP, 63000, 1398) buf := make([]byte, len(f)) b.SetBytes(int64(len(f))) @@ -20,7 +20,7 @@ func BenchmarkForwardGSO(b *testing.B) { } func BenchmarkForwardSmall(b *testing.B) { - l := benchLink() + l := benchLink(b) f := frame(ProtoUDP, GuestAddr, remote, false) buf := make([]byte, len(f)) b.SetBytes(int64(len(f))) @@ -34,7 +34,7 @@ func BenchmarkForwardSmall(b *testing.B) { } func BenchmarkDeliver(b *testing.B) { - l := benchLink() + l := benchLink(b) l.forward(frame(ProtoTCP, GuestAddr, remote, false)) d := reply(remote) inner, _, _ := parseVXLAN(d, 7) @@ -43,6 +43,7 @@ func BenchmarkDeliver(b *testing.B) { l.r.pool.Put(bp) } }() + b.Cleanup(func() { close(l.send) }) b.SetBytes(int64(len(inner))) b.ReportAllocs() for b.Loop() { @@ -52,13 +53,16 @@ func BenchmarkDeliver(b *testing.B) { } } -func benchLink() *link { +func benchLink(b *testing.B) *link { + b.Helper() r := New(nopUnder{}, testOverlay(1450)) c := Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }} r.Arm(c) a := r.claims[claimAddr] l := &link{r: r, a: a, mtu: 1450, merge: true, send: make(chan *[]byte, sendQueue), done: make(chan struct{})} - a.attach(l) + if !a.attach(l) { + b.Fatal("the bench claim was disarmed") + } return l } diff --git a/sandboxd/tunnet/router_test.go b/sandboxd/tunnet/router_test.go index 6b10bd5da..78a9bf783 100644 --- a/sandboxd/tunnet/router_test.go +++ b/sandboxd/tunnet/router_test.go @@ -7,9 +7,11 @@ import ( "io" "net" "net/netip" + "os" "sync" "sync/atomic" "testing" + "testing/synctest" "time" ) @@ -20,336 +22,409 @@ var ( ) func TestRouterAnnouncesRewritesAndPassesOnlyReplies(t *testing.T) { - u, r := newRouter(t) - var records []FlowRecord - var mu sync.Mutex - r.Arm(Claim{ - ID: "c1", Addr: claimAddr, - Allow: func(_ netip.Addr, port uint16) bool { return port == 443 }, - Record: func(fr FlowRecord) { mu.Lock(); records = append(records, fr); mu.Unlock() }, + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + var records []FlowRecord + var mu sync.Mutex + r.Arm(Claim{ + ID: "c1", Addr: claimAddr, + Allow: func(_ netip.Addr, port uint16) bool { return port == 443 }, + Record: func(fr FlowRecord) { mu.Lock(); records = append(records, fr); mu.Unlock() }, + }) + if inner, etype, to := u.sent(t); etype != etherARP || to.Addr() != vtep { + t.Fatalf("arm sent %v to %v, want a gratuitous ARP to the exit", etype, to) + } else if _, _, target, ok := arpRequest(inner); !ok || target != claimAddr { + t.Errorf("gratuitous ARP announces %v", target) + } + guest := serve(t, r, claimAddr) + if h := guestHello(t, guest); h.Flags&FlagOn == 0 || h.MTU != 1450 || h.Offloads&OffloadTSO4 == 0 { + t.Fatalf("host hello = %+v", h) + } + sendFrame(t, guest, frame(ProtoTCP, netip.MustParseAddr("100.80.0.99"), remote, true)) + inner, etype, _ := u.sent(t) + pkt := append(make([]byte, VnetHdrLen), inner...) + if p, _ := Parse(pkt); etype != etherIPv4 || p.Src != claimAddr || !validIP(pkt) || !validL4(pkt) { + t.Fatalf("sent src %v, want the claim address with full checksums", p.Src) + } + denied := frame(ProtoTCP, GuestAddr, remote, false) + binary.BigEndian.PutUint16(denied[VnetHdrLen+20+2:], 22) + sendFrame(t, guest, denied) + sendFrame(t, guest, frame(ProtoTCP, GuestAddr, netip.MustParseAddr("100.80.0.5"), false)) + stranger := reply(netip.MustParseAddr("198.51.100.1")) + binary.BigEndian.PutUint16(stranger[Overhead+20:], 22) + u.inject(t, stranger) + u.inject(t, reply(remote)) + got := readFrame(t, guest) + if p, _ := Parse(got); p.Dst != GuestAddr || p.Src != remote || !validIP(got) { + t.Fatalf("delivered %+v, want the reply rewritten to the guest", p) + } + synctest.Wait() + if got := r.Stats().Drops[DropPolicy]; got != 3 { + t.Fatalf("policy drops = %d, want 3", got) + } + if n := len(u.out); n != 0 { + t.Errorf("%d more datagrams sent, want none for denied traffic", n) + } + _ = guest.Close() + r.Disarm(claimAddr) + synctest.Wait() + mu.Lock() + defer mu.Unlock() + if len(records) != 2 { + t.Errorf("recorded %d flows after the disarm, want 2", len(records)) + } }) - if inner, etype, to := u.sent(t); etype != etherARP || to.Addr() != vtep { - t.Fatalf("arm sent %v to %v, want a gratuitous ARP to the exit", etype, to) - } else if _, _, target, ok := arpRequest(inner); !ok || target != claimAddr { - t.Errorf("gratuitous ARP announces %v", target) - } - guest := serve(t, r, claimAddr) - if h := guestHello(t, guest); h.Flags&FlagOn == 0 || h.MTU != 1450 || h.Offloads&OffloadTSO4 == 0 { - t.Fatalf("host hello = %+v", h) - } - sendFrame(t, guest, frame(ProtoTCP, netip.MustParseAddr("100.80.0.99"), remote, true)) - inner, etype, _ := u.sent(t) - pkt := append(make([]byte, VnetHdrLen), inner...) - if p, _ := Parse(pkt); etype != etherIPv4 || p.Src != claimAddr || !validIP(pkt) || !validL4(pkt) { - t.Fatalf("sent src %v, want the claim address with full checksums", p.Src) - } - denied := frame(ProtoTCP, GuestAddr, remote, false) - binary.BigEndian.PutUint16(denied[VnetHdrLen+20+2:], 22) - sendFrame(t, guest, denied) - sendFrame(t, guest, frame(ProtoTCP, GuestAddr, netip.MustParseAddr("100.80.0.5"), false)) - stranger := reply(netip.MustParseAddr("198.51.100.1")) - binary.BigEndian.PutUint16(stranger[Overhead+20:], 22) - u.inject(t, stranger) - u.inject(t, reply(remote)) - got := readFrame(t, guest) - if p, _ := Parse(got); p.Dst != GuestAddr || p.Src != remote || !validIP(got) { - t.Fatalf("delivered %+v, want the reply rewritten to the guest", p) - } - waitFor(t, func() bool { return r.Stats().Drops[DropPolicy] == 3 }) - if n := len(u.out); n != 0 { - t.Errorf("%d more datagrams sent, want none for denied traffic", n) - } - _ = guest.Close() - r.Disarm(claimAddr) - waitFor(t, func() bool { mu.Lock(); defer mu.Unlock(); return len(records) == 2 }) } func TestRouterAnswersARPOnlyForArmedClaims(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr}) - u.sent(t) - asker := netip.MustParseAddr("100.80.0.1") - for _, target := range []netip.Addr{netip.MustParseAddr("100.80.0.8"), claimAddr} { + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr}) + u.sent(t) + asker := netip.MustParseAddr("100.80.0.1") + for _, target := range []netip.Addr{netip.MustParseAddr("100.80.0.8"), claimAddr} { + b := appendHeader(nil, 7, broadcast, exitMAC, etherARP) + u.inject(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, target)) + } + inner, _, to := u.sent(t) + if to.Addr() != vtep || binary.BigEndian.Uint16(inner[6:]) != arpReply || netip.AddrFrom4([4]byte(inner[14:18])) != claimAddr { + t.Fatalf("arp reply to %v: %x", to, inner) + } b := appendHeader(nil, 7, broadcast, exitMAC, etherARP) - u.inject(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, target)) - } - inner, _, to := u.sent(t) - if to.Addr() != vtep || binary.BigEndian.Uint16(inner[6:]) != arpReply || netip.AddrFrom4([4]byte(inner[14:18])) != claimAddr { - t.Fatalf("arp reply to %v: %x", to, inner) - } - b := appendHeader(nil, 7, broadcast, exitMAC, etherARP) - u.injectFrom(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, claimAddr), netip.MustParseAddrPort("10.0.0.77:4789")) - waitFor(t, func() bool { return r.Stats().Drops[DropSender] == 1 }) - r.Disarm(claimAddr) - b = appendHeader(nil, 7, broadcast, exitMAC, etherARP) - u.inject(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, claimAddr)) - sentinel := netip.MustParseAddr("100.80.0.8") - r.Arm(Claim{ID: "c2", Addr: sentinel}) - u.sent(t) - b = appendHeader(nil, 7, broadcast, exitMAC, etherARP) - u.inject(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, sentinel)) - if inner, _, _ := u.sent(t); netip.AddrFrom4([4]byte(inner[14:18])) != sentinel { - t.Fatalf("first answer after the disarm announces %x, want only the sentinel's", inner[14:18]) - } - if len(u.out) != 0 { - t.Error("a disarmed claim, or a sender that is no exit, got an ARP answer") - } + u.injectFrom(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, claimAddr), netip.MustParseAddrPort("10.0.0.77:4789")) + synctest.Wait() + if got := r.Stats().Drops[DropSender]; got != 1 { + t.Fatalf("sender drops = %d, want 1", got) + } + r.Disarm(claimAddr) + b = appendHeader(nil, 7, broadcast, exitMAC, etherARP) + u.inject(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, claimAddr)) + sentinel := netip.MustParseAddr("100.80.0.8") + r.Arm(Claim{ID: "c2", Addr: sentinel}) + u.sent(t) + b = appendHeader(nil, 7, broadcast, exitMAC, etherARP) + u.inject(t, appendARP(b, arpReq, exitMAC, asker, [6]byte{}, sentinel)) + if inner, _, _ := u.sent(t); netip.AddrFrom4([4]byte(inner[14:18])) != sentinel { + t.Fatalf("first answer after the disarm announces %x, want only the sentinel's", inner[14:18]) + } + synctest.Wait() + if len(u.out) != 0 { + t.Error("a disarmed claim, or a sender that is no exit, got an ARP answer") + } + }) } func TestRouterSegmentsGSOIntoOneBatch(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 1400)) - for i := range 3 { - inner, _, _ := u.sent(t) - if pkt := append(make([]byte, VnetHdrLen), inner...); !validIP(pkt) || !validL4(pkt) { - t.Errorf("segment %d invalid", i) + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 1400)) + for i := range 3 { + inner, _, _ := u.sent(t) + if pkt := append(make([]byte, VnetHdrLen), inner...); !validIP(pkt) || !validL4(pkt) { + t.Errorf("segment %d invalid", i) + } } - } - if b := u.batches.Load(); b != 2 { - t.Errorf("sends = %d, want the ARP and one batch for the GSO frame", b) - } + if b := u.batches.Load(); b != 2 { + t.Errorf("sends = %d, want the ARP and one batch for the GSO frame", b) + } + }) } func TestRouterRefusesUnarmedAndDisabledAndDisarmEndsStream(t *testing.T) { - _, r := newRouter(t) - if h := guestHello(t, serve(t, r, claimAddr)); h.Flags&FlagOn != 0 { - t.Error("an unarmed address got on") - } - r.Arm(Claim{ID: "c1", Addr: claimAddr, Enabled: func() bool { return false }}) - off := serve(t, r, claimAddr) - if h := guestHello(t, off); h.Flags&FlagOn != 0 { - t.Error("a disabled claim got on") - } - if _, err := off.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { - t.Errorf("stream after off: %v", err) - } - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) - on := serve(t, r, claimAddr) - guestHello(t, on) - waitFor(t, func() bool { return r.claims[claimAddr].live() != nil }) - r.Disarm(claimAddr) - if _, err := on.Read(make([]byte, 1)); err == nil { - t.Error("stream still open after disarm") - } + synctest.Test(t, func(t *testing.T) { + _, r := newRouter(t) + if h := guestHello(t, serve(t, r, claimAddr)); h.Flags&FlagOn != 0 { + t.Error("an unarmed address got on") + } + r.Arm(Claim{ID: "c1", Addr: claimAddr, Enabled: func() bool { return false }}) + off := serve(t, r, claimAddr) + if h := guestHello(t, off); h.Flags&FlagOn != 0 { + t.Error("a disabled claim got on") + } + _ = off.SetReadDeadline(time.Now().Add(2 * time.Second)) + if _, err := off.Read(make([]byte, 1)); !errors.Is(err, io.EOF) { + t.Errorf("stream after off: %v", err) + } + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) + on := serve(t, r, claimAddr) + guestHello(t, on) + synctest.Wait() + if r.claims[claimAddr].live() == nil { + t.Fatal("the armed claim's stream never went live") + } + r.Disarm(claimAddr) + _ = on.SetReadDeadline(time.Now().Add(2 * time.Second)) + if _, err := on.Read(make([]byte, 1)); err == nil || errors.Is(err, os.ErrDeadlineExceeded) { + t.Errorf("stream still open after disarm: %v", err) + } + }) } func TestRouterQuotaStopsTraffic(t *testing.T) { - u, r := newRouter(t) - used := new(atomic.Int64) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Used: used, Limits: Limits{QuotaBytes: 100}, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - f := frame(ProtoTCP, GuestAddr, remote, false) - for range 3 { - sendFrame(t, guest, f) - } - u.sent(t) - waitFor(t, func() bool { return r.Stats().Drops[DropLimit] == 2 }) - if used.Load() > 100 { - t.Errorf("used %d past the quota", used.Load()) - } + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + used := new(atomic.Int64) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Used: used, Limits: Limits{QuotaBytes: 100}, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + f := frame(ProtoTCP, GuestAddr, remote, false) + for range 3 { + sendFrame(t, guest, f) + } + u.sent(t) + synctest.Wait() + if got := r.Stats().Drops[DropLimit]; got != 2 { + t.Fatalf("limit drops = %d, want 2", got) + } + if used.Load() > 100 { + t.Errorf("used %d past the quota", used.Load()) + } + }) } func TestRouterChargesNothingForAFrameWithNoExit(t *testing.T) { - _, r := newRouter(t) - 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) - } + synctest.Test(t, func(t *testing.T) { + _, r := newRouter(t) + 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)) + synctest.Wait() + if got := r.Stats().Drops[DropNoExit]; got != 1 { + t.Fatalf("no-exit drops = %d, want 1", got) + } + 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(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) - u.sent(t) - bad := dataSegment(5000, 1000) - bad[len(bad)-1] ^= 0xff - 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 { - t.Fatalf("frame gso %d size %d len %d, want one TCPv4 GSO frame of 3400 payload bytes at 1000", merged[1], binary.LittleEndian.Uint16(merged[4:]), len(ip)) - } - if got, want := binary.BigEndian.Uint16(ip[36:]), fold(pseudoSum(ip, len(ip)-20)); got != want || !validIP(merged) { - t.Errorf("merged checksum %#x want the pseudo-header sum %#x, ip valid %v", got, want, validIP(merged)) - } - if p, _ := Parse(merged); p.Dst != GuestAddr { - t.Errorf("merged frame goes to %v", p.Dst) - } - if lone := readFrame(t, guest); lone[1] != gsoNone || validL4(lone) { - t.Error("a segment with a bad checksum was merged or repaired instead of passed through") - } + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) + u.sent(t) + bad := dataSegment(5000, 1000) + bad[len(bad)-1] ^= 0xff + 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 { + t.Fatalf("frame gso %d size %d len %d, want one TCPv4 GSO frame of 3400 payload bytes at 1000", merged[1], binary.LittleEndian.Uint16(merged[4:]), len(ip)) + } + if got, want := binary.BigEndian.Uint16(ip[36:]), fold(pseudoSum(ip, len(ip)-20)); got != want || !validIP(merged) { + t.Errorf("merged checksum %#x want the pseudo-header sum %#x, ip valid %v", got, want, validIP(merged)) + } + if p, _ := Parse(merged); p.Dst != GuestAddr { + t.Errorf("merged frame goes to %v", p.Dst) + } + if lone := readFrame(t, guest); lone[1] != gsoNone || validL4(lone) { + t.Error("a segment with a bad checksum was merged or repaired instead of passed through") + } + }) } func TestRouterChargesSegmentsAndRefusesOversizedPackets(t *testing.T) { - u, r := newRouter(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 - } - }() - guest := serve(t, r, claimAddr) - guestHello(t, guest) - sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 1420)) - big := gsoFrame(ProtoUDP, 1500, 1500) - big[1] = gsoNone - sendFrame(t, guest, big) - sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 8)) - sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 40)) - sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 40)) - waitFor(t, func() bool { s := r.Stats(); return s.Drops[DropSize] == 3 && s.Drops[DropLimit] == 1 }) - if s := r.Stats(); s.DgramsOut != 100 { - t.Errorf("sent %d datagrams, want the 100 segments of the one frame the packet bucket admitted", s.DgramsOut) - } + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(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 { + select { + case <-u.out: + case <-u.done: + return + } + } + }() + guest := serve(t, r, claimAddr) + guestHello(t, guest) + sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 1420)) + big := gsoFrame(ProtoUDP, 1500, 1500) + big[1] = gsoNone + sendFrame(t, guest, big) + sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 8)) + sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 40)) + sendFrame(t, guest, gsoFrame(ProtoTCP, 4000, 40)) + synctest.Wait() + s := r.Stats() + if s.Drops[DropSize] != 3 { + t.Errorf("size drops = %d, want 3", s.Drops[DropSize]) + } + if s.Drops[DropLimit] != 1 { + t.Errorf("limit drops = %d, want 1", s.Drops[DropLimit]) + } + if s.DgramsOut != 100 { + t.Errorf("sent %d datagrams, want the 100 segments of the one frame the packet bucket admitted", s.DgramsOut) + } + }) } func TestRouterLimitsOutliveAReconnect(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Limits: Limits{NewFlowsPerSecond: 0.01}, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - first := serve(t, r, claimAddr) - guestHello(t, first) - sendFrame(t, first, frame(ProtoTCP, GuestAddr, remote, false)) - u.sent(t) - _ = first.Close() - second := serve(t, r, claimAddr) - guestHello(t, second) - other := frame(ProtoTCP, GuestAddr, remote, false) - binary.BigEndian.PutUint16(other[VnetHdrLen+20:], 40001) - sendFrame(t, second, other) - waitFor(t, func() bool { return r.Stats().Drops[DropLimit] == 1 }) + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Limits: Limits{NewFlowsPerSecond: 0.01}, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + first := serve(t, r, claimAddr) + guestHello(t, first) + sendFrame(t, first, frame(ProtoTCP, GuestAddr, remote, false)) + u.sent(t) + _ = first.Close() + second := serve(t, r, claimAddr) + guestHello(t, second) + other := frame(ProtoTCP, GuestAddr, remote, false) + binary.BigEndian.PutUint16(other[VnetHdrLen+20:], 40001) + sendFrame(t, second, other) + synctest.Wait() + if got := r.Stats().Drops[DropLimit]; got != 1 { + t.Errorf("limit drops = %d, want 1", got) + } + }) } func TestRouterHandsAnICMPErrorBackWithTheQuotedGuestAddress(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) - inner, _, _ := u.sent(t) - 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) { - t.Fatalf("quoted source %v, checksums icmp %#x quoted %#x", netip.AddrFrom4([4]byte(ip[28+srcAt:])), fold(onesSum(ip[20:], 0)), fold(onesSum(ip[28:48], 0))) - } + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) + inner, _, _ := u.sent(t) + 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) { + t.Fatalf("quoted source %v, checksums icmp %#x quoted %#x", netip.AddrFrom4([4]byte(ip[28+srcAt:])), fold(onesSum(ip[20:], 0)), fold(onesSum(ip[28:48], 0))) + } + }) } func TestRouterResumesAStreamOpenedBeforeTheArmButRefusesAnOpener(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(_ netip.Addr, port uint16) bool { return port == 443 }}) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - waitFor(t, func() bool { return r.claims[claimAddr].live() != nil }) - syn := reply(remote) - syn[Overhead+20+13] = tcpSyn - u.inject(t, syn) - waitFor(t, func() bool { return r.Stats().Drops[DropPolicy] == 1 }) - u.inject(t, reply(remote)) - if p, _ := Parse(readFrame(t, guest)); p.Src != remote || p.Dst != GuestAddr { - t.Fatalf("delivered %+v, want the stream's next segment", p) - } + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(_ netip.Addr, port uint16) bool { return port == 443 }}) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + synctest.Wait() + if r.claims[claimAddr].live() == nil { + t.Fatal("the armed claim's stream never went live") + } + syn := reply(remote) + syn[Overhead+20+13] = tcpSyn + u.inject(t, syn) + synctest.Wait() + if got := r.Stats().Drops[DropPolicy]; got != 1 { + t.Fatalf("policy drops = %d, want 1", got) + } + u.inject(t, reply(remote)) + if p, _ := Parse(readFrame(t, guest)); p.Src != remote || p.Dst != GuestAddr { + t.Fatalf("delivered %+v, want the stream's next segment", p) + } + }) } func TestRouterKeepsTheMTUAStreamWasToldUntilItsNextHello(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - r.Reload(testOverlay(1410)) - u.sent(t) - full := gsoFrame(ProtoUDP, 1412, 1412) - full[0], full[1] = 0, gsoNone - sendFrame(t, guest, full) - if inner, _, _ := u.sent(t); len(inner) != 1440 { - t.Errorf("sent %d bytes, want the 1440-byte packet the stream's MTU allows", len(inner)) - } + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + r.Reload(testOverlay(1410)) + u.sent(t) + full := gsoFrame(ProtoUDP, 1412, 1412) + full[0], full[1] = 0, gsoNone + sendFrame(t, guest, full) + if inner, _, _ := u.sent(t); len(inner) != 1440 { + t.Errorf("sent %d bytes, want the 1440-byte packet the stream's MTU allows", len(inner)) + } + }) } func TestEndFlowsRecordsLiveFlowsWithIPBytes(t *testing.T) { - u, r := newRouter(t) - var mu sync.Mutex - var records []FlowRecord - r.Arm(Claim{ - ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }, - Record: func(fr FlowRecord) { mu.Lock(); records = append(records, fr); mu.Unlock() }, + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + var mu sync.Mutex + var records []FlowRecord + r.Arm(Claim{ + ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }, + Record: func(fr FlowRecord) { mu.Lock(); records = append(records, fr); mu.Unlock() }, + }) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + out := frame(ProtoTCP, GuestAddr, remote, false) + sendFrame(t, guest, out) + u.sent(t) + in := reply(remote) + u.inject(t, in) + readFrame(t, guest) + r.EndFlows() + mu.Lock() + defer mu.Unlock() + if len(records) != 1 || records[0].Up != int64(len(out)-VnetHdrLen) || records[0].Down != int64(len(in)-Overhead) { + t.Fatalf("records %+v, want one flow with %d up and %d down IP bytes", records, len(out)-VnetHdrLen, len(in)-Overhead) + } }) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - out := frame(ProtoTCP, GuestAddr, remote, false) - sendFrame(t, guest, out) - u.sent(t) - in := reply(remote) - u.inject(t, in) - readFrame(t, guest) - r.EndFlows() - mu.Lock() - defer mu.Unlock() - if len(records) != 1 || records[0].Up != int64(len(out)-VnetHdrLen) || records[0].Down != int64(len(in)-Overhead) { - t.Fatalf("records %+v, want one flow with %d up and %d down IP bytes", records, len(out)-VnetHdrLen, len(in)-Overhead) - } } func TestDeliverMergesOnlyForAGuestThatOfferedTSO(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - host, guest := net.Pipe() - t.Cleanup(func() { _ = guest.Close() }) - go func() { _ = r.Serve(host, claimAddr) }() - h := Hello{Version: Version, Offloads: OffloadCSUM}.Encode() - if _, err := guest.Write(h[:]); err != nil { - t.Fatal(err) - } - guestHello(t, guest) - sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) - u.sent(t) - 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)) + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + host, guest := net.Pipe() + t.Cleanup(func() { _ = guest.Close() }) + go func() { _ = r.Serve(host, claimAddr) }() + h := Hello{Version: Version, Offloads: OffloadCSUM}.Encode() + if _, err := guest.Write(h[:]); err != nil { + t.Fatal(err) } - } + guestHello(t, guest) + sendFrame(t, guest, frame(ProtoTCP, GuestAddr, remote, false)) + u.sent(t) + 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)) + } + } + }) } func TestRouterPointsAGuestICMPErrorAtTheClaimAddress(t *testing.T) { - u, r := newRouter(t) - r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) - u.sent(t) - guest := serve(t, r, claimAddr) - guestHello(t, guest) - quoted := frame(ProtoUDP, remote, GuestAddr, false)[VnetHdrLen:][:28] - 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))) - } + synctest.Test(t, func(t *testing.T) { + u, r := newRouter(t) + r.Arm(Claim{ID: "c1", Addr: claimAddr, Allow: func(netip.Addr, uint16) bool { return true }}) + u.sent(t) + guest := serve(t, r, claimAddr) + guestHello(t, guest) + quoted := frame(ProtoUDP, remote, GuestAddr, false)[VnetHdrLen:][:28] + 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))) + } + }) } func newRouter(t *testing.T) (*fakeUnder, *Router) { @@ -455,16 +530,6 @@ func unreachable(src, dst netip.Addr, code byte, quoted []byte) []byte { 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) { - if ok() { - return - } - } - t.Fatal("condition not reached") -} - type sent struct { d []byte to netip.AddrPort @@ -520,6 +585,7 @@ func (u *fakeUnder) injectBatch(t *testing.T, ds ...[]byte) { func (u *fakeUnder) sent(t *testing.T) ([]byte, uint16, netip.AddrPort) { t.Helper() + synctest.Wait() select { case s := <-u.out: inner, etype, ok := parseVXLAN(s.d, 7) @@ -527,7 +593,7 @@ func (u *fakeUnder) sent(t *testing.T) ([]byte, uint16, netip.AddrPort) { t.Fatalf("sent a datagram that is not VXLAN for vni 7: %x", s.d) } return inner, etype, s.to - case <-time.After(2 * time.Second): + default: t.Fatal("nothing sent") return nil, 0, netip.AddrPort{} } diff --git a/sandboxd/tunnet/udp/udp_linux_test.go b/sandboxd/tunnet/udp/udp_linux_test.go index c48aee670..0584ef25f 100644 --- a/sandboxd/tunnet/udp/udp_linux_test.go +++ b/sandboxd/tunnet/udp/udp_linux_test.go @@ -3,7 +3,9 @@ package udp import ( "net" "net/netip" + "slices" "testing" + "time" ) func TestSendBatchArrivesAsSegments(t *testing.T) { @@ -17,13 +19,14 @@ func TestSendBatchArrivesAsSegments(t *testing.T) { if err = u.Send(batch, 1000, dst, 7); err != nil { t.Fatal(err) } + _ = u.recv.SetReadDeadline(time.Now().Add(5 * time.Second)) var sizes []int for len(sizes) < 4 { if err = u.Recv(func(d []byte, _ netip.AddrPort) { sizes = append(sizes, len(d)) }); err != nil { t.Fatal(err) } } - if sizes[0] != 1000 || sizes[3] != 400 { + if !slices.Equal(sizes, []int{1000, 1000, 1000, 400}) { t.Errorf("datagram sizes %v, want 1000 x3 then 400", sizes) } } diff --git a/sandboxd/types/env_test.go b/sandboxd/types/env_test.go index 1194812f4..971d3962f 100644 --- a/sandboxd/types/env_test.go +++ b/sandboxd/types/env_test.go @@ -152,8 +152,17 @@ func TestEnvInjectMatchesAndCompares(t *testing.T) { a := Env{"K": {Value: "v", Guest: new(false), Inject: inj}} b := Env{"K": {Value: "v", Guest: new(false), Inject: &EnvInject{Hosts: []string{"api.example.com"}, Header: "X-Key"}}} c := Env{"K": {Value: "v", Guest: new(false), Inject: &EnvInject{Hosts: inj.Hosts, Header: "X-Key", Placeholder: "@K@"}}} - if a.Equal(b) || a.Equal(c) || !a.Equal(Env{"K": {Value: "v", Guest: new(false), Inject: inj}}) || !a.SameGuest(b) { - t.Error("Equal must see an inject or placeholder change, and SameGuest must not") + if a.Equal(b) { + t.Error("Equal missed an inject hosts change") + } + if a.Equal(c) { + t.Error("Equal missed a placeholder change") + } + if !a.Equal(Env{"K": {Value: "v", Guest: new(false), Inject: inj}}) { + t.Error("Equal rejected an identical env") + } + if !a.SameGuest(b) { + t.Error("SameGuest saw an inject change") } q := Env{"K": {Value: "v", Guest: new(false), Inject: &EnvInject{Hosts: inj.Hosts, Query: "key", Placeholder: "@K@"}}} qb := Env{"K": {Value: "v", Guest: new(false), Inject: &EnvInject{Hosts: inj.Hosts, Query: "key", Body: true, Placeholder: "@K@"}}} diff --git a/sandboxd/types/types_test.go b/sandboxd/types/types_test.go index a8d3357d2..32787e92a 100644 --- a/sandboxd/types/types_test.go +++ b/sandboxd/types/types_test.go @@ -47,14 +47,14 @@ func TestValidateSizes(t *testing.T) { } } -func TestXLargeSpec(t *testing.T) { +func TestXLargeSpecIsFourCPUsAndEightGiB(t *testing.T) { spec, ok := SizeXLarge.Spec() if !ok || spec.CPU != 4 || spec.Memory != "8G" { t.Errorf("got %+v ok=%v, want {4 8G} true", spec, ok) } } -func TestMedium4GSpec(t *testing.T) { +func TestMedium4GSpecIsTwoCPUsAndFourGiB(t *testing.T) { spec, ok := SizeMedium4G.Spec() if !ok || spec.CPU != 2 || spec.Memory != "4G" || spec.MemoryBytes != 4<<30 { t.Errorf("got %+v ok=%v, want {2 4G 4GiB} true", spec, ok) diff --git a/sandboxd/usage/pg/pg_test.go b/sandboxd/usage/pg/pg_test.go index 1924a797a..76c5c3883 100644 --- a/sandboxd/usage/pg/pg_test.go +++ b/sandboxd/usage/pg/pg_test.go @@ -128,6 +128,7 @@ func TestLandWaitsForTheCellLock(t *testing.T) { if err != nil { t.Fatalf("begin: %v", err) } + t.Cleanup(func() { _ = tx.Rollback(context.WithoutCancel(ctx)) }) if _, err = tx.Exec(ctx, cellLock, "c1"); err != nil { t.Fatalf("take the cell lock: %v", err) } diff --git a/sdk/go/checkpoint_test.go b/sdk/go/checkpoint_test.go index 225e543be..d60b23a41 100644 --- a/sdk/go/checkpoint_test.go +++ b/sdk/go/checkpoint_test.go @@ -141,8 +141,9 @@ func TestCheckpointNewRedirectFallbackHeals(t *testing.T) { } func TestCheckpointNewRedirectNeverYieldsEmptyID(t *testing.T) { + dead := retiredAddr(t) entry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _ = json.MarshalWrite(w, claimResponse{Redirect: []string{"127.0.0.1:1"}}) + _ = json.MarshalWrite(w, claimResponse{Redirect: []string{dead}}) })) t.Cleanup(entry.Close) @@ -158,8 +159,9 @@ func TestCheckpointNewRedirectNeverYieldsEmptyID(t *testing.T) { } func TestCheckpointNewSecondLevelRedirectFails(t *testing.T) { + dead := retiredAddr(t) nodeB := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - _ = json.MarshalWrite(w, claimResponse{Redirect: []string{"127.0.0.1:1"}}) + _ = json.MarshalWrite(w, claimResponse{Redirect: []string{dead}}) })) t.Cleanup(nodeB.Close) diff --git a/sdk/go/client_test.go b/sdk/go/client_test.go index 193a59cef..cecad6933 100644 --- a/sdk/go/client_test.go +++ b/sdk/go/client_test.go @@ -194,13 +194,15 @@ func TestAMovedSandboxRetriesAStreamCallAtItsNewOwner(t *testing.T) { return err }, } { - sb, adopter := movedSandbox(t) - if err := call(sb); err != nil { - t.Errorf("%s after the move: %v", name, err) - } - if got := sb.Owner(); got != adopter { - t.Errorf("%s: owner %s, want the adopter %s", name, got, adopter) - } + t.Run(name, func(t *testing.T) { + sb, adopter := movedSandbox(t) + if err := call(sb); err != nil { + t.Errorf("after the move: %v", err) + } + if got := sb.Owner(); got != adopter { + t.Errorf("owner %s, want the adopter %s", got, adopter) + } + }) } } @@ -372,7 +374,11 @@ func TestCloseReportsARetiredNodeWhenNoNodeHoldsTheSandbox(t *testing.T) { func TestNewFollowsRedirect(t *testing.T) { var nodeB *httptest.Server - nodeB = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + var gotNoRedirect bool + nodeB = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var req claimRequest + _ = json.UnmarshalRead(r.Body, &req) + gotNoRedirect = req.NoRedirect _ = json.MarshalWrite(w, claimResponse{ ID: "sb_2", Token: "tok", OwnerAddr: addrOf(nodeB), }) @@ -391,6 +397,9 @@ func TestNewFollowsRedirect(t *testing.T) { if sb.owner != addrOf(nodeB) { t.Errorf("owner %q, want node B", sb.owner) } + if !gotNoRedirect { + t.Error("redirected claim did not set no_redirect") + } } func TestDeleteTemplateFollowsRedirect(t *testing.T) { @@ -445,36 +454,12 @@ func TestDeleteTemplateRetryPolicy(t *testing.T) { }) } -func TestRedirectSetsNoRedirect(t *testing.T) { - var gotNoRedirect bool - target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - var req claimRequest - _ = json.UnmarshalRead(r.Body, &req) - gotNoRedirect = req.NoRedirect - _ = json.MarshalWrite(w, claimResponse{ID: "sb_3", Token: "t"}) - })) - t.Cleanup(target.Close) - - entry := redirectEntry(t, addrOf(target)) - - sb, err := testClient(t, entry).New(t.Context(), "rt:24.04") - if err != nil { - t.Fatalf("New: %v", err) - } - if sb.ID != "sb_3" { - t.Errorf("id %q, want sb_3", sb.ID) - } - if !gotNoRedirect { - t.Error("retry did not set no_redirect") - } -} - func TestRedirectWalksPastACandidateThatRefusedTheClaim(t *testing.T) { for _, tt := range []struct { name string first func(*testing.T) string }{ - {"unreachable", func(*testing.T) string { return "127.0.0.1:1" }}, + {"unreachable", retiredAddr}, {"forbidden", func(t *testing.T) string { return addrOf(statusServer(t, http.StatusForbidden, nil)) }}, } { t.Run(tt.name, func(t *testing.T) { diff --git a/sdk/go/drain_test.go b/sdk/go/drain_test.go index 1515088ac..5a081f610 100644 --- a/sdk/go/drain_test.go +++ b/sdk/go/drain_test.go @@ -18,7 +18,7 @@ func TestDrainCordonVerbs(t *testing.T) { post := r.Method == http.MethodPost _ = json.MarshalWrite(w, NodeInfo{Claimed: 3, Draining: post && r.URL.Path == "/v1/drain", Cordoned: post && r.URL.Path == "/v1/cordon"}) })) - defer ts.Close() + t.Cleanup(ts.Close) c := testClient(t, ts, WithAPIToken("root")) for _, tt := range []struct { diff --git a/sdk/go/git_test.go b/sdk/go/git_test.go index b67090eb7..c8b1620da 100644 --- a/sdk/go/git_test.go +++ b/sdk/go/git_test.go @@ -59,10 +59,10 @@ func TestGitPushPullPlumbing(t *testing.T) { } })) if err := sb.GitPush(t.Context(), "/repo", ""); err != nil { - t.Errorf("push: %v", err) + t.Fatalf("push: %v", err) } if err := sb.GitPull(t.Context(), "/repo", "tok"); err != nil { - t.Errorf("pull: %v", err) + t.Fatalf("pull: %v", err) } if push, ok := (<-reqs).(*wire.GitPush); !ok || push.Path != "/repo" || push.Auth != "" { t.Errorf("guest saw %+v, want a git push of /repo", push) diff --git a/sdk/go/keepalive_test.go b/sdk/go/keepalive_test.go index e0aba6c46..c0ab68bb3 100644 --- a/sdk/go/keepalive_test.go +++ b/sdk/go/keepalive_test.go @@ -146,7 +146,7 @@ func TestPeerHangUpIsNoticedBeforeReuse(t *testing.T) { } } -func TestPeerQuiet(t *testing.T) { +func TestProbeSeesUnpromptedBytesAndHangUpWithoutConsumingThem(t *testing.T) { if !canProbe { t.Skip("no parked-connection probe on this platform") } diff --git a/sdk/go/pools_test.go b/sdk/go/pools_test.go index 93beb763d..884e82af2 100644 --- a/sdk/go/pools_test.go +++ b/sdk/go/pools_test.go @@ -20,7 +20,7 @@ func TestSetPools(t *testing.T) { _ = json.UnmarshalRead(r.Body, &got) _ = json.MarshalWrite(w, NodeInfo{Claimed: 2}) })) - defer ts.Close() + t.Cleanup(ts.Close) c := testClient(t, ts, WithAPIToken("root")) info, err := c.SetPools(t.Context(), []PoolSpec{{Template: "rt:24.04", Net: NetNone, Size: Small, Warm: 3}}) @@ -47,7 +47,7 @@ func TestSetPoolsClusterFansOut(t *testing.T) { _ = json.MarshalWrite(w, NodeInfo{}) } peer := httptest.NewServer(http.HandlerFunc(pools)) - defer peer.Close() + t.Cleanup(peer.Close) peerAddr := addrOf(peer) entry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -57,7 +57,7 @@ func TestSetPoolsClusterFansOut(t *testing.T) { } pools(w, r) })) - defer entry.Close() + t.Cleanup(entry.Close) c := testClient(t, entry, WithAPIToken("root")) results, err := c.SetPoolsCluster(t.Context(), []PoolSpec{{Template: "rt:24.04", Net: NetNone, Size: Small, Warm: 5}}) @@ -94,7 +94,7 @@ func TestSetPoolsClusterPeerDiscoveryFails(t *testing.T) { applied++ _ = json.MarshalWrite(w, NodeInfo{}) })) - defer entry.Close() + t.Cleanup(entry.Close) c := testClient(t, entry, WithAPIToken("root")) results, err := c.SetPoolsCluster(t.Context(), []PoolSpec{{Template: "rt:24.04", Net: NetNone, Size: Small, Warm: 5}}) diff --git a/sdk/go/tenants_test.go b/sdk/go/tenants_test.go index f37cab5e2..9e3adc0f2 100644 --- a/sdk/go/tenants_test.go +++ b/sdk/go/tenants_test.go @@ -42,7 +42,7 @@ func TestTenantVerbsSendTheOperatorTokenAndEscapeTheName(t *testing.T) { want request }{ {"put", func() error { return c.PutTenant(t.Context(), TenantSpec{Name: "u:42/west", Token: "w", MaxClaims: 3}) }, request{http.MethodPut, "/v1/tenants/u:42%2Fwest", "Bearer root", `{"token":"w","max_claims":3}`}}, - {"patch cap", func() error { return c.PutTenant(t.Context(), TenantSpec{Name: "acme", MaxClaims: 1}) }, request{http.MethodPut, "/v1/tenants/acme", "Bearer root", `{"max_claims":1}`}}, + {"put cap only", func() error { return c.PutTenant(t.Context(), TenantSpec{Name: "acme", MaxClaims: 1}) }, request{http.MethodPut, "/v1/tenants/acme", "Bearer root", `{"max_claims":1}`}}, {"class", func() error { return c.PutTenant(t.Context(), TenantSpec{Name: "acme", EgressClass: "desk"}) }, request{http.MethodPut, "/v1/tenants/acme", "Bearer root", `{"egress_class":"desk"}`}}, {"delete", func() error { return c.DeleteTenant(t.Context(), "acme") }, request{http.MethodDelete, "/v1/tenants/acme", "Bearer root", ""}}, {"page", func() error { _, err := c.Tenants(t.Context(), "u:1", 50); return err }, request{http.MethodGet, "/v1/tenants?after=u%3A1&limit=50", "Bearer root", ""}}, @@ -63,11 +63,16 @@ func TestTenantVerbsSendTheOperatorTokenAndEscapeTheName(t *testing.T) { func TestDeleteTenantClusterCountsAMissingTenantAsDone(t *testing.T) { var mu sync.Mutex deletes := map[string]int{} + count := func(r *http.Request) { + mu.Lock() + defer mu.Unlock() + if r.Method == http.MethodDelete { + deletes[r.Host]++ + } + } answer := func(status int) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - mu.Lock() - deletes[r.Host]++ - mu.Unlock() + count(r) w.WriteHeader(status) if status == http.StatusNotFound { _, _ = io.WriteString(w, `{"error":"unknown tenant"}`) @@ -75,11 +80,14 @@ func TestDeleteTenantClusterCountsAMissingTenantAsDone(t *testing.T) { } } gone := httptest.NewServer(answer(http.StatusNotFound)) - defer gone.Close() + t.Cleanup(gone.Close) broken := httptest.NewServer(answer(http.StatusInternalServerError)) - defer broken.Close() - old := httptest.NewServer(http.NotFoundHandler()) - defer old.Close() + t.Cleanup(broken.Close) + old := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + count(r) + http.NotFound(w, r) + })) + t.Cleanup(old.Close) peers := []string{addrOf(gone), addrOf(broken), addrOf(old)} entry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/v1/peers" { @@ -88,7 +96,7 @@ func TestDeleteTenantClusterCountsAMissingTenantAsDone(t *testing.T) { } answer(http.StatusNoContent)(w, r) })) - defer entry.Close() + t.Cleanup(entry.Close) results, err := testClient(t, entry, WithAPIToken("root")).DeleteTenantCluster(t.Context(), "acme") if err != nil { @@ -106,4 +114,10 @@ func TestDeleteTenantClusterCountsAMissingTenantAsDone(t *testing.T) { if len(results) != 4 || !maps.Equal(failed, want) { t.Errorf("failures %v over %d nodes, want the broken peer and the node without the route to retry: %v", failed, len(results), want) } + mu.Lock() + defer mu.Unlock() + wantDeletes := map[string]int{addrOf(entry): 1, peers[0]: 1, peers[1]: 1, peers[2]: 1} + if !maps.Equal(deletes, wantDeletes) { + t.Errorf("DELETEs per node %v, want exactly one each: %v", deletes, wantDeletes) + } } diff --git a/sdk/go/volumes_test.go b/sdk/go/volumes_test.go index efd554d9e..69b9aaabd 100644 --- a/sdk/go/volumes_test.go +++ b/sdk/go/volumes_test.go @@ -117,7 +117,7 @@ func TestTemplateNewVolumeClaimFollowsRedirect(t *testing.T) { } } -func TestClientVolumes(t *testing.T) { +func TestVolumesDecodesTheCatalogIncludingWritableEntries(t *testing.T) { imagenet := VolumeInfo{Name: "imagenet", DefaultMount: "/volumes/imagenet", SizeBytes: 42, Available: true, Nodes: 3} tests := []struct { name string diff --git a/sdk/go/watch_test.go b/sdk/go/watch_test.go index a44c256e0..856bae8a6 100644 --- a/sdk/go/watch_test.go +++ b/sdk/go/watch_test.go @@ -12,7 +12,7 @@ import ( "github.com/cocoonstack/sandbox/protocol/wire" ) -func TestWatchDeliversEventsUntilClose(t *testing.T) { +func TestWatchRelaysTheFakesSyntheticEventAndClosesCleanly(t *testing.T) { sb := fakeSandbox(t) ctx := t.Context() if err := sb.Mkdir(ctx, "/work", true); err != nil { diff --git a/sdk/openai/tests/test_adapter.py b/sdk/openai/tests/test_adapter.py index eda52dd9f..601a953bb 100644 --- a/sdk/openai/tests/test_adapter.py +++ b/sdk/openai/tests/test_adapter.py @@ -12,8 +12,6 @@ from cocoonsandbox import ProtocolError, SilkdError from cocoonsandbox_openai import CocoonSandboxClient, CocoonSandboxClientOptions, CocoonSandboxSessionState -CLAIMS: list = [] - class FakeNode(BaseHTTPRequestHandler): def do_POST(self): @@ -43,13 +41,18 @@ def log_message(self, *args): @pytest.fixture -def node(): +def node_server(): server = HTTPServer(("127.0.0.1", 0), FakeNode) - server.claims = CLAIMS - CLAIMS.clear() + server.claims = [] threading.Thread(target=server.serve_forever, daemon=True).start() - yield f"127.0.0.1:{server.server_port}" + yield server server.shutdown() + server.server_close() + + +@pytest.fixture +def node(node_server): + return f"127.0.0.1:{node_server.server_port}" def test_create_claims_and_state_round_trips(node): @@ -69,14 +72,14 @@ async def go(): asyncio.run(go()) -def test_default_lease_outlives_an_agent_run(node): +def test_default_lease_outlives_an_agent_run(node_server, node): async def go(): client = CocoonSandboxClient() session = await client.create(options=CocoonSandboxClientOptions(addr=node)) await client.delete(session) asyncio.run(go()) - assert CLAIMS[0]["ttl_seconds"] == 3600, CLAIMS + assert node_server.claims[0]["ttl_seconds"] == 3600, node_server.claims def test_operations_share_one_sandbox_handle(node): diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 859deb2d9..ddef5814f 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -3,24 +3,25 @@ import json import os import pathlib +import queue import resource import socket import threading import time from collections.abc import Callable from http.server import BaseHTTPRequestHandler, HTTPServer -from typing import BinaryIO +from typing import BinaryIO, TypeVar import pytest from cocoonsandbox import Client, Sandbox FIXTURES = pathlib.Path(__file__).parents[3] / "protocol" / "wire" / "fixtures" / "v1" +GUARD = 10.0 +_T = TypeVar("_T") -class FakeNode(BaseHTTPRequestHandler): - routes = {} - last_headers = {} +class FakeNode(BaseHTTPRequestHandler): def do_POST(self): self._dispatch("POST") @@ -40,10 +41,11 @@ def log_message(self, *args): pass def _dispatch(self, method): - FakeNode.last_headers = dict(self.headers) + self.server.last_headers.clear() + self.server.last_headers.update(self.headers) length = int(self.headers.get("Content-Length") or 0) body = json.loads(self.rfile.read(length)) if length else {} - handler = self.routes.get((method, self.path.split("?")[0])) + handler = self.server.routes.get((method, self.path.split("?")[0])) if handler is None: self._reply(404, {"error": "no route"}) return @@ -99,6 +101,25 @@ def accept_upgrade(conn: socket.socket) -> BinaryIO: return reader +def bounded(call: Callable[[], _T]) -> _T: + outcome: queue.Queue[tuple[_T | None, BaseException | None]] = queue.Queue() + + def run() -> None: + try: + outcome.put((call(), None)) + except BaseException as exc: + outcome.put((None, exc)) + + threading.Thread(target=run, daemon=True).start() + try: + value, error = outcome.get(timeout=GUARD) + except queue.Empty: + raise AssertionError(f"call still running after {GUARD}s") from None + if error is not None: + raise error + return value + + def wait_until(cond: Callable[[], bool], message: str) -> None: deadline = time.monotonic() + 3 while time.monotonic() < deadline: @@ -125,9 +146,10 @@ def fd_floor(request): def spawn_node(): servers = [] - def spawn(routes): - handler = type("Node", (FakeNode,), {"routes": routes}) - server = HTTPServer(("127.0.0.1", 0), handler) + def spawn(routes, last_headers=None): + server = HTTPServer(("127.0.0.1", 0), FakeNode) + server.routes = routes + server.last_headers = {} if last_headers is None else last_headers threading.Thread(target=server.serve_forever, daemon=True).start() servers.append(server) return f"127.0.0.1:{server.server_port}" @@ -135,6 +157,7 @@ def spawn(routes): yield spawn for server in servers: server.shutdown() + server.server_close() @pytest.fixture @@ -177,6 +200,7 @@ def start(script, *args) -> str: @pytest.fixture def black_hole(): sock = socket.socket() + sock.setsockopt(socket.SOL_SOCKET, socket.SO_RCVBUF, 4096) sock.bind(("127.0.0.1", 0)) sock.listen(8) yield f"127.0.0.1:{sock.getsockname()[1]}" diff --git a/sdk/python/tests/test_checkpoint.py b/sdk/python/tests/test_checkpoint.py index 1b573ecee..d01296c55 100644 --- a/sdk/python/tests/test_checkpoint.py +++ b/sdk/python/tests/test_checkpoint.py @@ -28,43 +28,6 @@ def redirect(body, path): assert seen[0]["no_redirect"] is True -def test_checkpoint_new_all_candidates_fail(spawn_node): - path = "/v1/checkpoints/ck_2/claim" - broken = spawn_node({("POST", path): lambda body, p: (500, {"error": "boom"})}) - - calls = [] - - def entry_claim(body, p): - calls.append(body) - if len(calls) == 1: - return 200, {"redirect": [broken]} - return 409, {"error": "origin also failed"} - - entry = spawn_node({("POST", path): entry_claim}) - with pytest.raises(APIError) as exc: - Client(entry).checkpoint("ck_2").new() - assert exc.value.status == 409 and "origin also failed" in exc.value.message - assert len(calls) == 2 and calls[1]["no_redirect"] is True - - -def test_checkpoint_new_redirect_fallback_heals(spawn_node): - path = "/v1/checkpoints/ck_5/claim" - busy = spawn_node({("POST", path): lambda body, p: (503, {"error": "healing"})}) - - calls = [] - - def entry_claim(body, p): - calls.append(body) - if len(calls) == 1: - return 200, {"redirect": [busy]} - return 200, {"id": "sb_healed", "token": "t"} - - entry = spawn_node({("POST", path): entry_claim}) - sb = Client(entry).checkpoint("ck_5").new() - assert sb.id == "sb_healed" and sb.owner == entry - assert len(calls) == 2 and calls[1]["no_redirect"] is True - - def test_checkpoint_new_redirect_never_yields_empty_id(spawn_node, dead_addr): entry = spawn_node({("POST", "/v1/checkpoints/ck_3/claim"): lambda body, path: (200, {"redirect": [dead_addr]})}) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index 85c81f5af..c244f1c5d 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -6,37 +6,43 @@ import pytest from cocoonsandbox import APIError, Client, Template -from conftest import FakeNode, sandbox_at, wait_until +from conftest import FakeNode, bounded, sandbox_at, wait_until class KeptNode(FakeNode): protocol_version = "HTTP/1.1" - conns = 0 - drop_after_reply = False - closed = 0 def setup(self): super().setup() - KeptNode.conns += 1 + self.server.conns += 1 def finish(self): super().finish() self.connection.close() - KeptNode.closed += 1 + self.server.closed += 1 def _reply(self, code, payload): super()._reply(code, payload) - self.close_connection = self.drop_after_reply + self.close_connection = self.server.drop_after_reply @pytest.fixture -def node(spawn_node): - FakeNode.routes = {} - return spawn_node(FakeNode.routes) +def routes(): + return {} -def test_claim_returns_the_sandbox_with_its_owner_and_digest(node): - FakeNode.routes[("POST", "/v1/claim")] = lambda body, path: ( +@pytest.fixture +def headers(): + return {} + + +@pytest.fixture +def node(spawn_node, routes, headers): + return spawn_node(routes, headers) + + +def test_claim_returns_the_sandbox_with_its_owner_and_digest(node, routes): + routes[("POST", "/v1/claim")] = lambda body, path: ( 200, {"id": "sb_1", "token": "tok", "owner_addr": node, "template_digest": "sha256:task"}, ) @@ -45,8 +51,9 @@ def test_claim_returns_the_sandbox_with_its_owner_and_digest(node): assert sb.template_digest == "sha256:task" -def test_claim_sends_volumes(node): +def test_claim_sends_volumes(node, routes): seen = recording_claim( + routes, { "id": "sb_1", "token": "tok", @@ -54,7 +61,7 @@ def test_claim_sends_volumes(node): {"name": "imagenet", "mount": "/volumes/imagenet"}, {"name": "weights-llama", "mount": "/models"}, ], - } + }, ) sb = Client(node).new("rt:24.04", volumes=["imagenet", {"name": "weights-llama", "mount": "/models"}]) assert seen == [ @@ -89,49 +96,35 @@ def test_claim_rejects_a_bare_volume_string(node, new): new(node) -def test_claim_sends_volume_mode_rw(node): - seen = recording_claim( - {"id": "sb_1", "token": "tok", "volumes": [{"name": "scratch", "mount": "/data", "mode": "rw"}]} - ) - sb = Client(node).new("rt:24.04", volumes=[{"name": "scratch", "mount": "/data", "mode": "rw"}]) - assert seen == [ - { - "template": "rt:24.04", - "volumes": [ - {"name": "scratch", "mount": "/data", "mode": "rw"}, - ], - } - ] - assert sb.volumes == [{"name": "scratch", "mount": "/data", "mode": "rw"}] - - -def test_claim_preserves_explicit_volume_mode(node): - seen = recording_claim({"id": "sb_1", "token": "tok"}) - Client(node).new("rt:24.04", volumes=[{"name": "imagenet", "mode": "ro"}]) - Client(node).new("rt:24.04", volumes=[{"name": "imagenet", "mode": ""}]) - assert seen[0]["volumes"] == [{"name": "imagenet", "mode": "ro"}] - assert seen[1]["volumes"] == [{"name": "imagenet", "mode": ""}] - - -def test_claim_leaves_volume_policy_to_server(node): - seen = recording_claim({"id": "sb_1", "token": "tok"}) - entry = {"name": "imagenet", "mount": "/datasets", "attach_only": True, "mode": "future-mode"} - Client(node).new("rt:24.04", volumes=[entry]) +@pytest.mark.parametrize( + "entry", + [ + {"name": "scratch", "mount": "/data", "mode": "rw"}, + {"name": "imagenet", "mode": "ro"}, + {"name": "imagenet", "mode": ""}, + {"name": "imagenet", "mount": "/datasets", "attach_only": True, "mode": "future-mode"}, + ], + ids=["rw", "ro", "empty_mode", "future_mode"], +) +def test_claim_passes_the_volume_entry_through(node, routes, entry): + seen = recording_claim(routes, {"id": "sb_1", "token": "tok", "volumes": [entry]}) + sb = Client(node).new("rt:24.04", volumes=[entry]) Template(Client(node), node, "task:v1", "none", "small").new(volumes=[entry]) assert [request["volumes"] for request in seen] == [[entry], [entry]] + assert sb.volumes == [entry] -def test_template_claim_asks_for_the_promoted_template(node): - seen = recording_claim({"id": "sb_9", "token": "tok"}) +def test_template_claim_asks_for_the_promoted_template(node, routes): + seen = recording_claim(routes, {"id": "sb_9", "token": "tok"}) Template(Client(node), node, "task:v1", "none", "small").new() assert seen == [ {"template": "task:v1", "net": "none", "size": "small", "no_redirect": True, "require_promoted": True} ] -def test_template_claim_sends_volumes(node): +def test_template_claim_sends_volumes(node, routes): seen = recording_claim( - {"id": "sb_2", "token": "tok", "volumes": [{"name": "imagenet", "mount": "/datasets/imagenet"}]} + routes, {"id": "sb_2", "token": "tok", "volumes": [{"name": "imagenet", "mount": "/datasets/imagenet"}]} ) sb = Template(Client(node), node, "task:v1", "none", "small").new( volumes=[{"name": "imagenet", "mount": "/datasets/imagenet"}] @@ -148,27 +141,35 @@ def test_template_claim_sends_volumes(node): assert sb.volumes == [{"name": "imagenet", "mount": "/datasets/imagenet"}] -def test_template_volume_claim_follows_redirect(node): +@pytest.mark.parametrize( + ("new", "promoted"), + [ + (lambda node, volumes: Client(node).new("rt:24.04", volumes=volumes), False), + (lambda node, volumes: Template(Client(node), node, "task:v1", "none", "small").new(volumes=volumes), True), + ], + ids=["client", "template"], +) +def test_volume_claim_follows_redirect_with_no_redirect(node, routes, new, promoted): seen = [] def claim(body, path): seen.append(body) if len(seen) == 1: - return 200, {"redirect": [node], "require_promoted": True} + return 200, {"redirect": [node], "require_promoted": promoted} return 200, {"id": "sb_2", "token": "tok", "volumes": body["volumes"]} - FakeNode.routes[("POST", "/v1/claim")] = claim - sb = Template(Client(node), node, "task:v1", "none", "small").new( - volumes=[{"name": "imagenet", "mount": "/datasets/imagenet"}] - ) + routes[("POST", "/v1/claim")] = claim + volumes = [{"name": "imagenet", "mount": "/datasets/imagenet"}] + sb = new(node, volumes) + assert sb.id == "sb_2" assert "no_redirect" not in seen[0] assert seen[1]["no_redirect"] is True - assert seen[1]["require_promoted"] is True - assert seen[1]["volumes"] == [{"name": "imagenet", "mount": "/datasets/imagenet"}] - assert sb.volumes == seen[1]["volumes"] + assert seen[1].get("require_promoted", False) is promoted + assert seen[0]["volumes"] == seen[1]["volumes"] == volumes + assert sb.volumes == volumes -def test_volume_catalog_surfaces_writable(node): +def test_volume_catalog_surfaces_writable(node, routes): want = [ { "name": "scratch", @@ -186,19 +187,19 @@ def test_volume_catalog_surfaces_writable(node): "nodes": 3, }, ] - FakeNode.routes[("GET", "/v1/volumes")] = lambda body, path: (200, {"volumes": want}) + routes[("GET", "/v1/volumes")] = lambda body, path: (200, {"volumes": want}) got = Client(node).volumes() assert got == want assert got[0]["writable"] is True assert "writable" not in got[1] -def test_promote_returns_content_digest(node): - FakeNode.routes[("POST", "/v1/claim")] = lambda body, path: ( +def test_promote_returns_content_digest(node, routes): + routes[("POST", "/v1/claim")] = lambda body, path: ( 200, {"id": "sb_1", "token": "tok", "owner_addr": node}, ) - FakeNode.routes[("POST", "/v1/sandboxes/sb_1/promote")] = lambda body, path: ( + routes[("POST", "/v1/sandboxes/sb_1/promote")] = lambda body, path: ( 200, { "key": {"template": "task:v1", "net": "none", "size": "small"}, @@ -212,23 +213,23 @@ def test_promote_returns_content_digest(node): @pytest.mark.parametrize(("ttl", "sent"), [(90, {"ttl_seconds": 90}), (0, {})]) -def test_renew_sends_the_sandbox_token_and_records_the_grant(node, ttl, sent): +def test_renew_sends_the_sandbox_token_and_records_the_grant(node, routes, headers, ttl, sent): seen = [] def renew(body, path): seen.append(body) return 200, {"deadline": "2026-09-23T12:00:00Z"} - FakeNode.routes[("POST", "/v1/sandboxes/sb_1/renew")] = renew + routes[("POST", "/v1/sandboxes/sb_1/renew")] = renew sb = sandbox_at(node, api_token="api") assert sb.renew(ttl) == "2026-09-23T12:00:00Z" assert sb.deadline == "2026-09-23T12:00:00Z" assert seen == [sent] - assert FakeNode.last_headers["Authorization"] == "Bearer tok" + assert headers["Authorization"] == "Bearer tok" -def test_renew_refusal_leaves_the_deadline(node): - FakeNode.routes[("POST", "/v1/sandboxes/sb_1/renew")] = lambda body, path: (409, {"error": "sandbox archived"}) +def test_renew_refusal_leaves_the_deadline(node, routes): + routes[("POST", "/v1/sandboxes/sb_1/renew")] = lambda body, path: (409, {"error": "sandbox archived"}) sb = sandbox_at(node) sb.deadline = "2026-09-23T11:00:00Z" with pytest.raises(APIError) as refused: @@ -237,30 +238,6 @@ def test_renew_refusal_leaves_the_deadline(node): assert sb.deadline == "2026-09-23T11:00:00Z" -def test_claim_follows_redirect_with_no_redirect(node): - seen = [] - - def claim(body, path): - seen.append(body) - if len(seen) == 1: - return 200, {"redirect": [node]} - return 200, {"id": "sb_2", "token": "tok"} - - FakeNode.routes[("POST", "/v1/claim")] = claim - volumes = [{"name": "imagenet", "mount": "/datasets/imagenet"}] - sb = Client(node).new("rt:24.04", volumes=volumes) - assert sb.id == "sb_2" - assert "no_redirect" not in seen[0] - assert seen[1]["no_redirect"] is True - assert ( - seen[0]["volumes"] - == seen[1]["volumes"] - == [ - {"name": "imagenet", "mount": "/datasets/imagenet"}, - ] - ) - - @pytest.mark.parametrize( ("verb", "method", "path"), [ @@ -270,27 +247,27 @@ def claim(body, path): ("uncordon", "DELETE", "/v1/cordon"), ], ) -def test_node_hold_verbs_hit_their_routes(node, verb, method, path): - FakeNode.routes[(method, path)] = lambda body, path: (200, {"claimed": 3}) +def test_node_hold_verbs_hit_their_routes(node, routes, verb, method, path): + routes[(method, path)] = lambda body, path: (200, {"claimed": 3}) assert getattr(Client(node), verb)() == {"claimed": 3} -def test_api_error_carries_server_message(node): - FakeNode.routes[("POST", "/v1/claim")] = lambda body, path: (409, {"error": "no egress"}) +def test_api_error_carries_server_message(node, routes): + routes[("POST", "/v1/claim")] = lambda body, path: (409, {"error": "no egress"}) with pytest.raises(APIError) as exc: Client(node).new("rt:24.04", net="egress") assert exc.value.status == 409 and "no egress" in exc.value.message -def test_checkpoint_listing_binds_handles(node): - FakeNode.routes[("GET", "/v1/checkpoints")] = lambda body, path: ( +def test_checkpoint_listing_binds_handles(node, routes): + routes[("GET", "/v1/checkpoints")] = lambda body, path: ( 200, {"checkpoints": [{"id": "ck_0011223344556677", "name": "s1", "sandbox_id": "sb_1"}]}, ) ckpts = Client(node).checkpoints() assert len(ckpts) == 1 and ckpts[0].id == "ck_0011223344556677" - FakeNode.routes[("POST", "/v1/checkpoints/ck_0011223344556677/claim")] = lambda body, path: ( + routes[("POST", "/v1/checkpoints/ck_0011223344556677/claim")] = lambda body, path: ( 200, {"id": "sb_branch", "token": "t2"}, ) @@ -298,77 +275,74 @@ def test_checkpoint_listing_binds_handles(node): assert branch.id == "sb_branch" -def test_claim_refuses_a_spent_deadline(node): - seen = recording_claim({"id": "sb_1", "token": "tok"}) +def test_claim_refuses_a_spent_deadline(node, routes): + seen = recording_claim(routes, {"id": "sb_1", "token": "tok"}) with pytest.raises(TimeoutError): Client(node).new("rt:24.04", deadline=time.monotonic() - 1) assert seen == [], "a spent deadline still reached the node" -def test_claim_deadline_bounds_the_redirect_walk(node, black_hole): - FakeNode.routes[("POST", "/v1/claim")] = lambda body, path: (200, {"redirect": [black_hole, black_hole]}) - client = Client(node, timeout=2.0) - started = time.monotonic() +def test_claim_deadline_bounds_the_redirect_walk(node, routes, black_hole): + routes[("POST", "/v1/claim")] = lambda body, path: (200, {"redirect": [black_hole, black_hole]}) + client = Client(node, timeout=60) with pytest.raises(TimeoutError): - client.new("rt:24.04", deadline=started + 0.3) - elapsed = time.monotonic() - started - assert elapsed < 1.5, elapsed + bounded(lambda: client.new("rt:24.04", deadline=time.monotonic() + 0.3)) def test_claim_deadline_bounds_the_entry_node(black_hole): - client = Client(black_hole, timeout=2.0) - started = time.monotonic() + client = Client(black_hole, timeout=60) with pytest.raises(TimeoutError): - client.new("rt:24.04", deadline=started + 0.3) - elapsed = time.monotonic() - started - assert elapsed < 1.5, elapsed + bounded(lambda: client.new("rt:24.04", deadline=time.monotonic() + 0.3)) @pytest.mark.parametrize(("keep_alive", "conns"), [(30.0, 1), (0, 2)]) def test_control_calls_reuse_one_connection(kept_node, keep_alive, conns, fd_floor): - KeptNode.routes[("POST", "/v1/claim")] = lambda body, path: (200, {"id": "sb_1", "token": "tok"}) - client = Client(kept_node, keep_alive=keep_alive) + kept_node.routes[("POST", "/v1/claim")] = lambda body, path: (200, {"id": "sb_1", "token": "tok"}) + client = Client(kept_node.addr, keep_alive=keep_alive) client.new("rt:24.04") client.new("rt:24.04") - assert KeptNode.conns == conns, KeptNode.conns + assert kept_node.conns == conns, kept_node.conns def test_a_parked_connection_the_node_closed_is_not_reused(kept_node, fd_floor): - KeptNode.routes[("POST", "/v1/claim")] = lambda body, path: (200, {"id": "sb_1", "token": "tok"}) - KeptNode.drop_after_reply = True - client = Client(kept_node) + kept_node.routes[("POST", "/v1/claim")] = lambda body, path: (200, {"id": "sb_1", "token": "tok"}) + kept_node.drop_after_reply = True + client = Client(kept_node.addr) client.new("rt:24.04") - wait_until(lambda: KeptNode.closed == 1, "the node never closed the connection") + wait_until(lambda: kept_node.closed == 1, "the node never closed the connection") assert client.new("rt:24.04").id == "sb_1" - assert KeptNode.conns == 2 + assert kept_node.conns == 2 def test_close_releases_the_parked_connections(kept_node): - KeptNode.routes[("POST", "/v1/claim")] = lambda body, path: (200, {"id": "sb_1", "token": "tok"}) - with Client(kept_node) as client: + kept_node.routes[("POST", "/v1/claim")] = lambda body, path: (200, {"id": "sb_1", "token": "tok"}) + with Client(kept_node.addr) as client: client.new("rt:24.04") - assert KeptNode.closed == 0 - wait_until(lambda: KeptNode.closed == 1, "close left the parked connection open") + assert kept_node.closed == 0 + wait_until(lambda: kept_node.closed == 1, "close left the parked connection open") client.close() assert client.new("rt:24.04").id == "sb_1" - assert KeptNode.conns == 2 + assert kept_node.conns == 2 @pytest.fixture def kept_node(): server = ThreadingHTTPServer(("127.0.0.1", 0), KeptNode) - KeptNode.routes, KeptNode.conns, KeptNode.closed, KeptNode.drop_after_reply = {}, 0, 0, False + server.routes, server.last_headers = {}, {} + server.conns, server.closed, server.drop_after_reply = 0, 0, False + server.addr = f"127.0.0.1:{server.server_port}" threading.Thread(target=server.serve_forever, daemon=True).start() - yield f"127.0.0.1:{server.server_port}" + yield server server.shutdown() + server.server_close() -def recording_claim(reply): +def recording_claim(routes, reply): seen = [] def claim(body, path): seen.append(body) return 200, reply - FakeNode.routes[("POST", "/v1/claim")] = claim + routes[("POST", "/v1/claim")] = claim return seen diff --git a/sdk/python/tests/test_fault_matrix.py b/sdk/python/tests/test_fault_matrix.py index 5ee137f03..e7c400dbd 100644 --- a/sdk/python/tests/test_fault_matrix.py +++ b/sdk/python/tests/test_fault_matrix.py @@ -4,10 +4,20 @@ import http.client import socket import threading -import time +import cocoonsandbox.client import pytest from cocoonsandbox import APIError, Client +from conftest import bounded + +ENTRY_POINTS = pytest.mark.parametrize( + ("path", "claim"), + [ + ("/v1/claim", lambda client: client.new("rt:24.04")), + ("/v1/checkpoints/ck_1/claim", lambda client: client.checkpoint("ck_1").new()), + ], + ids=["claim", "checkpoint"], +) @pytest.mark.parametrize( @@ -40,16 +50,8 @@ def unreachable(conn): assert sb.id == "sb_ok" and sb.owner == good -def test_claim_redirect_stops_when_a_candidate_may_have_claimed(spawn_node): - server = socket.create_server(("127.0.0.1", 0)) - - def drop_reply(): - conn, _ = server.accept() - conn.recv(65536) - conn.close() - - threading.Thread(target=drop_reply, daemon=True).start() - lost = f"127.0.0.1:{server.getsockname()[1]}" +def test_claim_redirect_stops_when_a_candidate_may_have_claimed(spawn_node, script_server): + lost = script_server(drop_reply) good_calls = [] good = spawn_node( {("POST", "/v1/claim"): lambda body, path: good_calls.append(body) or (200, {"id": "x", "token": "t"})} @@ -58,54 +60,54 @@ def drop_reply(): entry = spawn_node( {("POST", "/v1/claim"): lambda body, path: entry_calls.append(body) or (200, {"redirect": [lost, good]})} ) - try: - with pytest.raises(APIError): - Client(entry).new("rt:24.04") - finally: - server.close() + with pytest.raises(APIError): + Client(entry).new("rt:24.04") assert not good_calls and len(entry_calls) == 1 -def test_claim_redirect_all_candidates_fail_then_origin_fallback_fails(spawn_node): - a = spawn_node({("POST", "/v1/claim"): lambda body, path: (404, {"error": "gone a"})}) - b = spawn_node({("POST", "/v1/claim"): lambda body, path: (404, {"error": "gone b"})}) +@ENTRY_POINTS +@pytest.mark.parametrize("status", [404, 500]) +def test_claim_redirect_all_candidates_fail_then_origin_fallback_fails(spawn_node, path, claim, status): + a = spawn_node({("POST", path): lambda body, p: (status, {"error": "gone a"})}) + b = spawn_node({("POST", path): lambda body, p: (status, {"error": "gone b"})}) calls = [] - def entry_claim(body, path): + def entry_claim(body, p): calls.append(body) if len(calls) == 1: return 200, {"redirect": [a, b]} return 409, {"error": "origin also failed"} - entry = spawn_node({("POST", "/v1/claim"): entry_claim}) + entry = spawn_node({("POST", path): entry_claim}) with pytest.raises(APIError) as exc: - Client(entry).new("rt:24.04") + claim(Client(entry)) assert exc.value.status == 409 and "origin also failed" in exc.value.message assert "gone b" in exc.value.message assert len(calls) == 2 and calls[1]["no_redirect"] is True -@pytest.mark.parametrize(("status", "message"), [(429, "full"), (401, "invalid api token")]) -def test_claim_redirect_retryable_candidate_falls_back_to_origin(spawn_node, status, message): +@ENTRY_POINTS +@pytest.mark.parametrize(("status", "message"), [(429, "full"), (401, "invalid api token"), (503, "healing")]) +def test_claim_redirect_retryable_candidate_falls_back_to_origin(spawn_node, path, claim, status, message): candidate_calls = [] - def candidate_claim(body, path): + def candidate_claim(body, p): candidate_calls.append(body) return status, {"error": message} - candidate = spawn_node({("POST", "/v1/claim"): candidate_claim}) + candidate = spawn_node({("POST", path): candidate_claim}) calls = [] - def entry_claim(body, path): + def entry_claim(body, p): calls.append(body) if len(calls) == 1: return 200, {"redirect": [candidate]} return 200, {"id": "sb_healed", "token": "t"} - entry = spawn_node({("POST", "/v1/claim"): entry_claim}) - sb = Client(entry).new("rt:24.04") + entry = spawn_node({("POST", path): entry_claim}) + sb = claim(Client(entry)) assert sb.id == "sb_healed" and sb.owner == entry assert len(calls) == 2 and calls[1]["no_redirect"] is True assert len(candidate_calls) == 1 @@ -171,7 +173,7 @@ def ok(body, path): @pytest.mark.parametrize("peer", ["dead_addr", "black_hole"]) -def test_lookup_unreachable_peer_resolves_fast(spawn_node, request, peer): +def test_lookup_resolves_past_an_unreachable_peer(spawn_node, request, peer): unreachable = request.getfixturevalue(peer) owner = spawn_node({("GET", "/v1/sandboxes/sb_1/owner"): lambda body, path: (200, {"owner_addr": "10.0.0.9:7777"})}) entry = spawn_node( @@ -180,11 +182,32 @@ def test_lookup_unreachable_peer_resolves_fast(spawn_node, request, peer): ("GET", "/v1/peers"): lambda body, path: (200, {"peers": [unreachable, owner]}), } ) - start = time.monotonic() sb = Client(entry, timeout=10.0).lookup("sb_1", "tok") - elapsed = time.monotonic() - start assert sb.owner == "10.0.0.9:7777" - assert elapsed < 2.0, elapsed + + +def test_lookup_returns_the_first_owner_without_waiting_for_a_slow_peer(spawn_node, monkeypatch): + monkeypatch.setattr(cocoonsandbox.client, "_PEERS_TIMEOUT", 60.0) + gate = threading.Event() + + def slow(body, path): + gate.wait(60) + return 404, {"error": "not here"} + + slow_peer = spawn_node({("GET", "/v1/sandboxes/sb_1/owner"): slow}) + owner = spawn_node({("GET", "/v1/sandboxes/sb_1/owner"): lambda body, path: (200, {"owner_addr": "10.0.0.9:7777"})}) + entry = spawn_node( + { + ("GET", "/v1/sandboxes/sb_1/owner"): lambda body, path: (404, {"error": "not here"}), + ("GET", "/v1/peers"): lambda body, path: (200, {"peers": [slow_peer, owner]}), + } + ) + try: + sb = bounded(lambda: Client(entry, timeout=60.0).lookup("sb_1", "tok")) + assert not gate.is_set() + finally: + gate.set() + assert sb.owner == "10.0.0.9:7777" def test_lookup_all_miss(spawn_node, dead_addr): @@ -274,3 +297,9 @@ def test_close_of_a_sandbox_no_node_holds_is_not_an_error(spawn_node): } ) Client(gone).attach(gone, "sb_1", "tok").close() + + +def drop_reply(server: socket.socket) -> None: + conn, _ = server.accept() + conn.recv(65536) + conn.close() diff --git a/sdk/python/tests/test_fixtures.py b/sdk/python/tests/test_fixtures.py index 145f807e9..2110112c6 100644 --- a/sdk/python/tests/test_fixtures.py +++ b/sdk/python/tests/test_fixtures.py @@ -1,4 +1,4 @@ -"""Round-trips the protocol fixture corpus, the same drift guard the Rust and Go implementations run.""" +"""Round-trips the protocol corpus's request frames and decodes each response frame whole.""" import base64 import json @@ -9,9 +9,11 @@ def fixture_files(prefix): - files = sorted(FIXTURES.glob(prefix + "*.json")) - assert files, f"no {prefix} fixtures under {FIXTURES}" - return files + return sorted(FIXTURES.glob(prefix + "*.json")) + + +def test_corpus_counts_are_pinned(): + assert (len(fixture_files("req_")), len(fixture_files("resp_"))) == (41, 22) @pytest.mark.parametrize("path", fixture_files("req_"), ids=lambda p: p.stem) @@ -26,7 +28,6 @@ def test_request_round_trip(path): def test_response_decodes(path): raw = path.read_text().encode() want = json.loads(raw) - frame = frames.decode_response(raw) - assert frame["type"] == want["type"] if "data" in want: - assert frame["data"] == base64.b64decode(want["data"]) + want["data"] = base64.b64decode(want["data"]) + assert frames.decode_response(raw) == want diff --git a/sdk/python/tests/test_hardening.py b/sdk/python/tests/test_hardening.py index 1f6a3dec0..3c781bc86 100644 --- a/sdk/python/tests/test_hardening.py +++ b/sdk/python/tests/test_hardening.py @@ -21,7 +21,7 @@ ) from cocoonsandbox.conn import Conn, dial_agent, remaining_timeout from cocoonsandbox.frames import BULK_CHUNK -from conftest import sandbox_at +from conftest import sandbox_at, wait_until def test_dial_agent_rejects_control_chars_in_identity(): @@ -131,7 +131,7 @@ def drain(): assert all(len(base64.b64decode(f["data"])) == BULK_CHUNK for f in frames) -def test_session_and_lsp_close_twice(monkeypatch): +def test_session_and_lsp_close_ignore_only_not_found(monkeypatch): sb = sandbox_at("127.0.0.1:1") calls = [] @@ -160,7 +160,7 @@ def test_watcher_error_stays_none_after_close(conn_pair): events = [] consumer = threading.Thread(target=lambda: events.extend(watcher), daemon=True) consumer.start() - time.sleep(0.1) + wait_until(conn.quiet, "the consumer never read the ready frame") watcher.close() consumer.join(5) assert not consumer.is_alive() @@ -201,13 +201,9 @@ def slow_headers() -> None: threading.Thread(target=slow_headers, daemon=True).start() addr = f"127.0.0.1:{server.getsockname()[1]}" - started = time.monotonic() with server, pytest.raises(SandboxTimeout), dial_agent(addr, "sb_x", "tok", timeout=0.2) as conn: conn.send("info") conn.recv() - assert time.monotonic() - started < 0.6, ( - "each header arrived under the per-read timeout; only a whole-dial budget stops this" - ) @pytest.fixture diff --git a/sdk/python/tests/test_keepalive.py b/sdk/python/tests/test_keepalive.py index 536e997bd..4ff9825cf 100644 --- a/sdk/python/tests/test_keepalive.py +++ b/sdk/python/tests/test_keepalive.py @@ -62,8 +62,6 @@ def _serve(self, conn: socket.socket) -> None: self.closed += 1 def _answer(self, op: str) -> list: - if op == "info": - return [{"type": "info", "version": "fake", "uptime_secs": 0, "procs": 0}] if op == "fs_stat": return [{"type": "stat", "info": {"kind": "dir", "size": 0, "mode": 0o755, "mtime_epoch_secs": 0}}] if op == "fs_find": @@ -144,13 +142,9 @@ def serve(): with conn, contextlib.suppress(OSError): reader = accept_upgrade(conn) for line in iter(reader.readline, b""): - op = json.loads(line)["op"] - if op == "info": - info = {"type": "info", "version": "fake", "uptime_secs": 0} - conn.sendall(json.dumps(info).encode() + b"\n") - elif op == "exec": + if json.loads(line)["op"] == "exec": conn.sendall(b'{"type":"started","pid":1}\n{"type":"exit","code":0}\n') - release.wait(10) + release.wait(60) return threading.Thread(target=serve, daemon=True).start() @@ -161,7 +155,7 @@ def serve(): ) try: call.start() - call.join(2) + call.join(10) assert codes == [0], "run outlived its timeout behind a stdin the guest stopped reading" assert sb._pool.take() is None finally: diff --git a/sdk/python/tests/test_proxy.py b/sdk/python/tests/test_proxy.py index 7f0c27dbe..f219a787b 100644 --- a/sdk/python/tests/test_proxy.py +++ b/sdk/python/tests/test_proxy.py @@ -55,6 +55,8 @@ def close(self) -> None: @pytest.mark.parametrize("local", ["127.0.0.1:0", "[::1]:0"]) def test_proxy_port_accepts_and_pipes(monkeypatch, local): + if local.startswith("[") and not ipv6_loopback(): + pytest.skip("no IPv6 loopback") sb = sandbox_at("127.0.0.1:1") monkeypatch.setattr(sb, "dial_port", lambda port: FakePortConn()) @@ -93,7 +95,20 @@ def test_proxy_port_retires_its_accept_thread_when_the_listener_closes(): sb = sandbox_at("127.0.0.1:1", keep_alive=0) known = set(threading.enumerate()) listener = sb.proxy_port("127.0.0.1:0", 80) - (accept_loop,) = set(threading.enumerate()) - known + (accept_loop,) = [ + thread + for thread in threading.enumerate() + if thread not in known and thread.name.endswith("(_proxy_accept_loop)") + ] listener.close() accept_loop.join(5) assert not accept_loop.is_alive() + + +def ipv6_loopback() -> bool: + try: + with socket.socket(socket.AF_INET6) as probe: + probe.bind(("::1", 0)) + except OSError: + return False + return True diff --git a/sdk/python/tests/test_stream.py b/sdk/python/tests/test_stream.py index bae509584..6c6d70e09 100644 --- a/sdk/python/tests/test_stream.py +++ b/sdk/python/tests/test_stream.py @@ -9,7 +9,7 @@ from cocoonsandbox import ProtocolError, Sandbox, SandboxTimeout from cocoonsandbox import conn as conn_module from cocoonsandbox import sandbox as sandbox_module -from conftest import accept_upgrade, sandbox_at +from conftest import accept_upgrade, bounded, sandbox_at TIMEOUT = 0.2 @@ -25,31 +25,33 @@ def test_port_stream_outlives_the_client_timeout(script_server): def test_run_timeout_cuts_a_silent_command_and_kills_it(script_server): kills: list[dict] = [] - sb = short_timeout_sandbox(script_server(serve_started_then_hang, 5 * TIMEOUT, kills)) - started = time.monotonic() + sb = long_timeout_sandbox(script_server(serve_started_then_hang, kills)) with pytest.raises(TimeoutError): - sb.run(["sleep", "9"], timeout=TIMEOUT) - assert time.monotonic() - started < 3 * TIMEOUT + bounded(lambda: sb.run(["sleep", "9"], timeout=TIMEOUT)) assert kills == [{"v": 1, "op": "kill", "pid": 7}], kills def test_run_timeout_bounds_the_kill_it_sends(monkeypatch, script_server): monkeypatch.setattr(sandbox_module, "_KILL_WAIT_SECONDS", TIMEOUT) kills: list[dict] = [] - sb = short_timeout_sandbox(script_server(serve_started_then_trickle_the_kill_reply, kills)) - started = time.monotonic() + sb = long_timeout_sandbox(script_server(serve_started_then_trickle_the_kill_reply, kills)) with pytest.raises(TimeoutError): - sb.run(["sleep", "9"], timeout=TIMEOUT) - assert time.monotonic() - started < 4 * TIMEOUT + bounded(lambda: sb.run(["sleep", "9"], timeout=TIMEOUT)) assert kills == [{"v": 1, "op": "kill", "pid": 7}], kills -def test_run_timeout_cuts_a_blocked_exec_send(black_hole): - sb = sandbox_at(black_hole, timeout=30, keep_alive=0) - started = time.monotonic() +def test_run_timeout_cuts_a_blocked_exec_send(monkeypatch, black_hole): + connect = socket.create_connection + + def small_send_buffer(*args, **kwargs): + sock = connect(*args, **kwargs) + sock.setsockopt(socket.SOL_SOCKET, socket.SO_SNDBUF, 4096) + return sock + + monkeypatch.setattr(socket, "create_connection", small_send_buffer) + sb = long_timeout_sandbox(black_hole) with pytest.raises(TimeoutError): - sb.run(["echo", "x" * (7 << 20)], timeout=TIMEOUT) - assert time.monotonic() - started < 5 * TIMEOUT + bounded(lambda: sb.run(["echo", "x" * (1 << 20)], timeout=TIMEOUT)) def test_run_rejects_a_non_positive_timeout(): @@ -60,10 +62,8 @@ def test_run_rejects_a_non_positive_timeout(): def test_dial_is_still_bounded_by_the_client_timeout(script_server): sb = short_timeout_sandbox(script_server(serve_silence)) - started = time.monotonic() with pytest.raises(SandboxTimeout): - sb.dial_port(5000) - assert time.monotonic() - started < 3 * TIMEOUT + bounded(lambda: sb.dial_port(5000)) @pytest.mark.parametrize(("exc", "want"), [(TimeoutError(), SandboxTimeout), (ConnectionResetError(), ProtocolError)]) @@ -75,6 +75,10 @@ def short_timeout_sandbox(addr: str) -> Sandbox: return sandbox_at(addr, timeout=TIMEOUT, keep_alive=0) +def long_timeout_sandbox(addr: str) -> Sandbox: + return sandbox_at(addr, timeout=60, keep_alive=0) + + def serve_port_forward(server: socket.socket, quiet: float, ops: list[str]) -> None: conn, _ = server.accept() reader = accept_upgrade(conn) @@ -85,7 +89,7 @@ def serve_port_forward(server: socket.socket, quiet: float, ops: list[str]) -> N conn.close() -def serve_started_then_hang(server: socket.socket, quiet: float, kills: list[dict]) -> None: +def serve_started_then_hang(server: socket.socket, kills: list[dict]) -> None: conn, _ = server.accept() reader = accept_upgrade(conn) reader.readline() @@ -94,7 +98,7 @@ def serve_started_then_hang(server: socket.socket, quiet: float, kills: list[dic kills.append(json.loads(accept_upgrade(killer).readline())) killer.sendall(b'{"type":"done"}\n') killer.close() - time.sleep(quiet) + reader.read() conn.close() @@ -106,15 +110,16 @@ def serve_started_then_trickle_the_kill_reply(server: socket.socket, kills: list killer, _ = server.accept() kills.append(json.loads(accept_upgrade(killer).readline())) with contextlib.suppress(OSError): - for byte in b'{"type":"done"}\n': + while True: time.sleep(0.75 * TIMEOUT) - killer.sendall(bytes([byte])) + killer.sendall(b" ") killer.close() conn.close() def serve_silence(server: socket.socket) -> None: conn, _ = server.accept() - conn.recv(4096) - time.sleep(5 * TIMEOUT) + with contextlib.suppress(OSError): + while conn.recv(4096): + pass conn.close() diff --git a/sdk/python/tests/test_upgrade.py b/sdk/python/tests/test_upgrade.py index 6f0b11920..553fd8996 100644 --- a/sdk/python/tests/test_upgrade.py +++ b/sdk/python/tests/test_upgrade.py @@ -4,16 +4,12 @@ import json import socket import threading -import time import pytest from cocoonsandbox import APIError, SandboxTimeout -from conftest import sandbox_at +from conftest import bounded, sandbox_at -REPLIES = { - "info": {"type": "info", "version": "fake", "uptime_secs": 0, "procs": 0}, - "fs_mkdir": {"type": "done"}, -} +REPLIES = {"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"}' @@ -75,8 +71,6 @@ def test_rejected_upgrade_is_an_api_error(reply): def test_run_timeout_cuts_an_unanswered_upgrade(black_hole): - sb = sandbox_at(black_hole, timeout=30, keep_alive=0) - started = time.monotonic() + sb = sandbox_at(black_hole, timeout=60, keep_alive=0) with pytest.raises(SandboxTimeout): - sb.run(["true"], timeout=0.2) - assert time.monotonic() - started < 2, "the run deadline must cut a hung upgrade under a long client timeout" + bounded(lambda: sb.run(["true"], timeout=0.2)) diff --git a/sdk/python/tests/test_volume_controller.py b/sdk/python/tests/test_volume_controller.py index e41a09663..c60684904 100644 --- a/sdk/python/tests/test_volume_controller.py +++ b/sdk/python/tests/test_volume_controller.py @@ -4,7 +4,6 @@ import pytest from cocoonsandbox import Client, VolumeError -from conftest import FakeNode from examples.volume_controller import VolumeHooks, hibernate_volume, resume_volume PATH = "/v1/sandboxes/sb_1" @@ -29,6 +28,7 @@ def controller(spawn_node, monkeypatch): replies = [] fail = set() gate = {"open": True} + headers = {} def hook(name): def run(): @@ -43,7 +43,7 @@ def run(): return run def patch(body, path): - assert FakeNode.last_headers["Authorization"] == "Bearer root-token" + assert headers["Authorization"] == "Bearer root-token" assert not gate["open"] events.append(("patch", body)) if replies: @@ -51,12 +51,12 @@ def patch(body, path): return 200, EMPTY if "remove" in body else ATTACHED def hibernate(body, path): - assert FakeNode.last_headers["Authorization"] == "Bearer sandbox-token" + assert headers["Authorization"] == "Bearer sandbox-token" assert not gate["open"] events.append("hibernate") return 204, {} - addr = spawn_node({("PATCH", PATH + "/volumes"): patch, ("POST", PATH + "/hibernate"): hibernate}) + addr = spawn_node({("PATCH", PATH + "/volumes"): patch, ("POST", PATH + "/hibernate"): hibernate}, headers) client = Client(addr, api_token="root-token") sb = client.attach(addr, "sb_1", "sandbox-token") diff --git a/sdk/python/tests/test_volumes.py b/sdk/python/tests/test_volumes.py index e8f233380..5a36c48e6 100644 --- a/sdk/python/tests/test_volumes.py +++ b/sdk/python/tests/test_volumes.py @@ -4,7 +4,7 @@ import pytest from cocoonsandbox import APIError, VolumeError -from conftest import FakeNode, sandbox_at +from conftest import bounded, sandbox_at PATH = "/v1/sandboxes/sb_1/volumes" VIEW = { @@ -36,12 +36,13 @@ def test_volume_mapping_is_json_encoded(spawn_node): ) def test_volume_requests_use_owner_and_api_token(spawn_node, method, body, call): seen, reply = recording() - addr = spawn_node({(method, PATH): reply}) + headers = {} + addr = spawn_node({(method, PATH): reply}, headers) sb = sandbox_at(addr, api_token="operator-token") sb._client.addr = "unused.invalid:7777" got = call(sb) assert seen == [body] - assert FakeNode.last_headers["Authorization"] == "Bearer operator-token" + assert headers["Authorization"] == "Bearer operator-token" assert got == VIEW assert sb.volumes == VIEW["volumes"] assert sb.volumes_want == VIEW["volumes_want"] @@ -129,9 +130,9 @@ def reply(body, path): def test_volume_timeout_bounds_the_call(black_hole): - sb = sandbox_at(black_hole, timeout=2) + sb = sandbox_at(black_hole, timeout=60) with pytest.raises(APIError) as caught: - sb.get_volumes(timeout=0.02) + bounded(lambda: sb.get_volumes(timeout=0.02)) assert caught.value.status == 0 diff --git a/sdk/python/tests/test_wire_binding.py b/sdk/python/tests/test_wire_binding.py index 7fcf7146d..6e1c81b3b 100644 --- a/sdk/python/tests/test_wire_binding.py +++ b/sdk/python/tests/test_wire_binding.py @@ -1,5 +1,6 @@ """Drives each real Sandbox call site and asserts its frame carries exactly the golden fixture's fields.""" +import base64 import contextlib import json import socket @@ -70,10 +71,14 @@ [{"type": "started", "pid": 7}], lambda sb, f: sb.open_pty(cols=f["cols"], rows=f["rows"], cwd=f["cwd"], env=f["env"], user=f["user"]).close(), ), + ("req_stdin", [], lambda sb, f: write_pty(sb, base64.b64decode(f["data"]))), + ("req_stdin_close", [{"type": "started", "pid": 7}, {"type": "exit", "code": 0}], lambda sb, f: sb.run(["true"])), + ("req_data", [{"type": "done"}], lambda sb, f: sb.write_file("/f", base64.b64decode(f["data"]))), + ("req_data_end", [{"type": "done"}], lambda sb, f: sb.write_file("/f", b"x")), ] - UNSENT = {"req_session_create": {"id"}} +NO_CALL_SITE = {"req_info", "req_tun_up", "req_tun_down"} class BranchActionConn(FakeConn): @@ -94,16 +99,15 @@ def recv(self): return {"type": "done"} -@pytest.mark.parametrize("stem,replies,invoke", CASES, ids=lambda c: c if isinstance(c, str) else "") +@pytest.mark.parametrize("stem,replies,invoke", CASES, ids=[case[0] for case in CASES]) def test_call_site_matches_fixture(monkeypatch, stem, replies, invoke): fixture = json.loads((FIXTURES / f"{stem}.json").read_text()) sb, sent, thread = fake_sandbox(monkeypatch, replies) invoke(sb, fixture) thread.join(timeout=5) - assert sent, "no frame reached the guest" - frame = sent[0] - assert frame["op"] == fixture["op"] + frame = next((frame for frame in sent if frame["op"] == fixture["op"]), None) + assert frame is not None, f"no {fixture['op']} frame reached the guest: {sent}" assert frame["v"] == PROTO_VERSION for key, value in frame.items(): assert key in fixture, f"field {key!r} not in the golden corpus (typo?)" @@ -115,12 +119,10 @@ def test_call_site_matches_fixture(monkeypatch, stem, replies, invoke): assert key in frame, f"fixture field {key!r} was never emitted (dropped field?)" -def test_enum_value_sets_match_corpus(): - enums = json.loads((FIXTURES / "enums.json").read_text()) - assert set(enums["error_kind"]) == {"bad_request", "not_found", "unimplemented", "internal"} - assert set(enums["event_kind"]) == {"created", "modified", "deleted", "renamed"} - assert set(enums["file_kind"]) == {"file", "dir", "symlink", "other"} - assert set(enums["git_branch_action"]) == {"list", "create", "delete", "checkout"} +def test_every_request_fixture_has_a_call_site_or_an_exclusion(): + covered = {case[0] for case in CASES} + assert covered.isdisjoint(NO_CALL_SITE) + assert {path.stem for path in FIXTURES.glob("req_*.json")} == covered | NO_CALL_SITE def test_git_branch_actions_come_from_the_corpus(monkeypatch): @@ -136,6 +138,11 @@ def test_git_branch_actions_come_from_the_corpus(monkeypatch): assert set(sent) == set(enums["git_branch_action"]) +def write_pty(sb, data): + with Pty(sb, sb._dial(), 7) as pty: + pty.write(data) + + def fake_sandbox(monkeypatch, replies): """A Sandbox whose _dial yields a real Conn over a socketpair to a scripted guest thread.""" sent = [] diff --git a/silkd/tests/common/mod.rs b/silkd/tests/common/mod.rs index f566be0f7..bf5e5597a 100644 --- a/silkd/tests/common/mod.rs +++ b/silkd/tests/common/mod.rs @@ -132,6 +132,17 @@ pub async fn wait_for_path(path: &Path) { .await; } +#[track_caller] +pub fn require_perl() { + assert!( + std::process::Command::new("perl") + .arg("-e1") + .status() + .is_ok_and(|status| status.success()), + "this test needs perl on PATH" + ); +} + pub fn alive(pid: u32) -> bool { // SAFETY: kill(2) with signal 0 only probes whether the pid exists; it takes no pointers. if unsafe { libc::kill(pid as libc::pid_t, 0) } != 0 { diff --git a/silkd/tests/exec_e2e.rs b/silkd/tests/exec_e2e.rs index ddd5814f1..c5890efdb 100644 --- a/silkd/tests/exec_e2e.rs +++ b/silkd/tests/exec_e2e.rs @@ -13,7 +13,7 @@ use tokio::io::{AsyncReadExt, AsyncWriteExt, BufReader}; use common::{ alive, assert_error, b64, connect_with, exchange, frames_until, next_frame, one, proc_entry, - request, roundtrip, send, stdout_body, type_of, wait_for_path, + request, require_perl, roundtrip, send, stdout_body, type_of, wait_for_path, }; async fn detached_pid(state: &Arc, script: &str) -> u64 { @@ -97,32 +97,12 @@ async fn empty_argv_is_a_bad_request() { assert_error(&frames[0], "bad_request"); } -#[tokio::test] -async fn info_reports_the_version() { - let frames = roundtrip(r#"{"op":"info"}"#).await; - assert_eq!(type_of(&frames[0]), "info"); - assert!(frames[0]["version"].as_str().is_some()); -} - #[tokio::test] async fn kill_unknown_pid_is_not_found() { let frames = roundtrip(r#"{"op":"kill","pid":999999}"#).await; assert_error(&frames[0], "not_found"); } -#[tokio::test] -async fn daemonizing_child_does_not_wedge_the_exec() { - let frames = tokio::time::timeout( - Duration::from_secs(8), - roundtrip(r#"{"op":"exec","argv":["/bin/sh","-c","sleep 30 & exit 0"]}"#), - ) - .await - .expect("exec wedged on a daemonizing child"); - let last = frames.last().unwrap(); - assert_eq!(type_of(last), "exit"); - assert_eq!(last["code"], 0); -} - #[tokio::test] async fn daemonizer_exit_is_the_last_frame() { let frames = tokio::time::timeout( @@ -136,6 +116,7 @@ async fn daemonizer_exit_is_the_last_frame() { "exit", "exit must be last: {frames:?}" ); + assert_eq!(frames.last().unwrap()["code"], 0, "{frames:?}"); assert!( !stdout_body(&frames).contains("late"), "grandchild output must not leak after exit" @@ -282,9 +263,7 @@ async fn exec_unknown_user_is_rejected() { #[tokio::test] #[ignore = "needs root"] async fn exec_as_a_user_keeps_its_supplementary_groups() { - let Some(user) = group_member() else { - return; - }; + let user = group_member().expect("no /etc/group entry lists a member to exec as"); let want = std::process::Command::new("id") .args(["-G", &user]) .output() @@ -399,16 +378,20 @@ async fn disconnect_during_drain_publishes_real_exit_code() { assert_eq!(type_of(&started), "started"); let pid = started["pid"].as_u64().unwrap(); - let st = Arc::clone(&state); - let attach = tokio::spawn(async move { attach_frames(&st, pid).await }); - tokio::time::sleep(Duration::from_millis(200)).await; + let (_aw, mut attached, _) = connect_with(&state, json!({"op":"attach","pid":pid})).await; + let late = next_frame(&mut attached).await; + assert_eq!(type_of(&late), "stdout", "{late:?}"); drop(lines); drop(cw); - let frames = tokio::time::timeout(Duration::from_secs(8), attach) - .await - .expect("attach wedged") - .unwrap(); + let frames = tokio::time::timeout( + Duration::from_secs(8), + frames_until(&mut attached, |f| { + matches!(type_of(f), "exit" | "done" | "error") + }), + ) + .await + .expect("attach wedged"); let last = frames.last().unwrap(); assert_eq!( type_of(last), @@ -447,6 +430,7 @@ async fn stdin_held_by_a_forked_child_does_not_wedge_the_next_rpc() { #[tokio::test] async fn a_client_gone_mid_output_releases_the_stdin_a_forked_child_holds() { + require_perl(); let dir = tempfile::tempdir().unwrap(); let out = dir.path().join("count"); let state = Arc::new(State::new()); diff --git a/silkd/tests/forward_e2e.rs b/silkd/tests/forward_e2e.rs index c8a0cd51a..9cee0b85f 100644 --- a/silkd/tests/forward_e2e.rs +++ b/silkd/tests/forward_e2e.rs @@ -183,7 +183,7 @@ async fn forward_split_requests_do_not_wait_on_delayed_acks() { let state = Arc::new(State::new()); let (mut cw, mut lines) = forwarded(&state, port).await; let start = Instant::now(); - for _ in 0..20 { + for _ in 0..200 { send( &mut cw, json!({"v":1,"op":"data","data":b64(&[b'a'; 8192])}), @@ -199,8 +199,8 @@ async fn forward_split_requests_do_not_wait_on_delayed_acks() { } let elapsed = start.elapsed(); assert!( - elapsed < Duration::from_millis(200), - "20 split requests took {elapsed:?}: the guest-side write waits on delayed ACKs" + elapsed < Duration::from_secs(5), + "200 split requests took {elapsed:?}: the guest-side write waits on delayed ACKs" ); }) .await; diff --git a/silkd/tests/fs_e2e.rs b/silkd/tests/fs_e2e.rs index d0391871d..6fde8be13 100644 --- a/silkd/tests/fs_e2e.rs +++ b/silkd/tests/fs_e2e.rs @@ -1,5 +1,6 @@ //! Filesystem verb E2E over the in-memory duplex, against real temp paths. +#![allow(clippy::unwrap_used, clippy::expect_used)] mod common; use std::os::unix::fs::{MetadataExt, PermissionsExt}; diff --git a/silkd/tests/keepalive_e2e.rs b/silkd/tests/keepalive_e2e.rs index 2f928ddff..18df50ec7 100644 --- a/silkd/tests/keepalive_e2e.rs +++ b/silkd/tests/keepalive_e2e.rs @@ -1,5 +1,6 @@ //! RPCs back to back on one connection, and the frames that cross an RPC boundary. +#![allow(clippy::unwrap_used, clippy::expect_used)] mod common; use std::sync::Arc; @@ -12,7 +13,7 @@ use tokio::time::timeout; use common::{ DEADLINE, FrameLines, assert_error, b64, connect_with, expect_frame, frames_until, next_frame, - send, type_of, + roundtrip, send, type_of, }; async fn types_until(lines: &mut FrameLines, terminal: &str) -> Vec { @@ -41,6 +42,13 @@ async fn rpcs_run_back_to_back_on_one_connection() { .unwrap(); } +#[tokio::test] +async fn info_reports_the_version() { + let frames = roundtrip(r#"{"op":"info"}"#).await; + assert_eq!(type_of(&frames[0]), "info"); + assert_eq!(frames[0]["version"], env!("CARGO_PKG_VERSION")); +} + #[tokio::test] async fn exec_hands_the_connection_back_after_exit() { let state = Arc::new(State::new()); diff --git a/silkd/tests/pty_e2e.rs b/silkd/tests/pty_e2e.rs index 586c1fe84..c2fb9f303 100644 --- a/silkd/tests/pty_e2e.rs +++ b/silkd/tests/pty_e2e.rs @@ -175,14 +175,22 @@ async fn stdin_nobody_reads_after_the_shell_exits_does_not_wedge_the_next_reques } #[tokio::test] -async fn pty_disconnect_tears_down_without_spin() { +async fn pty_disconnect_removes_the_process() { let state = Arc::new(State::new()); - let (cw, mut lines, handle) = open_pty(&state).await; + let (mut cw, mut lines, handle) = open_pty(&state).await; let pid = read_until(&mut lines, |v| v["type"] == "started").await["pid"] .as_u64() .unwrap(); - tokio::time::sleep(Duration::from_millis(200)).await; + send( + &mut cw, + json!({"op":"stdin","data":b64(b"echo silk-pty-$((1+1))-ready\n")}), + ) + .await; + read_until(&mut lines, |v| { + v["type"] == "stdout" && String::from_utf8_lossy(&decode(v)).contains("silk-pty-2-ready") + }) + .await; drop(lines); drop(cw); diff --git a/silkd/tests/session_e2e.rs b/silkd/tests/session_e2e.rs index 5693e483b..6ea9c2a2d 100644 --- a/silkd/tests/session_e2e.rs +++ b/silkd/tests/session_e2e.rs @@ -10,7 +10,7 @@ use serde_json::{Value, json}; use silkd::server::State; use tokio::task::JoinHandle; -use common::{assert_error, one, stdout_body, type_of, wait_for_path}; +use common::{assert_error, one, require_perl, stdout_body, type_of, wait_for_path}; async fn create(state: &Arc) -> String { create_with(state, json!({"op":"session_create"})).await @@ -328,6 +328,7 @@ async fn session_merges_stderr_into_stdout() { #[tokio::test] async fn a_dead_session_command_leaves_a_recreated_session_alone() { + require_perl(); let state = Arc::new(State::new()); let id = create_with(&state, json!({"op":"session_create","id":"s"})).await; let bg = sh( diff --git a/silkd/tests/tree_e2e.rs b/silkd/tests/tree_e2e.rs index b1a168e27..5b33d89b9 100644 --- a/silkd/tests/tree_e2e.rs +++ b/silkd/tests/tree_e2e.rs @@ -100,21 +100,6 @@ async fn push_then_pull_roundtrips_a_tree() { assert_eq!(std::fs::read(out.path().join("t/d/f2")).unwrap(), b"two"); } -#[tokio::test] -async fn push_truncated_stream_errors() { - let dest = tempfile::tempdir().unwrap(); - let frames = exchange(&[ - json!({"op":"fs_push","dest":dest.path().join("p").to_str().unwrap()}).to_string(), - json!({"op":"data","data":b64(b"not-a-real-tar")}).to_string(), - ]) - .await; - assert_eq!( - type_of(frames.last().unwrap()), - "error", - "truncated push: {frames:?}" - ); -} - #[tokio::test] async fn push_bad_archive_reports_tar_failure() { let dest = tempfile::tempdir().unwrap(); diff --git a/silkd/tests/tun_e2e.rs b/silkd/tests/tun_e2e.rs index 71d2b45ec..831211946 100644 --- a/silkd/tests/tun_e2e.rs +++ b/silkd/tests/tun_e2e.rs @@ -9,25 +9,21 @@ use std::os::unix::net::UnixStream; use std::sync::Arc; use std::sync::atomic::{AtomicBool, AtomicU64, Ordering}; use std::thread::JoinHandle; -use std::time::{Duration, Instant}; +use std::time::Duration; use nix::sched::{CloneFlags, unshare}; -use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; -use tokio::net::{TcpListener, TcpStream, UdpSocket}; - use silkd::sysutil; use silkd::tun::{ self, FLAG_ON, Flow, HELLO_LEN, Hello, Io, PEER_ADDR, Tun, VERSION, VNET_HDR_LEN, }; +use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader}; +use tokio::net::{TcpListener, TcpStream, UdpSocket}; const CLAIM_ADDR: Ipv4Addr = Ipv4Addr::new(100, 65, 0, 7); const UPLOAD: u16 = 5001; const ECHO: u16 = 5002; const DOWNLOAD: u16 = 5003; const SINK: u16 = 5004; -const PPS_SINK: u16 = 5005; -const PPS_SOURCE: u16 = 5006; -const GUEST_PPS_PORT: u16 = 6000; const BULK: u64 = 256 << 20; const GSO_NONE: u8 = 0; const GSO_UDP_L4: u8 = 5; @@ -39,8 +35,6 @@ const PROTO_UDP: u8 = 17; const UDP_SEGMENT: libc::c_int = 103; const UDP_DATAGRAMS: usize = 32; const UDP_SEG: usize = 1000; -const SMALL: usize = 64; -const FLOOD: Duration = Duration::from_secs(2); #[derive(Default)] struct Stats { @@ -63,9 +57,12 @@ fn tcp_udp_and_gso_cross_a_rewriting_host() { for io in [Io::Uring, Io::Epoll] { let up = Arc::new(Stats::default()); let down = Arc::new(Stats::default()); - let (seen, accepts) = with_link(io, true, Some((up.clone(), down.clone())), |_| { - guest_workload() - }); + let (seen, accepts) = with_link( + io, + true, + (Arc::clone(&up), Arc::clone(&down)), + guest_workload, + ); assert_eq!(seen.upload_peer, CLAIM_ADDR.to_string(), "{io:?}"); assert_eq!(seen.echo_peers, vec![CLAIM_ADDR.to_string(); 16], "{io:?}"); assert_eq!(seen.downloaded, BULK, "{io:?}"); @@ -107,9 +104,12 @@ fn full_checksums_cross_when_offloads_are_off() { for io in [Io::Uring, Io::Epoll] { let up = Arc::new(Stats::default()); let down = Arc::new(Stats::default()); - let (seen, _) = with_link(io, false, Some((up.clone(), down.clone())), |_| { - guest_workload() - }); + let (seen, _) = with_link( + io, + false, + (Arc::clone(&up), Arc::clone(&down)), + guest_workload, + ); assert_eq!(seen.upload_peer, CLAIM_ADDR.to_string(), "{io:?}"); assert_eq!(seen.echo_peers, vec![CLAIM_ADDR.to_string(); 16], "{io:?}"); assert_eq!(seen.downloaded, BULK, "{io:?}"); @@ -145,104 +145,37 @@ fn host_off_answer_leaves_no_tun() { .unwrap(); } -#[test] -#[ignore = "needs root; prints throughput, asserts nothing about speed"] -fn throughput_and_per_packet_cost() { - let order = [ - (Io::Uring, true), - (Io::Epoll, true), - (Io::Uring, false), - (Io::Epoll, false), - (Io::Epoll, false), - (Io::Uring, false), - (Io::Epoll, true), - (Io::Uring, true), - ]; - for (io, offloads) in order { - let (r, _) = with_link(io, offloads, None, |pump_tid| { - runtime().block_on(async move { - let t = Instant::now(); - upload(SocketAddr::from((PEER_ADDR, UPLOAD)), 4 * BULK).await; - let up = gbits(4 * BULK, t.elapsed()); - let t = Instant::now(); - let got = download(SocketAddr::from((PEER_ADDR, DOWNLOAD))).await; - let down = gbits(got, t.elapsed()); - let cpu = pump_cpu_ns(pump_tid); - let sent = flood_up().await; - let up_pkts = query(PPS_SINK).await; - let up_cpu = pump_cpu_ns(pump_tid) - cpu; - let cpu = pump_cpu_ns(pump_tid); - let down_pkts = flood_down().await; - let down_cpu = pump_cpu_ns(pump_tid) - cpu; - (up, down, sent, up_pkts, up_cpu, down_pkts, down_cpu) - }) - }); - let (up, down, sent, up_pkts, up_cpu, down_pkts, down_cpu) = r; - let secs = FLOOD.as_secs_f64(); - eprintln!( - "{io:?} offloads={offloads}: tcp up {up:.2} Gbit/s, down {down:.2} Gbit/s; \ - udp{SMALL} up {:.0} kpps ({:.0} ns/pkt, {sent} sent), down {:.0} kpps ({:.0} ns/pkt)", - up_pkts as f64 / secs / 1e3, - up_cpu as f64 / up_pkts.max(1) as f64, - down_pkts as f64 / secs / 1e3, - down_cpu as f64 / down_pkts.max(1) as f64, - ); - } -} - -/// Runs `guest` in the guest namespace while the session runs; `relay` rewrites the guest address the way the host's shared TUN does. fn with_link( io: Io, offloads: bool, - relay: Option<(Arc, Arc)>, - guest: impl FnOnce(i32) -> T + Send + 'static, + (up, down): (Arc, Arc), + guest: impl FnOnce() -> T + Send + 'static, ) -> (T, u16) { - let (guest_end, host_end, relay_threads) = match relay { - Some((up, down)) => { - let (g, rg) = UnixStream::pair().unwrap(); - let (rh, h) = UnixStream::pair().unwrap(); - let threads = vec![ - forward( - rg.try_clone().unwrap(), - rh.try_clone().unwrap(), - up, - IP_SRC, - tun::GUEST_ADDR, - CLAIM_ADDR, - ), - forward(rh, rg, down, IP_DST, CLAIM_ADDR, tun::GUEST_ADDR), - ]; - (g, h, threads) - } - None => { - let (g, h) = UnixStream::pair().unwrap(); - (g, h, Vec::new()) - } - }; + let (guest_end, rg) = UnixStream::pair().unwrap(); + let (rh, host_end) = UnixStream::pair().unwrap(); + let relay_threads = [ + forward( + rg.try_clone().unwrap(), + rh.try_clone().unwrap(), + up, + IP_SRC, + tun::GUEST_ADDR, + CLAIM_ADDR, + ), + forward(rh, rg, down, IP_DST, CLAIM_ADDR, tun::GUEST_ADDR), + ]; let stop = Arc::new(AtomicBool::new(false)); let host_stop = guest_end.try_clone().unwrap(); - let (host, accepts) = spawn_host( - host_end, - io, - offloads, - relay_threads.is_empty(), - stop.clone(), - ); + let (host, accepts) = spawn_host(host_end, io, offloads, Arc::clone(&stop)); let out = in_netns(move || { let tun = Tun::open("tun0").unwrap(); tun.configure_guest().unwrap(); let shut = guest_end.try_clone().unwrap(); - let (tid_tx, tid_rx) = std::sync::mpsc::channel(); let session = std::thread::Builder::new() .name("guest-pump".into()) - .spawn(move || { - // SAFETY: gettid takes no arguments. - tid_tx.send(unsafe { libc::gettid() }).unwrap(); - tun::session(&tun, &guest_end, io) - }) + .spawn(move || tun::session(&tun, &guest_end, io)) .unwrap(); - let tid = tid_rx.recv().unwrap(); - let out = guest(tid); + let out = guest(); shut.shutdown(std::net::Shutdown::Both).unwrap(); assert_eq!(session.join().unwrap().unwrap(), Flow::Closed); out @@ -262,18 +195,13 @@ fn spawn_host( stream: UnixStream, io: Io, offloads: bool, - direct: bool, stop: Arc, ) -> (JoinHandle<()>, u16) { let (ready_tx, ready_rx) = std::sync::mpsc::channel(); let handle = in_netns(move || { let tun = Tun::open("tunh").unwrap(); - if direct { - sysutil::if_up("tunh", PEER_ADDR, tun::PREFIX).unwrap(); - } else { - sysutil::if_up("tunh", PEER_ADDR, 32).unwrap(); - sysutil::route_add("tunh", Ipv4Addr::new(100, 65, 0, 0), 16, None).unwrap(); - } + sysutil::if_up("tunh", PEER_ADDR, 32).unwrap(); + sysutil::route_add("tunh", Ipv4Addr::new(100, 65, 0, 0), 16, None).unwrap(); let shut = stream.try_clone().unwrap(); let (bound_tx, bound_rx) = std::sync::mpsc::channel(); let servers = std::thread::spawn(move || { @@ -282,14 +210,10 @@ fn spawn_host( let download = TcpListener::bind((PEER_ADDR, DOWNLOAD)).await.unwrap(); let echo = UdpSocket::bind((PEER_ADDR, ECHO)).await.unwrap(); let sink = UdpSocket::bind((PEER_ADDR, SINK)).await.unwrap(); - let pps = UdpSocket::bind((PEER_ADDR, PPS_SINK)).await.unwrap(); - let source = TcpListener::bind((PEER_ADDR, PPS_SOURCE)).await.unwrap(); tokio::spawn(serve_upload(upload)); tokio::spawn(serve_download(download)); tokio::spawn(serve_echo(echo)); - tokio::spawn(serve_count(sink, SINK, Some(UDP_SEG))); - tokio::spawn(serve_count(pps, PPS_SINK, None)); - tokio::spawn(serve_flood(source)); + tokio::spawn(serve_count(sink)); bound_tx.send(()).unwrap(); while !stop.load(Ordering::Relaxed) { tokio::time::sleep(Duration::from_millis(20)).await; @@ -351,7 +275,7 @@ fn guest_workload() -> Seen { assert_eq!(rc, 0, "UDP_SEGMENT: {}", std::io::Error::last_os_error()); sink.send_to(&[7u8; UDP_DATAGRAMS * UDP_SEG], (PEER_ADDR, SINK)) .unwrap(); - let sink_datagrams = query(SINK).await; + let sink_datagrams = sink_count(UDP_DATAGRAMS as u64).await; Seen { upload_peer, echo_peers, @@ -361,27 +285,6 @@ fn guest_workload() -> Seen { }) } -fn thread_cpu_ns(tid: i32) -> u64 { - std::fs::read_to_string(format!("/proc/self/task/{tid}/schedstat")) - .ok() - .and_then(|s| s.split_whitespace().next()?.parse().ok()) - .unwrap_or(0) -} - -fn pump_cpu_ns(pump_tid: i32) -> u64 { - let workers = format!("iou-wrk-{pump_tid}"); - let mut total = thread_cpu_ns(pump_tid); - for entry in std::fs::read_dir("/proc/self/task").unwrap().flatten() { - let comm = std::fs::read_to_string(entry.path().join("comm")).unwrap_or_default(); - if comm.trim() == workers - && let Ok(tid) = entry.file_name().to_string_lossy().parse::() - { - total += thread_cpu_ns(tid); - } - } - total -} - fn runtime() -> tokio::runtime::Runtime { tokio::runtime::Builder::new_current_thread() .enable_all() @@ -396,10 +299,6 @@ fn in_netns(f: impl FnOnce() -> T + Send + 'static) -> JoinHa }) } -fn gbits(bytes: u64, took: Duration) -> f64 { - bytes as f64 * 8.0 / took.as_secs_f64() / 1e9 -} - async fn upload(target: SocketAddr, bytes: u64) -> String { let mut s = TcpStream::connect(target).await.unwrap(); let chunk = vec![0x5au8; 1 << 20]; @@ -430,48 +329,16 @@ async fn download(target: SocketAddr) -> u64 { } } -async fn query(port: u16) -> u64 { - tokio::time::sleep(Duration::from_millis(200)).await; - let mut s = TcpStream::connect((PEER_ADDR, port)).await.unwrap(); - s.read_u64().await.unwrap() -} - -async fn flood_up() -> u64 { - tokio::task::spawn_blocking(|| { - let s = std::net::UdpSocket::bind((Ipv4Addr::UNSPECIFIED, 0)).unwrap(); - s.connect((PEER_ADDR, PPS_SINK)).unwrap(); - let payload = [1u8; SMALL]; - let deadline = Instant::now() + FLOOD; - let mut sent = 0u64; - while Instant::now() < deadline { - for _ in 0..64 { - if s.send(&payload).is_ok() { - sent += 1; - } - } - } - sent - }) - .await - .unwrap() -} - -async fn flood_down() -> u64 { - let sink = std::net::UdpSocket::bind((Ipv4Addr::UNSPECIFIED, GUEST_PPS_PORT)).unwrap(); - sink.set_read_timeout(Some(Duration::from_millis(300))) - .unwrap(); - let mut s = TcpStream::connect((PEER_ADDR, PPS_SOURCE)).await.unwrap(); - s.write_u8(1).await.unwrap(); - tokio::task::spawn_blocking(move || { - let mut buf = [0u8; 2048]; - let mut got = 0u64; - while sink.recv(&mut buf).is_ok() { - got += 1; +async fn sink_count(want: u64) -> u64 { + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + loop { + let mut s = TcpStream::connect((PEER_ADDR, SINK)).await.unwrap(); + let got = s.read_u64().await.unwrap(); + if got >= want || tokio::time::Instant::now() >= deadline { + return got; } - got - }) - .await - .unwrap() + tokio::time::sleep(Duration::from_millis(20)).await; + } } async fn serve_upload(l: TcpListener) { @@ -514,8 +381,8 @@ async fn serve_echo(s: UdpSocket) { } } -async fn serve_count(s: UdpSocket, port: u16, size: Option) { - let report = TcpListener::bind((PEER_ADDR, port)).await.unwrap(); +async fn serve_count(s: UdpSocket) { + let report = TcpListener::bind((PEER_ADDR, SINK)).await.unwrap(); let got = Arc::new(AtomicU64::new(0)); tokio::spawn({ let got = Arc::clone(&got); @@ -523,7 +390,7 @@ async fn serve_count(s: UdpSocket, port: u16, size: Option) { let mut buf = [0u8; 2048]; loop { let (n, _) = s.recv_from(&mut buf).await.unwrap(); - if size.is_none_or(|want| want == n) { + if n == UDP_SEG { got.fetch_add(1, Ordering::Relaxed); } } @@ -531,26 +398,7 @@ async fn serve_count(s: UdpSocket, port: u16, size: Option) { }); loop { let (mut c, _) = report.accept().await.unwrap(); - let _ = c.write_u64(got.swap(0, Ordering::Relaxed)).await; - } -} - -async fn serve_flood(l: TcpListener) { - loop { - let (mut c, peer) = l.accept().await.unwrap(); - let _ = c.read_u8().await; - let target = SocketAddr::from((peer.ip(), GUEST_PPS_PORT)); - tokio::task::spawn_blocking(move || { - let s = std::net::UdpSocket::bind((PEER_ADDR, 0)).unwrap(); - s.connect(target).unwrap(); - let payload = [2u8; SMALL]; - let deadline = Instant::now() + FLOOD; - while Instant::now() < deadline { - for _ in 0..64 { - let _ = s.send(&payload); - } - } - }); + let _ = c.write_u64(got.load(Ordering::Relaxed)).await; } } @@ -640,7 +488,6 @@ fn rewrite(frame: &mut [u8], at: usize, from: [u8; 4], to: [u8; 4]) { ip[field..field + 2].copy_from_slice(&next.to_be_bytes()); } -/// Moves a ones'-complement sum from one address to another (RFC 1624). fn adjust(sum: u16, from: [u8; 4], to: [u8; 4]) -> u16 { let mut s = u32::from(sum); for i in [0, 2] { diff --git a/silkd/tests/watch_e2e.rs b/silkd/tests/watch_e2e.rs index bbb77d0fd..6422fca74 100644 --- a/silkd/tests/watch_e2e.rs +++ b/silkd/tests/watch_e2e.rs @@ -1,5 +1,6 @@ //! fs.watch E2E: a connection-bound event stream ended by client disconnect. +#![allow(clippy::unwrap_used, clippy::expect_used)] mod common; use std::sync::Arc;