Skip to content

Commit

Permalink
cmd/run: Optimize 'enter' and 'run' in the non-fallback case
Browse files Browse the repository at this point in the history
Currently, the 'enter' command involves two extra invocations of
'podman exec' to detect if the user's chosen shell and current working
directory are present inside the toolbox container. Each invocation is
sufficiently expensive to add a noticeable overhead to the 'enter' and
'run' commands. Moreover, file system operations being inherently racy,
it's always better to detect errors and handle them instead of trying
to pre-emptively avoid them.

Therefore, this shuffles the code around to attempt the non-fallback
invocation, and then handle the errors by attempting a series of
fallbacks for the command and the current working directory.

Unfortunately, in case of a missing command, capsh(1) adds an extra
error message that seems difficult to get rid of:
  $ toolbox enter
  /bin/sh: /bin/zsh: No such file or directory
  Error: command /bin/zsh not found in container fedora-toolbox-34
  Using /bin/bash instead.

containers#813
  • Loading branch information
debarshiray committed Jun 25, 2021
1 parent 323afb9 commit 4536e2c
Showing 1 changed file with 75 additions and 72 deletions.
147 changes: 75 additions & 72 deletions src/cmd/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ var (
distro string
release string
}

runFallbackCommands = [][]string{{"/bin/bash", "-l"}}
runFallbackWorkDirs = []string{"" /* $HOME */}
)

var runCmd = &cobra.Command{
Expand Down Expand Up @@ -265,29 +268,14 @@ func runCommand(container string,

logrus.Debugf("Container %s is initialized", container)

if _, err := isCommandPresent(container, command[0]); err != nil {
if fallbackToBash {
fmt.Fprintf(os.Stderr,
"Error: command %s not found in container %s\n",
command[0],
container)
fmt.Fprintf(os.Stderr, "Using /bin/bash instead.\n")

command = []string{"/bin/bash", "-l"}
} else {
return fmt.Errorf("command %s not found in container %s", command[0], container)
}
if err := runCommandWithFallbacks(container, command, emitEscapeSequence, fallbackToBash); err != nil {
return err
}

if pathPresent, _ := isPathPresent(container, workingDirectory); !pathPresent {
fmt.Fprintf(os.Stderr, "Error: path %s not found in container %s\n",
workingDirectory, container)
fmt.Fprintf(os.Stderr, "Using %s instead.\n",
currentUser.HomeDir)

workingDirectory = currentUser.HomeDir
}
return nil
}

func runCommandWithFallbacks(container string, command []string, emitEscapeSequence, fallbackToBash bool) error {
logrus.Debug("Checking if 'podman exec' supports disabling the detach keys")

var detachKeysSupported bool
Expand All @@ -299,48 +287,82 @@ func runCommand(container string,

envOptions := utils.GetEnvOptionsForPreservedVariables()

execArgs := constructExecArgs(container, command, detachKeysSupported, envOptions, workingDirectory)
runFallbackCommandsIndex := 0
runFallbackWorkDirsIndex := 0
workDir := workingDirectory

if emitEscapeSequence {
fmt.Printf("\033]777;container;push;%s;toolbox;%s\033\\", container, currentUser.Uid)
}
for {
execArgs := constructExecArgs(container, command, detachKeysSupported, envOptions, workDir)

logrus.Debugf("Running in container %s:", container)
logrus.Debug("podman")
for _, arg := range execArgs {
logrus.Debugf("%s", arg)
}
if emitEscapeSequence {
fmt.Printf("\033]777;container;push;%s;toolbox;%s\033\\", container, currentUser.Uid)
}

exitCode, err := shell.RunWithExitCode("podman", os.Stdin, os.Stdout, nil, execArgs...)
logrus.Debugf("Running in container %s:", container)
logrus.Debug("podman")
for _, arg := range execArgs {
logrus.Debugf("%s", arg)
}

if emitEscapeSequence {
fmt.Printf("\033]777;container;pop;;;%s\033\\", currentUser.Uid)
}
exitCode, err := shell.RunWithExitCode("podman", os.Stdin, os.Stdout, nil, execArgs...)

switch exitCode {
case 0:
if err != nil {
panic("unexpected error: 'podman exec' finished successfully")
}
case 125:
err = fmt.Errorf("failed to invoke 'podman exec' in container %s", container)
case 126:
err = fmt.Errorf("failed to invoke command %s in container %s", command[0], container)
case 127:
if pathPresent, _ := isPathPresent(container, workingDirectory); !pathPresent {
err = fmt.Errorf("directory %s not found in container %s", workingDirectory, container)
} else {
err = fmt.Errorf("command %s not found in container %s", command[0], container)
if emitEscapeSequence {
fmt.Printf("\033]777;container;pop;;;%s\033\\", currentUser.Uid)
}
default:
err = nil
}

if err != nil {
return err
switch exitCode {
case 0:
if err != nil {
panic("unexpected error: 'podman exec' finished successfully")
}
return nil
case 125:
err = fmt.Errorf("failed to invoke 'podman exec' in container %s", container)
return err
case 126:
err = fmt.Errorf("failed to invoke command %s in container %s", command[0], container)
return err
case 127:
if pathPresent, _ := isPathPresent(container, workDir); !pathPresent {
if runFallbackWorkDirsIndex < len(runFallbackWorkDirs) {
fmt.Fprintf(os.Stderr,
"Error: directory %s not found in container %s\n",
workDir,
container)

workDir = runFallbackWorkDirs[runFallbackWorkDirsIndex]
if workDir == "" {
workDir = currentUser.HomeDir
}

fmt.Fprintf(os.Stderr, "Using %s instead.\n", workDir)
runFallbackWorkDirsIndex++
} else {
err = fmt.Errorf("directory %s not found in container %s", workDir, container)
return err
}
} else {
if fallbackToBash && runFallbackCommandsIndex < len(runFallbackCommands) {
fmt.Fprintf(os.Stderr,
"Error: command %s not found in container %s\n",
command[0],
container)

command = runFallbackCommands[runFallbackCommandsIndex]
fmt.Fprintf(os.Stderr, "Using %s instead.\n", command[0])

runFallbackCommandsIndex++
} else {
err = fmt.Errorf("command %s not found in container %s", command[0], container)
return err
}
}
default:
return nil
}
}

return nil
panic("code should not be reached")
}

func runHelp(cmd *cobra.Command, args []string) {
Expand Down Expand Up @@ -466,25 +488,6 @@ func getEntryPointAndPID(container string) (string, int, error) {
return entryPoint, entryPointPIDInt, nil
}

func isCommandPresent(container, command string) (bool, error) {
logrus.Debugf("Looking for command %s in container %s", command, container)

logLevelString := podman.LogLevel.String()
args := []string{
"--log-level", logLevelString,
"exec",
"--user", currentUser.Username,
container,
"sh", "-c", "command -v \"$1\"", "sh", command,
}

if err := shell.Run("podman", nil, nil, nil, args...); err != nil {
return false, err
}

return true, nil
}

func isPathPresent(container, path string) (bool, error) {
logrus.Debugf("Looking for path %s in container %s", path, container)

Expand Down

0 comments on commit 4536e2c

Please sign in to comment.