Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cache: enable gha cache backend if cache service v2 detected #3001

Merged
merged 2 commits into from
Feb 18, 2025

Conversation

crazy-max
Copy link
Member

@crazy-max crazy-max commented Feb 17, 2025

After testing latest RC, gha cache backend is always disabled when cache service v2 is detected. We forgot to check if url_v2 is set in #2983.

Also fixes gha cache url handling to always set both url and url_v2 if possible to match fallback of github actions tookit.

@crazy-max
Copy link
Member Author

Hum it fails to solve in our e2e test with: https://github.com/docker/buildx/actions/runs/13375533199/job/37353936009?pr=3001#step:9:388

#6 [xx internal] load metadata for docker.io/library/alpine:latest
#6 DONE 0.7s
ERROR: target ld64-static-tgz: failed to solve: failed to configure gha cache exporter: url not set for github actions cache

I think that's because we only set url_v2 but the gha cache exporter requires url to be set: https://github.com/moby/buildkit/blob/de39e457a2c91179913c2eb303543f240ca793b3/cache/remotecache/gha/gha.go#L66-L69

So we need a fix on BuildKit as well.

@crazy-max
Copy link
Member Author

crazy-max commented Feb 17, 2025

Hum it fails to solve in our e2e test with: https://github.com/docker/buildx/actions/runs/13375533199/job/37353936009?pr=3001#step:9:388

In this case it fails because we are pushing an empty url attribute against previous BuildKit version (0.19.0) that doesn't support url_v2. In such case we should always set the url attribute.

Looking at github toolkit source code https://github.com/actions/toolkit/blob/2b08dc18f261b9fdd978b70279b85cbef81af8bc/packages/cache/src/internal/config.ts#L28-L33 they set either ACTIONS_CACHE_URL or ACTIONS_RESULTS_URL if found. Although I doubt ACTIONS_RESULTS_URL would work with v1 🤔

I think we should do the same in buildctl https://github.com/moby/buildkit/blob/de39e457a2c91179913c2eb303543f240ca793b3/cmd/buildctl/build/util.go#L28-L41

@crazy-max crazy-max marked this pull request as ready for review February 17, 2025 18:18
@crazy-max crazy-max added this to the v0.21.0 milestone Feb 17, 2025
@crazy-max
Copy link
Member Author

@crazy-max crazy-max merged commit b04d394 into docker:master Feb 18, 2025
127 checks passed
@crazy-max crazy-max deleted the fix-gha-cache-v2 branch February 18, 2025 20:24
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