LGTM
/approve

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: QiWang19, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Please add a system test, possibly in test/system/030-run.bats, using something like what I messaged you earlier today. The e2e test here is not very useful: all it does is confirm that the option does not cause a syntax error. It does not test any actual functionality.

Please add a system test, possibly in test/system/030-run.bats, using something like what I messaged you earlier today. The e2e test here is not very useful: all it does is confirm that the option does not cause a syntax error. It does not test any actual functionality.

https://github.com/containers/libpod/pull/6625/files#diff-f1c2f651579cf49df98160fa0e2d50a7 test failed. @edsantiago Can you help me diagnose if it's because the test not correct? Otherwise, my implementation may not as expected.

@QiWang19 this is what the test should look like:

@test "podman run --preserve-fds" {
    skip_if_remote

    content=$(random_string 20)
    echo "$content" > $PODMAN_TMPDIR/tempfile

    run_podman run --rm -i --preserve-fds=2 $IMAGE sh -c "cat <&4" 4<$PODMAN_TMPDIR/tempfile
    is "$output" "$content" "container read input from fd 4"
}

Please use that. It still fails, but it does so for a real-problem reason which is being tracked in #6653

 2020/06/17 15:51:44 bolt.Close(): funlock error: bad file descriptor
time="2020-06-17T15:51:44-06:00" level=error msg="failed to close libpod db: \"db file close: close /var/lib/containers/storage/libpod/bolt_state.db: bad file descriptor\""
Error: error saving container 26cd8352b73d3c07b46343c63f3b3b5c00e37f28ed685ace08788745025a0704 state: write /var/lib/containers/storage/libpod/bolt_state.db: bad file descriptor

Please coordinate efforts to resolve that issue.

Explanation of what I changed:

• use FD 4, not 3: I forgot that BATS uses fd 3 for internal purposes.
• Remove the NONLOCAL_IMAGE stuff. You copy-pasted that from another test. It is not necessary or desired here (it introduces a point of failure because it relies on remote registry)
• use random content, not hardcoded strings which could easily appear via other ways
• use a file in $PODMAN_TMPDIR - otherwise "testfile" will pollute your current working directory
• use --rm in the run command, to avoid having to clean up

@edsantiago Thanks!

@QiWang19 it looks like #6653 might not get attention soon. Since I believe your PR is correct, and the real problem is elsewhere, I'd be OK with you adding skip "enable this once #6653 is fixed" to the BATS test and trying to get your PR committed.

Hold this until Monday, 2.0 is feature-frozen

@mheon Are you ok to merge this now?

LGTM, but would like final signoff from @giuseppe

Thanks.
@giuseppe PTAL.

/lgtm