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

Add a dangling image prefix when tag name is not specified. #1461

Merged
merged 1 commit into from
Nov 4, 2022

Conversation

manugupt1
Copy link
Contributor

Fixes: #1398

@manugupt1 manugupt1 force-pushed the noneimage branch 3 times, most recently from fd0e942 to f1a4107 Compare October 24, 2022 03:45
@manugupt1 manugupt1 marked this pull request as ready for review October 24, 2022 03:47
@manugupt1
Copy link
Contributor Author

@AkihiroSuda can I get a review on this please.
Thanks!

@ktock
Copy link
Member

ktock commented Oct 28, 2022

@manugupt1 Doesn't moby/buildkit#3212 affect this implementation?

@manugupt1
Copy link
Contributor Author

manugupt1 commented Oct 28, 2022

@manugupt1 Doesn't moby/buildkit#3212 affect this implementation?

It affects only scratch images or images with no layer. moby/buildkit#3212 (comment) ; unpacking should be skipped for when there are no layers I believe is the right solution in buildkit. Also, this bug comes only when images are built with no tag and has no layers.

There is a bug, however, when buildkit fixes that bug; we won't have to fix it on our end with this change later.

Tag present Image with no layers Works
Yes Yes Yes (worked earlier)
Yes No No
No Yes No (because of the bug)
No No Yes (did not work earlier, fixed now)

@manugupt1 manugupt1 force-pushed the noneimage branch 4 times, most recently from c24eeb1 to 6f81b76 Compare November 3, 2022 06:14
@manugupt1 manugupt1 requested review from fahedouch and removed request for ktock November 3, 2022 06:14
@manugupt1
Copy link
Contributor Author

@ktock PTAL, this and the PR (moby/buildkit#3251) in buildkit should fix all the concerns so far. LMK if you have questions.


func TestBuildNoTag(t *testing.T) {
testutil.RequiresBuild(t)
testutil.DockerIncompatible(t)
Copy link
Member

Choose a reason for hiding this comment

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

Why incompatible?

Fixes: containerd#1398
Signed-off-by: Manu Gupta <manugupt1@gmail.com>
@AkihiroSuda AkihiroSuda added this to the v1.0.1 milestone Nov 4, 2022
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 merged commit b71443b into containerd:master Nov 4, 2022
@manugupt1 manugupt1 deleted the noneimage branch November 4, 2022 14:35
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.

nerdctl built images, without a tag don't show up when running nerdctl images command
4 participants