Merge pull request #3934 from rhatdan/wait

Podman-remote run should wait for exit code

4 files changed, 73 insertions, 37 deletions

diff --git a/pkg/adapter/containers.go b/pkg/adapter/containers.go
index 41607145d..47db5c0dc 100644
--- a/pkg/adapter/containers.go
+++ b/pkg/adapter/containers.go
@@ -341,12 +341,7 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 		// if the container was created as part of a pod, also start its dependencies, if any.
 		if err := ctr.Start(ctx, c.IsSet("pod")); err != nil {
 			// This means the command did not exist
-			exitCode = 127
-			e := strings.ToLower(err.Error())
-			if strings.Contains(e, "permission denied") || strings.Contains(e, "operation not permitted") || strings.Contains(e, "file not found") || strings.Contains(e, "no such file or directory") {
-				exitCode = 126
-			}
-			return exitCode, err
+			return define.ExitCode(err), err
 		}
 
 		fmt.Printf("%s\n", ctr.ID())
@@ -401,21 +396,14 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 		// Do not perform cleanup, or wait for container exit code
 		// Just exit immediately
 		if errors.Cause(err) == define.ErrDetach {
-			exitCode = 0
-			return exitCode, nil
-		}
-		// This means the command did not exist
-		exitCode = 127
-		e := strings.ToLower(err.Error())
-		if strings.Contains(e, "permission denied") || strings.Contains(e, "operation not permitted") {
-			exitCode = 126
+			return 0, nil
 		}
 		if c.IsSet("rm") {
 			if deleteError := r.Runtime.RemoveContainer(ctx, ctr, true, false); deleteError != nil {
 				logrus.Debugf("unable to remove container %s after failing to start and attach to it", ctr.ID())
 			}
 		}
