cmd/run, root: Exit with exit code of invoked command

When a command is executed with toolbox run and it returns a non-zero
exit code, it is just ignored if that exit code is not handled. This
prevents users to identify errors when executing commands in toolbox.

With this fix, the exit codes of the invoked command are propagated
and returned by 'toolbox run'. This includes even exit codes returned
by Podman on error.

https://github.com/containers/toolbox/pull/1013

Co-authored-by: Ondřej Míchal <harrymichal@seznam.cz>

doc/toolbox-run.1.md

@@ -42,6 +42,41 @@ matches the host system.
 Run command inside a toolbox container for a different operating system
 RELEASE than the host.
 
+## EXIT STATUS
+
+The exit code gives information about why the command within the container
+failed to run or why it exited.
+
+**1** There was an internal error in Toolbox
+
+**125** There was an internal error in Podman
+
+**126** The run command could not be invoked
+
+```
+$ toolbox run /etc; echo $?
+/bin/sh: line 1: /etc: Is a directory
+/bin/sh: line 1: exec: /etc: cannot execute: Is a directory
+Error: failed to invoke command /etc in container fedora-toolbox-35
+126
+```
+
+**127** The run command cannot be found or the working directory does not exist
+
+```
+$ toolbox run foo; echo $?
+/bin/sh: line 1: exec: foo: not found
+Error: command foo not found in container fedora-toolbox-35
+127
+```
+
+**Exit code** The run command exit code
+
+```
+$ toolbox run false; echo $?
+1
+```
+
 ## EXAMPLES
 
 ### Run ls inside a toolbox container using the default image matching the host OS

src/cmd/root.go

@@ -61,8 +61,29 @@ var (
 	workingDirectory string
 )
 
