Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd, test/system: Retain exit codes when forwarding to host #1534

Merged

Conversation

debarshiray
Copy link
Member

Some changes by Debarshi Ray.

#957
#1052

Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/5db32d7e9d234250bec2e2e317f228d8

✔️ unit-test SUCCESS in 5m 28s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 42s
✔️ unit-test-restricted SUCCESS in 5m 26s
system-test-fedora-rawhide FAILURE in 1h 34m 29s
system-test-fedora-40 FAILURE in 1h 33m 21s
system-test-fedora-39 FAILURE in 1h 32m 22s

@debarshiray debarshiray force-pushed the wip/rishi/cmd-forward-stderr-02 branch from 1842899 to 035806c Compare September 5, 2024 12:16
debarshiray added a commit to debarshiray/toolbox that referenced this pull request Sep 5, 2024
Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/2817409884824cf3b1f5ea21f86a1b98

✔️ unit-test SUCCESS in 5m 26s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 13s
✔️ unit-test-restricted SUCCESS in 5m 28s
system-test-fedora-rawhide FAILURE in 1h 31m 44s
system-test-fedora-40 FAILURE in 1h 43m 07s
system-test-fedora-39 FAILURE in 1h 35m 17s

This will make it easier to propagate the exit codes of subordinate
processes through an exitError instance, when toolbox(1) is invoked
inside a container, and invocation is forwarded to the host.

Cobra doesn't honour the root command's SilenceErrors, if an error
occurred when parsing the command line for a command, even though the
command was found.  However, Cobra does honour SilenceErrors, if the
error occurred afterwards.

Therefore, to avoid setting SilenceErrors in each and every command, it
was set in the PersistentPreRunE hook (ie., preRun), which is called
after all command line parsing has been successfully completed.

containers#957
@debarshiray debarshiray force-pushed the wip/rishi/cmd-forward-stderr-02 branch from 035806c to 1a3f4ff Compare September 6, 2024 15:07
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/fab396b38ae542c4ac48bc9a28c50dce

✔️ unit-test SUCCESS in 5m 34s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 11s
✔️ unit-test-restricted SUCCESS in 5m 37s
✔️ system-test-fedora-rawhide SUCCESS in 1h 30m 19s
✔️ system-test-fedora-40 SUCCESS in 1h 29m 55s
✔️ system-test-fedora-39 SUCCESS in 1h 34m 24s

Copy link

Build failed.
https://softwarefactory-project.io/zuul/t/local/buildset/bb3be0c52d0746a2bf44de9e6df3078e

✔️ unit-test SUCCESS in 5m 38s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 39s
✔️ unit-test-restricted SUCCESS in 5m 43s
system-test-fedora-rawhide FAILURE in 1h 33m 46s
system-test-fedora-40 FAILURE in 1h 33m 03s
system-test-fedora-39 FAILURE in 1h 34m 06s

@debarshiray debarshiray force-pushed the wip/rishi/cmd-forward-stderr-02 branch from e3ff9ba to fa24660 Compare September 9, 2024 12:11
The test suite uses its own separate local container/storage store to
isolate itself from the default store, so that the tests' interactions
with containers and images don't affect anything else.  This is done by
using the CONTAINERS_STORAGE_CONF environment variable [1] to specify a
separate storage.conf(5) file [2].

Therefore, when running the test suite, the CONTAINERS_STORAGE_CONF
environment variable must be preserved when forwarding toolbox(1)
invocations inside containers to the host.  Otherwise, the initial
toolbox(1) invocation on the host and the forwarded invocation running
on the host won't use the same local container/storage store.

This problem only impacts test cases that cover toolbox(1) code paths
that invoke podman(1).

[1] https://docs.podman.io/en/latest/markdown/podman.1.html

[2] https://manpages.debian.org/testing/containers-storage/containers-storage.conf.5.en.html

containers#957
containers#1052
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/4e44d931c9154cc3bfd6999935525373

✔️ unit-test SUCCESS in 5m 34s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 09s
✔️ unit-test-restricted SUCCESS in 5m 22s
✔️ system-test-fedora-rawhide SUCCESS in 1h 33m 57s
✔️ system-test-fedora-40 SUCCESS in 1h 31m 35s
✔️ system-test-fedora-39 SUCCESS in 1h 36m 54s

Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/6926bd2e17694241a99bae97819f5fde

✔️ unit-test SUCCESS in 5m 41s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 18s
✔️ unit-test-restricted SUCCESS in 5m 32s
✔️ system-test-fedora-rawhide SUCCESS in 1h 34m 18s
✔️ system-test-fedora-40 SUCCESS in 1h 35m 18s
✔️ system-test-fedora-39 SUCCESS in 1h 33m 29s