-		return exitCode, err
+		return define.ExitCode(err), err
 	}
 
 	if ecode, err := ctr.Wait(); err != nil {
@@ -424,7 +412,7 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 			event, err := r.Runtime.GetLastContainerEvent(ctr.ID(), events.Exited)
 			if err != nil {
 				logrus.Errorf("Cannot get exit code: %v", err)
-				exitCode = 127
+				exitCode = define.ExecErrorCodeNotFound
 			} else {
 				exitCode = event.ContainerExitCode
 			}
@@ -576,7 +564,7 @@ func (r *LocalRuntime) Restore(ctx context.Context, c *cliconfig.RestoreValues)
 // Start will start a container
 func (r *LocalRuntime) Start(ctx context.Context, c *cliconfig.StartValues, sigProxy bool) (int, error) {
 	var (
-		exitCode  = 125
+		exitCode  = define.ExecErrorCodeGeneric
 		lastError error
 	)
 
@@ -636,7 +624,7 @@ func (r *LocalRuntime) Start(ctx context.Context, c *cliconfig.StartValues, sigP
 					event, err := r.Runtime.GetLastContainerEvent(ctr.ID(), events.Exited)
 					if err != nil {
 						logrus.Errorf("Cannot get exit code: %v", err)
-						exitCode = 127
+						exitCode = define.ExecErrorCodeNotFound
 					} else {
 						exitCode = event.ContainerExitCode
 					}
@@ -914,7 +902,7 @@ func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecVal
 		cmd []string
 	)
 	// default invalid command exit code
-	ec := 125
+	ec := define.ExecErrorCodeGeneric
 
 	if cli.Latest {
 		if ctr, err = r.GetLatestContainer(); err != nil {
diff --git a/pkg/adapter/containers_remote.go b/pkg/adapter/containers_remote.go
index 590fef43f..01e008e87 100644
--- a/pkg/adapter/containers_remote.go
+++ b/pkg/adapter/containers_remote.go
@@ -464,19 +464,22 @@ func (r *LocalRuntime) Run(ctx context.Context, c *cliconfig.RunValues, exitCode
 	results := shared.NewIntermediateLayer(&c.PodmanCommand, true)
 	cid, err := iopodman.CreateContainer().Call(r.Conn, results.MakeVarlink())
 	if err != nil {
-		return 0, err
+		return exitCode, err
 	}
 	if c.Bool("detach") {
-		_, err := iopodman.StartContainer().Call(r.Conn, cid)
+		if _, err := iopodman.StartContainer().Call(r.Conn, cid); err != nil {
+			return exitCode, err
+		}
 		fmt.Println(cid)
-		return 0, err
+		return 0, nil
 	}
-	errChan, err := r.attach(ctx, os.Stdin, os.Stdout, cid, true, c.String("detach-keys"))
+	exitChan, errChan, err := r.attach(ctx, os.Stdin, os.Stdout, cid, true, c.String("detach-keys"))
 	if err != nil {
-		return 0, err
+		return exitCode, err
 	}
+	exitCode = <-exitChan
 	finalError := <-errChan
-	return 0, finalError
+	return exitCode, finalError
 }
 
 func ReadExitFile(runtimeTmp, ctrID string) (int, error) {
@@ -572,7 +575,7 @@ func (r *LocalRuntime) Attach(ctx context.Context, c *cliconfig.AttachValues) er
 			return err
 		}
 	}
-	errChan, err := r.attach(ctx, inputStream, os.Stdout, c.InputArgs[0], false, c.DetachKeys)
+	_, errChan, err := r.attach(ctx, inputStream, os.Stdout, c.InputArgs[0], false, c.DetachKeys)
 	if err != nil {
 		return err
 	}
@@ -669,7 +672,7 @@ func (r *LocalRuntime) Restore(ctx context.Context, c *cliconfig.RestoreValues)
 func (r *LocalRuntime) Start(ctx context.Context, c *cliconfig.StartValues, sigProxy bool) (int, error) {
 	var (
 		finalErr error
-		exitCode = 125
+		exitCode = define.ExecErrorCodeGeneric
 	)
 	// TODO Figure out how to deal with exit codes
 	inputStream := os.Stdin
@@ -686,12 +689,13 @@ func (r *LocalRuntime) Start(ctx context.Context, c *cliconfig.StartValues, sigP
 	}
 	// start.go makes sure that if attach, there can be only one ctr
 	if c.Attach {
-		errChan, err := r.attach(ctx, inputStream, os.Stdout, containerIDs[0], true, c.DetachKeys)
+		exitChan, errChan, err := r.attach(ctx, inputStream, os.Stdout, containerIDs[0], true, c.DetachKeys)
 		if err != nil {
 			return exitCode, nil
 		}
+		exitCode := <-exitChan
 		err = <-errChan
-		return 0, err
+		return exitCode, err
 	}
 
 	// TODO the notion of starting a pod container and its deps still needs to be worked through
@@ -710,13 +714,13 @@ 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) {
+func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid string, start bool, detachKeys string) (chan int, chan error, error) {
 	var (
 		oldTermState *term.State
 	)
 	spec, err := r.Spec(cid)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
 	resize := make(chan remotecommand.TerminalSize, 5)
 	haveTerminal := terminal.IsTerminal(int(os.Stdin.Fd()))
@@ -726,7 +730,7 @@ func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid s
 	if haveTerminal && spec.Process.Terminal {
 		cancel, oldTermState, err := handleTerminalAttach(ctx, resize)
 		if err != nil {
-			return nil, err
+			return nil, nil, err
 		}
 		defer cancel()
 		defer restoreTerminal(oldTermState)
@@ -738,7 +742,7 @@ func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid s
 	reply, err := iopodman.Attach().Send(r.Conn, varlink.Upgrade, cid, detachKeys, start)
 	if err != nil {
 		restoreTerminal(oldTermState)
-		return nil, err
+		return nil, nil, err
 	}
 
 	// See if the server accepts the upgraded connection or returns an error
@@ -746,11 +750,12 @@ func (r *LocalRuntime) attach(ctx context.Context, stdin, stdout *os.File, cid s
 
 	if err != nil {
 		restoreTerminal(oldTermState)
-		return nil, err
+		return nil, nil, err
 	}
 
-	errChan := configureVarlinkAttachStdio(r.Conn.Reader, r.Conn.Writer, stdin, stdout, oldTermState, resize, nil)
-	return errChan, nil
+	ecChan := make(chan int, 1)
+	errChan := configureVarlinkAttachStdio(r.Conn.Reader, r.Conn.Writer, stdin, stdout, oldTermState, resize, ecChan)
+	return ecChan, errChan, nil
 }
 
 // PauseContainers pauses container(s) based on CLI inputs.
diff --git a/pkg/util/utils.go b/pkg/util/utils.go
index 2261934f0..583bf5d18 100644
--- a/pkg/util/utils.go
+++ b/pkg/util/utils.go
@@ -377,3 +377,19 @@ func ValidatePullType(pullType string) (PullType, error) {
 		return PullImageMissing, errors.Errorf("invalid pull type %q", pullType)
 	}
 }
+
+// ExitCode reads the error message when failing to executing container process
+// and then returns 0 if no error, 126 if command does not exist, or 127 for
+// all other errors
+func ExitCode(err error) int {
+	if err == nil {
+		return 0
+	}
+	e := strings.ToLower(err.Error())
+	if strings.Contains(e, "file not found") ||
+		strings.Contains(e, "no such file or directory") {
+		return 127
+	}
+
+	return 126
+}
diff --git a/pkg/varlinkapi/attach.go b/pkg/varlinkapi/attach.go
index 1f8d48eb9..3bd487849 100644
--- a/pkg/varlinkapi/attach.go
+++ b/pkg/varlinkapi/attach.go
@@ -9,7 +9,9 @@ import (
 	"github.com/containers/libpod/cmd/podman/varlink"
 	"github.com/containers/libpod/libpod"
 	"github.com/containers/libpod/libpod/define"
+	"github.com/containers/libpod/libpod/events"
 	"github.com/containers/libpod/pkg/varlinkapi/virtwriter"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"k8s.io/client-go/tools/remotecommand"
 )
@@ -79,11 +81,36 @@ func (i *LibpodAPI) Attach(call iopodman.VarlinkCall, name string, detachKeys st
 		finalErr = startAndAttach(ctr, streams, detachKeys, resize, errChan)
 	}
 
+	exitCode := define.ExitCode(finalErr)
 	if finalErr != define.ErrDetach && finalErr != nil {
 		logrus.Error(finalErr)
+	} else {
+		if ecode, err := ctr.Wait(); err != nil {
+			if errors.Cause(err) == define.ErrNoSuchCtr {
+				// Check events
+				event, err := i.Runtime.GetLastContainerEvent(ctr.ID(), events.Exited)
+				if err != nil {
+					logrus.Errorf("Cannot get exit code: %v", err)
+					exitCode = define.ExecErrorCodeNotFound
+				} else {
+					exitCode = event.ContainerExitCode
+				}
+			} else {
+				exitCode = define.ExitCode(err)
+			}
+		} else {
+			exitCode = int(ecode)
+		}
+	}
+
+	if ctr.AutoRemove() {
+		err := i.Runtime.RemoveContainer(getContext(), ctr, false, false)
+		if err != nil {
+			logrus.Errorf("Failed to remove container %s: %s", ctr.ID(), err.Error())
+		}
 	}
 
-	if err = virtwriter.HangUp(writer, 0); err != nil {
+	if err = virtwriter.HangUp(writer, uint32(exitCode)); err != nil {
 		logrus.Errorf("Failed to HANG-UP attach to %s: %s", ctr.ID(), err.Error())
 	}
 	return call.Writer.Flush()