Skip to content

libct/int: better error reporting#5232

Merged
rata merged 5 commits intoopencontainers:mainfrom
kolyshkin:int-wait-stderr
Apr 15, 2026
Merged

libct/int: better error reporting#5232
rata merged 5 commits intoopencontainers:mainfrom
kolyshkin:int-wait-stderr

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Apr 8, 2026

Saw the following failure (from here):

=== RUN   TestHook
time="2026-04-08T18:10:08Z" level=warning msg="cannot serialize hook of type configs.FuncHook, skipping"
....
time="2026-04-08T18:10:08Z" level=warning msg="cannot serialize hook of type configs.FuncHook, skipping"
    exec_test.go:1090: unexpected error: exit status 1
--- FAIL: TestHook (0.09s)

It looks like we're running commands and expect them to exit(0), but when they fail we have no way of knowing what went wrong.

Address this by showing process' stderr if something bad happens.

While at it,

  • remove buffers.Stdin (unused);
  • replace bytes.Buffer with strings.Builder.

Related to #5245

Copy link
Copy Markdown
Member

@rata rata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overall LGTM, left a simple comment about a typo pi vs pid that is fixed later in another commit.

Comment thread libcontainer/integration/exec_test.go Outdated
Comment thread libcontainer/integration/exec_test.go
@kolyshkin kolyshkin mentioned this pull request Apr 13, 2026
Since Wait returns an ExitError if process' exit status is not 0,
checking process status is redundant and this code is never reached.

Remove it.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
When running a process inside a container, make sure its stderr is not
nil (except for some trivial cases like cat). Modify waitProcess to show
failed command's stderr, if possible.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
By default, readlink is silent about any errors. Make it verbose so we
can better interpret any test failures.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
It is never used.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The latter is simpler and provides just enough functionality to be used
here.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin requested a review from rata April 15, 2026 00:06
@kolyshkin kolyshkin added the backport/1.5-todo A PR in main branch which needs to be backported to release-1.5 label Apr 15, 2026
Copy link
Copy Markdown
Member

@rata rata left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@rata rata merged commit 6f42442 into opencontainers:main Apr 15, 2026
55 checks passed
@kolyshkin kolyshkin added backport/1.5-done A PR in main branch which has been backported to release-1.5 and removed backport/1.5-todo A PR in main branch which needs to be backported to release-1.5 labels Apr 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci backport/1.5-done A PR in main branch which has been backported to release-1.5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants