Skip to content

perf(ci): use stable test ldflags for Go test cache#19394

Closed
davdhacs wants to merge 1 commit intomasterfrom
davdhacs/zversion-codegen
Closed

perf(ci): use stable test ldflags for Go test cache#19394
davdhacs wants to merge 1 commit intomasterfrom
davdhacs/zversion-codegen

Conversation

@davdhacs
Copy link
Contributor

@davdhacs davdhacs commented Mar 12, 2026 •
edited
Loading

Description

Enable Go build cache reuse across commits by stabilizing the -X ldflags
that Go includes in build ActionIDs.

The problem

scripts/go-tool.sh injects per-commit version values via -X ldflags:

  • -X MainVersion=4.11.x-317-gabcdef (changes every commit — includes commit count + SHA)
  • -X GitShortSha=abcdef (changes every commit)

Go includes -X ldflags in the link ActionID (exec.go:1404: linkflags %q).
When ldflags change, Go computes a new ActionID for every package that gets
linked, causing cache misses for:

  • Build binaries: the linked binary cache key changes (though Go patches
    -X values post-link, so the build cost is minimal ~1-2s)
  • Test binaries: the test binary ID changes, causing all test results
    to miss the cache     and re-run from scratch every commit

The fix

Split ldflags into two sets:

  • Build ldflags: full per-commit version (unchanged from today)
  • Test ldflags: stable base version tag (e.g. 4.11.x from git describe --tags --abbrev=0),
    GitShortSha skipped

This preserves the correct version string in all built binaries while giving
test binaries a stable ActionID across commits.

-buildvcs=false

Go 1.18 (adopted September 2022) silently enabled buildvcs=auto, which
stamps vcs.revision, vcs.time, vcs.modified into every binary via the
linker. No code in the repository reads this data — version stamping is done
independently via the -X ldflags from status.sh. The buildvcs data was
never intentionally enabled; it appeared as an unnoticed side effect of the
Go 1.18 upgrade. The only time it was previously noticed was July 2024
(PR #11935) when it broke a scanner CI workflow.

Adding -buildvcs=false removes this unused overhead.

What changes

One file: scripts/go-tool.sh

  • Split ldflags into build_ldflags (full version) and test_ldflags (stable base tag)
  • Tests get -X MainVersion=4.11.x instead of -X MainVersion=4.11.x-317-gabcdef
  • Tests skip -X GitShortSha (not needed, changes per commit)
  • -buildvcs=false added to both build and test

What is preserved

  • //XDef: annotations — unchanged
  • status.sh — unchanged
  • -X ldflags for builds — unchanged (full git describe version in binaries)
  • version_data.go — unchanged
  • Operator Dockerfile — unchanged
  • Built binary version strings — identical to today

Build cache impact

Builds already benefit from GOCACHE for compilation (.a files are keyed by
source hashes, not ldflags). The -X ldflags only affect linking, and Go
patches -X values post-link so there is minimal build cost (~1-2s).

The main benefit of this PR is enabling test result caching when combined
with PR #19395 (file mtime stabilization). Together they achieve:

Job Master warm Combined warm Speedup
go unit tests 35m ~8m 4.5x
go-postgres 31m ~36s 52x
sensor-integration 26m ~2.5m 11x

A combined test branch (davdhacs/test-cache-combined) with only these two
changes is being tested to provide clean timing data.

Partially generated by AI.

@openshift-ci
Copy link

openshift-ci bot commented Mar 12, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@davdhacs davdhacs mentioned this pull request Mar 12, 2026
Draft
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Mar 12, 2026 •
edited
Loading

Images are ready for the commit at 14b6d2b.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-311-g14b6d2b3e7.

@codecov
Copy link

codecov bot commented Mar 12, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.69%. Comparing base (6ac212c) to head (14b6d2b).
⚠️ Report is 8 commits behind head on master.

Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #19394   +/-   ##
=======================================
  Coverage   49.68%   49.69%           
=======================================
  Files        2700     2700           
  Lines      203278   203297   +19     
=======================================
+ Hits       100999   101024   +25     
+ Misses      94753    94749    -4     
+ Partials     7526     7524    -2
Flag Coverage Δ
go-unit-tests 49.69% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@davdhacs davdhacs force-pushed the davdhacs/zversion-codegen branch from 0dfd5e7 to 830929b Compare March 12, 2026 19:09
@davdhacs davdhacs changed the title chore(ci): replace -X ldflags with generated zversion.go perf(ci): use stable test ldflags for Go test cache Mar 12, 2026
@davdhacs @claude
perf(ci): use stable test ldflags for Go test cache
14b6d2b 
Go includes -X ldflags in the link ActionID (exec.go:1404), so
per-commit values (MainVersion, GitShortSha) change the test binary
ID, causing test cache misses on every commit.

Fix: for go test, replace per-commit MainVersion with the stable
base version tag (from git describe --tags --abbrev=0) and skip
GitShortSha. Builds continue using full per-commit ldflags unchanged.

Also adds -buildvcs=false which removes unused VCS stamping that
Go 1.18+ silently enabled (no code reads debug.ReadBuildInfo).

This is the minimal change to enable test caching — just go-tool.sh.
The existing XDef/status.sh mechanism is preserved for builds.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@davdhacs davdhacs force-pushed the davdhacs/zversion-codegen branch from 830929b to 14b6d2b Compare March 12, 2026 22:31
@davdhacs davdhacs mentioned this pull request Mar 12, 2026
Draft
@davdhacs
Copy link
Contributor Author

davdhacs commented Mar 13, 2026 •
edited
Loading

Closing in favor of a VERSION file or generated zversion.go approach that also enables Go's link cache for builds.

This PR's simpler approach (conditional test ldflags, keep XDef for builds) successfully enables test caching (92% hit rate, 3-9x test speedup), but leaves build speed on the table:

Link cache finding: Go caches linked binaries by ActionID, which includes -X ldflags. When -X MainVersion changes per commit, Go re-links all binaries every time. With stable ldflags (no per-commit -X), the link cache hits and builds are 2.2x faster (confirmed locally: 2.4s vs 5.2s for 8 binaries, 5 runs each).

@davdhacs davdhacs closed this Mar 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

ai-assisted area/ci area/operator do-not-merge/work-in-progress

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@davdhacs @rhacs-bot