exec: fix --preserve-fds

There were two problems with preserve fds.
libpod didn't open the fds before passing _OCI*PIPE to conmon. This caused libpod to talk on the preserved fds, rather than the pipes, with conmon talking on the pipes. This caused a hang.
Libpod also didn't convert an int to string correctly, so it would further fail.

Fix these and add a unit test to make sure we don't regress in the future

Note: this test will not pass on crun until crun supports --preserve-fds

Signed-off-by: Peter Hunt <pehunt@redhat.com>

2 files changed, 23 insertions, 7 deletions

diff --git a/libpod/oci_linux.go b/libpod/oci_linux.go
index 091b6d155..9ec074704 100644
--- a/libpod/oci_linux.go
+++ b/libpod/oci_linux.go
@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -199,7 +200,7 @@ func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty
 	args := r.sharedConmonArgs(c, sessionID, c.execBundlePath(sessionID), c.execPidPath(sessionID), c.execLogPath(sessionID), c.execExitFileDir(sessionID), ociLog)
 
 	if preserveFDs > 0 {
-		args = append(args, formatRuntimeOpts("--preserve-fds", string(preserveFDs))...)
+		args = append(args, formatRuntimeOpts("--preserve-fds", strconv.Itoa(preserveFDs))...)
 	}
 
 	for _, capability := range capAdd {
@@ -236,6 +237,12 @@ func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty
 		return -1, nil, err
 	}
 
+	if preserveFDs > 0 {
+		for fd := 3; fd < 3+preserveFDs; fd++ {
+			execCmd.ExtraFiles = append(execCmd.ExtraFiles, os.NewFile(uintptr(fd), fmt.Sprintf("fd-%d", fd)))
+		}
+	}
+
 	// we don't want to step on users fds they asked to preserve
 	// Since 0-2 are used for stdio, start the fds we pass in at preserveFDs+3
 	execCmd.Env = append(r.conmonEnv, fmt.Sprintf("_OCI_SYNCPIPE=%d", preserveFDs+3), fmt.Sprintf("_OCI_STARTPIPE=%d", preserveFDs+4), fmt.Sprintf("_OCI_ATTACHPIPE=%d", preserveFDs+5))
@@ -248,12 +255,6 @@ func (r *OCIRuntime) execContainer(c *Container, cmd, capAdd, env []string, tty
 		Setpgid: true,
 	}
 
-	if preserveFDs > 0 {
-		for fd := 3; fd < 3+preserveFDs; fd++ {
-			execCmd.ExtraFiles = append(execCmd.ExtraFiles, os.NewFile(uintptr(fd), fmt.Sprintf("fd-%d", fd)))
-		}
-	}
-
 	err = startCommandGivenSelinux(execCmd)
 
 	// We don't need children pipes  on the parent side
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index 670269eab..13fdabb81 100644
--- a/test/e2e/exec_test.go
+++ b/test/e2e/exec_test.go
@@ -2,6 +2,7 @@ package integration
 
 import (
 	"os"
+	"strings"
 
 	. "github.com/containers/libpod/test/utils"
 	. "github.com/onsi/ginkgo"
@@ -228,4 +229,18 @@ var _ = Describe("Podman exec", func() {
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(127))
 	})
+
+	It("podman exec preserve fds sanity check", func() {
+		// TODO: add this test once crun adds the --preserve-fds flag for exec
+		if strings.Contains(podmanTest.OCIRuntime, "crun") {
+			Skip("Test only works on crun")
+		}
+		setup := podmanTest.RunTopContainer("test1")
+		setup.WaitWithDefaultTimeout()
+		Expect(setup.ExitCode()).To(Equal(0))
+
+		session := podmanTest.Podman([]string{"exec", "--preserve-fds", "1", "test1", "ls"})
+		session.WaitWithDefaultTimeout()
+		Expect(session.ExitCode()).To(Equal(0))
+	})
 })