From 80639df27acd135e760fd552e6f4a4fb2ffbb116 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 16 Aug 2024 15:44:02 +0200 Subject: [PATCH] podman wait: allow waiting for removal of containers 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 8a943311db 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 --- docs/source/markdown/podman-wait.1.md.in | 5 +++-- libpod/container_api.go | 12 ++++++++---- test/system/800-config.bats | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/docs/source/markdown/podman-wait.1.md.in b/docs/source/markdown/podman-wait.1.md.in index 218ca04ba1..0d41ec82cf 100644 --- a/docs/source/markdown/podman-wait.1.md.in +++ b/docs/source/markdown/podman-wait.1.md.in @@ -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 diff --git a/libpod/container_api.go b/libpod/container_api.go index 1efa82800c..8a309c4cf9 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -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. @@ -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 } diff --git a/test/system/800-config.bats b/test/system/800-config.bats index 2ff116ae27..d099cde825 100644 --- a/test/system/800-config.bats +++ b/test/system/800-config.bats @@ -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