-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 backend for github actions #1974
Conversation
7d01995
to
11c977f
Compare
Many thanks for this feature @tonistiigi 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. |
11c977f
to
7e17c13
Compare
7e17c13
to
c87a0ae
Compare
c87a0ae
to
322a69b
Compare
@tonistiigi in regards to
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. |
322a69b
to
2554266
Compare
cache/remotecache/gha/gha.go
Outdated
) | ||
|
||
func init() { | ||
actionscache.Log = log.Printf |
There was a problem hiding this comment.
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()) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
2554266
to
2235ae6
Compare
needs rebase |
2235ae6
to
cb22431
Compare
There was a problem hiding this comment.
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 <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
Signed-off-by: Tonis Tiigi <[email protected]>
cb22431
to
b1c80cf
Compare
Woo hoo! |
moby/buildkit#1974 is merged. We no longer need extra manoeuvre to cache builds. See also https://github.com/docker/build-push-action/blob/master/docs/advanced/cache.md#github-cache
closes #1947
WIP version for early testing.
type=gha
, optionalscope
can be specified to namespace the cache.TODO: