Skip to content

Commit

Permalink
Merge pull request #3934 from rhatdan/wait
Browse files Browse the repository at this point in the history
Podman-remote run should wait for exit code
  • Loading branch information
openshift-merge-robot authored Sep 13, 2019
2 parents 5c09c4d + 82ac0d8 commit 7875e00
Show file tree
Hide file tree
Showing 14 changed files with 123 additions and 60 deletions.
4 changes: 2 additions & 2 deletions cmd/podman/containers_prune.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ 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
}
}
return err
}
if len(failures) > 0 {
exitCode = 125
exitCode = define.ExecErrorCodeGeneric
}
return printCmdResults(ok, failures)
}
8 changes: 5 additions & 3 deletions cmd/podman/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions cmd/podman/pause.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,15 @@ 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
}
}
return err
}
if len(failures) > 0 {
exitCode = 125
exitCode = define.ExecErrorCodeGeneric
}
return printCmdResults(ok, failures)
}
4 changes: 2 additions & 2 deletions cmd/podman/restart.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ 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
}
}
return err
}
if len(failures) > 0 {
exitCode = 125
exitCode = define.ExecErrorCodeGeneric
}
return printCmdResults(ok, failures)
}
4 changes: 2 additions & 2 deletions cmd/podman/unpause.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ 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
}
}
return err
}
if len(failures) > 0 {
exitCode = 125
exitCode = define.ExecErrorCodeGeneric
}
return printCmdResults(ok, failures)
}
13 changes: 11 additions & 2 deletions libpod/container_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
18 changes: 18 additions & 0 deletions libpod/define/exec_codes.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package define

import (
"strings"

"github.com/pkg/errors"
)

Expand Down Expand Up @@ -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
}
26 changes: 7 additions & 19 deletions pkg/adapter/containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
Expand Down Expand Up @@ -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
)

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down
39 changes: 22 additions & 17 deletions pkg/adapter/containers_remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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()))
Expand All @@ -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)
Expand All @@ -738,19 +742,20 @@ 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
_, err = reply()

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.
Expand Down
16 changes: 16 additions & 0 deletions pkg/util/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Loading

0 comments on commit 7875e00

Please sign in to comment.