From ed24f0b2ca87d6481546d07f97b52016e9ae9a25 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 16 Jun 2023 11:11:28 +0200 Subject: [PATCH] remote wait: fix "removed" condition The "removed" condition mapped to an undefined state which ultimately rendered the wait endpoint to return an incorrect exit code. Instead, map "removed" to "exited" to make sure Podman returns the expected exit code. Fixes: #18889 Signed-off-by: Valentin Rothberg --- pkg/api/handlers/utils/containers.go | 18 ++++++++++++++---- test/apiv2/26-containersWait.at | 11 ++++++++--- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/pkg/api/handlers/utils/containers.go b/pkg/api/handlers/utils/containers.go index 798dd8ff65..c937d61a03 100644 --- a/pkg/api/handlers/utils/containers.go +++ b/pkg/api/handlers/utils/containers.go @@ -189,11 +189,21 @@ var notRunningStates = []define.ContainerStatus{ } func waitRemoved(ctrWait containerWaitFn) (int32, error) { - code, err := ctrWait(define.ContainerStateUnknown) - if err != nil && errors.Is(err, define.ErrNoSuchCtr) { - return code, nil + var code int32 + for { + c, err := ctrWait(define.ContainerStateExited) + if errors.Is(err, define.ErrNoSuchCtr) { + // Make sure to wait until the container has been removed. + break + } + if err != nil { + return code, err + } + // If the container doesn't exist, the return code is -1, so + // only set it in case of success. + code = c } - return code, err + return code, nil } func waitNextExit(ctx context.Context, containerName string) (int32, error) { diff --git a/test/apiv2/26-containersWait.at b/test/apiv2/26-containersWait.at index 41938d5673..3bd4165d06 100644 --- a/test/apiv2/26-containersWait.at +++ b/test/apiv2/26-containersWait.at @@ -12,7 +12,8 @@ CTR="WaitTestingCtr" t POST "containers/nonExistent/wait?condition=next-exit" 404 -podman create --name "${CTR}" --entrypoint '["true"]' "${IMAGE}" +# Make sure to test a non-zero exit code (see #18889) +podman create --name "${CTR}" "${IMAGE}" sh -c "exit 3" t POST "containers/${CTR}/wait?condition=non-existent-cond" 400 @@ -24,7 +25,7 @@ child_pid=$! # This will block until the background job completes t POST "containers/${CTR}/wait?condition=next-exit" 200 \ - .StatusCode=0 \ + .StatusCode=3 \ .Error=null wait "${child_pid}" @@ -41,6 +42,10 @@ fi child_pid=$! t POST "containers/${CTR}/wait?condition=removed" 200 \ - .StatusCode=0 \ + .StatusCode=3 \ .Error=null +# Make sure the container has really been removed after waiting for +# "condition=removed". This check is racy but should flake in case it doesn't +# work correctly. +t POST "containers/${CTR}/wait?condition=next-exit" 404 wait "${child_pid}"