ci(race): add gcc to the Linux CI image and fix the one race we own - #171
Merged
Conversation
Prerequisites for enabling -race in CI, split from the workflow change so this can land green: the published runner-ci-linux image has no C compiler, so a ci.yml step running -race would fail on its first run. - images/runner-ci-linux/Dockerfile: gcc + libc6-dev. Minimal on purpose; build-essential would add g++/make/dpkg-dev and ~250MB for nothing. - magefile.go: TestRace target, deliberately NOT in `mage ci` so `mage ci` stays cgo-free and runnable on Windows dev boxes. - pkg/forgerunner/runner_test.go: a real race we own. Run returns the instant the deadline fires, leaving a FetchTask handler mid-flight on the httptest goroutine, and the assertion read fetchCount with no happens-before edge. Now atomic.Int64. Intermittent: 26/30 pre-fix, 0/30 post-fix, and it did not fire on the first full-suite run. - pkg/dind: quarantine TestPushHandlerEndToEnd under -race only. The race is in containerd, not us - authHandler.doBearerAuth reads cached expirationTime under ah.Lock while the fetching goroutine writes token/refreshToken/err/expirationTime from a defer holding no lock (core/remotes/docker/authorizer.go:289 vs :303, v2.2.2). Reproduces 10/10. Still unfixed on containerd main, so a bump will not clear it. Race detector run in WSL (gcc 15.2, Go 1.26.2): pkg/upgrade 20/20 clean - including #167's new watchdog code, with all 53 tests confirmed executing rather than GOOS-skipped - and pkg/scheduler 20/20 clean. 20 runs is not proof of absence for a rare interleaving.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prerequisites for running
-racein CI. Split from the workflow change so this can land green — the publishedrunner-ci-linuximage has no C compiler, so aci.ymlstep running-racewould fail on its first run. Merging this triggersbuild-images.yml(it watchesimages/**), which publishes an image with gcc; the workflow step follows in a second PR.-racewas not running anywheremage ci→go test -tags containers_image_openpgp -count=1 ./...(magefile.go:31). No-race, and no other workflow runs unit tests at all.Worth knowing separately: the macOS and Windows CI jobs run zero tests — they only
mage build:*and check--version. So there was never a Windows/macOS race job to break, and this leaves them alone. (runner-ci-windowsis servercore with no C toolchain; the macOS image is asset-staging only.)CGO_ENABLED=0is set only on cross-compile build targets, never on the test path — so cgo was not being suppressed, the compiler was simply absent.The detector was actually run — WSL, gcc 15.2, Go 1.26.2
pkg/upgrade(#167's new watchdog)pkg/schedulerpkg/dindpkg/forgerunner-count=5./...(post-fix)All 53
pkg/upgradetests were confirmed to execute on Linux rather than GOOS-skip, so those 20 clean runs cover the code #167 just added. 20 runs is not proof of absence for a rare interleaving.The race we own — fixed here
pkg/forgerunner/runner_test.go,TestPoll_ContextCancellation:Runreturns the instant the deadline fires, leaving aFetchTaskhandler mid-flight on the httptest goroutine, and the assertion read that handler'sfetchCountwith no happens-before edge. Nowatomic.Int64.Intermittent — 26/30 pre-fix, 0/30 post-fix, and it did not fire on the first full-suite run. Precisely the flake that would otherwise have landed later as an unexplained red.
The race we don't own — quarantined, with the reason
authHandler.doBearerAuthreads a cachedexpirationTimeunderah.Lockwhile the goroutine owning the in-flight fetch writestoken/refreshToken/err/expirationTimefrom adeferholding no lock (core/remotes/docker/authorizer.go:289read vs:303write, v2.2.2).TestPushHandlerEndToEndtrips it 10/10.containerd
mainwas checked — still unfixed upstream, so a dependency bump will not clear it. Quarantined under-raceonly, via build-tagged files, and verified in both directions: SKIP under-race, still PASS in a normalmage ci. Worth filing upstream.Changes
images/runner-ci-linux/Dockerfile—gcc+libc6-dev,--no-install-recommends. Minimal deliberately;build-essentialwould pull g++/make/dpkg-dev and ~250 MB for nothing.magefile.go— aTestRacetarget, deliberately not inmage cisomage cistays cgo-free and runnable on Windows dev boxes.pkg/dind/racedetector_{race,norace}_test.go+ the skip inregistry_e2e_test.go.pkg/forgerunner/runner_test.go— the atomic fix.Not touched
Two pre-existing failures, both environmental and both failing without
-racetoo:pkg/runner/TestFindTarball_MatchesPlatform(dev checkout has a stale Windows runner zip and no Linux tarball; CI'sdownload.Allfixes it) andpkg/workflow/TestSniffGitInfo_CurrentRepo(fails inside a git worktree, passes in a normal clone).No image was built or pushed, and no workflow run was triggered.