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

tests/int: runc delete: fix flake, enable for rootless #3392

Merged
merged 1 commit into from
Mar 22, 2022

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Feb 22, 2022

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.

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 right
fix, 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

@kolyshkin kolyshkin force-pushed the test-runc-delete-flake branch 4 times, most recently from 8b67ba9 to 49109c7 Compare February 23, 2022 19:40
@kolyshkin kolyshkin marked this pull request as draft February 23, 2022 19:53
@kolyshkin kolyshkin force-pushed the test-runc-delete-flake branch from 49109c7 to 90b4699 Compare February 23, 2022 21:47
@kolyshkin kolyshkin mentioned this pull request Mar 1, 2022
@kolyshkin kolyshkin force-pushed the test-runc-delete-flake branch from 90b4699 to d6dfca4 Compare March 1, 2022 19:23
@kolyshkin kolyshkin requested a review from AkihiroSuda March 1, 2022 19:36
@kolyshkin kolyshkin marked this pull request as ready for review March 2, 2022 01:30
@kolyshkin
Copy link
Contributor Author

No longer a draft; PTAL @AkihiroSuda

@kolyshkin kolyshkin force-pushed the test-runc-delete-flake branch from d6dfca4 to b33ca61 Compare March 7, 2022 23:32
@kolyshkin kolyshkin changed the title tests/int: fix flake in runc delete test tests/int: fix flake in runc delete test, enable it for rootless Mar 7, 2022
@kolyshkin kolyshkin changed the title tests/int: fix flake in runc delete test, enable it for rootless tests/int: runc delete: fix flake, enable for rootless Mar 7, 2022
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 <[email protected]>
@kolyshkin kolyshkin force-pushed the test-runc-delete-flake branch from b33ca61 to 728571c Compare March 7, 2022 23:33
@kolyshkin
Copy link
Contributor Author

@opencontainers/runc-maintainers an we please have this merged? This fixes a [rare] flake in CI, and enables runc delete for rootless.

@hqhq hqhq merged commit c258ed0 into opencontainers:main Mar 22, 2022
@kolyshkin kolyshkin mentioned this pull request Jul 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

not ok 42 runc delete: cgroup not cleaned up correctly
3 participants