From a6f8cad545fa3242a2778c9a0d25da4da0a51ce2 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 30 May 2022 15:25:57 +0200 Subject: fix bad import path for cmd/podman/utils Libpod or packages under /pkg should never import from /cmd/... This will quickly result in import cycles and weird code paths. Also there is no reason to use this special code we can just use syscall.SIGHUB as SIGNAL. Signed-off-by: Paul Holzinger --- cmd/podman/utils/signals_linux.go | 15 --------------- cmd/podman/utils/signals_windows.go | 15 --------------- 2 files changed, 30 deletions(-) delete mode 100644 cmd/podman/utils/signals_linux.go delete mode 100644 cmd/podman/utils/signals_windows.go (limited to 'cmd/podman') diff --git a/cmd/podman/utils/signals_linux.go b/cmd/podman/utils/signals_linux.go deleted file mode 100644 index dd0507c0e..000000000 --- a/cmd/podman/utils/signals_linux.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build !windows -// +build !windows - -package utils - -import ( - "os" - - "golang.org/x/sys/unix" -) - -// Platform specific signal synonyms -var ( - SIGHUP os.Signal = unix.SIGHUP -) diff --git a/cmd/podman/utils/signals_windows.go b/cmd/podman/utils/signals_windows.go deleted file mode 100644 index e6fcc1b32..000000000 --- a/cmd/podman/utils/signals_windows.go +++ /dev/null @@ -1,15 +0,0 @@ -//go:build windows -// +build windows - -package utils - -import ( - "os" - - "golang.org/x/sys/windows" -) - -// Platform specific signal synonyms -var ( - SIGHUP os.Signal = windows.SIGHUP -) -- cgit v1.2.3-54-g00ecf From 0e58636c3a599bb3d7d5ef06ab2773befe8a0150 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 30 May 2022 15:15:57 +0200 Subject: podman machine ssh: set correct exit code Forward the ssh exit code to the podman caller. This is useful for scripts. Use the same logic as podman unshare. Fixes #14401 Signed-off-by: Paul Holzinger --- cmd/podman/machine/ssh.go | 4 +++- cmd/podman/system/unshare.go | 21 ++---------------- cmd/podman/utils/error.go | 32 ++++++++++++++++++++++++++++ docs/source/markdown/podman-machine-ssh.1.md | 30 ++++++++++++++++++++++++++ pkg/machine/e2e/config.go | 8 +++++++ pkg/machine/e2e/ssh_test.go | 7 ++++++ 6 files changed, 82 insertions(+), 20 deletions(-) (limited to 'cmd/podman') diff --git a/cmd/podman/machine/ssh.go b/cmd/podman/machine/ssh.go index 4a86da67a..8261f3607 100644 --- a/cmd/podman/machine/ssh.go +++ b/cmd/podman/machine/ssh.go @@ -9,6 +9,7 @@ import ( "github.com/containers/common/pkg/completion" "github.com/containers/common/pkg/config" "github.com/containers/podman/v4/cmd/podman/registry" + "github.com/containers/podman/v4/cmd/podman/utils" "github.com/containers/podman/v4/pkg/machine" "github.com/pkg/errors" "github.com/spf13/cobra" @@ -89,7 +90,8 @@ func ssh(cmd *cobra.Command, args []string) error { if err != nil { return errors.Wrapf(err, "vm %s not found", vmName) } - return vm.SSH(vmName, sshOpts) + err = vm.SSH(vmName, sshOpts) + return utils.HandleOSExecError(err) } func remoteConnectionUsername() (string, error) { diff --git a/cmd/podman/system/unshare.go b/cmd/podman/system/unshare.go index 0ae5b81ad..1ed08eac3 100644 --- a/cmd/podman/system/unshare.go +++ b/cmd/podman/system/unshare.go @@ -2,10 +2,10 @@ package system import ( "os" - "os/exec" "github.com/containers/common/pkg/completion" "github.com/containers/podman/v4/cmd/podman/registry" + "github.com/containers/podman/v4/cmd/podman/utils" "github.com/containers/podman/v4/pkg/domain/entities" "github.com/containers/podman/v4/pkg/rootless" "github.com/pkg/errors" @@ -60,22 +60,5 @@ func unshare(cmd *cobra.Command, args []string) error { } err := registry.ContainerEngine().Unshare(registry.Context(), args, unshareOptions) - if err != nil { - if exitError, ok := err.(*exec.ExitError); ok { - // the user command inside the unshare env has failed - // we set the exit code, do not return the error to the user - // otherwise "exit status X" will be printed - registry.SetExitCode(exitError.ExitCode()) - return nil - } - // cmd.Run() can return fs.ErrNotExist, fs.ErrPermission or exec.ErrNotFound - // follow podman run/exec standard with the exit codes - if errors.Is(err, os.ErrNotExist) || errors.Is(err, exec.ErrNotFound) { - registry.SetExitCode(127) - } else if errors.Is(err, os.ErrPermission) { - registry.SetExitCode(126) - } - return err - } - return nil + return utils.HandleOSExecError(err) } diff --git a/cmd/podman/utils/error.go b/cmd/podman/utils/error.go index 2aaa71373..3efff0301 100644 --- a/cmd/podman/utils/error.go +++ b/cmd/podman/utils/error.go @@ -4,10 +4,12 @@ import ( "errors" "fmt" "os" + "os/exec" "strconv" "strings" buildahCLI "github.com/containers/buildah/pkg/cli" + "github.com/containers/podman/v4/cmd/podman/registry" ) type OutputErrors []error @@ -43,3 +45,33 @@ func ExitCodeFromBuildError(errorMsg string) (int, error) { } return buildahCLI.ExecErrorCodeGeneric, errors.New("message does not contains a valid exit code") } + +// HandleOSExecError checks the given error for an exec.ExitError error and +// sets the same podman exit code as the error. +// No error will be returned in this case to make sure things like podman +// unshare false work correctly without extra output. +// When the exec file does not exists we set the exit code to 127, for +// permission errors 126 is used as exit code. In this case we still return +// the error so the user gets an error message. +// If the error is nil it returns nil. +func HandleOSExecError(err error) error { + if err == nil { + return nil + } + var exitError *exec.ExitError + if errors.As(err, &exitError) { + // the user command inside the unshare/ssh env has failed + // we set the exit code, do not return the error to the user + // otherwise "exit status X" will be printed + registry.SetExitCode(exitError.ExitCode()) + return nil + } + // cmd.Run() can return fs.ErrNotExist, fs.ErrPermission or exec.ErrNotFound + // follow podman run/exec standard with the exit codes + if errors.Is(err, os.ErrNotExist) || errors.Is(err, exec.ErrNotFound) { + registry.SetExitCode(127) + } else if errors.Is(err, os.ErrPermission) { + registry.SetExitCode(126) + } + return err +} diff --git a/docs/source/markdown/podman-machine-ssh.1.md b/docs/source/markdown/podman-machine-ssh.1.md index db0350961..6a1455df1 100644 --- a/docs/source/markdown/podman-machine-ssh.1.md +++ b/docs/source/markdown/podman-machine-ssh.1.md @@ -14,6 +14,7 @@ first argument must be the virtual machine name. The optional command to execute can then follow. If no command is provided, an interactive session with the virtual machine is established. +The exit code from ssh command will be forwarded to the podman machine ssh caller, see [Exit Codes](#Exit-Codes). ## OPTIONS @@ -25,6 +26,35 @@ Print usage statement. Username to use when SSH-ing into the VM. +## Exit Codes + +The exit code from `podman machine ssh` gives information about why the command failed. +When `podman machine ssh` commands exit with a non-zero code, +the exit codes follow the `chroot` standard, see below: + + **125** The error is with podman **_itself_** + + $ podman machine ssh --foo; echo $? + Error: unknown flag: --foo + 125 + + **126** Executing a _contained command_ and the _command_ cannot be invoked + + $ podman machine ssh /etc; echo $? + Error: fork/exec /etc: permission denied + 126 + + **127** Executing a _contained command_ and the _command_ cannot be found + + $ podman machine ssh foo; echo $? + Error: fork/exec /usr/bin/bogus: no such file or directory + 127 + + **Exit code** _contained command_ exit code + + $ podman machine ssh /bin/sh -c 'exit 3'; echo $? + 3 + ## EXAMPLES To get an interactive session with the default virtual machine: diff --git a/pkg/machine/e2e/config.go b/pkg/machine/e2e/config.go index c17b840d3..248a2f0ad 100644 --- a/pkg/machine/e2e/config.go +++ b/pkg/machine/e2e/config.go @@ -85,6 +85,14 @@ func (ms *machineSession) outputToString() string { return strings.Join(fields, " ") } +// errorToString returns the error output from a session in string form +func (ms *machineSession) errorToString() string { + if ms == nil || ms.Err == nil || ms.Err.Contents() == nil { + return "" + } + return string(ms.Err.Contents()) +} + // newMB constructor for machine test builders func newMB() (*machineTestBuilder, error) { mb := machineTestBuilder{ diff --git a/pkg/machine/e2e/ssh_test.go b/pkg/machine/e2e/ssh_test.go index 155d39a64..9ee31ac26 100644 --- a/pkg/machine/e2e/ssh_test.go +++ b/pkg/machine/e2e/ssh_test.go @@ -56,5 +56,12 @@ var _ = Describe("podman machine ssh", func() { Expect(err).To(BeNil()) Expect(sshSession).To(Exit(0)) Expect(sshSession.outputToString()).To(ContainSubstring("Fedora CoreOS")) + + // keep exit code + sshSession, err = mb.setName(name).setCmd(ssh.withSSHComand([]string{"false"})).run() + Expect(err).To(BeNil()) + Expect(sshSession).To(Exit(1)) + Expect(sshSession.outputToString()).To(Equal("")) + Expect(sshSession.errorToString()).To(Equal("")) }) }) -- cgit v1.2.3-54-g00ecf