Skip to content

c8d/list: Don't exclude non-container images#48399

Merged
vvoland merged 1 commit intomoby:masterfrom
vvoland:c8d-list-nonimage
Aug 29, 2024
Merged

c8d/list: Don't exclude non-container images#48399
vvoland merged 1 commit intomoby:masterfrom
vvoland:c8d-list-nonimage

Conversation

@vvoland
Copy link
Contributor

@vvoland vvoland commented Aug 29, 2024

Before this, the image list would not show images that are not a valid container image, but could be a valid artifact.

While they're not directly usable by docker, we should still show them so the user can still discover them and at least be able to delete them.

- What I did

- How I did it

- How to verify it
TestImageList

and

$ docker pull tianon/test:text-plain
$ docker images

- Description for the changelog

containerd image store: Fix non-container images being hidden in the `docker images` output

- A picture of a cute animal (not mandatory but encouraged)

Before this, the image list would not show images that are not a valid
container image, but could be a valid artifact.

While they're not directly usable by docker, we should still show them
so the user can still discover them and at least be able to delete them.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland added area/images Image Distribution kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration process/cherry-pick/27.2 labels Aug 29, 2024
@vvoland vvoland added this to the 28.0.0 milestone Aug 29, 2024
@vvoland vvoland self-assigned this Aug 29, 2024
@vvoland vvoland requested review from laurazard and thaJeztah August 29, 2024 12:26
Comment on lines -390 to -393
// TODO we should probably show *something* for images we've pulled
// but are 100% shallow or an empty manifest list/index
// ("tianon/scratch:index" is an empty example image index and
// "tianon/scratch:list" is an empty example manifest list)
Copy link
Member

Choose a reason for hiding this comment

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

LOL did I write this?? 😂 ❤️

Copy link
Member

Choose a reason for hiding this comment

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

At least I'm 100% consistently on-brand, right? 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep! 😄

Copy link
Member

@tianon tianon left a comment

Choose a reason for hiding this comment

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

This seems really sane to me, especially in the containerd integration -- these are valid OCI objects, so being able to successfully pull them and list them, even though they'll fail to run, seems like sane behavior IMO, especially if we someday do something interesting with #44369. 👀

Copy link
Member

@laurazard laurazard left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/images Image Distribution containerd-integration Issues and PRs related to containerd integration kind/bugfix PR's that fix bugs process/cherry-picked

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants