Skip to content
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

Use containerd's label package #1131

Merged
merged 1 commit into from
Mar 2, 2023
Merged

Conversation

ktock
Copy link
Member

@ktock ktock commented Mar 1, 2023

This fixes cri-related code to use containerd's exported label handling logic.
This PR also adds a helper function to append extra labels used for advanced functionalities like IPFS.

lazy pulling supported but no IPFS support:

ctdsnapshotters.AppendInfoHandlerWrapper(ref)

lazy pulling supported and IPFS supported:

source.AppendExtraLabelsHandler(prefetchSize, ctdsnapshotters.AppendInfoHandlerWrapper(ref))

Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
// targetImageURLsLabelPrefix is a label prefix which constructs a map from the layer index to
// urls of the layer descriptor. This isn't contained in the set of the labels passed from CRI plugin but
// some clients (e.g. nerdctl) passes this for preserving url field in the OCI descriptor.
targetImageURLsLabelPrefix = "containerd.io/snapshot/remote/urls."
Copy link
Member

Choose a reason for hiding this comment

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

Should nerdctl use the CRI labels too?

Ok to preserve old labels for compatibility sake though

Copy link
Member Author

Choose a reason for hiding this comment

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

nerdctl shoudl use containerd CRI labels (targetRefLabel+targetLayerDigestLabel+targetImageLayersLabel) + extra labels (targetImageURLsLabelPrefix+targetURLsLabel; they aren't defined/used by CRI). All of them should be needed for nerdctl to perform registry-based lazy pulling as well as IPFS-based lazy pulling.

@AkihiroSuda AkihiroSuda merged commit d944efd into containerd:main Mar 2, 2023
@ktock ktock deleted the ctdlabels branch March 3, 2023 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants