cache backend for github actions by tonistiigi · Pull Request #1974 · moby/buildkit (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation16 Commits3 Checks0 Files changed

Conversation

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

[ Show hidden characters]({{ revealButtonHref }})

closes #1947

WIP version for early testing. type=gha, optional scope can be specified to namespace the cache.

TODO:

only tested some basic cases.
exporting always overwrites cache for the current writable scope. The actual version should merge all caches together and periodically clean up the branches that point to layers that github has pruned.

crazy-max, ralph, ovr, geekdave, davetapley, peimanja, bryantbiggs, dezren39, Juice10, TheCatLady, and 30 more reacted with heart emoji Vlaaaaaaad, 0x2b3bfa0, hertzg, jim80net, GaryGSC, tjovicic, and johanneswuerbach reacted with rocket emoji

Many thanks for this feature @tonistiigi

I don't see it but I assume cache exporter already prune/gc remote blobs?

I don't see it but I assume cache exporter already prune/gc remote blobs?

There is no API to remove cache blobs in Github afaics. I think Github just deletes objects whenever usage goes over the cap. I'm not sure of the exact order of how blobs are deleted (see #1947 (comment)) . Exporting will push a new manifest and currently, if it doesn't refer to the old blob anymore it will remain dangling until github cleans it up. I hope to change that so that there are periodical checks to determine what objects still exists and the old cache that still points to existing objects is merged with the new one.

Niek mentioned this pull request

Apr 12, 2021

@tonistiigi in regards to

I think Github just deletes objects whenever usage goes over the cap. I'm not sure of the exact order of how blobs are deleted (see #1947 (comment))

We delete the blobs in order of least recently accessed as they go over the cap and any blob that has not been access in 7 days will be deleted. We should have an API in the near future to delete cache blobs as well.

)

func init() {
actionscache.Log = log.Printf

Choose a reason for hiding this comment

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

nit: why not logrus

if ce == nil {
return nil, errors.Errorf("blob not found")
}
rac := ce.Download(context.TODO())

Choose a reason for hiding this comment

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

Can't we pass the ctx here?

Choose a reason for hiding this comment

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

This context is long-running. Canceling it (allowed after this function returns) would cancel the ReaderAt that it returns. It should use the tracing context though, or the library should take a HTTP client so that tracing transport can be used.

Choose a reason for hiding this comment

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

Tested on my side and LGTM:

  time="2021-07-06T22:44:49Z" level=debug msg="load cache https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/cache?keys=buildkit-blob-1-sha256%3A5843afab387455b37944e709ee8c78d7520df80f8d01cf7f861aae63beeddb6b&version=693bb7016429d80366022f036f84856888c9f13e00145f5f6f4dce303a38d6f2"
  time="2021-07-06T22:44:49Z" level=debug msg="load cache https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/cache?keys=buildkit-blob-1-sha256%3Adc728e95acca44552fe2f7a2fba71b75c7b2a9a5572c42f2c6291edb4a11fb37&version=693bb7016429d80366022f036f84856888c9f13e00145f5f6f4dce303a38d6f2"
  time="2021-07-06T22:44:49Z" level=debug msg="load cache https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/cache?keys=buildkit-blob-1-sha256%3Af03631b5b56f414fd215d7b32bd09d35631f9e851c5809f8cf07edfb8e3aae44&version=693bb7016429d80366022f036f84856888c9f13e00145f5f6f4dce303a38d6f2"
  time="2021-07-06T22:44:49Z" level=debug msg="load cache https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/cache?keys=index-buildkit-1-49c8f7d5%23&version=693bb7016429d80366022f036f84856888c9f13e00145f5f6f4dce303a38d6f2"
  time="2021-07-06T22:44:49Z" level=debug msg="load cache https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/cache?keys=index-buildkit-1-49c8f7d5%23&version=693bb7016429d80366022f036f84856888c9f13e00145f5f6f4dce303a38d6f2"
  time="2021-07-06T22:44:49Z" level=debug msg="save cache req https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/caches body={\"key\":\"index-buildkit-1-49c8f7d5#6\",\"version\":\"693bb7016429d80366022f036f84856888c9f13e00145f5f6f4dce303a38d6f2\"}"
  time="2021-07-06T22:44:49Z" level=debug msg="save cache resp: {\"cacheId\":65}"
  time="2021-07-06T22:44:49Z" level=debug msg="upload cache chunk https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/caches/65, range 0-3049"
  time="2021-07-06T22:44:49Z" level=debug msg="commit cache https://artifactcache.actions.githubusercontent.com/iC2459jfgdOW9d7EC3lNVmpsYRV1wmgoJboSD3iyUzxb76QZip/_apis/artifactcache/caches/65, size 3050"
  time="2021-07-06T22:44:49Z" level=debug msg="session finished: <nil>"

Would need to update the README with a new section about this cache exporter.

🤦 I forgot to merge this for v0.9-rc1. Writing changelog and wondered why my tool didn't detect this.

The plan was to merge this even though it is not fully complete to ease testing and gathering feedback. I guess this means rc2.

@tonistiigi That's fine. It will be ok in rc2. In the meantime can you rebase and merge the PR to be able to test from public repos? I already have a workflow ready.

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

crazy-max merged commit 0304821 into moby:master yesterday

Woo hoo!

shyouhei added a commit to ruby/ruby-ci-image that referenced this pull request

Sep 29, 2022