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>

6 files changed, 37 insertions, 21 deletions

diff --git a/cmd/podman/exec.go b/cmd/podman/exec.go
index 2e9b9e47e..649a7b0db 100644
--- a/cmd/podman/exec.go
+++ b/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
 }
diff --git a/libpod/container_api.go b/libpod/container_api.go
index 0cce6ca22..cd020e429 100644
--- a/libpod/container_api.go
+++ b/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())
diff --git a/libpod/define/exec_codes.go b/libpod/define/exec_codes.go
new file mode 100644
index 000000000..90a68cf93
--- /dev/null
+++ b/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
+)
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index 47f1b091e..a8bff187c 100644
--- a/pkg/adapter/containers.go
+++ b/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
diff --git a/pkg/adapter/containers_remote.go b/pkg/adapter/containers_remote.go
index 7a9cf94b7..7fce30378 100644
--- a/pkg/adapter/containers_remote.go
+++ b/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
diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go
index a70a87675..fa17bed84 100644
--- a/pkg/varlinkapi/containers.go
+++ b/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())
 	}