diff options
-rw-r--r-- | cmd/podman/containers_prune.go | 4 | ||||
-rw-r--r-- | cmd/podman/main.go | 8 | ||||
-rw-r--r-- | cmd/podman/pause.go | 4 | ||||
-rw-r--r-- | cmd/podman/restart.go | 4 | ||||
-rw-r--r-- | cmd/podman/unpause.go | 4 | ||||
-rw-r--r-- | libpod/container_api.go | 13 | ||||
-rw-r--r-- | libpod/define/exec_codes.go | 18 | ||||
-rw-r--r-- | pkg/adapter/containers.go | 26 | ||||
-rw-r--r-- | pkg/adapter/containers_remote.go | 39 | ||||
-rw-r--r-- | pkg/util/utils.go | 16 | ||||
-rw-r--r-- | pkg/varlinkapi/attach.go | 29 | ||||
-rw-r--r-- | test/e2e/run_exit_test.go | 15 | ||||
-rw-r--r-- | test/e2e/run_selinux_test.go | 2 | ||||
-rw-r--r-- | test/system/030-run.bats | 1 |
14 files changed, 123 insertions, 60 deletions
diff --git a/cmd/podman/containers_prune.go b/cmd/podman/containers_prune.go index b8a84a0e3..3d0fef37d 100644 --- a/cmd/podman/containers_prune.go +++ b/cmd/podman/containers_prune.go @@ -53,7 +53,7 @@ func pruneContainersCmd(c *cliconfig.PruneContainersValues) error { if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { if len(c.InputArgs) > 1 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } else { exitCode = 1 } @@ -61,7 +61,7 @@ func pruneContainersCmd(c *cliconfig.PruneContainersValues) error { return err } if len(failures) > 0 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } return printCmdResults(ok, failures) } diff --git a/cmd/podman/main.go b/cmd/podman/main.go index 2b808b2bc..b83ccd9a5 100644 --- a/cmd/podman/main.go +++ b/cmd/podman/main.go @@ -8,6 +8,7 @@ import ( "github.com/containers/libpod/cmd/podman/cliconfig" "github.com/containers/libpod/libpod" + "github.com/containers/libpod/libpod/define" _ "github.com/containers/libpod/pkg/hooks/0.1.0" "github.com/containers/libpod/pkg/rootless" "github.com/containers/libpod/version" @@ -20,7 +21,7 @@ import ( // This is populated by the Makefile from the VERSION file // in the repository var ( - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric Ctx context.Context span opentracing.Span closer io.Closer @@ -152,11 +153,12 @@ func main() { if err := rootCmd.Execute(); err != nil { outputError(err) } else { - // The exitCode modified from 125, indicates an application + // The exitCode modified from define.ExecErrorCodeGeneric, + // indicates an application // running inside of a container failed, as opposed to the // podman command failed. Must exit with that exit code // otherwise command exited correctly. - if exitCode == 125 { + if exitCode == define.ExecErrorCodeGeneric { exitCode = 0 } diff --git a/cmd/podman/pause.go b/cmd/podman/pause.go index 3a8f4edb5..247a480e3 100644 --- a/cmd/podman/pause.go +++ b/cmd/podman/pause.go @@ -56,7 +56,7 @@ func pauseCmd(c *cliconfig.PauseValues) error { if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { if len(c.InputArgs) > 1 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } else { exitCode = 1 } @@ -64,7 +64,7 @@ func pauseCmd(c *cliconfig.PauseValues) error { return err } if len(failures) > 0 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } return printCmdResults(ok, failures) } diff --git a/cmd/podman/restart.go b/cmd/podman/restart.go index 494a9ec06..c97fb0dc1 100644 --- a/cmd/podman/restart.go +++ b/cmd/podman/restart.go @@ -61,7 +61,7 @@ func restartCmd(c *cliconfig.RestartValues) error { if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { if len(c.InputArgs) > 1 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } else { exitCode = 1 } @@ -69,7 +69,7 @@ func restartCmd(c *cliconfig.RestartValues) error { return err } if len(failures) > 0 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } return printCmdResults(ok, failures) } diff --git a/cmd/podman/unpause.go b/cmd/podman/unpause.go index 382b64e97..ae24b0e66 100644 --- a/cmd/podman/unpause.go +++ b/cmd/podman/unpause.go @@ -55,7 +55,7 @@ func unpauseCmd(c *cliconfig.UnpauseValues) error { if err != nil { if errors.Cause(err) == define.ErrNoSuchCtr { if len(c.InputArgs) > 1 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } else { exitCode = 1 } @@ -63,7 +63,7 @@ func unpauseCmd(c *cliconfig.UnpauseValues) error { return err } if len(failures) > 0 { - exitCode = 125 + exitCode = define.ExecErrorCodeGeneric } return printCmdResults(ok, failures) } diff --git a/libpod/container_api.go b/libpod/container_api.go index 9bf97c5d4..4f0d5301c 100644 --- a/libpod/container_api.go +++ b/libpod/container_api.go @@ -216,8 +216,8 @@ func (c *Container) Kill(signal uint) error { } // Exec starts a new process inside the container -// Returns an exit code and an error. If Exec was not able to exec in the container before a failure, an exit code of 126 is returned. -// If another generic error happens, an exit code of 125 is returned. +// Returns an exit code and an error. If Exec was not able to exec in the container before a failure, an exit code of define.ExecErrorCodeCannotInvoke is returned. +// If another generic error happens, an exit code of define.ExecErrorCodeGeneric is returned. // Sometimes, the $RUNTIME exec call errors, and if that is the case, the exit code is the exit code of the call. // Otherwise, the exit code will be the exit code of the executed call inside of the container. // TODO investigate allowing exec without attaching @@ -821,3 +821,12 @@ func (c *Container) Restore(ctx context.Context, options ContainerCheckpointOpti defer c.newContainerEvent(events.Restore) return c.restore(ctx, options) } + +// AutoRemove indicates whether the container will be removed after it is executed +func (c *Container) AutoRemove() bool { + spec := c.config.Spec + if spec.Annotations == nil { + return false + } + return c.Spec().Annotations[InspectAnnotationAutoremove] == InspectResponseTrue +} diff --git a/libpod/define/exec_codes.go b/libpod/define/exec_codes.go index 7184f1e59..33d631326 100644 --- a/libpod/define/exec_codes.go +++ b/libpod/define/exec_codes.go @@ -1,6 +1,8 @@ package define import ( + "strings" + "github.com/pkg/errors" ) @@ -28,3 +30,19 @@ func TranslateExecErrorToExitCode(originalEC int, err error) int { } return originalEC } + +// ExitCode reads the error message when failing to executing container process +// and then returns 0 if no error, ExecErrorCodeNotFound if command does not exist, or ExecErrorCodeCannotInvoke 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 ExecErrorCodeNotFound + } + + return ExecErrorCodeCannotInvoke +} 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() diff --git a/test/e2e/run_exit_test.go b/test/e2e/run_exit_test.go index 861d6b3b7..374705879 100644 --- a/test/e2e/run_exit_test.go +++ b/test/e2e/run_exit_test.go @@ -1,10 +1,9 @@ -// +build !remoteclient - package integration import ( "os" + "github.com/containers/libpod/libpod/define" . "github.com/containers/libpod/test/utils" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" @@ -34,22 +33,22 @@ var _ = Describe("Podman run exit", func() { }) - It("podman run exit 125", func() { + It("podman run exit define.ExecErrorCodeGeneric", func() { result := podmanTest.Podman([]string{"run", "--foobar", ALPINE, "ls", "$tmp"}) result.WaitWithDefaultTimeout() - Expect(result.ExitCode()).To(Equal(125)) + Expect(result.ExitCode()).To(Equal(define.ExecErrorCodeGeneric)) }) - It("podman run exit 126", func() { + It("podman run exit ExecErrorCodeCannotInvoke", func() { result := podmanTest.Podman([]string{"run", ALPINE, "/etc"}) result.WaitWithDefaultTimeout() - Expect(result.ExitCode()).To(Equal(126)) + Expect(result.ExitCode()).To(Equal(define.ExecErrorCodeCannotInvoke)) }) - It("podman run exit 127", func() { + It("podman run exit ExecErrorCodeNotFound", func() { result := podmanTest.Podman([]string{"run", ALPINE, "foobar"}) result.WaitWithDefaultTimeout() - Expect(result.ExitCode()).To(Equal(127)) + Expect(result.ExitCode()).To(Equal(define.ExecErrorCodeNotFound)) }) It("podman run exit 0", func() { diff --git a/test/e2e/run_selinux_test.go b/test/e2e/run_selinux_test.go index dfe71531a..0c78ab15b 100644 --- a/test/e2e/run_selinux_test.go +++ b/test/e2e/run_selinux_test.go @@ -162,7 +162,7 @@ var _ = Describe("Podman run", func() { session = podmanTest.Podman([]string{"run", "-it", "--security-opt", "label=type:spc_t", "--security-opt", "label=filetype:foobar", fedoraMinimal, "ls", "-Z", "/dev"}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Equal(127)) + Expect(session.ExitCode()).To(Equal(126)) }) }) diff --git a/test/system/030-run.bats b/test/system/030-run.bats index f279a0c75..760ed6a18 100644 --- a/test/system/030-run.bats +++ b/test/system/030-run.bats @@ -60,7 +60,6 @@ echo $rand | 0 | $rand # 'run --rm' goes through different code paths and may lose exit status. # See https://github.com/containers/libpod/issues/3795 @test "podman run --rm" { - skip_if_remote "podman-remote does not handle exit codes" run_podman 0 run --rm $IMAGE /bin/true run_podman 1 run --rm $IMAGE /bin/false |