With --rm option remove container if podman run fails

[rhatdan]

Fixes #15049

Signed-off-by: Daniel J Walsh [email protected]

Does this PR introduce a user-facing change?

podman run --rm ... commands that fail will no longer leave the container behind, it will be removed.

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

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:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[mheon]

LGTM

[edsantiago]

Test looks weird

[edsantiago]

225??? That seems like a nonsensical exit code

[edsantiago]

why nginx? Can't we use $IMAGE?

[rhatdan]

I thought we actually needed a process to bind, but it looks like we can do it without. Besides this is the image (sortof) that the user reported. Since $IMAGE will cause the same error, I will change.

[Luap99]

The bind is done by podman on the host, it will fail with all images.

[edsantiago]

Comment seems wrong

[Luap99]

this removes the container even when opts.Rm is not set
also we should not ignore the error

[rhatdan]

Fixed.

[Luap99]

same here

[rhatdan]

opts.RM is checked. removeContainer is logging the debug message itself.

[edsantiago]

Test is still broken, no point in CI right now. Suggestion:

@test "podman run failed --rm " {
    port=$(random_free_port)

    # Run two containers with the same port bindings. The second must fail
    run_podman     run -p $port:80 --rm -d --name c_ok    $IMAGE top
    run_podman 126 run -p $port:80 --rm -d --name c_notok $IMAGE top
    # Prior to #15060, the second container would still show up in ps -a
    run_podman ps -a --format '{{.Image}}--{{.Names}}'
    is "$output" "$IMAGE--c_ok" "podman ps -a shows only the OK container"

    run_podman container rm -f -t 0 c_ok
}

Another suggestion: you can run system tests on your own system with, e.g.,

$ hack/bats 030:"run failed"   <--- 030 = 030-run.bats, "run failed" is filter substring on test name

I've confirmed that this test passes with your PR, and fails on main

[edsantiago]

Better suggestion:

@test "podman run failed --rm " {
    port=$(random_free_port)

    # Run two containers with the same port bindings. The second must fail
    run_podman     run -p $port:80 --rm -d --name c_ok           $IMAGE top
    run_podman 126 run -p $port:80      -d --name c_fail_no_rm   $IMAGE top
    run_podman 126 run -p $port:80 --rm -d --name c_fail_with_rm $IMAGE top
    # Prior to #15060, the third container would still show up in ps -a
    run_podman ps -a --sort names --format '{{.Image}}--{{.Names}}'
    is "$output" "$IMAGE--c_fail_no_rm
$IMAGE--c_ok" \
       "podman ps -a shows running & failed containers, but not failed-with-rm"

    run_podman container rm -f -t 0 c_ok c_fail_no_rm
}

This one confirms that the error container is only removed with --rm

[edsantiago]

LGTM but I'd say this merits a release note...?