Merge pull request #3624 from haircommander/conmon-exec-with-remote-exec

Add remote exec

13 files changed, 291 insertions, 193 deletions

diff --git a/API.md b/API.md
index 31bffeb16..71d09fed8 100755
--- a/API.md
+++ b/API.md
@@ -1591,6 +1591,8 @@ user [?string](#?string)
 workdir [?string](#?string)
 
 env [?[]string](#?[]string)
+
+detachKeys [?string](#?string)
 ### <a name="Image"></a>type Image
 
 
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/cmd/podman/varlink/io.podman.varlink b/cmd/podman/varlink/io.podman.varlink
index cc66e7df0..f5f3250f7 100644
--- a/cmd/podman/varlink/io.podman.varlink
+++ b/cmd/podman/varlink/io.podman.varlink
@@ -514,7 +514,9 @@ type ExecOpts(
     # workdir to run command in container
     workdir: ?string,
     # slice of keyword=value environment variables
-    env: ?[]string
+    env: ?[]string,
+    # string of detach keys
+    detachKeys: ?string
 )
 
 # GetVersion returns version and build information of the podman service
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..7184f1e59
--- /dev/null
+++ b/libpod/define/exec_codes.go
@@ -0,0 +1,30 @@
+package define
+
+import (
+	"github.com/pkg/errors"
+)
+
+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
+)
+
+// TranslateExecErrorToExitCode takes an error and checks whether it
+// has a predefined exit code associated. If so, it returns that, otherwise it returns
+// the exit code originally stated in libpod.Exec()
+func TranslateExecErrorToExitCode(originalEC int, err error) int {
+	if errors.Cause(err) == ErrOCIRuntimePermissionDenied {
+		return ExecErrorCodeCannotInvoke
+	}
+	if errors.Cause(err) == ErrOCIRuntimeNotFound {
+		return ExecErrorCodeNotFound
+	}
+	return originalEC
+}
diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index 47f1b091e..faaef3e60 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -1000,7 +1000,8 @@ 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)
+	return define.TranslateExecErrorToExitCode(ec, err), err
 }
 
 // Prune removes stopped containers
diff --git a/pkg/adapter/containers_remote.go b/pkg/adapter/containers_remote.go
index 6b9fc8ee7..5a26f537f 100644
--- a/pkg/adapter/containers_remote.go
+++ b/pkg/adapter/containers_remote.go
@@ -3,6 +3,7 @@
 package adapter
 
 import (
+	"bufio"
 	"context"
 	"encoding/json"
 	"fmt"
@@ -555,93 +556,6 @@ func (r *LocalRuntime) Ps(c *cliconfig.PsValues, opts shared.PsOptions) ([]share
 	return psContainers, nil
 }
 
-func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid string, start bool, detachKeys string) (chan error, error) {
-	var (
-		oldTermState *term.State
-	)
-	errChan := make(chan error)
-	spec, err := r.Spec(cid)
-	if err != nil {
-		return nil, err
-	}
-	resize := make(chan remotecommand.TerminalSize, 5)
-	haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd()))
-
-	// Check if we are attached to a terminal. If we are, generate resize
-	// events, and set the terminal to raw mode
-	if haveTerminal && spec.Process.Terminal {
-		logrus.Debugf("Handling terminal attach")
-
-		subCtx, cancel := context.WithCancel(ctx)
-		defer cancel()
-
-		resizeTty(subCtx, resize)
-		oldTermState, err = term.SaveState(os.Stdin.Fd())
-		if err != nil {
-			return nil, errors.Wrapf(err, "unable to save terminal state")
-		}
-
-		logrus.SetFormatter(&RawTtyFormatter{})
-		term.SetRawTerminal(os.Stdin.Fd())
-
-	}
-	// TODO add detach keys support
-	reply, err := iopodman.Attach().Send(r.Conn, varlink.Upgrade, cid, detachKeys, start)
-	if err != nil {
-		restoreTerminal(oldTermState)
-		return nil, err
-	}
-
-	// See if the server accepts the upgraded connection or returns an error
-	_, err = reply()
-
-	if err != nil {
-		restoreTerminal(oldTermState)
-		return nil, err
-	}
-
-	// These are the varlink sockets
-	reader := r.Conn.Reader
-	writer := r.Conn.Writer
-
-	// These are the special writers that encode input from the client.
-	varlinkStdinWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.ToStdin)
-	varlinkResizeWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.TerminalResize)
-
-	go func() {
-		// Read from the wire and direct to stdout or stderr
-		err := virtwriter.Reader(reader, stdout, os.Stderr, nil, nil)
-		defer restoreTerminal(oldTermState)
-		errChan <- err
-	}()
-
-	go func() {
-		for termResize := range resize {
-			b, err := json.Marshal(termResize)
-			if err != nil {
-				defer restoreTerminal(oldTermState)
-				errChan <- err
-			}
-			_, err = varlinkResizeWriter.Write(b)
-			if err != nil {
-				defer restoreTerminal(oldTermState)
-				errChan <- err
-			}
-		}
-	}()
-
-	// Takes stdinput and sends it over the wire after being encoded
-	go func() {
-		if _, err := io.Copy(varlinkStdinWriter, stdin); err != nil {
-			defer restoreTerminal(oldTermState)
-			errChan <- err
-		}
-
-	}()
-	return errChan, nil
-
-}
-
 // Attach to a remote terminal
 func (r *LocalRuntime) Attach(ctx context.Context, c *cliconfig.AttachValues) error {
 	ctr, err := r.LookupContainer(c.InputArgs[0])
@@ -796,6 +710,49 @@ func (r *LocalRuntime) Start(ctx context.Context, c *cliconfig.StartValues, sigP
 	return exitCode, finalErr
 }
 
+func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid string, start bool, detachKeys string) (chan error, error) {
+	var (
+		oldTermState *term.State
+	)
+	spec, err := r.Spec(cid)
+	if err != nil {
+		return nil, err
+	}
+	resize := make(chan remotecommand.TerminalSize, 5)
+	haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd()))
+
+	// Check if we are attached to a terminal. If we are, generate resize
+	// events, and set the terminal to raw mode
+	if haveTerminal && spec.Process.Terminal {
+		cancel, oldTermState, err := handleTerminalAttach(ctx, resize)
+		if err != nil {
+			return nil, err
+		}
+		defer cancel()
+		defer restoreTerminal(oldTermState)
+
+		logrus.SetFormatter(&RawTtyFormatter{})
+		term.SetRawTerminal(os.Stdin.Fd())
+	}
+
+	reply, err := iopodman.Attach().Send(r.Conn, varlink.Upgrade, cid, detachKeys, start)
+	if err != nil {
+		restoreTerminal(oldTermState)
+		return nil, err
+	}
+
+	// See if the server accepts the upgraded connection or returns an error
+	_, err = reply()
+
+	if err != nil {
+		restoreTerminal(oldTermState)
+		return nil, err
+	}
+
+	errChan := configureVarlinkAttachStdio(r.Conn.Reader, r.Conn.Writer, stdin, stdout, oldTermState, resize, nil)
+	return errChan, nil
+}
+
 // PauseContainers pauses container(s) based on CLI inputs.
 func (r *LocalRuntime) PauseContainers(ctx context.Context, cli *cliconfig.PauseValues) ([]string, map[string]error, error) {
 	var (
@@ -1037,8 +994,11 @@ func (r *LocalRuntime) Commit(ctx context.Context, c *cliconfig.CommitValues, co
 
 // ExecContainer executes a command in the container
 func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecValues) (int, error) {
+	var (
+		oldTermState *term.State
+		ec           int = define.ExecErrorCodeGeneric
+	)
 	// default invalid command exit code
-	ec := 125
 	// Validate given environment variables
 	env := map[string]string{}
 	if err := parse.ReadKVStrings(env, []string{}, cli.Env); err != nil {
@@ -1051,6 +1011,23 @@ func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecVal
 		envs = append(envs, fmt.Sprintf("%s=%s", k, v))
 	}
 
+	resize := make(chan remotecommand.TerminalSize, 5)
+	haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd()))
+
+	// Check if we are attached to a terminal. If we are, generate resize
+	// events, and set the terminal to raw mode
+	if haveTerminal && cli.Tty {
+		cancel, oldTermState, err := handleTerminalAttach(ctx, resize)
+		if err != nil {
+			return ec, err
+		}
+		defer cancel()
+		defer restoreTerminal(oldTermState)
+
+		logrus.SetFormatter(&RawTtyFormatter{})
+		term.SetRawTerminal(os.Stdin.Fd())
+	}
+
 	opts := iopodman.ExecOpts{
 		Name:       cli.InputArgs[0],
 		Tty:        cli.Tty,
@@ -1059,18 +1036,79 @@ func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecVal
 		User:       &cli.User,
 		Workdir:    &cli.Workdir,
 		Env:        &envs,
+		DetachKeys: &cli.DetachKeys,
+	}
+
+	inputStream := os.Stdin
+	if !cli.Interactive {
+		inputStream = nil
 	}
 
-	receive, err := iopodman.ExecContainer().Send(r.Conn, varlink.Upgrade, opts)
+	reply, err := iopodman.ExecContainer().Send(r.Conn, varlink.Upgrade, opts)
 	if err != nil {
 		return ec, errors.Wrapf(err, "Exec failed to contact service for %s", cli.InputArgs)
 	}
 
-	_, err = receive()
+	_, err = reply()
 	if err != nil {
 		return ec, errors.Wrapf(err, "Exec operation failed for %s", cli.InputArgs)
 	}
+	ecChan := make(chan int, 1)
+	errChan := configureVarlinkAttachStdio(r.Conn.Reader, r.Conn.Writer, inputStream, os.Stdout, oldTermState, resize, ecChan)
+
+	ec = <-ecChan
+	err = <-errChan
+
+	return ec, err
+}
+
+func configureVarlinkAttachStdio(reader *bufio.Reader, writer *bufio.Writer, stdin *os.File, stdout *os.File, oldTermState *term.State, resize chan remotecommand.TerminalSize, ecChan chan int) chan error {
+	errChan := make(chan error, 1)
+	// These are the special writers that encode input from the client.
+	varlinkStdinWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.ToStdin)
+	varlinkResizeWriter := virtwriter.NewVirtWriteCloser(writer, virtwriter.TerminalResize)
+
+	go func() {
+		// Read from the wire and direct to stdout or stderr
+		err := virtwriter.Reader(reader, stdout, os.Stderr, nil, nil, ecChan)
+		defer restoreTerminal(oldTermState)
+		sendGenericError(ecChan)
+		errChan <- err
+	}()
+
+	go func() {
+		for termResize := range resize {
+			b, err := json.Marshal(termResize)
+			if err != nil {
+				defer restoreTerminal(oldTermState)
+				sendGenericError(ecChan)
+				errChan <- err
+			}
+			_, err = varlinkResizeWriter.Write(b)
+			if err != nil {
+				defer restoreTerminal(oldTermState)
+				sendGenericError(ecChan)
+				errChan <- err
+			}
+		}
+	}()
+
+	if stdin != nil {
+		// Takes stdinput and sends it over the wire after being encoded
+		go func() {
+			if _, err := io.Copy(varlinkStdinWriter, stdin); err != nil {
+				defer restoreTerminal(oldTermState)
+				sendGenericError(ecChan)
+				errChan <- err
+			}
+
+		}()
+	}
+	return errChan
+}
 
