Skip to content

Commit

Permalink
podman wait: allow waiting for removal of containers
Browse files Browse the repository at this point in the history
By default wait only waits for the exit of a container, there is really
no way to make it wait for the removal too when the container was
created with --rm. I though I found a clever way in 8a94331 but this
is not working race free. While it works most of the time any other
parallel process might call syncContainer() before the cleanup process
holds the lock until it removes it. As such the wait hack to only update
the state and not sync the exit file did not work so we can drop that.

However the test wants to wait for the removal to happen by the cleanup
process and we can already say --condition=removing to do this but this
will throw an error if the ctr was removed instead of counting this as
success so fix that as well.

Fixes #23640

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
  • Loading branch information
Luap99 committed Aug 16, 2024
1 parent e8410b8 commit 80639df
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 7 deletions.
5 changes: 3 additions & 2 deletions docs/source/markdown/podman-wait.1.md.in
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ such as those created by `podman kube play`, the containers may be repeatedly
exiting and restarting, possibly with different exit codes. `podman wait` will
only display and detect the first exit after the wait command was started.

When running a container with podman run --rm wait should wait for the
container to be fully removed as well before it returns.
When running a container with podman run --rm wait does not wait for the
container to be fully removed. To wait for the removal of a container use
`--condition=removing`.

## OPTIONS

Expand Down
12 changes: 8 additions & 4 deletions libpod/container_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -546,10 +546,7 @@ func (c *Container) WaitForExit(ctx context.Context, pollInterval time.Duration)
c.lock.Lock()
defer c.lock.Unlock()

// Note this is one of the rare cases where we do not like to use syncContainer() as it does read the exit file
// We like to avoid that here because that means we might read it before container cleanup run and possible
// removed the container
if err := c.runtime.state.UpdateContainer(c); err != nil {
if err := c.syncContainer(); err != nil {
if errors.Is(err, define.ErrNoSuchCtr) {
// if the container is not valid at this point as it was deleted,
// check if the exit code was recorded in the db.
Expand Down Expand Up @@ -705,6 +702,13 @@ func (c *Container) WaitForConditionWithInterval(ctx context.Context, waitTimeou
if len(wantedStates) > 0 {
state, err := c.State()
if err != nil {
// If the we wait for removing and the container is removed do not return this as error.
// This allows callers to actually wait for the ctr to be removed.
if wantedStates[define.ContainerStateRemoving] &&
(errors.Is(err, define.ErrNoSuchCtr) || errors.Is(err, define.ErrCtrRemoved)) {
trySend(-1, nil)
return
}
trySend(-1, err)
return
}
Expand Down
2 changes: 1 addition & 1 deletion test/system/800-config.bats
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ EOF
# container completes fast, and the cleanup *did* happen properly
# the container is now gone. So, we need to ignore "no such
# container" errors from podman wait.
CONTAINERS_CONF="$conf_tmp" run_podman '?' wait "$cid"
CONTAINERS_CONF="$conf_tmp" run_podman '?' wait --condition=removing "$cid"
if [[ $status != 0 ]]; then
is "$output" "Error:.*no such container" "unexpected error from podman wait"
fi
Expand Down

0 comments on commit 80639df

Please sign in to comment.