runc icon indicating copy to clipboard operation
runc copied to clipboard

tests/int: use `run -N|!` for runc

Open kolyshkin opened this issue 2 months ago • 8 comments

This is the second part of tests/integration cleanup I wanted to do for quite some time. For the first part, see #4945.

  1. Improve runc wrapper

    1. Add status check support (same as in bats' run helper).

    2. Add PRE_CMD support (so we can use commands like taskset or timeout).

    3. Drop sane_helper since the output of the command is shown in case of an error, and we show the command itself in runc wrapper (unless -N or ! is provided -- in this case the command is shown by bats, together with the error). This does not show the output of successful commands which IMO is a net positive since we are almost always interested in failed command output only.

    4. Use the new functionality in cpu_affinity.bats and start.bats as a showcase (the test of refactoring is in a separate commit).

  2. refactor to use runc status checks

  3. add missing runc status checks

kolyshkin avatar Oct 18 '25 23:10 kolyshkin

CI failure in almalinux9 is a known flake https://github.com/opencontainers/runc/issues/4437. Restarted.

kolyshkin avatar Oct 20 '25 16:10 kolyshkin

Now we do:

runc start foo
[ $status -eq 0 ]

and runc hides a call to run.

With this PR, we now do

runc -0 start foo

I.e. we still hide run inside but allow runc to accept some of run options (-N and ! for now).

The alternative to that is use run explicitly:

run -0 runc start foo

I am open to any suggestions.

kolyshkin avatar Oct 26 '25 22:10 kolyshkin

The alternative to that is use run explicitly:

run -0 runc start foo

I like this better, it's also trivial to add taskset, etc. I think having a bash function named runc (instead of runc being the binary) is just confusing and it doesn't really add much. IMHO if we want such function, we should call it something like rrun.

Can we use run instead of the env var for the taskset commands?

But if you or others prefers as it is in this PR now, I'm fine too :)

rata avatar Oct 30 '25 15:10 rata

The alternative to that is use run explicitly:

run -0 runc start foo

I like this better, it's also trivial to add taskset, etc. I think having a bash function named runc (instead of runc being the binary) is just confusing and it doesn't really add much. IMHO if we want such function, we should call it something like rrun.

Can we use run instead of the env var for the taskset commands?

But if you or others prefers as it is in this PR now, I'm fine too :)

I'd prefer run -0 runc start foo over whatever we have now or this PR, even if it's more verbose (mostly because no one understands that runc in bats means run runc ....

One issue is, we can't mix bash functions and binaries together, IOW something like run -0 taskset xxx runc start foo won't work.

I'll try to come up with something better.

kolyshkin avatar Oct 30 '25 23:10 kolyshkin

Drop sane_helper since the output of the command is shown in case of an error, and we show the command itself in runc wrapper (unless -N or ! is provided -- in this case the command is shown by bats, together with the error). This does not show the output of successful commands which IMO is a net positive since we are almost always interested in failed command output only.

I have often found when debugging test failures that successful output is quite useful -- not least of which when the failing line is [[ "$output" == "foo" ]]. Even if you migrated all of the tests to \| grep -Fx foo, you still wouldn't be able to capture that information in the failing case. Maybe if there was a helper for this that only output on failure? (But then we're sacrificing readability again.)

cyphar avatar Oct 31 '25 01:10 cyphar

One issue is, we can't mix bash functions and binaries together, IOW something like run -0 taskset xxx runc start foo won't work.

Why? I mean, if runc is not a function, why wouldn't that work? If runc is a binary, wouldn't that work?

rata avatar Nov 03 '25 13:11 rata

One issue is, we can't mix bash functions and binaries together, IOW something like run -0 taskset xxx runc start foo won't work.

Why? I mean, if runc is not a function, why wouldn't that work? If runc is a binary, wouldn't that work?

If runc is a binary that works. The issue is, in our case runc is a function, as we have to add a few parameters like --root.

kolyshkin avatar Nov 03 '25 20:11 kolyshkin

I have often found when debugging test failures that successful output is quite useful -- not least of which when the failing line is [[ "$output" == "foo" ]].

This is what I wanted to work on next -- there's a bats helper with functions like assert_output which makes things way better as it prints what it got and what it wanted (in case of an error). Alas it's not part of bats-core and thus we need to add some installation steps.

kolyshkin avatar Nov 03 '25 20:11 kolyshkin