Revert 'Fix conmon exec exit status handling'#590
Conversation
test/06-exec-exit-status.bats
Outdated
| fi | ||
|
|
||
| # Test 1: Success case | ||
| if ! podman exec "$container_id" true; then |
There was a problem hiding this comment.
Does this work if podman is run without the --conmon option?
There was a problem hiding this comment.
Just re-pushed the PR. The test now explicitly uses the built conmon binary via --conmon flag, so it will work correctly regardless of system configuration and always tests the code being developed. This applies for local testing - the CI will always use the freshly built conmon binary regardless. But it's good to keep them both testing the development version.
Honny1
left a comment
There was a problem hiding this comment.
The new test appears correct; however, I cannot comment on the other parts of the pull request.
The original fix introduced a regression where exec operations always returned exit code 0 instead of the actual command exit code. This broke podman health checks and exec exit code propagation. The original code was working correctly - container_status contains the actual exit code of executed commands, while runtime_status only indicates whether the runtime successfully executed the command. Testing shows: - Before fc0a342: exec commands return correct exit codes - OK - After fc0a342: exec commands always return 0 - NOT OK - After this revert: exec commands return correct exit codes - OK Related issue: containers/podman#26981 Fixes: containers#589 Signed-off-by: Jindrich Novy <jnovy@redhat.com>
Luap99
left a comment
There was a problem hiding this comment.
reverting LGTM, I cannot really comment on the testing details here.
| fmt: | ||
| git ls-files -z \*.c \*.h | xargs -0 clang-format -i | ||
|
|
||
| git ls-files -z '*.bats' | xargs -0 sed -i 's/[[:space:]]\+$$//' |
There was a problem hiding this comment.
This one is just missing format fix for BATS tests to avoid trailing spaces - intentional.
The original fix introduced a regression where exec operations always returned exit code 0 instead of the actual command exit code. This broke podman health checks and exec exit code propagation.
The original code was working correctly - container_status contains the actual exit code of executed commands, while runtime_status only indicates whether the runtime successfully executed the command.
Testing shows:
Related issue: containers/podman#26981
Fixes: #589