Revert "exec: get the exit code from sync pipe instead of file"

This reverts commit 4b72f9e4013411208751df2a92ab9f322d4da5b2.

Continues what began with revert of
d3d97a25e8c87cf741b2e24ac01ef84962137106 in previous commit.

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

1 files changed, 28 insertions, 54 deletions

diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index 8da6db09f..800f89603 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -595,29 +595,31 @@ func (r *ConmonOCIRuntime) AttachResize(ctr *Container, newSize remotecommand.Te
 
 // ExecContainer executes a command in a running container
 // TODO: Split into Create/Start/Attach/Wait
-func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options *ExecOptions) (chan DataAndErr, chan error, error) {
+func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options *ExecOptions) (int, chan error, error) {
 	if options == nil {
-		return nil, nil, errors.Wrapf(define.ErrInvalidArg, "must provide an ExecOptions struct to ExecContainer")
+		return -1, nil, errors.Wrapf(define.ErrInvalidArg, "must provide an ExecOptions struct to ExecContainer")
 	}
 	if len(options.Cmd) == 0 {
-		return nil, nil, errors.Wrapf(define.ErrInvalidArg, "must provide a command to execute")
+		return -1, nil, errors.Wrapf(define.ErrInvalidArg, "must provide a command to execute")
 	}
 
 	if sessionID == "" {
-		return nil, nil, errors.Wrapf(define.ErrEmptyID, "must provide a session ID for exec")
+		return -1, nil, errors.Wrapf(define.ErrEmptyID, "must provide a session ID for exec")
 	}
 
 	// create sync pipe to receive the pid
 	parentSyncPipe, childSyncPipe, err := newPipe()
 	if err != nil {
-		return nil, nil, errors.Wrapf(err, "error creating socket pair")
+		return -1, nil, errors.Wrapf(err, "error creating socket pair")
 	}
 
+	defer errorhandling.CloseQuiet(parentSyncPipe)
+
 	// create start pipe to set the cgroup before running
 	// attachToExec is responsible for closing parentStartPipe
 	childStartPipe, parentStartPipe, err := newPipe()
 	if err != nil {
-		return nil, nil, errors.Wrapf(err, "error creating socket pair")
+		return -1, nil, errors.Wrapf(err, "error creating socket pair")
 	}
 
 	// We want to make sure we close the parent{Start,Attach}Pipes if we fail
@@ -636,7 +638,7 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
 	// attachToExec is responsible for closing parentAttachPipe
 	parentAttachPipe, childAttachPipe, err := newPipe()
 	if err != nil {
-		return nil, nil, errors.Wrapf(err, "error creating socket pair")
+		return -1, nil, errors.Wrapf(err, "error creating socket pair")
 	}
 
 	defer func() {
@@ -656,7 +658,7 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
 
 	runtimeDir, err := util.GetRuntimeDir()
 	if err != nil {
-		return nil, nil, err
+		return -1, nil, err
 	}
 
 	finalEnv := make([]string, 0, len(options.Env))
@@ -666,7 +668,7 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
 
 	processFile, err := prepareProcessExec(c, options.Cmd, finalEnv, options.Terminal, options.Cwd, options.User, sessionID)
 	if err != nil {
-		return nil, nil, err
+		return -1, nil, err
 	}
 
 	var ociLog string
@@ -715,7 +717,7 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
 
 	conmonEnv, extraFiles, err := r.configureConmonEnv(runtimeDir)
 	if err != nil {
-		return nil, nil, err
+		return -1, nil, err
 	}
 
 	if options.PreserveFDs > 0 {
@@ -746,10 +748,10 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
 	childrenClosed = true
 
 	if err != nil {
-		return nil, nil, errors.Wrapf(err, "cannot start container %s", c.ID())
+		return -1, nil, errors.Wrapf(err, "cannot start container %s", c.ID())
 	}
 	if err := r.moveConmonToCgroupAndSignal(c, execCmd, parentStartPipe); err != nil {
-		return nil, nil, err
+		return -1, nil, err
 	}
 
 	if options.PreserveFDs > 0 {
@@ -772,16 +774,9 @@ func (r *ConmonOCIRuntime) ExecContainer(c *Container, sessionID string, options
 	}()
 	attachToExecCalled = true
 
-	dataChan := make(chan DataAndErr)
-	go func() {
-		// read the exec pid
-		dataChan <- readConmonPipeData(parentSyncPipe, ociLog)
-		// read the exec exit code
-		dataChan <- readConmonPipeData(parentSyncPipe, ociLog)
-		errorhandling.CloseQuiet(parentSyncPipe)
-	}()
+	pid, err := readConmonPipeData(parentSyncPipe, ociLog)
 
-	return dataChan, attachChan, err
+	return pid, attachChan, err
 }
 
 // ExecStopContainer stops a given exec session in a running container.
@@ -1211,14 +1206,14 @@ func (r *ConmonOCIRuntime) createOCIContainer(ctr *Container, restoreOptions *Co
 		return err
 	}
 
-	pipeData := readConmonPipeData(parentSyncPipe, ociLog)
-	if pipeData.err != nil {
+	pid, err := readConmonPipeData(parentSyncPipe, ociLog)
+	if err != nil {
 		if err2 := r.DeleteContainer(ctr); err2 != nil {
 			logrus.Errorf("Error removing container %s from runtime after creation failed", ctr.ID())
 		}
-		return pipeData.err
+		return err
 	}
-	ctr.state.PID = pipeData.data
+	ctr.state.PID = pid
 
 	conmonPID, err := readConmonPidFile(ctr.config.ConmonPidFile)
 	if err != nil {
@@ -1530,7 +1525,7 @@ func readConmonPidFile(pidFile string) (int, error) {
 }
 
 // readConmonPipeData attempts to read a syncInfo struct from the pipe
-func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
+func readConmonPipeData(pipe *os.File, ociLog string) (int, error) {
 	// syncInfo is used to return data from monitor process to daemon
 	type syncInfo struct {
 		Data    int    `json:"data"`
@@ -1566,17 +1561,11 @@ func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
 				if err == nil {
 					var ociErr ociError
 					if err := json.Unmarshal(ociLogData, &ociErr); err == nil {
-						return DataAndErr{
-							data: -1,
-							err:  getOCIRuntimeError(ociErr.Msg),
-						}
+						return -1, getOCIRuntimeError(ociErr.Msg)
 					}
 				}
 			}
-			return DataAndErr{
-				data: -1,
-				err:  errors.Wrapf(ss.err, "container create failed (no logs from conmon)"),
-			}
+			return -1, errors.Wrapf(ss.err, "container create failed (no logs from conmon)")
 		}
 		logrus.Debugf("Received: %d", ss.si.Data)
 		if ss.si.Data < 0 {
@@ -1585,36 +1574,21 @@ func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
 				if err == nil {
 					var ociErr ociError
 					if err := json.Unmarshal(ociLogData, &ociErr); err == nil {
-						return DataAndErr{
-							data: ss.si.Data,
-							err:  getOCIRuntimeError(ociErr.Msg),
-						}
+						return ss.si.Data, getOCIRuntimeError(ociErr.Msg)
 					}
 				}
 			}
 			// If we failed to parse the JSON errors, then print the output as it is
 			if ss.si.Message != "" {
-				return DataAndErr{
-					data: ss.si.Data,
-					err:  getOCIRuntimeError(ss.si.Message),
-				}
-			}
-			return DataAndErr{
-				data: ss.si.Data,
-				err:  errors.Wrapf(define.ErrInternal, "container create failed"),
+				return ss.si.Data, getOCIRuntimeError(ss.si.Message)
 			}
+			return ss.si.Data, errors.Wrapf(define.ErrInternal, "container create failed")
 		}
 		data = ss.si.Data
 	case <-time.After(define.ContainerCreateTimeout):
-		return DataAndErr{
-			data: -1,
-			err:  errors.Wrapf(define.ErrInternal, "container creation timeout"),
-		}
-	}
-	return DataAndErr{
-		data: data,
-		err:  nil,
+		return -1, errors.Wrapf(define.ErrInternal, "container creation timeout")
 	}
+	return data, nil
 }
 
 // writeConmonPipeData writes nonse data to a pipe