-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Ensure that container still exists when removing #10476
Ensure that container still exists when removing #10476
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mheon 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 |
Is this to fix THE race? |
Nope. Separate BZ, unfortunately. Would not be surprised if it fixes some races in CI, though. |
After containers#8906, there is a potential race condition in container removal of running containers with `--rm`. Running containers must first be stopped, which was changed to unlock the container to allow commands like `podman ps` to continue to run while stopping; however, this also means that the cleanup process can potentially run before we re-lock, and remove the container from under us, resulting in error messages from `podman rm`. The end result is unchanged, the container is still cleanly removed, but the `podman rm` command will seem to have failed. Work around this by pinging the database after we stop the container to make sure it still exists. If it doesn't, our job is done and we can exit cleanly. Signed-off-by: Matthew Heon <[email protected]>
9a8d9ca
to
fad6e1d
Compare
FWIW, addresses https://bugzilla.redhat.com/show_bug.cgi?id=1964852 |
/lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/hold cancel
LGTM
After #8906, there is a potential race condition in container removal of running containers with
--rm
. Running containers must first be stopped, which was changed to unlock the container to allow commands likepodman ps
to continue to run while stopping; however, this also means that the cleanup process can potentially run before we re-lock, and remove the container from under us, resulting in error messages frompodman rm
. The end result is unchanged, the container is still cleanly removed, but thepodman rm
command will seem to have failed.Work around this by pinging the database after we stop the container to make sure it still exists. If it doesn't, our job is done and we can exit cleanly.
[NO TESTS NEEDED] because the race doesn't trigger easily and I can't make a test to reproduce it on demand anywhere.