Merge pull request #14417 from Luap99/machine-ssh

podman machine ssh: set correct exit code

10 files changed, 86 insertions, 53 deletions

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/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
-)
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/domain/infra/runtime_libpod.go b/pkg/domain/infra/runtime_libpod.go
index ac557e9de..daa6f0cbf 100644
--- a/pkg/domain/infra/runtime_libpod.go
+++ b/pkg/domain/infra/runtime_libpod.go
@@ -9,9 +9,9 @@ import (
 	"os"
 	"os/signal"
 	"sync"
+	"syscall"
 
 	"github.com/containers/common/pkg/cgroups"
-	"github.com/containers/podman/v4/cmd/podman/utils"
 	"github.com/containers/podman/v4/libpod"
 	"github.com/containers/podman/v4/pkg/domain/entities"
 	"github.com/containers/podman/v4/pkg/namespaces"
@@ -375,7 +375,7 @@ func ParseIDMapping(mode namespaces.UsernsMode, uidMapSlice, gidMapSlice []strin
 func StartWatcher(rt *libpod.Runtime) {
 	// Setup the signal notifier
 	ch := make(chan os.Signal, 1)
-	signal.Notify(ch, utils.SIGHUP)
+	signal.Notify(ch, syscall.SIGHUP)
 
 	go func() {
 		for {
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(""))
 	})
 })
diff --git a/pkg/machine/qemu/machine.go b/pkg/machine/qemu/machine.go
index f9119af94..b9f23662e 100644
--- a/pkg/machine/qemu/machine.go
+++ b/pkg/machine/qemu/machine.go
@@ -952,7 +952,8 @@ func (v *MachineVM) SSH(_ string, opts machine.SSHOptions) error {
 	sshDestination := username + "@localhost"
 	port := strconv.Itoa(v.Port)
 
-	args := []string{"-i", v.IdentityPath, "-p", port, sshDestination, "-o", "UserKnownHostsFile=/dev/null", "-o", "StrictHostKeyChecking=no"}
+	args := []string{"-i", v.IdentityPath, "-p", port, sshDestination, "-o", "UserKnownHostsFile=/dev/null",
+		"-o", "StrictHostKeyChecking=no", "-o", "LogLevel=ERROR"}
 	if len(opts.Args) > 0 {
 		args = append(args, opts.Args...)
 	} else {