-	// TODO return exit code from exec call
-	return 0, nil
+func sendGenericError(ecChan chan int) {
+	if ecChan != nil {
+		ecChan <- define.ExecErrorCodeGeneric
+	}
 }
diff --git a/pkg/adapter/terminal.go b/pkg/adapter/terminal.go
index 373c78322..51b747d23 100644
--- a/pkg/adapter/terminal.go
+++ b/pkg/adapter/terminal.go
@@ -7,6 +7,7 @@ import (
 
 	"github.com/docker/docker/pkg/signal"
 	"github.com/docker/docker/pkg/term"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"k8s.io/client-go/tools/remotecommand"
 )
@@ -76,3 +77,25 @@ func (f *RawTtyFormatter) Format(entry *logrus.Entry) ([]byte, error) {
 
 	return bytes, err
 }
+
+func handleTerminalAttach(ctx context.Context, resize chan remotecommand.TerminalSize) (context.CancelFunc, *term.State, error) {
+	logrus.Debugf("Handling terminal attach")
+
+	subCtx, cancel := context.WithCancel(ctx)
+
+	resizeTty(subCtx, resize)
+
+	oldTermState, err := term.SaveState(os.Stdin.Fd())
+	if err != nil {
+		// allow caller to not have to do any cleaning up if we error here
+		cancel()
+		return nil, nil, errors.Wrapf(err, "unable to save terminal state")
+	}
+
+	logrus.SetFormatter(&RawTtyFormatter{})
+	if _, err := term.SetRawTerminal(os.Stdin.Fd()); err != nil {
+		return cancel, nil, err
+	}
+
+	return cancel, oldTermState, nil
+}
diff --git a/pkg/adapter/terminal_linux.go b/pkg/adapter/terminal_linux.go
index de2600b75..26cfd7b5e 100644
--- a/pkg/adapter/terminal_linux.go
+++ b/pkg/adapter/terminal_linux.go
@@ -6,7 +6,6 @@ import (
 	"os"
 
 	"github.com/containers/libpod/libpod"
-	"github.com/docker/docker/pkg/term"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"golang.org/x/crypto/ssh/terminal"
@@ -108,25 +107,3 @@ func StartAttachCtr(ctx context.Context, ctr *libpod.Container, stdout, stderr,
 
 	return nil
 }
-
-func handleTerminalAttach(ctx context.Context, resize chan remotecommand.TerminalSize) (context.CancelFunc, *term.State, error) {
-	logrus.Debugf("Handling terminal attach")
-
-	subCtx, cancel := context.WithCancel(ctx)
-
-	resizeTty(subCtx, resize)
-
-	oldTermState, err := term.SaveState(os.Stdin.Fd())
-	if err != nil {
-		// allow caller to not have to do any cleaning up if we error here
-		cancel()
-		return nil, nil, errors.Wrapf(err, "unable to save terminal state")
-	}
-
-	logrus.SetFormatter(&RawTtyFormatter{})
-	if _, err := term.SetRawTerminal(os.Stdin.Fd()); err != nil {
-		return cancel, nil, err
-	}
-
-	return cancel, oldTermState, nil
-}
diff --git a/pkg/varlinkapi/attach.go b/pkg/varlinkapi/attach.go
index 97ba525a5..1f8d48eb9 100644
--- a/pkg/varlinkapi/attach.go
+++ b/pkg/varlinkapi/attach.go
@@ -68,7 +68,7 @@ func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string, detachKeys st
 	reader, writer, _, pw, streams := setupStreams(call)
 
 	go func() {
-		if err := virtwriter.Reader(reader, nil, nil, pw, resize); err != nil {
+		if err := virtwriter.Reader(reader, nil, nil, pw, resize, nil); err != nil {
 			errChan <- err
 		}
 	}()
@@ -83,7 +83,7 @@ func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string, detachKeys st
 		logrus.Error(finalErr)
 	}
 
-	if err = virtwriter.HangUp(writer); err != nil {
+	if err = virtwriter.HangUp(writer, 0); err != nil {
 		logrus.Errorf("Failed to HANG-UP attach to %s: %s", ctr.ID(), err.Error())
 	}
 	return call.Writer.Flush()
diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go
index 19a8bfd2e..cd5f305c9 100644
--- a/pkg/varlinkapi/containers.go
+++ b/pkg/varlinkapi/containers.go
@@ -782,6 +782,9 @@ func (i *LibpodAPI) ExecContainer(call iopodman.VarlinkCall, opts iopodman.ExecO
 			fmt.Sprintf("exec requires a running container, %s is %s", ctr.ID(), state.String()))
 	}
 
+	// ACK the client upgrade request
+	call.ReplyExecContainer()
+
 	envs := []string{}
 	if opts.Env != nil {
 		envs = *opts.Env
@@ -797,44 +800,52 @@ func (i *LibpodAPI) ExecContainer(call iopodman.VarlinkCall, opts iopodman.ExecO
 		workDir = *opts.Workdir
 	}
 
+	var detachKeys string
+	if opts.DetachKeys != nil {
+		detachKeys = *opts.DetachKeys
+	}
+
 	resizeChan := make(chan remotecommand.TerminalSize)
-	errChan := make(chan error)
 
 	reader, writer, _, pipeWriter, streams := setupStreams(call)
 
+	type ExitCodeError struct {
+		ExitCode uint32
+		Error    error
+	}
+	ecErrChan := make(chan ExitCodeError, 1)
+
 	go func() {
-		fmt.Printf("ExecContainer Start Reader\n")
-		if err := virtwriter.Reader(reader, nil, nil, pipeWriter, resizeChan); err != nil {
-			fmt.Printf("ExecContainer Reader err %s, %s\n", err.Error(), errors.Cause(err).Error())
-			errChan <- err
+		if err := virtwriter.Reader(reader, nil, nil, pipeWriter, resizeChan, nil); err != nil {
+			ecErrChan <- ExitCodeError{
+				define.ExecErrorCodeGeneric,
+				err,
+			}
 		}
 	}()
 
-	// Debugging...
-	time.Sleep(5 * time.Second)
-
 	go func() {
-		fmt.Printf("ExecContainer Start ctr.Exec\n")
-		// TODO detach keys and resize
-		// TODO add handling for exit code
-		// TODO capture exit code and return to main thread
-		_, err := ctr.Exec(opts.Tty, opts.Privileged, envs, opts.Cmd, user, workDir, streams, 0, nil, "")
+		ec, err := ctr.Exec(opts.Tty, opts.Privileged, envs, opts.Cmd, user, workDir, streams, 0, resizeChan, detachKeys)
 		if err != nil {
-			fmt.Printf("ExecContainer Exec err %s, %s\n", err.Error(), errors.Cause(err).Error())
-			errChan <- errors.Wrapf(err, "ExecContainer failed for container %s", ctr.ID())
+			logrus.Errorf(err.Error())
+		}
+		ecErrChan <- ExitCodeError{
+			uint32(ec),
+			err,
 		}
 	}()
 
-	execErr := <-errChan
+	ecErr := <-ecErrChan
 
-	if execErr != nil && errors.Cause(execErr) != io.EOF {
-		fmt.Printf("ExecContainer err: %s\n", execErr.Error())
-		return call.ReplyErrorOccurred(execErr.Error())
-	}
+	exitCode := define.TranslateExecErrorToExitCode(int(ecErr.ExitCode), ecErr.Error)
 
-	if err = virtwriter.HangUp(writer); err != nil {
-		fmt.Printf("ExecContainer hangup err: %s\n", err.Error())
+	if err = virtwriter.HangUp(writer, uint32(exitCode)); err != nil {
 		logrus.Errorf("ExecContainer failed to HANG-UP on %s: %s", ctr.ID(), err.Error())
 	}
-	return call.Writer.Flush()
+
+	if err := call.Writer.Flush(); err != nil {
+		logrus.Errorf("Exec Container err: %s", err.Error())
+	}
+
+	return ecErr.Error
 }
diff --git a/pkg/varlinkapi/virtwriter/virtwriter.go b/pkg/varlinkapi/virtwriter/virtwriter.go
index 0da2a91fc..27ecd1f52 100644
--- a/pkg/varlinkapi/virtwriter/virtwriter.go
+++ b/pkg/varlinkapi/virtwriter/virtwriter.go
@@ -89,10 +89,14 @@ func (v VirtWriteCloser) Write(input []byte) (int, error) {
 }
 
 // Reader decodes the content that comes over the wire and directs it to the proper destination.
-func Reader(r *bufio.Reader, output io.Writer, errput io.Writer, input io.Writer, resize chan remotecommand.TerminalSize) error {
+func Reader(r *bufio.Reader, output, errput, input io.Writer, resize chan remotecommand.TerminalSize, execEcChan chan int) error {
 	var messageSize int64
 	headerBytes := make([]byte, 8)
 
+	if r == nil {
+		return errors.Errorf("Reader must not be nil")
+	}
+
 	for {
 		n, err := io.ReadFull(r, headerBytes)
 		if err != nil {
@@ -106,35 +110,43 @@ func Reader(r *bufio.Reader, output io.Writer, errput io.Writer, input io.Writer
 
 		switch IntToSocketDest(int(headerBytes[0])) {
 		case ToStdout:
-			_, err := io.CopyN(output, r, messageSize)
-			if err != nil {
-				return err
+			if output != nil {
+				_, err := io.CopyN(output, r, messageSize)
+				if err != nil {
+					return err
+				}
 			}
 		case ToStderr:
-			_, err := io.CopyN(errput, r, messageSize)
-			if err != nil {
-				return err
+			if errput != nil {
+				_, err := io.CopyN(errput, r, messageSize)
+				if err != nil {
+					return err
+				}
 			}
 		case ToStdin:
-			_, err := io.CopyN(input, r, messageSize)
-			if err != nil {
-				return err
-			}
-		case TerminalResize:
-			out := make([]byte, messageSize)
-			if messageSize > 0 {
-				_, err = io.ReadFull(r, out)
-
+			if input != nil {
+				_, err := io.CopyN(input, r, messageSize)
 				if err != nil {
 					return err
 				}
 			}
-			// Resize events come over in bytes, need to be reserialized
-			resizeEvent := remotecommand.TerminalSize{}
-			if err := json.Unmarshal(out, &resizeEvent); err != nil {
-				return err
+		case TerminalResize:
+			if resize != nil {
+				out := make([]byte, messageSize)
+				if messageSize > 0 {
+					_, err = io.ReadFull(r, out)
+
+					if err != nil {
+						return err
+					}
+				}
+				// Resize events come over in bytes, need to be reserialized
+				resizeEvent := remotecommand.TerminalSize{}
+				if err := json.Unmarshal(out, &resizeEvent); err != nil {
+					return err
+				}
+				resize <- resizeEvent
 			}
-			resize <- resizeEvent
 		case Quit:
 			out := make([]byte, messageSize)
 			if messageSize > 0 {
@@ -144,6 +156,10 @@ func Reader(r *bufio.Reader, output io.Writer, errput io.Writer, input io.Writer
 					return err
 				}
 			}
+			if execEcChan != nil {
+				ecInt := binary.BigEndian.Uint32(out)
+				execEcChan <- int(ecInt)
+			}
 			return nil
 
 		default:
@@ -154,9 +170,11 @@ func Reader(r *bufio.Reader, output io.Writer, errput io.Writer, input io.Writer
 }
 
 // HangUp sends message to peer to close connection
-func HangUp(writer *bufio.Writer) (err error) {
+func HangUp(writer *bufio.Writer, ec uint32) (err error) {
 	n := 0
-	msg := []byte("HANG-UP")
+	msg := make([]byte, 4)
+
+	binary.BigEndian.PutUint32(msg, ec)
 
 	writeQuit := NewVirtWriteCloser(writer, Quit)
 	if n, err = writeQuit.Write(msg); err != nil {
diff --git a/test/e2e/exec_test.go b/test/e2e/exec_test.go
index 76515cf3e..6cf78a25c 100644
--- a/test/e2e/exec_test.go
+++ b/test/e2e/exec_test.go
@@ -1,5 +1,3 @@
-// +build !remoteclient
-
 package integration
 
 import (
@@ -67,6 +65,8 @@ var _ = Describe("Podman exec", func() {
 	})
 
 	It("podman exec simple command using latest", func() {
+		// the remote client doesn't use latest
+		SkipIfRemote()
 		setup := podmanTest.RunTopContainer("test1")
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
@@ -81,27 +81,35 @@ var _ = Describe("Podman exec", func() {
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
 
-		session := podmanTest.Podman([]string{"exec", "-l", "--env", "FOO=BAR", "printenv", "FOO"})
+		session := podmanTest.Podman([]string{"exec", "--env", "FOO=BAR", "test1", "printenv", "FOO"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ := session.GrepString("BAR")
 		Expect(match).Should(BeTrue())
 
-		session = podmanTest.Podman([]string{"exec", "-l", "--env", "PATH=/bin", "printenv", "PATH"})
+		session = podmanTest.Podman([]string{"exec", "--env", "PATH=/bin", "test1", "printenv", "PATH"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ = session.GrepString("/bin")
 		Expect(match).Should(BeTrue())
+	})
+
+	It("podman exec os.Setenv env", func() {
+		// remote doesn't properly interpret os.Setenv
+		SkipIfRemote()
+		setup := podmanTest.RunTopContainer("test1")
+		setup.WaitWithDefaultTimeout()
+		Expect(setup.ExitCode()).To(Equal(0))
 
 		os.Setenv("FOO", "BAR")
-		session = podmanTest.Podman([]string{"exec", "-l", "--env", "FOO", "printenv", "FOO"})
+		session := podmanTest.Podman([]string{"exec", "--env", "FOO", "test1", "printenv", "FOO"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
-		match, _ = session.GrepString("BAR")
+		match, _ := session.GrepString("BAR")
 		Expect(match).Should(BeTrue())
 		os.Unsetenv("FOO")
-
 	})
+
 	It("podman exec exit code", func() {
 		setup := podmanTest.RunTopContainer("test1")
 		setup.WaitWithDefaultTimeout()
@@ -143,13 +151,13 @@ var _ = Describe("Podman exec", func() {
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
 
-		session := podmanTest.Podman([]string{"exec", "-l", "--workdir", "/tmp", "pwd"})
+		session := podmanTest.Podman([]string{"exec", "--workdir", "/tmp", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ := session.GrepString("/tmp")
 		Expect(match).Should(BeTrue())
 
-		session = podmanTest.Podman([]string{"exec", "-l", "-w", "/tmp", "pwd"})
+		session = podmanTest.Podman([]string{"exec", "-w", "/tmp", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 		match, _ = session.GrepString("/tmp")
@@ -161,11 +169,11 @@ var _ = Describe("Podman exec", func() {
 		setup.WaitWithDefaultTimeout()
 		Expect(setup.ExitCode()).To(Equal(0))
 
-		session := podmanTest.Podman([]string{"exec", "-l", "--workdir", "/missing", "pwd"})
+		session := podmanTest.Podman([]string{"exec", "--workdir", "/missing", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(1))
 
-		session = podmanTest.Podman([]string{"exec", "-l", "-w", "/missing", "pwd"})
+		session = podmanTest.Podman([]string{"exec", "-w", "/missing", "test1", "pwd"})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(1))
 	})