+type exitError struct {
+	Code int
+	err  error
+}
+
+func (e *exitError) Error() string {
+	if e.err != nil {
+		return e.err.Error()
+	} else {
+		return ""
+	}
+}
+
 func Execute() {
 	if err := rootCmd.Execute(); err != nil {
+		var errExit *exitError
+		if errors.As(err, &errExit) {
+			if errExit.err != nil {
+				fmt.Fprintf(os.Stderr, "Error: %s\n", errExit)
+			}
+			os.Exit(errExit.Code)
+		}
+
 		os.Exit(1)
 	}
 

src/cmd/root_test.go

@@ -0,0 +1,75 @@
+/*
+ * Copyright © 2022 Ondřej Míchal
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package cmd
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func getExitError(err error, rc int) error {
+	return &exitError{rc, err}
+}
+
+func TestExitError(t *testing.T) {
+	t.Run("correct error interface implementation", func(t *testing.T) {
+		var err error = &exitError{0, nil}
+		assert.Implements(t, (*error)(nil), err)
+	})
+
+	testCases := []struct {
+		name string
+		err  error
+		rc   int
+	}{
+		{
+			"errmsg empty; return code 0; casting from Error",
+			nil,
+			0,
+		},
+		{
+			"errmsg empty; return code > 0; casting from Error",
+			nil,
+			42,
+		},
+		{
+			"errmsg full; return code 0; casting from Error",
+			errors.New("this is an error message"),
+			0,
+		},
+		{
+			"errmsg full; return code > 0; casting from Error",
+			errors.New("this is an error message"),
+			42,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			err := getExitError(tc.err, tc.rc)
+			var errExit *exitError
+
+			assert.ErrorAs(t, err, &errExit)
+			assert.Equal(t, tc.rc, errExit.Code)
+			if tc.err != nil {
+				assert.Equal(t, tc.err.Error(), errExit.Error())
+			}
+		})
+	}
+}

src/cmd/run.go

@@ -159,6 +159,15 @@ func run(cmd *cobra.Command, args []string) error {
 		false,
 		false,
 		true); err != nil {
+		// runCommand returns exitError for the executed commands to properly
+		// propagate return codes. Cobra prints all non-nil errors which in
+		// that case is not desirable. In that scenario silence the errors and
+		// leave the error handling to the root command.
+		var errExit *exitError
+		if errors.As(err, &errExit) {
+			cmd.SilenceErrors = true
+		}
+
 		return err
 	}
 
@@ -338,9 +347,9 @@ func runCommandWithFallbacks(container string, command []string, emitEscapeSeque
 			}
 			return nil
 		case 125:
-			return fmt.Errorf("failed to invoke 'podman exec' in container %s", container)
+			return &exitError{exitCode, fmt.Errorf("failed to invoke 'podman exec' in container %s", container)}
 		case 126:
-			return fmt.Errorf("failed to invoke command %s in container %s", command[0], container)
+			return &exitError{exitCode, fmt.Errorf("failed to invoke command %s in container %s", command[0], container)}
 		case 127:
 			if pathPresent, _ := isPathPresent(container, workDir); !pathPresent {
 				if runFallbackWorkDirsIndex < len(runFallbackWorkDirs) {
@@ -357,7 +366,7 @@ func runCommandWithFallbacks(container string, command []string, emitEscapeSeque
 					fmt.Fprintf(os.Stderr, "Using %s instead.\n", workDir)
 					runFallbackWorkDirsIndex++
 				} else {
-					return fmt.Errorf("directory %s not found in container %s", workDir, container)
+					return &exitError{exitCode, fmt.Errorf("directory %s not found in container %s", workDir, container)}
 				}
 			} else if _, err := isCommandPresent(container, command[0]); err != nil {
 				if fallbackToBash && runFallbackCommandsIndex < len(runFallbackCommands) {
@@ -371,13 +380,13 @@ func runCommandWithFallbacks(container string, command []string, emitEscapeSeque
 
 					runFallbackCommandsIndex++
 				} else {
-					return fmt.Errorf("command %s not found in container %s", command[0], container)
+					return &exitError{exitCode, fmt.Errorf("command %s not found in container %s", command[0], container)}
 				}
 			} else {
 				return nil
 			}
 		default:
-			return nil
+			return &exitError{exitCode, nil}
 		}
 	}
 }

test/system/104-run.bats

@@ -121,3 +121,48 @@ teardown() {
   assert_success
   assert_output --partial "uid=0(root)"
 }
+
+@test "run: Run command exiting with zero code in the default container" {
+  create_default_container
+
+  run $TOOLBOX run /bin/sh -c 'exit 0'
+
+  assert_success
+  assert_output ""
+}
+
+@test "run: Run command exiting with non-zero code in the default container" {
+  create_default_container
+
+  run $TOOLBOX run /bin/sh -c 'exit 2'
+  assert_failure
+  assert [ $status -eq 2 ]
+  assert_output ""
+}
+
+@test "run: Try to run non-existent command in the default container" {
+  local cmd="non-existent-command"
+
+  create_default_container
+
+  run $TOOLBOX run $cmd
+
+  assert_failure
+  assert [ $status -eq 127 ]
+  assert_line --index 0 "/bin/sh: line 1: exec: $cmd: not found"
+  assert_line --index 1 "Error: command $cmd not found in container $(get_latest_container_name)"
+  assert [ ${#lines[@]} -eq 2 ]
+}
+
+@test "run: Try to run /etc as a command in the deault container" {
+  create_default_container
+
+  run $TOOLBOX run /etc
+
+  assert_failure
+  assert [ $status -eq 126 ]
+  assert_line --index 0 "/bin/sh: line 1: /etc: Is a directory"
+  assert_line --index 1 "/bin/sh: line 1: exec: /etc: cannot execute: Is a directory"
+  assert_line --index 2 "Error: failed to invoke command /etc in container $(get_latest_container_name)"
+  assert [ ${#lines[@]} -eq 3 ]
+}

test/system/libs/helpers.bash

@@ -391,6 +391,12 @@ function stop_container() {
 }
 
 
+# Returns the name of the latest created container
+function get_latest_container_name() {
+  $PODMAN ps -l --format "{{ .Names }}"
+}
+
+
 function list_images() {
   $PODMAN images --all --quiet | wc -l
 }