fix(test): reclaim fixture scratch dirs stranded in /tmp (#598)

* fix(test): reclaim fixture scratch dirs stranded in /tmp

The shared predastore fixture and the integration suite's JetStream store both
outlive any individual test, so neither can use t.TempDir() - the first test to
finish would delete files the daemon still has open. Both used os.MkdirTemp
with nothing removing the directory afterwards, so every run stranded one, and
a run killed by a timeout panic or the OOM killer stranded it regardless. /tmp
is a tmpfs on the dev boxes, so the leak is charged to RAM and swap, and
swapped tmpfs pages have no backing store to be dropped to.

Add a shared sweep that reclaims a directory only when it matches both the
prefix and an age no live run can reach. The NATS store also has a clean
shutdown point, so TestMain now removes it outright and the sweep is left to
cover the killed-run case. The predastore daemon runs until the process exits
and has no such point, so the sweep is its only mechanism.

* chore: repin viperblock to the dev tip carrying the scratch-dir fix

Test-only upstream change, so the build output is unchanged - this keeps
go.mod aligned with the submodule pointer rather than delivering a fix.
This commit is contained in:
Josh Siviour
2026-07-29 14:57:50 +10:00
committed by GitHub
parent fc6e055ca0
commit 7d2106bfb5
7 changed files with 150 additions and 8 deletions
+1 -1
View File
@@ -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
+2 -2
View File
@@ -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=
+11 -1
View File
@@ -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)
}
+57
View File
@@ -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)
}
}
+50
View File
@@ -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)
})
}
+4
View File
@@ -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)
}
+25 -4
View File
@@ -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