Skip to content

Commit

Permalink
libpod: fix race condition rm'ing stopping containers
Browse files Browse the repository at this point in the history
do not allow removing containers that are in the stopping state,
otherwise it can lead to a race condition where a "podman rm" removes
the container from the storage while another process is stopping the
same container.

Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2155828

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
  • Loading branch information
giuseppe committed Jan 4, 2023
1 parent a4edd0d commit 6886e80
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
2 changes: 1 addition & 1 deletion libpod/container_internal.go
Expand Up @@ -2210,7 +2210,7 @@ func (c *Container) checkReadyForRemoval() error {
return fmt.Errorf("container %s is in invalid state: %w", c.ID(), define.ErrCtrStateInvalid)
}

if c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused) && !c.IsInfra() {
if c.ensureState(define.ContainerStateRunning, define.ContainerStatePaused, define.ContainerStateStopping) && !c.IsInfra() {
return fmt.Errorf("cannot remove container %s as it is %s - running or paused containers cannot be removed without force: %w", c.ID(), c.state.State.String(), define.ErrCtrStateInvalid)
}

Expand Down
46 changes: 46 additions & 0 deletions test/system/055-rm.bats
Expand Up @@ -113,4 +113,50 @@ load helpers
is "$output" "" "Should print no output"
}

@test "podman container rm doesn't affect stopping containers" {
local cname=c$(random_string 30)
run_podman run -d --name $cname \
--health-cmd /bin/false \
--health-interval 1s \
--health-retries 2 \
--health-timeout 1s \
--health-on-failure=stop \
--stop-timeout=2 \
--health-start-period 0 \
--stop-signal SIGTERM \
$IMAGE sleep infinity
local cid=$output

# We'll use the PID later to confirm that container is not running
run_podman inspect --format '{{.State.Pid}}' $cname
local pid=$output

# rm without -f should fail, because container is running/stopping.
# We have no way to guarantee that we see 'stopping', but at a very
# minimum we need to check at least one rm failure
local rm_failures=0
for i in {1..10}; do
run_podman '?' rm $cname
if [[ $status -eq 0 ]]; then
break
fi

# rm failed. Confirm that it's for the right reason.
assert "$output" =~ "Error: cannot remove container $cid as it is .* - running or paused containers cannot be removed without force: container state improper" \
"Expected error message from podman rm"
rm_failures=$((rm_failures + 1))
sleep 1
done

# At this point, container should be gone
run_podman 1 container exists $cname
run_podman 1 container exists $cid

assert "$rm_failures" -gt 0 "we want at least one failure from podman-rm"

if kill -0 $pid; then
die "Container $cname process is still running (pid $pid)"
fi
}

# vim: filetype=sh

0 comments on commit 6886e80

Please sign in to comment.