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

Fix image output #357

Merged
merged 1 commit into from Sep 28, 2021
Merged

Conversation

fahedouch
Copy link
Member

avoid creating digest images for each manifest in the index.

fixing #177

Comment on lines 69 to 68
imgs, err := client.Import(ctx, in, containerd.WithDigestRef(archive.DigestTranslator(sn)), containerd.WithAllPlatforms(clicontext.Bool("all-platforms")))
imgs, err := client.Import(ctx, in, containerd.WithAllPlatforms(clicontext.Bool("all-platforms")))
Copy link
Member

Choose a reason for hiding this comment

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

This change makes all non-tagged images loaded by nerdctl load get GC-ed immediately.

Example:

nerdctl pull busybox:latest
nerdctl save busybox:latest > /tmp/busybox.tar
mkdir /tmp/busybox
tar -C /tmp/busybox -xf /tmp/busybox.tar
jq 'del(.manifests[].annotations)' /tmp/busybox/index.json > /tmp/busybox/index.json.back
mv /tmp/busybox/index.json.back /tmp/busybox/index.json
jq 'del(.[].RepoTags)' /tmp/busybox/manifest.json > /tmp/busybox/manifest.json.back
mv /tmp/busybox/manifest.json.back /tmp/busybox/manifest.json
(...clear containerd cache...)
tar -C /tmp/busybox/ -c . | nerdctl load
(no image loaded here)

@ktock
Copy link
Member

ktock commented Sep 14, 2021

I believe containerd/containerd#5997 will be needed for solve it. PTAL.

@AkihiroSuda AkihiroSuda marked this pull request as draft September 14, 2021 17:42
@AkihiroSuda
Copy link
Member

containerd/containerd#5997

Merged

@fahedouch
Copy link
Member Author

thank you @AkihiroSuda , waiting for next containerd release to update the PR.

@ktock
Copy link
Member

ktock commented Sep 22, 2021

waiting for next containerd release to update the PR.

@fahedouch Do we need to wait for the next release? That patch changes only client library so we can work on it with the main branch of containerd.

@AkihiroSuda
Copy link
Member

@ktock Would you be interested in carrying this PR?

@fahedouch
Copy link
Member Author

@AkihiroSuda @ktock
if you allow, I can take care of it

@AkihiroSuda
Copy link
Member

Thanks @fahedouch , yes, please

@ktock
Copy link
Member

ktock commented Sep 27, 2021

Thanks @fahedouch !

@fahedouch fahedouch force-pushed the fix-image-output branch 2 times, most recently from ef07255 to 6619357 Compare September 27, 2021 20:15
Signed-off-by: fahed dorgaa <fahed.dorgaa@gmail.com>
@fahedouch fahedouch marked this pull request as ready for review September 27, 2021 20:16
@fahedouch
Copy link
Member Author

@ktock @AkihiroSuda is it good ?

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Thanks

@AkihiroSuda AkihiroSuda added this to the v0.12.0 milestone Sep 28, 2021
@AkihiroSuda AkihiroSuda merged commit 180053d into containerd:master Sep 28, 2021
@AkihiroSuda AkihiroSuda added enhancement New feature or request and removed kind/external labels Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants