diff --git a/images/runner-ci-linux/Dockerfile b/images/runner-ci-linux/Dockerfile index 555c9097..f2222db7 100644 --- a/images/runner-ci-linux/Dockerfile +++ b/images/runner-ci-linux/Dockerfile @@ -23,6 +23,19 @@ ARG TARGETARCH USER root +# C toolchain. Required by `go test -race`: the race detector is implemented in +# a cgo runtime shim, so without a C compiler every -race invocation dies with +# "-race requires cgo" / `C compiler "gcc" not found`. The base image +# (mcr.microsoft.com/dotnet/runtime-deps:8.0-noble via actions-runner) ships +# runtime libs only and has no compiler. +# +# gcc + libc6-dev is the minimal set that satisfies the race runtime. Deliberately +# not build-essential: that adds g++, make, dpkg-dev and ~250MB of image for +# nothing we build here. ephemerd itself is still compiled CGO_ENABLED=0. +RUN apt-get update && \ + apt-get install -y --no-install-recommends gcc libc6-dev && \ + rm -rf /var/lib/apt/lists/* + RUN curl -fsSL "https://go.dev/dl/go${GO_VERSION}.linux-${TARGETARCH}.tar.gz" | tar -C /usr/local -xz ENV PATH="/usr/local/go/bin:/usr/local/bin:${PATH}" diff --git a/magefile.go b/magefile.go index a289ac62..81f482d9 100644 --- a/magefile.go +++ b/magefile.go @@ -31,6 +31,22 @@ func Test() error { return sh.RunV("go", "test", "-tags", "containers_image_openpgp", "-count=1", "./...") } +// TestRace runs all Go tests under the race detector. +// +// Deliberately NOT part of `mage ci`: -race requires cgo, and the Windows dev +// toolchain in this project cannot preprocess the cgo deps (see AGENTS.md), so +// folding it into `mage ci` would break the "run CI locally before pushing" +// rule for every Windows engineer. CI calls this target directly on the Linux +// runner, which is the only image with a C compiler (images/runner-ci-linux). +// +// Runs the whole tree rather than a hand-picked package list: the full suite +// takes ~1 minute under -race, and a scoped list silently stops covering +// whatever concurrency gets added next. +func TestRace() error { + mg.Deps(download.All) + return sh.RunV("go", "test", "-race", "-tags", "containers_image_openpgp", "-count=1", "./...") +} + // Lint runs golangci-lint (downloads linter and embedded deps first if needed). func Lint() error { mg.Deps(download.Golangcilint, download.All) diff --git a/pkg/dind/racedetector_norace_test.go b/pkg/dind/racedetector_norace_test.go new file mode 100644 index 00000000..28e39a95 --- /dev/null +++ b/pkg/dind/racedetector_norace_test.go @@ -0,0 +1,7 @@ +//go:build !race + +package dind + +// raceDetectorEnabled reports whether this test binary was built with -race. +// See racedetector_race_test.go. +const raceDetectorEnabled = false diff --git a/pkg/dind/racedetector_race_test.go b/pkg/dind/racedetector_race_test.go new file mode 100644 index 00000000..fae0c30f --- /dev/null +++ b/pkg/dind/racedetector_race_test.go @@ -0,0 +1,8 @@ +//go:build race + +package dind + +// raceDetectorEnabled reports whether this test binary was built with -race. +// Go exposes no runtime API for this, so it is derived from the build tag the +// toolchain sets for us. +const raceDetectorEnabled = true diff --git a/pkg/dind/registry_e2e_test.go b/pkg/dind/registry_e2e_test.go index 0c71ce11..2108061b 100644 --- a/pkg/dind/registry_e2e_test.go +++ b/pkg/dind/registry_e2e_test.go @@ -45,6 +45,29 @@ func TestPushHandlerEndToEnd(t *testing.T) { if testing.Short() { t.Skip("skipping push e2e in short mode") } + if raceDetectorEnabled { + // Quarantined under -race because of a data race in containerd itself, + // not in ephemerd. containerd's authHandler.doBearerAuth reads the + // cached authResult's expirationTime under ah.Lock: + // + // if r, exist := ah.scopedTokens[scoped]; exist && + // (r.expirationTime == nil || r.expirationTime.After(time.Now())) + // + // while the goroutine that owns the in-flight fetch writes + // r.token/r.refreshToken/r.err/r.expirationTime from a defer that holds + // no lock at all. Any concurrent push of two blobs needing the same + // token scope trips it, which is exactly what this test does — it + // reproduces 10 out of 10 runs. + // + // core/remotes/docker/authorizer.go:289 (read) vs :303 (write), + // containerd v2.2.2. Still unfixed on containerd main as of this commit, + // so bumping the dependency will not clear it. + // + // Nothing here is ephemerd's to fix; the alternative to this skip is + // dropping pkg/dind from the race job entirely, which would give up + // race coverage of dind's own concurrency to hide one upstream bug. + t.Skip("upstream containerd bearer-auth data race (authorizer.go doBearerAuth); see comment") + } const ( loginUser = "ephpm" diff --git a/pkg/forgerunner/runner_test.go b/pkg/forgerunner/runner_test.go index 7e6ce8a5..5b357d2f 100644 --- a/pkg/forgerunner/runner_test.go +++ b/pkg/forgerunner/runner_test.go @@ -259,7 +259,13 @@ jobs: } func TestPoll_ContextCancellation(t *testing.T) { - fetchCount := 0 + // Atomic, not a plain int: Run returns the moment the context deadline + // fires, which can leave a FetchTask handler still executing on the + // httptest server's goroutine. The assertion below then reads this counter + // with no happens-before edge to that write — a genuine data race, and one + // the detector catches intermittently (it fired on a full-suite -race run + // but not on a package-scoped one). + var fetchCount atomic.Int64 srv := newTestForge(t, map[string]http.HandlerFunc{ "Register": func(w http.ResponseWriter, _ *http.Request) { jsonResponse(w, `{"runner":{"id":1,"uuid":"u","name":"n","token":"t"}}`) @@ -268,7 +274,7 @@ func TestPoll_ContextCancellation(t *testing.T) { jsonResponse(w, `{}`) }, "FetchTask": func(w http.ResponseWriter, _ *http.Request) { - fetchCount++ + fetchCount.Add(1) jsonResponse(w, `{"tasksVersion":0}`) }, }) @@ -290,7 +296,7 @@ func TestPoll_ContextCancellation(t *testing.T) { if !errors.Is(err, context.DeadlineExceeded) { t.Errorf("expected context.DeadlineExceeded, got %v", err) } - if fetchCount == 0 { + if fetchCount.Load() == 0 { t.Error("expected at least one FetchTask call") } }