Skip to content
This repository was archived by the owner on Jul 18, 2019. It is now read-only.

Commit 8c759b3

Browse files
poetteringkeszybz
authored andcommitted
tests: when running a manager object in a test, migrate to private cgroup subroot first (systemd#6576)
Without this "meson test" will end up running all tests in the same cgroup root, and they all will try to manage it. Which usually isn't too bad, except when they end up clearing up each other's cgroups. This race is hard to trigger but has caused various CI runs to fail spuriously. With this change we simply move every test that runs a manager object into their own private cgroup. Note that we don't clean up the cgroup at the end, we leave that to the cgroup manager around it. This fixes races that become visible by test runs throwing out errors like this: ``` exec-systemcallfilter-failing.service: Passing 0 fds to service exec-systemcallfilter-failing.service: About to execute: /bin/echo 'This should not be seen' exec-systemcallfilter-failing.service: Forked /bin/echo as 5693 exec-systemcallfilter-failing.service: Changed dead -> start exec-systemcallfilter-failing.service: Failed to attach to cgroup /exec-systemcallfilter-failing.service: No such file or directory Received SIGCHLD from PID 5693 ((echo)). Child 5693 ((echo)) died (code=exited, status=219/CGROUP) exec-systemcallfilter-failing.service: Child 5693 belongs to exec-systemcallfilter-failing.service exec-systemcallfilter-failing.service: Main process exited, code=exited, status=219/CGROUP exec-systemcallfilter-failing.service: Changed start -> failed exec-systemcallfilter-failing.service: Unit entered failed state. exec-systemcallfilter-failing.service: Failed with result 'exit-code'. exec-systemcallfilter-failing.service: cgroup is empty Assertion 'service->main_exec_status.status == status_expected' failed at ../src/src/test/test-execute.c:71, function check(). Aborting. ``` BTW, I tracked this race down by using perf: ``` # perf record -e cgroup:cgroup_mkdir,cgroup_rmdir … # perf script ``` Thanks a lot @iaguis, @alban for helping me how to use perf for this. Fixes systemd#5895.
1 parent e85a690 commit 8c759b3

File tree

10 files changed

+71
-7
lines changed

10 files changed

+71
-7
lines changed

src/test/meson.build

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ tests += [
4242
[],
4343
[]],
4444

45-
[['src/test/test-engine.c'],
45+
[['src/test/test-engine.c',
46+
'src/test/test-helper.c'],
4647
[libcore,
4748
libudev,
4849
libsystemd_internal],
@@ -105,7 +106,8 @@ tests += [
105106
[],
106107
'ENABLE_EFI'],
107108

108-
[['src/test/test-unit-name.c'],
109+
[['src/test/test-unit-name.c',
110+
'src/test/test-helper.c'],
109111
[libcore,
110112
libshared],
111113
[threads,
@@ -115,7 +117,8 @@ tests += [
115117
libmount,
116118
libblkid]],
117119

118-
[['src/test/test-unit-file.c'],
120+
[['src/test/test-unit-file.c',
121+
'src/test/test-helper.c'],
119122
[libcore,
120123
libshared],
121124
[threads,
@@ -456,7 +459,8 @@ tests += [
456459
'', 'manual'],
457460

458461

459-
[['src/test/test-cgroup-mask.c'],
462+
[['src/test/test-cgroup-mask.c',
463+
'src/test/test-helper.c'],
460464
[libcore,
461465
libshared],
462466
[threads,
@@ -486,7 +490,8 @@ tests += [
486490
[],
487491
[]],
488492

489-
[['src/test/test-path.c'],
493+
[['src/test/test-path.c',
494+
'src/test/test-helper.c'],
490495
[libcore,
491496
libshared],
492497
[threads,
@@ -496,7 +501,8 @@ tests += [
496501
libmount,
497502
libblkid]],
498503

499-
[['src/test/test-execute.c'],
504+
[['src/test/test-execute.c',
505+
'src/test/test-helper.c'],
500506
[libcore,
501507
libshared],
502508
[threads,
@@ -524,7 +530,8 @@ tests += [
524530
[],
525531
[]],
526532

527-
[['src/test/test-sched-prio.c'],
533+
[['src/test/test-sched-prio.c',
534+
'src/test/test-helper.c'],
528535
[libcore,
529536
libshared],
530537
[threads,

src/test/test-cgroup-mask.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ static int test_cgroup_mask(void) {
3434
FDSet *fdset = NULL;
3535
int r;
3636

37+
enter_cgroup_subroot();
38+
3739
/* Prepare the manager. */
3840
assert_se(set_unit_path(get_testdata_dir("")) >= 0);
3941
assert_se(runtime_dir = setup_fake_runtime_dir());

src/test/test-engine.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,8 @@ int main(int argc, char *argv[]) {
3737
Job *j;
3838
int r;
3939

40+
enter_cgroup_subroot();
41+
4042
/* prepare the test */
4143
assert_se(set_unit_path(get_testdata_dir("")) >= 0);
4244
assert_se(runtime_dir = setup_fake_runtime_dir());

src/test/test-execute.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,8 @@ int main(int argc, char *argv[]) {
526526
return EXIT_TEST_SKIP;
527527
}
528528

529+
enter_cgroup_subroot();
530+
529531
assert_se(setenv("XDG_RUNTIME_DIR", "/tmp/", 1) == 0);
530532
assert_se(set_unit_path(get_testdata_dir("/test-execute")) >= 0);
531533

src/test/test-helper.c

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/***
2+
This file is part of systemd.
3+
4+
Copyright 2017 Lennart Poettering
5+
6+
systemd is free software; you can redistribute it and/or modify it
7+
under the terms of the GNU Lesser General Public License as published by
8+
the Free Software Foundation; either version 2.1 of the License, or
9+
(at your option) any later version.
10+
11+
systemd is distributed in the hope that it will be useful, but
12+
WITHOUT ANY WARRANTY; without even the implied warranty of
13+
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
14+
Lesser General Public License for more details.
15+
16+
You should have received a copy of the GNU Lesser General Public License
17+
along with systemd; If not, see <http://www.gnu.org/licenses/>.
18+
***/
19+
20+
#include "test-helper.h"
21+
#include "random-util.h"
22+
#include "alloc-util.h"
23+
#include "cgroup-util.h"
24+
25+
void enter_cgroup_subroot(void) {
26+
_cleanup_free_ char *cgroup_root = NULL, *cgroup_subroot = NULL;
27+
CGroupMask supported;
28+
29+
assert_se(cg_pid_get_path(NULL, 0, &cgroup_root) >= 0);
30+
assert_se(asprintf(&cgroup_subroot, "%s/%" PRIx64, cgroup_root, random_u64()) >= 0);
31+
assert_se(cg_mask_supported(&supported) >= 0);
32+
33+
/* If this fails, then we don't mind as the later cgroup operations will fail too, and it's fine if we handle
34+
* any errors at that point. */
35+
36+
if (cg_create_everywhere(supported, _CGROUP_MASK_ALL, cgroup_subroot) < 0)
37+
return;
38+
39+
if (cg_attach_everywhere(supported, cgroup_subroot, 0, NULL, NULL) < 0)
40+
return;
41+
}

src/test/test-helper.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,5 @@
3939
-ENOENT, \
4040
-ENOMEDIUM /* cannot determine cgroup */ \
4141
)
42+
43+
void enter_cgroup_subroot(void);

src/test/test-path.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ static int setup_test(Manager **m) {
4545

4646
assert_se(m);
4747

48+
enter_cgroup_subroot();
49+
4850
r = manager_new(UNIT_FILE_USER, true, &tmp);
4951
if (MANAGER_SKIP_TEST(r)) {
5052
log_notice_errno(r, "Skipping test: manager_new: %m");

src/test/test-sched-prio.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ int main(int argc, char *argv[]) {
3434
FDSet *fdset = NULL;
3535
int r;
3636

37+
enter_cgroup_subroot();
38+
3739
/* prepare the test */
3840
assert_se(set_unit_path(get_testdata_dir("")) >= 0);
3941
assert_se(runtime_dir = setup_fake_runtime_dir());

src/test/test-unit-file.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -848,6 +848,8 @@ int main(int argc, char *argv[]) {
848848
log_parse_environment();
849849
log_open();
850850

851+
enter_cgroup_subroot();
852+
851853
assert_se(runtime_dir = setup_fake_runtime_dir());
852854

853855
r = test_unit_file_get_set();

src/test/test-unit-name.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,8 @@ int main(int argc, char* argv[]) {
469469
log_parse_environment();
470470
log_open();
471471

472+
enter_cgroup_subroot();
473+
472474
assert_se(runtime_dir = setup_fake_runtime_dir());
473475

474476
test_unit_name_is_valid();

0 commit comments

Comments
 (0)