From f8e21a31b317a65f57773c280926aa471ae060ff Mon Sep 17 00:00:00 2001 From: Oliver Gutierrez Date: Mon, 30 Aug 2021 14:47:45 +0100 Subject: [PATCH] cmd/run, root: Exit with exit code of invoked command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- doc/toolbox-run.1.md | 35 ++++++++++++++++ src/cmd/root.go | 21 ++++++++++ src/cmd/root_test.go | 75 +++++++++++++++++++++++++++++++++++ src/cmd/run.go | 19 ++++++--- test/system/104-run.bats | 45 +++++++++++++++++++++ test/system/libs/helpers.bash | 6 +++ 6 files changed, 196 insertions(+), 5 deletions(-) create mode 100644 src/cmd/root_test.go diff --git a/doc/toolbox-run.1.md b/doc/toolbox-run.1.md index e0dc098..2ed8ded 100644 --- a/doc/toolbox-run.1.md +++ b/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 diff --git a/src/cmd/root.go b/src/cmd/root.go index 4d8ebc9..f2f91e1 100644 --- a/src/cmd/root.go +++ b/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) } diff --git a/src/cmd/root_test.go b/src/cmd/root_test.go new file mode 100644 index 0000000..ac59eea --- /dev/null +++ b/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()) + } + }) + } +} diff --git a/src/cmd/run.go b/src/cmd/run.go index 38df538..d65143a 100644 --- a/src/cmd/run.go +++ b/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} } } } diff --git a/test/system/104-run.bats b/test/system/104-run.bats index c509214..4afa881 100644 --- a/test/system/104-run.bats +++ b/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 ] +} diff --git a/test/system/libs/helpers.bash b/test/system/libs/helpers.bash index e9dfc16..ef2bd8c 100644 --- a/test/system/libs/helpers.bash +++ b/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 }