Add --preservefds to podman run

[QiWang19]

Add --preservefds to podman run. close #6458

Signed-off-by: Qi Wang [email protected]

[rhatdan]

LGTM
/approve

[openshift-ci-robot]

[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

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

[edsantiago]

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.

[mheon]

Recommend rewording to "Pass a number of additional file descriptors into the container"

[mheon]

Can you add a comment that this is not in the JSON because it is not supported for remote connections?

[mheon]

Oh, wait, nevermind - this needs to actually have a JSON tag here, not "-" - that should only be in Specgen. This is for the DB, and we do want to save this to the container in the DB.

[mheon]

Suggest "WithPreserveFDs forwards from the process running Libpod into the container the given number of extra FDs (starting after the standard streams) to the created container"

[QiWang19]

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.

[edsantiago]

@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

[QiWang19]

@edsantiago Thanks!

[edsantiago]

@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.

[mheon]

Hold this until Monday, 2.0 is feature-frozen

[rhatdan]

@mheon Are you ok to merge this now?

[mheon]

LGTM, but would like final signoff from @giuseppe

[QiWang19]

Thanks.
@giuseppe PTAL.

[giuseppe]

/lgtm