cmd/run: Optimize 'enter' and 'run' in the non-fallback case #813
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This will be used by the subsequent commit to optimize the 'enter' and 'run' commands in the non-fallback case, by attempting the fallback only if an error was encountered by the main 'podman exec' invocation, as opposed to pre-emptively setting up the fallback. containers#813
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Jun 25, 2021
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. containers#813
debarshiray
force-pushed
the
wip/rishi/run-optimize-fallbacks
branch
from
41bea8c to
43a3222
Compare
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Jun 25, 2021
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. containers#813
debarshiray
force-pushed
the
wip/rishi/run-optimize-fallbacks
branch
from
43a3222 to
3b57aac
Compare
|
Build failed.
|
debarshiray
added a commit
to debarshiray/toolbox
that referenced
this pull request
Jun 25, 2021
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
debarshiray
force-pushed
the
wip/rishi/run-optimize-fallbacks
branch
from
3b57aac to
11051a6
Compare
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
debarshiray
force-pushed
the
wip/rishi/run-optimize-fallbacks
branch
from
11051a6 to
4536e2c
Compare
|
Build failed.
|
debarshiray
commented
Sep 9, 2021
| err = fmt.Errorf("directory %s not found in container %s", workDir, container) | ||
| return err | ||
| } | ||
| } else { |
There was a problem hiding this comment.
We can't assume that an exit code of 127 from podman exec means that either the working directory or the command was absent. The given shell might also return with 127, which will get forwarded by podman exec, if the last command that was attempted inside the shell was absent. In such cases this will create a loop.
See #872
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently, the
entercommand involves two extra invocations ofpodman execto detect if the user's chosen shell and current workingdirectory are present inside the toolbox container. Each invocation is
sufficiently expensive to add a noticeable overhead to the
enterandruncommands. 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 extraerror message that seems difficult to get rid of: