-
Notifications
You must be signed in to change notification settings - Fork 663
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
Add eStargz specification to OCI v1 (support lazy pulling) #877
base: main
Are you sure you want to change the base?
Conversation
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.
LGTM (IANAM)
@cyphar PTAL
annotations.md
Outdated
@@ -49,6 +49,7 @@ This specification defines the following annotation keys, intended for but not l | |||
* This SHOULD be the immediate image sharing zero-indexed layers with the image, such as from a Dockerfile `FROM` statement. | |||
* This SHOULD NOT reference any other images used to generate the contents of the image (e.g., multi-stage Dockerfile builds). | |||
* If the `image.base.name` annotation is specified, the `image.base.digest` annotation SHOULD be the digest of the manifest referenced by the `image.ref.name` annotation. | |||
* **org.opencontainers.image.toc.digest** Digest of the TOC JSON of [eStargz](estargz.md) layer. |
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.
The concept of TOC digest may be helpful for other usage cases, so is it ok to relax the support not bound to estargz? For example, it may help to enable e2e integrity verification for Nydus(https://github.com/dragonflyoss/image-service).
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.
Are there other formats which use TOC?
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.
The nydus project extracts all filesystem metadata to a dedicated blob in binary mode, which solves the same role as TOC for estargz. So hope we could reuse the same annotation too.
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.
Thanks for the suggestion. But if the "filesystem metadata" format is different from the TOC, shouldn't we have a separated key? Or, in following-up PRs, we can define "filesystem metadata" you mention then relax the scope of this key.
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 the annotation is specific to eStargz, should the key be org.opencontainers.image.estargz.digest
?
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.
@sudo-bmitch Thank you for the suggestion. Modified the key to org.opencontainers.image.estargz.toc.digest
. I left toc
here for futural extensibility (e.g. we may want other toc-related annotations in the future like estargz.toc.size
or something)
LGTM on the idea of adding this, but I haven't given this a detailed read. I think we need to have the maintainers read this one in detail. We may also need a description of how to fetch these in the distribution-spec. |
Thank you for the comments.
Lazy pulling depends on HTTP range request which is already described in the distribution-spec so we don't need changes on the distribution-spec but can add notes that the range request is used by lazy pulling. |
Needs rebase to make CI green |
Rebased. |
Signed-off-by: Kohei Tokunaga <[email protected]>
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.
While this is really interesting, I'm a (soft) NACK on including it in the image-spec itself -- I think it's probably more productive to maintain this outside the spec, especially as there are now multiple (somewhat incompatible) implementations of similar ideas.
Thank you for your comments.
Yes, there are several lazy pulling implementations in the community. So it would be great if OCI spec officially describes the format for lazy pulling so that runtimes and builders can adopt it and maintain the implementation while keeping interoperability among them. |
Resolves #815
This PR adds support of lazy pulling to OCI Image Spec v1 by standardizing eStargz image format as an optional extension.
No need to introduce a new layer media type, because eStargz is fully compatible with
application/vnd.oci.image.layer.v1.tar+gz
.eStargz has been adopted by tools in the community, including containerd(stargz-snapshotter), Podman, k3s, BuildKit, Kaniko, ko, buildpacks.io, Harbor(acceleration-service). Please refer to containerd/stargz-snapshotter#258 for the complete list.
The following is the summary of this PR:
estargz.md
) from stargz-snapshotter repo to this repo.estargz.md
in the existing documents.org.opencontainers.image.toc.digest
defined inestargz.md
tospecs-go
.