Skip to content

Commit

Permalink
cmd/run, root: Exit with exit code of invoked command
Browse files Browse the repository at this point in the history
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.

containers#867
  • Loading branch information
olivergs authored and HarryMichal committed Feb 20, 2022
1 parent 2af0b30 commit 750db4b
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 5 deletions.
33 changes: 33 additions & 0 deletions doc/toolbox-run.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,39 @@ 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.

**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
Expand Down
21 changes: 21 additions & 0 deletions src/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
73 changes: 73 additions & 0 deletions src/cmd/root_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*
* 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 {
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())
}
}
}
19 changes: 14 additions & 5 deletions src/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,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
}

Expand Down Expand Up @@ -322,9 +331,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) {
Expand All @@ -341,7 +350,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) {
Expand All @@ -355,13 +364,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}
}
}
}
Expand Down
49 changes: 49 additions & 0 deletions test/system/104-run.bats
Original file line number Diff line number Diff line change
Expand Up @@ -83,3 +83,52 @@ teardown() {
assert_success
assert_output --partial "uid=0(root)"
}

@test "run: Run command exiting with zero code in the default container" {
local cmd="/bin/sh -c 'exit 0'"

create_default_container

run $TOOLBOX run $cmd

assert_success
assert_output ""
}

@test "run: Run command exiting with non-zero code in the default container" {
local cmd="/bin/sh -c 'exit 2'"

create_default_container

run $TOOLBOX run $cmd
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_output "/bin/sh: line1: exec: $cmd: not found
Error: command $cmd not found in container $(get_latest_container_name)"
}

@test "run: Try to run /etc as a command in the deault container" {
local cmd="/etc"

create_default_container

run $TOOLBOX run $cmd

assert_failure
assert [ $status -eq 126 ]
assert_output "/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 $(get_latest_container_name)"
}
6 changes: 6 additions & 0 deletions test/system/libs/helpers.bash
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,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
}
Expand Down

0 comments on commit 750db4b

Please sign in to comment.