tests/int: runc delete: fix flake, enable for rootless#3392
Merged
hqhq merged 1 commit intoopencontainers:mainfrom Mar 22, 2022
Merged
tests/int: runc delete: fix flake, enable for rootless#3392hqhq merged 1 commit intoopencontainers:mainfrom
hqhq merged 1 commit intoopencontainers:mainfrom
Conversation
8b67ba9 to
49109c7
Compare
49109c7 to
90b4699
Compare
Merged
90b4699 to
d6dfca4
Compare
Contributor
Author
|
No longer a draft; PTAL @AkihiroSuda |
d6dfca4 to
b33ca61
Compare
The following failure was observed in CI (on centos-stream-8 in integration-cgroup suite): not ok 42 runc delete (from function `fail' in file tests/integration/helpers.bash, line 338, in test file tests/integration/delete.bats, line 30) `[ "$output" = "" ] || fail "cgroup not cleaned up correctly: $output"' failed .... cgroup not cleaned up correctly: /sys/fs/cgroup/pids/system.slice/tmp-bats\x2drun\x2d68012-runc.IPOypI-state-testbusyboxdelete-runc.zriC8C.mount /sys/fs/cgroup/cpu,cpuacct/system.slice/tmp-bats\x2drun\x2d68012-runc.IPOypI-state-testbusyboxdelete-runc.zriC8C.mount ... Apparently, this is a cgroup systemd creates for a mount unit which appears then runc does internal /proc/self/exe bind-mount. The test case should not take it into account. The second problem with this test is it does not check that cgroup actually exists when the container is running (so checking that it was removed after makes less sense). For example, in rootless mode the cgroup might not have been created. Fix the find arguments to look for a specific cgroup name, and add a check that these arguments are correct (i.e. the cgroup is found when the container is running). Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
b33ca61 to
728571c
Compare
Contributor
Author
|
@opencontainers/runc-maintainers an we please have this merged? This fixes a [rare] flake in CI, and enables |
AkihiroSuda
approved these changes
Mar 22, 2022
hqhq
approved these changes
Mar 22, 2022
Merged
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The following failure was observed in CI (on centos-stream-8 in
integration-cgroup suite):
Apparently, this is a cgroup systemd creates for a mount unit which
appears then runc does internal /proc/self/exe bind-mount. The test
case should not take it into account.
Fix the find arguments to look for a specific cgroup name, and add
a check that these arguments are correct (i.e. the cgroup is found
when the container is running).
This added correctness check reveals another problem -- for rootless,
the test is not able to create a cgroup, so the test case is not checking anything.
The obvious fix would be to add
requires rootless_cgroups. It is not the rightfix, because for rootless + fs cgroup driver runc does not actually create a cgroup
(it is done by tests/rootless.sh). So, require systemd (which allows to create
user cgroups), and do not test the fs driver.
Fixes: #3391