The lines were getting too wide to fit within a vertical 1080x1920
display.  Therefore, restrict them to 120 characters per line.

Fallout from f8e21a3

containers#1534
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/a918ad0ba4b74e23846d01ae0953fad8

✔️ unit-test SUCCESS in 5m 32s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 09s
✔️ unit-test-restricted SUCCESS in 5m 35s
✔️ system-test-fedora-rawhide SUCCESS in 1h 37m 12s
✔️ system-test-fedora-40 SUCCESS in 1h 34m 29s
✔️ system-test-fedora-39 SUCCESS in 1h 40m 02s

When 'toolbox run' is invoked on the host, an exit code of 126 from
'podman exec' means that the specified command couldn't be invoked
because it's not an executable.  eg., the command was actually a
directory.  Note that this doesn't mean that the command couldn't be
found.  That's denoted by exit code 127.

Secondly, Toolbx containers always have an executable toolbox(1) binary
at /usr/bin/toolbox and it's assumed that /usr/bin is always part of the
PATH environment variable.

When 'toolbox run toolbox ...' is invoked, the inner toolbox(1)
invocation will be forwarded back to the host by the Toolbx container's
/usr/bin/toolbox, which is always present as an executable.  Hence, if
the outer 'podman exec' on the host fails with an exit code of 126,
then it doesn't mean that the container didn't have a working toolbox(1)
executable, but that some subordinate process started by the container's
toolbox(1) failed with that exit code.

Therefore, handle this as a special case to avoid showing an extra error
message.  Otherwise, it leads to:
  $ toolbox run toolbox run /etc
  bash: line 1: /etc: Is a directory
  bash: line 1: exec: /etc: cannot execute: Is a directory
  Error: failed to invoke command /etc in container fedora-toolbox-40
  Error: failed to invoke command toolbox in container fedora-toolbox-40
  $ echo "$?"
  126

Instead, it will now be:
  $ toolbox run toolbox run /etc
  bash: line 1: /etc: Is a directory
  bash: line 1: exec: /etc: cannot execute: Is a directory
  Error: failed to invoke command /etc in container fedora-toolbox-40
  $ echo "$?"
  126

containers#957
containers#1052
When 'toolbox run' is invoked on the host, an exit code of 127 from
'podman exec' means either that the specified command couldn't be found
or that the working directory didn't exist.  The only way to tell these
two scenarios apart is to actually look inside the container.

Secondly, Toolbx containers always have an executable toolbox(1) binary
at /usr/bin/toolbox and it's assumed that /usr/bin is always part of the
PATH environment variable.

When 'toolbox run toolbox ...' is invoked, the inner toolbox(1)
invocation will be forwarded back to the host by the Toolbx container's
/usr/bin/toolbox, which is always present as an executable.  Hence, if
the outer 'podman exec' on the host fails with an exit code of 127,
then it doesn't mean that the container didn't have a toolbox(1)
executable, but that some subordinate process started by the container's
toolbox(1) failed with that exit code.

Therefore, handle this as a special case to avoid losing the exit code.
Otherwise, it leads to:
  $ toolbox run toolbox run non-existent-command
  bash: line 1: exec: non-existent-command: not found
  Error: command non-existent-command not found in container
      fedora-toolbox-40
  $ echo "$?"
  0

Instead, it will now be:
  $ toolbox run toolbox run non-existent-command
  bash: line 1: exec: non-existent-command: not found
  Error: command non-existent-command not found in container
      fedora-toolbox-40
  $ echo "$?"
  127

containers#957
containers#1052
Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/a8712b93f5d94ce99da25eddd0943094

✔️ unit-test SUCCESS in 5m 41s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 46s
✔️ unit-test-restricted SUCCESS in 5m 30s
✔️ system-test-fedora-rawhide SUCCESS in 1h 35m 27s
✔️ system-test-fedora-40 SUCCESS in 1h 32m 37s
✔️ system-test-fedora-39 SUCCESS in 1h 38m 54s

Copy link

Build succeeded.
https://softwarefactory-project.io/zuul/t/local/buildset/5835a211329844c281c983e3b7ffb274

✔️ unit-test SUCCESS in 5m 28s
✔️ unit-test-migration-path-for-coreos-toolbox SUCCESS in 3m 49s
✔️ unit-test-restricted SUCCESS in 5m 32s
✔️ system-test-fedora-rawhide SUCCESS in 1h 37m 19s
✔️ system-test-fedora-40 SUCCESS in 1h 36m 29s
✔️ system-test-fedora-39 SUCCESS in 1h 36m 41s

@debarshiray debarshiray merged commit 26a76f2 into containers:main Sep 10, 2024
3 checks passed
@debarshiray debarshiray deleted the wip/rishi/cmd-forward-stderr-02 branch September 10, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants