move editing of exitCode to runtime

There's no way to get the error if we successfully get an exit code (as it's just printed to stderr instead).
instead of relying on the error to be passed to podman, and edit based on the error code, process it on the varlink side instead

Also move error codes to define package

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

cmd/podman/exec.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"github.com/containers/libpod/cmd/podman/cliconfig"
-	"github.com/containers/libpod/libpod/define"
 	"github.com/containers/libpod/pkg/adapter"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
@@ -70,11 +69,5 @@ func execCmd(c *cliconfig.ExecValues) error {
 	defer runtime.DeferredShutdown(false)
 
 	exitCode, err = runtime.ExecContainer(getContext(), c)
-	if errors.Cause(err) == define.ErrOCIRuntimePermissionDenied {
-		exitCode = 126
-	}
-	if errors.Cause(err) == define.ErrOCIRuntimeNotFound {
-		exitCode = 127
-	}
 	return err
 }

libpod/container_api.go

@@ -18,11 +18,6 @@ import (
 	"k8s.io/client-go/tools/remotecommand"
 )
 
-const (
-	defaultExecExitCode             = 125
-	defaultExecExitCodeCannotInvoke = 126
-)
-
 // Init creates a container in the OCI runtime
 func (c *Container) Init(ctx context.Context) (err error) {
 	span, _ := opentracing.StartSpanFromContext(ctx, "containerInit")
@@ -234,7 +229,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 		defer c.lock.Unlock()
 
 		if err := c.syncContainer(); err != nil {
-			return defaultExecExitCodeCannotInvoke, err
+			return define.ExecErrorCodeCannotInvoke, err
 		}
 	}
 
@@ -242,7 +237,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 
 	// TODO can probably relax this once we track exec sessions
 	if conState != define.ContainerStateRunning {
-		return defaultExecExitCodeCannotInvoke, errors.Wrapf(define.ErrCtrStateInvalid, "cannot exec into container that is not running")
+		return define.ExecErrorCodeCannotInvoke, errors.Wrapf(define.ErrCtrStateInvalid, "cannot exec into container that is not running")
 	}
 
 	if privileged || c.config.Privileged {
@@ -269,7 +264,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 
 	logrus.Debugf("Creating new exec session in container %s with session id %s", c.ID(), sessionID)
 	if err := c.createExecBundle(sessionID); err != nil {
-		return defaultExecExitCodeCannotInvoke, err
+		return define.ExecErrorCodeCannotInvoke, err
 	}
 
 	defer func() {
@@ -281,7 +276,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 
 	pid, attachChan, err := c.ociRuntime.execContainer(c, cmd, capList, env, tty, workDir, user, sessionID, streams, preserveFDs, resize, detachKeys)
 	if err != nil {
-		ec := defaultExecExitCode
+		ec := define.ExecErrorCodeGeneric
 		// 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.
@@ -303,7 +298,7 @@ func (c *Container) Exec(tty, privileged bool, env, cmd []string, user, workDir
 	if err := c.save(); err != nil {
 		// Now we have a PID but we can't save it in the DB
 		// TODO handle this better
-		return defaultExecExitCode, errors.Wrapf(err, "error saving exec sessions %s for container %s", sessionID, c.ID())
+		return define.ExecErrorCodeGeneric, errors.Wrapf(err, "error saving exec sessions %s for container %s", sessionID, c.ID())
 	}
 	c.newContainerEvent(events.Exec)
 	logrus.Debugf("Successfully started exec session %s in container %s", sessionID, c.ID())

libpod/define/exec_codes.go

@@ -0,0 +1,13 @@
+package define
+
+const (
+	// ExecErrorCodeGeneric is the default error code to return from an exec session if libpod failed
+	// prior to calling the runtime
+	ExecErrorCodeGeneric = 125
+	// ExecErrorCodeCannotInvoke is the error code to return when the runtime fails to invoke a command
+	// an example of this can be found by trying to execute a directory:
+	// `podman exec -l /etc`
+	ExecErrorCodeCannotInvoke = 126
+	// ExecErrorCodeNotFound is the error code to return when a command cannot be found
+	ExecErrorCodeNotFound = 127
+)

pkg/adapter/containers.go

@@ -1000,7 +1000,14 @@ func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecVal
 	streams.AttachOutput = true
 	streams.AttachError = true
 
-	return ExecAttachCtr(ctx, ctr.Container, cli.Tty, cli.Privileged, envs, cmd, cli.User, cli.Workdir, streams, cli.PreserveFDs, cli.DetachKeys)
+	ec, err = ExecAttachCtr(ctx, ctr.Container, cli.Tty, cli.Privileged, envs, cmd, cli.User, cli.Workdir, streams, cli.PreserveFDs, cli.DetachKeys)
+	if errors.Cause(err) == define.ErrOCIRuntimePermissionDenied {
+		ec = 126
+	}
+	if errors.Cause(err) == define.ErrOCIRuntimeNotFound {
+		ec = 127
+	}
+	return ec, err
 }
 
 // Prune removes stopped containers

pkg/adapter/containers_remote.go

@@ -997,7 +997,7 @@ func (r *LocalRuntime) Commit(ctx context.Context, c *cliconfig.CommitValues, co
 func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecValues) (int, error) {
 	var (
 		oldTermState *term.State
-		ec           int = 125
+		ec           int = define.ExecErrorCodeGeneric
 	)
 	// default invalid command exit code
 	// Validate given environment variables

pkg/varlinkapi/containers.go

@@ -812,7 +812,7 @@ func (i *LibpodAPI) ExecContainer(call iopodman.VarlinkCall, opts iopodman.ExecO
 	go func() {
 		if err := virtwriter.Reader(reader, nil, nil, pipeWriter, resizeChan, nil); err != nil {
 			ecErrChan <- ExitCodeError{
-				125, //TODO FIXME magic number, define package?
+				define.ExecErrorCodeGeneric,
 				err,
 			}
 		}
@@ -833,7 +833,15 @@ func (i *LibpodAPI) ExecContainer(call iopodman.VarlinkCall, opts iopodman.ExecO
 	ecErr := <-ecErrChan
 
 	// TODO FIXME prevent all of these conversions
-	if err = virtwriter.HangUp(writer, int(ecErr.ExitCode)); err != nil {
+	exitCode := int(ecErr.ExitCode)
+	if errors.Cause(ecErr.Error) == define.ErrOCIRuntimePermissionDenied {
+		exitCode = define.ExecErrorCodeCannotInvoke
+	}
+	if errors.Cause(ecErr.Error) == define.ErrOCIRuntimeNotFound {
+		exitCode = define.ExecErrorCodeNotFound
+	}
+
+	if err = virtwriter.HangUp(writer, exitCode); err != nil {
 		logrus.Errorf("ExecContainer failed to HANG-UP on %s: %s", ctr.ID(), err.Error())
 	}