diff --git a/go.mod b/go.mod index 79449fa39..6a73038ec 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,7 @@ require ( github.com/miekg/dns v1.1.72 github.com/mulgadc/northstar v1.14.0 github.com/mulgadc/predastore v1.14.0 - github.com/mulgadc/viperblock v1.14.0 + github.com/mulgadc/viperblock v1.14.1-0.20260729042700-9b253946d091 github.com/nats-io/nats-server/v2 v2.14.3 github.com/nats-io/nats.go v1.52.0 github.com/opencontainers/runtime-spec v1.3.0 diff --git a/go.sum b/go.sum index d4450b899..877ae2a23 100644 --- a/go.sum +++ b/go.sum @@ -1321,8 +1321,8 @@ github.com/mulgadc/northstar v1.14.0 h1:8JzDcS4k3SN5PF3jBdB8x9FOWC24eITCClmOdwpo github.com/mulgadc/northstar v1.14.0/go.mod h1:6EArbWIQUrCFQ+XZ5ZJrGS/ep/u/rF6l+dKZh5vVIRs= github.com/mulgadc/predastore v1.14.0 h1:vG+egKIP08SLql4hDUwke31ritValUYaDbyH39Ta6qw= github.com/mulgadc/predastore v1.14.0/go.mod h1:C/mlefzQgqRAXNMbKc/rmW5BVSoB3R0OPHsgOi/islI= -github.com/mulgadc/viperblock v1.14.0 h1:Q6REPTPHh+3fnkjbyBSfLXWSppgiLsEcxwuvaHow0LY= -github.com/mulgadc/viperblock v1.14.0/go.mod h1:5ENQb0OrjTplIRiLShRW/oNWOzbFvSCFgo+PkNH4858= +github.com/mulgadc/viperblock v1.14.1-0.20260729042700-9b253946d091 h1:P+KZWeIYh3uVmje6wad6TuCqAkV0Fn/4oVM4ldBaED0= +github.com/mulgadc/viperblock v1.14.1-0.20260729042700-9b253946d091/go.mod h1:5ENQb0OrjTplIRiLShRW/oNWOzbFvSCFgo+PkNH4858= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mwitkow/go-conntrack v0.0.0-20161129095857-cc309e4a2223/go.mod h1:qRWi+5nqEBWmkhHvq77mSJWrCKwh8bxhgT7d/eI7P4U= diff --git a/tests/fixtures/predastore/predastore.go b/tests/fixtures/predastore/predastore.go index 799a370b4..218f2bdc1 100644 --- a/tests/fixtures/predastore/predastore.go +++ b/tests/fixtures/predastore/predastore.go @@ -35,6 +35,7 @@ import ( "github.com/aws/aws-sdk-go/service/s3" "github.com/mulgadc/predastore/quic/quicclient" predastoreserver "github.com/mulgadc/predastore/s3" + "github.com/mulgadc/spinifex/tests/fixtures/scratch" ) // Fixed connection details for the shared predastore fixture daemon started @@ -84,6 +85,10 @@ var ( fixture *Fixture ) +// fixtureDirPrefix names each run's fixture directory. It is a constant +// because the sweep in Start matches on it. +const fixtureDirPrefix = "predastore-fixture-" + // Start starts a real predastore daemon the first time it's called within a // test binary and returns connection details for it; every later call in // the same process returns the already-running fixture. The daemon @@ -100,7 +105,12 @@ func Start(t *testing.T) *Fixture { return fixture } - testDir, err := os.MkdirTemp("", "predastore-fixture-*") //nolint:usetesting // shared daemon outlives individual tests + // The daemon runs until the process exits, so there is no point at which + // this run can remove its own directory. Reclaiming it is left to a later + // run's sweep, which is also what recovers a run killed mid-flight. + scratch.SweepAbandoned(os.TempDir(), fixtureDirPrefix, scratch.DefaultMaxAge) + + testDir, err := os.MkdirTemp("", fixtureDirPrefix+"*") //nolint:usetesting // shared daemon outlives individual tests if err != nil { t.Fatalf("predastore fixture: create temp dir: %v", err) } diff --git a/tests/fixtures/scratch/scratch.go b/tests/fixtures/scratch/scratch.go new file mode 100644 index 000000000..c06b9f445 --- /dev/null +++ b/tests/fixtures/scratch/scratch.go @@ -0,0 +1,57 @@ +// Package scratch reclaims test scratch directories stranded in the system +// temp directory by earlier runs. +// +// Fixtures whose daemon outlives any individual test cannot use t.TempDir(): +// the first test to finish would delete files the shared daemon still has +// open. They use os.MkdirTemp instead, which nothing then removes when the +// process dies without running its cleanups — a test timeout panic or an OOM +// kill. /tmp is a tmpfs on the dev boxes, so a stranded directory is charged +// to RAM and swap rather than disk, and swapped tmpfs pages have no backing +// store to be dropped to: they are never reclaimed until the files go. +package scratch + +import ( + "fmt" + "os" + "path/filepath" + "strings" + "time" +) + +// DefaultMaxAge is how old a stranded directory must be before it is +// reclaimed. A full run takes minutes, so a directory this old cannot belong +// to a live test binary. +const DefaultMaxAge = 6 * time.Hour + +// SweepAbandoned removes directories under tmpDir that match prefix and are +// older than maxAge. +// +// Deleting shared scratch deserves caution, so a candidate must match on both +// the prefix and the age: a concurrently running test binary's directory is +// minutes old at most and can never qualify. Failures are reported and +// otherwise ignored — not reclaiming disk must never fail the run. +func SweepAbandoned(tmpDir, prefix string, maxAge time.Duration) { + entries, err := os.ReadDir(tmpDir) + if err != nil { + fmt.Fprintf(os.Stderr, "scratch sweep: read %s: %v\n", tmpDir, err) + return + } + + cutoff := time.Now().Add(-maxAge) + for _, entry := range entries { + if !entry.IsDir() || !strings.HasPrefix(entry.Name(), prefix) { + continue + } + info, err := entry.Info() + if err != nil || info.ModTime().After(cutoff) { + continue + } + + path := filepath.Join(tmpDir, entry.Name()) + if err := os.RemoveAll(path); err != nil { + fmt.Fprintf(os.Stderr, "scratch sweep: remove %s: %v\n", path, err) + continue + } + fmt.Fprintf(os.Stderr, "scratch sweep: reclaimed abandoned scratch dir %s\n", path) + } +} diff --git a/tests/fixtures/scratch/scratch_test.go b/tests/fixtures/scratch/scratch_test.go new file mode 100644 index 000000000..f506f51cd --- /dev/null +++ b/tests/fixtures/scratch/scratch_test.go @@ -0,0 +1,50 @@ +package scratch + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// The sweep runs against a directory shared with other processes, so what it +// declines to touch matters as much as what it reclaims: only a directory that +// matches the prefix AND is far older than any live run may go. +func TestSweepAbandoned(t *testing.T) { + tmpDir := t.TempDir() + stale := time.Now().Add(-DefaultMaxAge - time.Hour) + + mkdir := func(name string, modTime time.Time) string { + path := filepath.Join(tmpDir, name) + require.NoError(t, os.MkdirAll(filepath.Join(path, "data"), 0o750)) + require.NoError(t, os.Chtimes(path, modTime, modTime)) + return path + } + + abandoned := mkdir("fixture-1234567890", stale) + concurrent := mkdir("fixture-0987654321", time.Now()) + otherPrefix := mkdir("some-other-tool-cache", stale) + + // A stale FILE sharing the prefix is not scratch and is not ours to delete. + stalefile := filepath.Join(tmpDir, "fixture-notadir") + require.NoError(t, os.WriteFile(stalefile, []byte("x"), 0o600)) + require.NoError(t, os.Chtimes(stalefile, stale, stale)) + + SweepAbandoned(tmpDir, "fixture-", DefaultMaxAge) + + assert.NoDirExists(t, abandoned, "a stale scratch dir from a killed run must be reclaimed") + assert.DirExists(t, concurrent, "a freshly-modified scratch dir may belong to a concurrent test binary") + assert.DirExists(t, otherPrefix, "the sweep must not touch directories outside its own prefix") + assert.FileExists(t, stalefile, "the sweep must only remove directories") +} + +// A missing or unreadable directory is not a test failure: the sweep exists to +// reclaim disk, and failing to do so must never take the run down with it. +func TestSweepAbandonedToleratesUnreadableDir(t *testing.T) { + assert.NotPanics(t, func() { + SweepAbandoned(filepath.Join(t.TempDir(), "does-not-exist"), "fixture-", DefaultMaxAge) + }) +} diff --git a/tests/integration/main_test.go b/tests/integration/main_test.go index 20798f69c..2e742bd61 100644 --- a/tests/integration/main_test.go +++ b/tests/integration/main_test.go @@ -44,5 +44,9 @@ func TestMain(m *testing.M) { h.closeAll() h.srv.Shutdown() h.srv.WaitForShutdown() + + // Only reached on a clean exit; a killed run leaves the store for the + // sweep in startSharedNATS to reclaim. + h.removeStore() os.Exit(code) } diff --git a/tests/integration/shared_nats.go b/tests/integration/shared_nats.go index 34ca5d6b1..ac285f2f8 100644 --- a/tests/integration/shared_nats.go +++ b/tests/integration/shared_nats.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "github.com/mulgadc/spinifex/tests/fixtures/scratch" "github.com/nats-io/nats-server/v2/server" "github.com/nats-io/nats.go" "github.com/nats-io/nats.go/jetstream" @@ -27,17 +28,26 @@ var sharedNATSHarness *sharedNATS // connection into it — is shared across the whole package rather than // booted fresh per test. type sharedNATS struct { - srv *server.Server - auth *accountAuthenticator + srv *server.Server + auth *accountAuthenticator + storeDir string connsMu sync.Mutex conns []*nats.Conn } +// natsStorePrefix names each run's JetStream store directory. It is a constant +// because the sweep below matches on it. +const natsStorePrefix = "spinifex-integration-nats-" + // startSharedNATS boots the one embedded, JetStream-enabled NATS server the // whole package's tests connect into. func startSharedNATS() (*sharedNATS, error) { - storeDir, err := os.MkdirTemp("", "spinifex-integration-nats-") + // The store cannot be a t.TempDir(): the server outlives every individual + // test, so the first test to finish would delete it out from under it. + scratch.SweepAbandoned(os.TempDir(), natsStorePrefix, scratch.DefaultMaxAge) + + storeDir, err := os.MkdirTemp("", natsStorePrefix) if err != nil { return nil, fmt.Errorf("nats store tempdir: %w", err) } @@ -63,7 +73,18 @@ func startSharedNATS() (*sharedNATS, error) { return nil, fmt.Errorf("nats server not ready for connections") } - return &sharedNATS{srv: ns, auth: auth}, nil + return &sharedNATS{srv: ns, auth: auth, storeDir: storeDir}, nil +} + +// removeStore reclaims the JetStream store once the server is fully down. +// Called after WaitForShutdown so nothing is still writing into it. +func (h *sharedNATS) removeStore() { + if h.storeDir == "" { + return + } + if err := os.RemoveAll(h.storeDir); err != nil { + fmt.Fprintf(os.Stderr, "tests/integration: remove nats store %s: %v\n", h.storeDir, err) + } } // connectIsolated opens a fresh connection scoped to its own dynamically