tests/int: use run -N|! for runc
#4946
Draft
Conversation
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
1. Remove the devicemapper driver mentions, and is it no longer supported by docker (or podman). 2. Remove the test example -- we have plenty of real ones. 3. Add a link to (well written and extensive) bats documentation. 4. Fix capitalization in a sentence. Signed-off-by: Kir Kolyshkin <[email protected]>
This is a bit opinionated, but some comments in integration tests do not really help to understand the nature of the tests being performed by stating something very obvious, like # run busybox detached runc run -d busybox To make things worse, these not-so-helpful messages are being copy/pasted over and over, and that is the main reason to remove them. Signed-off-by: Kir Kolyshkin <[email protected]>
Move the repetitive code and comment into setup_netns. Signed-off-by: Kir Kolyshkin <[email protected]>
...instead of an explicit or absent status check. Signed-off-by: Kir Kolyshkin <[email protected]>
Commands that are not run via "run" helper (cat, mkdir, __runc) do not set $status, so it makes no sense to check it. Fixes: 94505a0, ed54837 Signed-off-by: Kir Kolyshkin <[email protected]>
This misleading comment is obviously a copy/paste from the previous test. Fix it. Fixes: dd69623 Signed-off-by: Kir Kolyshkin <[email protected]>
In our bats tests, runc itself is a wrapper which calls bats run helper, so using "run runc" is wrong as it results in calling run helper twice. Fixes: 8d180e9 Signed-off-by: Kir Kolyshkin <[email protected]>
The "runc delete --force [paused container]" test case does not check runc pause exit code, and if added, the test fails in rootless tests, because: - not all rootless tests have access to cgroups; - rootless containers doesn't have default cgroups path. To fix, add: - setup for rootless case; - require cgroups_freezer; - runc pause exit code check. Signed-off-by: Kir Kolyshkin <[email protected]>
This is mostly to improve readability. While at it, make the script more robust by adding -e option to shell. The exception is echo $pid which is opportunistic and may fail depending on the order of pids in the file. Also, remove the empty comment and a shellcheck annotation. Fixes: c91fe9a Signed-off-by: Kir Kolyshkin <[email protected]>
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). Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
These places should check runc exit code but they don't. Signed-off-by: Kir Kolyshkin <[email protected]>
|
CI failure in almalinux9 is a known flake #4437. Restarted. |
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 currently includes #4945; draft until that one is merged.
This is the second part of tests/integration cleanup I wanted to do for quite some time.
For the first part, see #4945.
Improve runc wrapper
Add status check support (same as in bats' run helper).
Add PRE_CMD support (so we can use commands like taskset or timeout).
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.
Use the new functionality in cpu_affinity.bats and start.bats as a
showcase (the test of refactoring is in a separate commit).
refactor to use runc status checks
add missing runc status checks