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

[v5.2-rhel] Fix podman stop and podman run --rmi #24197

Open
wants to merge 1 commit into
base: v5.2-rhel
Choose a base branch
from

Conversation

TomSweeneyRedHat
Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat commented Oct 8, 2024

This started off as an attempt to make podman stop on a container started with --rm actually remove the container, instead of just cleaning it up and waiting for the cleanup process to finish the removal.

In the process, I realized that podman run --rmi was rather broken. It was only done as part of the Podman CLI, not the cleanup process (meaning it only worked with attached containers) and the way it was wired meant that I was fairly confident that it wouldn't work if I did a podman stop on an attached container run with --rmi. I rewired it to use the same mechanism that podman run --rm uses, so it should be a lot more durable now, and I also wired it into podman inspect so you can tell that a container will remove its image.

Tests have been added for the changes to podman run --rmi. No tests for stop on a run --rm container as that would be racy.

Fixes #22852
Fixes RHEL-39513

For this branch
Backport of: 458ba5a
Fixes: https://issues.redhat.com/browse/RHEL-61667

Does this PR introduce a user-facing change?

None

Copy link
Contributor

openshift-ci bot commented Oct 8, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TomSweeneyRedHat

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 8, 2024
@github-actions github-actions bot added the kind/api-change Change to remote API; merits scrutiny label Oct 8, 2024
Copy link

Ephemeral COPR build failed. @containers/packit-build please check.

@giuseppe
Copy link
Member

giuseppe commented Oct 8, 2024

Could you add a reference to the commit that this is a backport of 458ba5a?

Have you used git cherry-pick to produce this patch? It would add the reference automatically if you specify -x.

@TomSweeneyRedHat
Copy link
Member Author

I did do a git cherry-pick 458ba5a8afedae15acfec0a1f64195f41d80edcf from the command line, but was unaware of the -x option. I will look into that and try to remember to use that going forward.

This started off as an attempt to make `podman stop` on a
container started with `--rm` actually remove the container,
instead of just cleaning it up and waiting for the cleanup
process to finish the removal.

In the process, I realized that `podman run --rmi` was rather
broken. It was only done as part of the Podman CLI, not the
cleanup process (meaning it only worked with attached containers)
and the way it was wired meant that I was fairly confident that
it wouldn't work if I did a `podman stop` on an attached
container run with `--rmi`. I rewired it to use the same
mechanism that `podman run --rm` uses, so it should be a lot more
durable now, and I also wired it into `podman inspect` so you can
tell that a container will remove its image.

Tests have been added for the changes to `podman run --rmi`. No
tests for `stop` on a `run --rm` container as that would be racy.

Fixes containers#22852
Fixes RHEL-39513

For this branch
Backport of: [458ba5a](containers@458ba5a)
Fixes: https://issues.redhat.com/browse/RHEL-61667

Signed-off-by: Matt Heon <[email protected]>
Signed-off-by: tomsweeneyredhat <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/api-change Change to remote API; merits scrutiny release-note-none
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants