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

Fail to remove image with command "podman container cleanup --rmi xxx" #15640

Closed
fhwsuper opened this issue Sep 6, 2022 · 9 comments · Fixed by #19482
Closed

Fail to remove image with command "podman container cleanup --rmi xxx" #15640

fhwsuper opened this issue Sep 6, 2022 · 9 comments · Fixed by #19482
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Comments

@fhwsuper
Copy link

fhwsuper commented Sep 6, 2022

This issue is found in Podman 3.4.4.

The code logic is that remove image directly, but the container is still using the image. I suppose it should remove the container first and then remove the image in code. Or is it work as design?

func (ic *ContainerEngine) ContainerCleanup()

Ubuntu Linux 20.04
root@fanhuawei-Standard-PC-i440FX-PIIX-1996:/home/fanhuawei/podman/podman-3.4.4/bin# ./podman container cleanup --rmi b11c6311eaf7
ERRO[0000] Error removing image: Image used by b11c6311eaf74ea64ca500781fb96207ec41ec01a5d8311ccf20e93c4b4a6401: image is in use by a container
Error: Image used by b11c6311eaf74ea64ca500781fb96207ec41ec01a5d8311ccf20e93c4b4a6401: image is in use by a container

@vrothberg
Copy link
Member

Thanks for reaching out, @fhwsuper.

@mheon WDYT?

@mheon
Copy link
Member

mheon commented Sep 6, 2022

This sounds like a bug, yeah. Also points out that our testing for the --rmi flag is probably not working as expected.

@mheon
Copy link
Member

mheon commented Sep 6, 2022

To be clear, the solution of swapping the ordering - remove the container first, image second - sounds completely correct.

@vrothberg
Copy link
Member

@mheon the ordering in the backend looks good to me. I think --rmi can only work with --rm. Not sure if --rmi should imply --rm or if we should check that on the CLI (or backend) and error out.

@mheon
Copy link
Member

mheon commented Sep 9, 2022

Given that podman container cleanup is only really meant to be invoked automatically by Conmon, I'd be perfectly fine making --rmi require --rm. We'll have to catch this in podman run and podman create as well, though, because otherwise the error messages will be lost.

@boaz0
Copy link
Collaborator

boaz0 commented Sep 12, 2022

What if we have two containers: container1 and container2 and both are using image image1.
I would rather podman container cleanup --rm --rmi container1 to fail instead of deleting container1, container2 and image1.

@github-actions
Copy link

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Jul 30, 2023

@mheon @boaz0 @fhwsuper @vrothberg Is this still an issue?

@mheon
Copy link
Member

mheon commented Aug 1, 2023

Yep, still an issue. podman run -t -i --rmi fedora ls / fails to successfully remove the image because the container still exists.

rhatdan added a commit to rhatdan/podman that referenced this issue Aug 2, 2023
Forcing users to set --rm when setting --rmi is just bad UI.
If I want the image to be removed, it implies that I want the
container removed that I am creating.

Fixes: containers#15640

Signed-off-by: Daniel J Walsh <[email protected]>
@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 Nov 2, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
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.

5 participants