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

podman manifest rm deletes referenced image instead of manifest list #11344

Closed
stu-gott opened this issue Aug 27, 2021 · 16 comments · Fixed by #11406
Closed

podman manifest rm deletes referenced image instead of manifest list #11344

stu-gott opened this issue Aug 27, 2021 · 16 comments · Fixed by #11406
Assignees
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@stu-gott
Copy link

stu-gott commented Aug 27, 2021

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

UPDATE: this description is out of date. clarification of the actual problem is in the first few comments.

I created a manifest, then attempted to delete it using podman rmi (this is likely wrong, but I couldn't figure out how to use podman manifest remove), and it deleted something I didn't expect it to.

Steps to reproduce the issue:

I pulled the amd64 and arch64 versions of fedora, then created a manifest and added them to it. here's the terminal transcript:

$ podman pull --arch=amd64 fedora:latest
Resolved "fedora" as an alias (/etc/containers/registries.conf.d/000-shortnames.conf)
Trying to pull registry.fedoraproject.org/fedora:latest...
Getting image source signatures
Copying blob ecfb9899f4ce done
Copying config 37e5619f4a done
Writing manifest to image destination
Storing signatures
37e5619f4a8ca9dbc4d6c0ae7890625674a10dbcfb76201399e2aaddb40da17d
$ podman tag 37e5619f4a8ca9dbc4d6c0ae7890625674a10dbcfb76201399e2aaddb40da17d localhost/fedora:latest.amd64
$ podman pull --arch=arm64 fedora:latest
Resolved "fedora" as an alias (/etc/containers/registries.conf.d/000-shortnames.conf)
Trying to pull registry.fedoraproject.org/fedora:latest...
Getting image source signatures
Copying blob 679663bf9a1d done
Copying config aace6e1f04 done
Writing manifest to image destination
Storing signatures
aace6e1f04bbdc9795884eb67c63514abd53b87f647630fa4263c07b931da5a5
$ podman tag aace6e1f04bbdc9795884eb67c63514abd53b87f647630fa4263c07b931da5a5 localhost/fedora:latest.arm64
$ podman manifest create localhost/fedora:latest
69c3faec36e64a3a5b521cd011893e9c16f445810ef888d7e69c158067fdfb3b
$ podman manifest add localhost/fedora:latest containers-storage:localhost/fedora:latest.amd64
69c3faec36e64a3a5b521cd011893e9c16f445810ef888d7e69c158067fdfb3b
$ podman manifest add localhost/fedora:latest containers-storage:localhost/fedora:latest.arm64
69c3faec36e64a3a5b521cd011893e9c16f445810ef888d7e69c158067fdfb3b
$ podman rmi localhost/fedora:latest
Untagged: localhost/fedora:latest.amd64

Deleted: 37e5619f4a8ca9dbc4d6c0ae7890625674a10dbcfb76201399e2aaddb40da17d
$ podman rmi localhost/fedora:latest
Untagged: localhost/fedora:latest
Deleted: 69c3faec36e64a3a5b521cd011893e9c16f445810ef888d7e69c158067fdfb3b

Describe the results you received:

The first invocation of podman rmi deleted the amd64 (my arch) version of the image referenced in the manifest.
The second invocation of podman rmi deleted the manifest itself.

Describe the results you expected:

podman rmi should have either deleted the manifest instead of the image, or should have refused to delete anything at all and asked the user if they really meant "podman manifest remove" (which I couldn't figure out, but that's not a bug lol).

Additional information you deem important (e.g. issue happens only occasionally):

Output of podman version:

podman version
Version:      3.3.0
API Version:  3.3.0
Go Version:   go1.16.6
Built:        Fri Aug 20 15:36:14 2021
OS/Arch:      linux/amd64

Output of podman info --debug:

Package info (e.g. output of rpm -q podman or apt list podman):

podman-3.3.0-1.fc34.x86_64

Have you tested with the latest version of Podman and have you checked the Podman Troubleshooting Guide? (https://github.com/containers/podman/blob/master/troubleshooting.md)

Yes

Additional environment details (AWS, VirtualBox, physical, etc.):

physical host.

@openshift-ci openshift-ci bot added the kind/bug Categorizes issue or PR as related to a bug. label Aug 27, 2021
@vrothberg
Copy link
Member

Thanks for reaching out, @stu-gott!

I think that podman rmi $image behaves correctly: if $image is a manifest list and if there's a local instance (member) of the manifest list that matches the local platform, then this instance will be selected and removed.

However, it seems like podman manifest rm behaves the same but should. manifest rm should remove the manifest list, just as described in the help text and the man page.

@flouthoc, since you're working in this area at the moment, do you want to tackle this bug? We need to make sure to add regression tests to nail down the behavior.

@flouthoc
Copy link
Collaborator

flouthoc commented Aug 30, 2021

@vrothberg Sure can look at this. This issue should be transferred to buildah right ?

@vrothberg
Copy link
Member

vrothberg commented Aug 30, 2021

@vrothberg Sure can look at this. This issue should be transferred to buildah right ?

No, podman and buildah are both using libimage but they behave in the same way, so I think we need to fix it in buildah as well. Note that manifest *remove* is meant to remove specific instances from the manifest list.

@flouthoc
Copy link
Collaborator

@vrothberg then buildah manifest rm is different from podman manifest rm cause buildah manpage says it is supposed to remove entire list while podman manpage says it is supposed to delete specific instance. I'll check what's happening inside code.

@vrothberg
Copy link
Member

Can you elaborate on the difference? To me, man buildah-manifest-rm and man podman-manifest-rm look almost identical and man podman-manifest-rm does not include the term "instance" at all.

Are you sure you looked at rm and not remove? As mentioned above, remove is for instances and rm is for lists.

@flouthoc
Copy link
Collaborator

My bad 🤦 I looked at remove.

@stu-gott
Copy link
Author

Thank you to both of you for looking into this. After retrying this, I can confirm @vrothberg's observation: podman manifest rm currently does the wrong thing.

@stu-gott stu-gott changed the title podman rmi on manifest deletes something unexpected podman manifest rm deletes referenced image instead of manifest list Aug 30, 2021
@flouthoc
Copy link
Collaborator

/assign

@rhatdan
Copy link
Member

rhatdan commented Aug 31, 2021

buildah manifest rm and podman manifest rm, should work the same and in the end the manifest list should be removed, whether or not the images the manifest list refer to should be removed, is debatable. I would think the default is they get removed if they are not in use.

@stu-gott
Copy link
Author

I tend to think the opposite would be ideal. The images can be managed individually using well established commands. At this time there's no way to remove the manifest without affecting the associated images.

I propose that manifest rm operates on just the manifest, but adding a "--all" or "--follow" flag would also delete any referenced images that are present.

@rhatdan
Copy link
Member

rhatdan commented Sep 1, 2021

Well I guess I can go along with that. '--all' makes the most sense to me.

@flouthoc
Copy link
Collaborator

flouthoc commented Sep 1, 2021

I spent some time on this after having few discussions and brainstorming with @vrothberg on a similar topic.

What i think is we should do operations only on manifest if podman manifest is invoked since there can be cases where image is still under use and afaik once tagged and saved to store i don't think it has a dependency on its original manifest list.

@flouthoc
Copy link
Collaborator

flouthoc commented Sep 1, 2021

But sure we can have a case to remove all referenced images when --all is used. @vrothberg what would be your take on this ?

@vrothberg
Copy link
Member

podman manifest rm --force maybe? But that really is a separate issue/feature request.

In retrospective, wished there was no manifest rm or manifest remove. We could have wired it into rmi. It is getting really confusing.

@stu-gott
Copy link
Author

stu-gott commented Sep 1, 2021

@vrothberg, that's a very good point. We've conflated two separate conversations here. I think it would be sensible to handle decisions regarding [--all|--force|--do-what-I-mean-not-what-I-say] outside of the scope of this issue, which is about the fact that podman manifest rm deletes an associated image.

@flouthoc
Copy link
Collaborator

flouthoc commented Sep 2, 2021

@stu-gott Following should be closed by #11406

@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 21, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/bug Categorizes issue or PR as related to a bug. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants