From 479eeac62cd74e32cbe74fc8afbfc82d4d8a8abd Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Mon, 22 Jul 2019 15:12:29 -0400 Subject: 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 --- libpod/container_api.go | 15 +++++---------- libpod/define/exec_codes.go | 13 +++++++++++++ 2 files changed, 18 insertions(+), 10 deletions(-) create mode 100644 libpod/define/exec_codes.go (limited to 'libpod') 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 +) -- cgit v1.2.3-54-g00ecf From 01a8483a59eed8bc706b5219b903704544b66c10 Mon Sep 17 00:00:00 2001 From: Peter Hunt Date: Tue, 23 Jul 2019 15:30:05 -0400 Subject: refactor to reduce duplicated error parsing Signed-off-by: Peter Hunt --- libpod/define/exec_codes.go | 17 +++++++++++++++++ pkg/adapter/containers.go | 8 +------- pkg/varlinkapi/containers.go | 10 ++-------- 3 files changed, 20 insertions(+), 15 deletions(-) (limited to 'libpod') diff --git a/libpod/define/exec_codes.go b/libpod/define/exec_codes.go index 90a68cf93..7184f1e59 100644 --- a/libpod/define/exec_codes.go +++ b/libpod/define/exec_codes.go @@ -1,5 +1,9 @@ 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 @@ -11,3 +15,16 @@ const ( // 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 a8bff187c..faaef3e60 100644 --- a/pkg/adapter/containers.go +++ b/pkg/adapter/containers.go @@ -1001,13 +1001,7 @@ func (r *LocalRuntime) ExecContainer(ctx context.Context, cli *cliconfig.ExecVal streams.AttachError = true 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 + return define.TranslateExecErrorToExitCode(ec, err), err } // Prune removes stopped containers diff --git a/pkg/varlinkapi/containers.go b/pkg/varlinkapi/containers.go index 6e2b26c0e..cd5f305c9 100644 --- a/pkg/varlinkapi/containers.go +++ b/pkg/varlinkapi/containers.go @@ -837,15 +837,9 @@ func (i *LibpodAPI) ExecContainer(call iopodman.VarlinkCall, opts iopodman.ExecO ecErr := <-ecErrChan - exitCode := ecErr.ExitCode - if errors.Cause(ecErr.Error) == define.ErrOCIRuntimePermissionDenied { - exitCode = define.ExecErrorCodeCannotInvoke - } - if errors.Cause(ecErr.Error) == define.ErrOCIRuntimeNotFound { - exitCode = define.ExecErrorCodeNotFound - } + exitCode := define.TranslateExecErrorToExitCode(int(ecErr.ExitCode), ecErr.Error) - if err = virtwriter.HangUp(writer, exitCode); err != nil { + if err = virtwriter.HangUp(writer, uint32(exitCode)); err != nil { logrus.Errorf("ExecContainer failed to HANG-UP on %s: %s", ctr.ID(), err.Error()) } -- cgit v1.2.3-54-g00ecf