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>
This commit is contained in:
Peter Hunt
2020-03-03 15:35:29 -05:00
parent 4b72f9e401
commit d3d97a25e8
3 changed files with 13 additions and 5 deletions

View File

@ -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. // 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 // 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. // 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 ec = -1 * pid
} }
return ec, err return ec, err

View File

@ -1,6 +1,7 @@
package define package define
import ( import (
"math"
"strings" "strings"
"github.com/pkg/errors" "github.com/pkg/errors"
@ -17,6 +18,11 @@ const (
ExecErrorCodeCannotInvoke = 126 ExecErrorCodeCannotInvoke = 126
// ExecErrorCodeNotFound is the error code to return when a command cannot be found // ExecErrorCodeNotFound is the error code to return when a command cannot be found
ExecErrorCodeNotFound = 127 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 // TranslateExecErrorToExitCode takes an error and checks whether it

View File

@ -1557,7 +1557,7 @@ func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
ch <- syncStruct{si: si} ch <- syncStruct{si: si}
}() }()
data := -1 data := define.ErrorConmonRead
select { select {
case ss := <-ch: case ss := <-ch:
if ss.err != nil { if ss.err != nil {
@ -1567,14 +1567,14 @@ func readConmonPipeData(pipe *os.File, ociLog string) DataAndErr {
var ociErr ociError var ociErr ociError
if err := json.Unmarshal(ociLogData, &ociErr); err == nil { if err := json.Unmarshal(ociLogData, &ociErr); err == nil {
return DataAndErr{ return DataAndErr{
data: -1, data: data,
err: getOCIRuntimeError(ociErr.Msg), err: getOCIRuntimeError(ociErr.Msg),
} }
} }
} }
} }
return DataAndErr{ return DataAndErr{
data: -1, data: data,
err: errors.Wrapf(ss.err, "container create failed (no logs from conmon)"), 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 data = ss.si.Data
case <-time.After(define.ContainerCreateTimeout): case <-time.After(define.ContainerCreateTimeout):
return DataAndErr{ return DataAndErr{
data: -1, data: data,
err: errors.Wrapf(define.ErrInternal, "container creation timeout"), err: errors.Wrapf(define.ErrInternal, "container creation timeout"),
} }
} }