-
Notifications
You must be signed in to change notification settings - Fork 405
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
Introduce KOCACHE #269
Introduce KOCACHE #269
Conversation
@mattmoor boilerplate requires |
@jonjohnsonjr boilerplate requests whatever is in |
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.
If we were to start making tarball.LayerFromOpener
optionally return an estargz layer, what about this would need to change?
pkg/build/gobuild.go
Outdated
hasher := md5.New() //nolint: gosec // No strong cryptography needed. | ||
hasher.Write([]byte(strings.Join(args, " ") + " " + strings.Join(defaultEnv, " "))) | ||
|
||
tmpDir = filepath.Join(os.TempDir(), "ko", ip, hashInputs(args, defaultEnv)) |
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.
should you factor in the working directory? What if I have multiple ko
workspaces? 🤔
pkg/build/gobuild.go
Outdated
if os.Getenv("KO_STABLE_OUTPUT") == "" { | ||
os.RemoveAll(tmpDir) | ||
} |
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.
Why cache if it is an error?
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.
Because we don't want to delete the other metadata in that directory, even though the out
binary is bad (or never got built). I think I want to rework this directory structure a bit before we merge it, e.g. maybe buildid-to-diffid
and diffid-to-descriptor
are global and have the same layout as go env GOCACHE
.
This Pull Request is stale because it has been open for 90 days with |
/remove-lifecycle stale I still want this. |
I don't think we have a bot that actually obeys us. I've just been manually changing the labels. |
723be95
to
3f433e7
Compare
Codecov Report
@@ Coverage Diff @@
## main #269 +/- ##
==========================================
- Coverage 50.60% 47.88% -2.73%
==========================================
Files 41 41
Lines 2051 2170 +119
==========================================
+ Hits 1038 1039 +1
- Misses 838 955 +117
- Partials 175 176 +1
Continue to review full report at Codecov.
|
f6da708
to
4ea7b57
Compare
Cache binaries under $KOCACHE/<import path>/<platform> Cache metdata mapping buildid to diffid and diffid to descriptor.
4ea7b57
to
021fa01
Compare
Want to take a look and fiddle with it? |
I'm tempted to namespace stuff a little bit like Then we could cache base image metadata under |
a4bb483
to
42179d5
Compare
This makes things a little cleaner by having a single place that calls buildLayer and passing a thunk down into the cache logic to call that on a cache miss. Also, remove the debug logging to make the code easier to follow (if you need to recompile anyway, it's easy enough to add log lines).
42179d5
to
8098926
Compare
f4f5608
to
9a9ece2
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.
🚀
Start of #264
Currently, use this by setting
KOCACHE=/tmp/ko
(or any directory you want). We can default to doing this in the future.We will cache binaries and metdata under
$KOCACHE/bin/<import path>/<platform>
:In
buildid-to-diffid
, we maintain a map from the binary's buildid to the diffid we produced from it:You can determine the buildid from the binary:
Which allows us to skip computing the diffid (tar + sha256) at all.
In
diffid-to-descriptor
, we maintain a map from diffid to a layer descriptor:... which is enough info to HEAD the blob in-registry without having to actually re-gzip it.
This is a separate structure from
buildid-to-diffid
so that we can decouplediffid-to-descriptor
from go builds and reuse it for the kodata layers (or really anything -- docker does very similar stuff here): #262@mattmoor @imjasonh
Numbers
Warm + cache miss (and fill) + registry miss:
Warm + cache miss (and fill) + registry hits:
Warm + cache hits + registry miss:
Warm + cache hits + registry hits:
What more can we do? Well... about half of this time is actually spent fetching the base image. If we pull that down to a local registry:
Starting towards achieving that in #525