pkg/shell: Preserve error types in shell command execution#1780
Open
DaliborKr wants to merge 1 commit into
Open
pkg/shell: Preserve error types in shell command execution#1780DaliborKr wants to merge 1 commit into
DaliborKr wants to merge 1 commit into
Conversation
The existing RunContextWithExitCode() wraps all errors from exec.Command in generic "failed to invoke" messages, which prevents callers from distinguishing between actual error types. Add RunContextWithExitCode2() and RunWithExitCode2() that return errors with their original types intact, including for ExitError. This allows callers to use errors.Is() and errors.As() to handle specific failure modes. For example, detecting a missing skopeo binary (exec.ErrNotFound) or an ENOEXEC error from inside non native containers, when an emulator is not set correctly. These new functions are meant to replace its original versions in the future. containers#1780 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
DaliborKr
force-pushed
the
pr01-shell-error-types
branch
from
588461c to
d28cb8c
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces RunContextWithExitCode2 and RunWithExitCode2 to the shell package. The feedback suggests refactoring the code to reduce duplication between the new and existing functions, as well as simplifying the error handling logic to be more idiomatic by removing redundant variables.
Comment on lines
+101
to
+113
| exitCode := 1 | ||
|
|
||
| if ctxErr := ctx.Err(); ctxErr != nil { | ||
| return 1, ctxErr | ||
| } | ||
|
|
||
| var exitErr *exec.ExitError | ||
| if errors.As(err, &exitErr) { | ||
| exitCode = exitErr.ExitCode() | ||
| return exitCode, err | ||
| } | ||
|
|
||
| return exitCode, err |
There was a problem hiding this comment.
The error handling logic can be simplified by removing the redundant exitCode variable and returning values directly. This makes the function more idiomatic and easier to read.
if ctxErr := ctx.Err(); ctxErr != nil {
return 1, ctxErr
}
var exitErr *exec.ExitError
if errors.As(err, &exitErr) {
return exitErr.ExitCode(), err
}
return 1, err
|
Build succeeded. ✔️ unit-test SUCCESS in 2m 13s |
This was referenced Apr 23, 2026
DaliborKr
added a commit
to DaliborKr/toolbox
that referenced
this pull request
Apr 24, 2026
Add the --arch and --arch-emulator-path flags to the init-container command, passed from the create command when creating a cross-architecture container. The --arch flag defaults to the host architecture ID so that existing native containers continue to work without changes. When the container's architecture differs from the host, the init-container entry point configures QEMU emulation inside the container before any foreign-architecture binaries can run: 1. Validate QEMU emulation by running the 'true' command, which fails with ENOEXEC if the host's binfmt_misc registration is not working (detected via RunWithExitCode2() added in [1]), because it is necessary to have host emulation working to emulate the binfmt_misc registration in the following step. 2. Mount a fresh binfmt_misc filesystem inside the container via MountBinfmtMisc() (added in [2]) to create a sandboxed binfmt_misc registration with the C flag. 3. Validate architecture support via IsArchSupportedOnInitialization() (added in [3]), which verifies the QEMU interpreter at the host-mounted path under /run/host. 4. Register the QEMU interpreter with the C flag via RegisterBinfmtMisc() (added and explained in [2]) The binfmt_misc registration is performed inside the container rather than relying on the host's registration, as explained in [2]. Update showEntryPointLog() in run.go to propagate lines prefixed with 'Warning:' to stderr on the host, instead of treating them as errors. This is needed because the cross-architecture initialization may emit warnings that should be visible to the user but are not fatal. [1] containers#1780 [2] containers#1782 [3] containers#1783 containers#1788 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
DaliborKr
added a commit
to DaliborKr/toolbox
that referenced
this pull request
Apr 24, 2026
Add the --arch and --arch-emulator-path flags to the init-container command, passed from the create command when creating a cross-architecture container. The --arch flag defaults to the host architecture ID so that existing native containers continue to work without changes. When the container's architecture differs from the host, the init-container entry point configures QEMU emulation inside the container before any foreign-architecture binaries can run: 1. Validate QEMU emulation by running the 'true' command, which fails with ENOEXEC if the host's binfmt_misc registration is not working (detected via RunWithExitCode2() added in [1]), because it is necessary to have host emulation working to emulate the binfmt_misc registration in the following step. 2. Mount a fresh binfmt_misc filesystem inside the container via MountBinfmtMisc() (added in [2]) to create a sandboxed binfmt_misc registration with the C flag. 3. Validate architecture support via IsArchSupportedOnInitialization() (added in [3]), which verifies the QEMU interpreter at the host-mounted path under /run/host. 4. Register the QEMU interpreter with the C flag via RegisterBinfmtMisc() (added and explained in [2]) The binfmt_misc registration is performed inside the container rather than relying on the host's registration, as explained in [2]. Update showEntryPointLog() in run.go to propagate lines prefixed with 'Warning:' to stderr on the host, instead of treating them as errors. This is needed because the cross-architecture initialization may emit warnings that should be visible to the user but are not fatal. [1] containers#1780 [2] containers#1782 [3] containers#1783 containers#1788 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
DaliborKr
added a commit
to DaliborKr/toolbox
that referenced
this pull request
Apr 24, 2026
Add the --arch and --arch-emulator-path flags to the init-container command, passed from the create command when creating a cross-architecture container. The --arch flag defaults to the host architecture ID so that existing native containers continue to work without changes. When the container's architecture differs from the host, the init-container entry point configures QEMU emulation inside the container before any foreign-architecture binaries can run: 1. Validate QEMU emulation by running the 'true' command, which fails with ENOEXEC if the host's binfmt_misc registration is not working (detected via RunWithExitCode2() added in [1]), because it is necessary to have host emulation working to emulate the binfmt_misc registration in the following step. 2. Mount a fresh binfmt_misc filesystem inside the container via MountBinfmtMisc() (added in [2]) to create a sandboxed binfmt_misc registration with the C flag. 3. Validate architecture support via IsArchSupportedOnInitialization() (added in [3]), which verifies the QEMU interpreter at the host-mounted path under /run/host. 4. Register the QEMU interpreter with the C flag via RegisterBinfmtMisc() (added and explained in [2]) The binfmt_misc registration is performed inside the container rather than relying on the host's registration, as explained in [2]. Update showEntryPointLog() in run.go to propagate lines prefixed with 'Warning:' to stderr on the host, instead of treating them as errors. This is needed because the cross-architecture initialization may emit warnings that should be visible to the user but are not fatal. [1] containers#1780 [2] containers#1782 [3] containers#1783 containers#1788 Signed-off-by: Dalibor Kricka <dalidalk@seznam.cz>
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This is PR 1/10 in a series adding cross-architecture container support using QEMU and binfmt_misc.
Summary
RunContextWithExitCode2()andRunWithExitCode2()to the shell packageexec.ErrNotFound,exec.ExitError,syscall.ENOEXEC, etc.) instead of wrapping them in generic messageserrors.Is()anderrors.As()to handle specific failure modesThese new functions are meant to replace its original versions in the future.