Exec: use ErrorConmonRead

Before, we were using -1 as a bogus value in podman to signify something went wrong when reading from a conmon pipe. However, conmon uses negative values to indicate the runtime failed, and return the runtime's exit code.

instead, we should use a bogus value that is actually bogus. Define that value in the define package as MinInt32 (-1<< 31 - 1), which is outside of the range of possible pids (-1 << 31)

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

3 files changed, 13 insertions, 5 deletions

diff --git a/libpod/container_api.go b/libpod/container_api.go
index 37a05bb75..356da12d0 100644
--- a/libpod/container_api.go
+++ b/libpod/container_api.go
@@ -297,7 +297,9 @@ func (c *Container) Exec(tty, privileged bool, env map[string]string, cmd []stri
 		// Conmon will pass a non-zero exit code from the runtime as a pid here.
 		// we differentiate a pid with an exit code by sending it as negative, so reverse
 		// that change and return the exit code the runtime failed with.
-		if pid < 0 {
+		// Make sure the value is not ErrorConmonRead, as that is a podman set bogus value
+		// and not sent by conmon (and thus has no special meaning)
+		if pid < 0 && pid != define.ErrorConmonRead {
 			ec = -1 * pid
 		}
 		return ec, err
diff --git a/libpod/define/exec_codes.go b/libpod/define/exec_codes.go
index f94616b33..c2ec08666 100644
--- a/libpod/define/exec_codes.go
+++ b/libpod/define/exec_codes.go
@@ -1,6 +1,7 @@
 package define
 
 import (
+	"math"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -17,6 +18,11 @@ const (
 	ExecErrorCodeCannotInvoke = 126
 	// ExecErrorCodeNotFound is the error code to return when a command cannot be found
 	ExecErrorCodeNotFound = 127
+	// ErrorConmonRead is a bogus value that can neither be a valid PID or exit code. It is
+	// used because conmon will send a negative value when sending a PID back over a pipe FD
+	// to signify something went wrong in the runtime. We need to differentiate between that
+	// value and a failure on the podman side of reading that value. Thus, we use ErrorConmonRead
+	ErrorConmonRead = math.MinInt32 - 1
 )
 
 // TranslateExecErrorToExitCode takes an error and checks whether it
diff --git a/libpod/oci_conmon_linux.go b/libpod/oci_conmon_linux.go
index 8da6db09f..f260e3a39 100644
--- a/libpod/oci_conmon_linux.go
+++ b/libpod/oci_conmon_linux.go
@@ -1557,7 +1557,7 @@ func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
 		ch <- syncStruct{si: si}
 	}()
 
-	data := -1
+	data := define.ErrorConmonRead
 	select {
 	case ss := <-ch:
 		if ss.err != nil {
@@ -1567,14 +1567,14 @@ func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
 					var ociErr ociError
 					if err := json.Unmarshal(ociLogData, &ociErr); err == nil {
 						return DataAndErr{
-							data: -1,
+							data: data,
 							err:  getOCIRuntimeError(ociErr.Msg),
 						}
 					}
 				}
 			}
 			return DataAndErr{
-				data: -1,
+				data: data,
 				err:  errors.Wrapf(ss.err, "container create failed (no logs from conmon)"),
 			}
 		}
@@ -1607,7 +1607,7 @@ func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
 		data = ss.si.Data
 	case <-time.After(define.ContainerCreateTimeout):
 		return DataAndErr{
-			data: -1,
+			data: data,
 			err:  errors.Wrapf(define.ErrInternal, "container creation timeout"),
 		}
 	}