Skip to content

fix(ci): save all Go caches on master#19069

Merged
davdhacs merged 2 commits intomasterfrom
davdhacs/gha-cache-save-all
Feb 19, 2026
Merged

fix(ci): save all Go caches on master#19069
davdhacs merged 2 commits intomasterfrom
davdhacs/gha-cache-save-all

Conversation

@davdhacs
Copy link
Copy Markdown
Contributor

@davdhacs davdhacs commented Feb 18, 2026

With increased cache limits, there's no reason to skip saves for test and style jobs. Re-enable saves on master for all jobs (~19 GB/week, ~76-95 GB over 30 days).

Partially generated by AI.

Testing and quality

How I validated my change

change me!

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue, and left some high level feedback:

  • The new generic go-build-v1- restore key removes the GOARCH/TAG scoping, which may cause incompatible cache reuse across Go versions or other tag changes; consider including at least the Go version (or equivalent) in this fallback key to avoid subtle build issues after upgrades.
  • With the removal of with: save: false in the workflows, the behavior now depends on the default for inputs.save in cache-go-dependencies; double-check that the default matches the intended "save on master only" semantics for all callers to avoid unexpectedly writing caches on non-default branches.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new generic `go-build-v1-` restore key removes the `GOARCH`/`TAG` scoping, which may cause incompatible cache reuse across Go versions or other tag changes; consider including at least the Go version (or equivalent) in this fallback key to avoid subtle build issues after upgrades.
- With the removal of `with: save: false` in the workflows, the behavior now depends on the default for `inputs.save` in `cache-go-dependencies`; double-check that the default matches the intended "save on master only" semantics for all callers to avoid unexpectedly writing caches on non-default branches.

## Individual Comments

### Comment 1
<location> `.github/actions/cache-go-dependencies/action.yaml:46` </location>
<code_context>
-        restore-keys: go-build-v1-${{ github.job }}-${{ steps.cache-paths.outputs.GOARCH }}-
+        restore-keys: |
+          go-build-v1-${{ github.job }}-${{ steps.cache-paths.outputs.GOARCH }}-
+          go-build-v1-

     - name: Cache Go Build (restore)
</code_context>

<issue_to_address>
**suggestion (bug_risk):** Consider whether the generic `go-build-v1-` restore key could lead to suboptimal or incorrect cache reuse across jobs/architectures.

This broader restore key allows caches from other jobs, architectures, or Go versions to be reused, which can both reduce cache effectiveness (due to mismatches) and introduce subtle, hard-to-debug issues from stale artifacts. Consider scoping the fallback key by at least GOARCH (e.g. `go-build-v1-${{ steps.cache-paths.outputs.GOARCH }}-`) and possibly Go version so fallback restores stay within compatible builds.

Suggested implementation:

```
        key: go-build-v1-${{ github.job }}-${{ steps.cache-paths.outputs.GOARCH }}-${{ steps.cache-paths.outputs.TAG }}
        restore-keys: |
          go-build-v1-${{ github.job }}-${{ steps.cache-paths.outputs.GOARCH }}-
          go-build-v1-${{ steps.cache-paths.outputs.GOARCH }}-

```

```
        key: go-build-v1-${{ github.job }}-${{ steps.cache-paths.outputs.GOARCH }}-${{ steps.cache-paths.outputs.TAG }}
        restore-keys: |
          go-build-v1-${{ github.job }}-${{ steps.cache-paths.outputs.GOARCH }}-
          go-build-v1-${{ steps.cache-paths.outputs.GOARCH }}-

```

If you want to further scope by Go version, you can:
1. Add an output like `GOVERSION` in `cache-paths` (or another prior step), and
2. Extend the fallback key to include it, e.g. `go-build-v1-${{ steps.cache-paths.outputs.GOARCH }}-${{ steps.cache-paths.outputs.GOVERSION }}-`.
Ensure that `GOVERSION` is stable and accurately reflects the toolchain version used for the build.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

With increased cache limits, there's no reason to skip
saves for test and style jobs. Re-enable saves on master for all
jobs (~19 GB/week, ~76-95 GB over 30 days).

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@davdhacs davdhacs force-pushed the davdhacs/gha-cache-save-all branch from 2a9ce1f to bbbb67e Compare February 18, 2026 05:45
@davdhacs
Copy link
Copy Markdown
Contributor Author

@janisz now that we have more gha cache space, we can re-enable the cache saves I had disabled.

I still think we should keep the gha caches only saved on the master branch since branch jobs cannot load caches saved by other non-master branches and the keys must be unique (they are not overwritten, and so they aren't replaced by master saves until expired/purged).

@rhacs-bot
Copy link
Copy Markdown
Contributor

rhacs-bot commented Feb 18, 2026

Images are ready for the commit at 45ea6ab.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-135-g45ea6ab9a8.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.51%. Comparing base (843afe3) to head (45ea6ab).
⚠️ Report is 12 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #19069      +/-   ##
==========================================
- Coverage   49.52%   49.51%   -0.01%     
==========================================
  Files        2670     2670              
  Lines      201549   201549              
==========================================
- Hits        99811    99803       -8     
- Misses      94287    94293       +6     
- Partials     7451     7453       +2     
Flag Coverage Δ
go-unit-tests 49.51% <ø> (-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 changed the title fix(ci): save all Go caches on master, add cross-job restore fallback fix(ci): save all Go caches on master Feb 18, 2026
@davdhacs davdhacs enabled auto-merge (squash) February 18, 2026 15:05
@davdhacs
Copy link
Copy Markdown
Contributor Author

and the keys must be unique (they are not overwritten, and so they aren't replaced by master saves until expired/purged).

correction: "the uniqueness constraint is key + ref, not just key"
I was wrong. I had misunderstood and thought the keys were unique for the entire cache. However, in usage (now that we have enough space to see what is being saved) I see that we get the same keys saved from different branches.

@davdhacs davdhacs merged commit 74c8e05 into master Feb 19, 2026
100 checks passed
@davdhacs davdhacs deleted the davdhacs/gha-cache-save-all branch February 19, 2026 07:29
dashrews78 added a commit that referenced this pull request Feb 19, 2026
@dashrews78 dashrews78 mentioned this pull request Feb 19, 2026
9 tasks
davdhacs added a commit that referenced this pull request Feb 19, 2026
…files

PR #19069 removed all save: false, but check-generated-files
modifies the working tree (regenerates protos, removes mocks)
which breaks hashFiles('**/go.sum') in the GOMODCACHE post-step,
causing cache save failures. Reverted in #19108.

Re-enable saves for all other jobs (unit-tests, style-check,
scanner-db-integration-tests) while keeping save: false only for
check-generated-files where the hashFiles failure occurs.

Partially generated by AI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants