buildkit icon indicating copy to clipboard operation
buildkit copied to clipboard

OCI layout context improvements

Open jedevc opened this issue 3 years ago • 6 comments

Follow up to #2827 with:

  • Fix for consistently occurring 5-15s timeouts (depending on caching) when resolving OCI layout content (split out into #3122)
  • Allow using tags instead of only digests (WIP)
  • Refactoring for reference parsing to rely on containerd's and docker distribution's reference.Parse functions, instead of manual parsing wherever possible
    • One of these changes breaks API compatibility between the front-end and buildkit! (by prefixing the storeID hostname on the frontend, and storing it in LLB, instead of splitting that logic across the backend and the frontend). However, since we haven't released OCI sources in buildkit yet, I think this should be fine, but I've split it out into a separate commit so we can easily drop it if necessary. I don't this should affect the buildx release we've recently made.

CC @deitch

jedevc avatar Sep 16 '22 15:09 jedevc

Fix for consistently occurring 5-15s timeouts (depending on caching) when resolving OCI layout content

What was the source of that?

deitch avatar Sep 18 '22 09:09 deitch

since we haven't released OCI sources in buildkit yet, I think this should be fine, but I've split it out into a separate commit so we can easily drop it if necessary

It is in buildx, so we would need to update that too.

deitch avatar Sep 18 '22 09:09 deitch

May I make a suggestion? Split this into two PRs. The first to fix the timeout/delay issue and simplify the reference parsing, then a second for the tags. That way you can get the first one in quickly, and we can spend time on the tags discussion.

deitch avatar Sep 20 '22 09:09 deitch

FWIW, I think we very much should support full tags. Anything that is halfway sane is good.

deitch avatar Sep 20 '22 09:09 deitch

Nice job on the split; probably should remove the stuff in #3122 from here, just to make it easier to review (although once that merges in and you rebase, it all should go away).

deitch avatar Sep 20 '22 12:09 deitch

Have pushed an update to handle full tags, e.g. of the form oci-layout://<store>/<image>[:tag][@digest] as well as the existing oci-layout://<store>[:tag][@digest]. All images with this full form are normalized before looking them up in index.json - so an OCI index containing an image library/alpine won't be recognized, it would need to be docker.io/library/alpine:latest.

Handling this as well as the previous :tag case is fiddly, so going to add some more tests to check this. Importantly, we should support all the different OCI outputs that docker itself produces, though I've relaxed it a little bit to try and find a "best case" if those fail - that said, I don't think we should spend much effort on covering anything outside of what we produce.

jedevc avatar Sep 22 '22 15:09 jedevc