File 0001-tests-when-running-a-manager-object-in-a-t... of Package systemd

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-tests-when-running-a-manager-object-in-a-test-migrat.patch of Package systemd (Revision a47769d8fb254159c07462a5675e1695)

Currently displaying revision a47769d8fb254159c07462a5675e1695 , Show latest

From d8fcfb3b926db0d55f47dfb414c8896649ab1739 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 9 Aug 2017 15:42:49 +0200
Subject: [PATCH 01/51] tests: when running a manager object in a test, migrate
 to private cgroup subroot first (#6576)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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 #5895.

(cherry picked from commit 8c759b33a435c6a9390a18ebe9259402e2fc595e)

[fbui: adjust context]
[fbui: added the automake bits]
---
 Makefile.am                 | 21 ++++++++++++++-------
 src/test/meson.build        | 21 ++++++++++++++-------
 src/test/test-cgroup-mask.c |  2 ++
 src/test/test-engine.c      |  2 ++
 src/test/test-execute.c     |  2 ++
 src/test/test-helper.c      | 41 +++++++++++++++++++++++++++++++++++++++++
 src/test/test-helper.h      |  2 ++
 src/test/test-path.c        |  2 ++
 src/test/test-sched-prio.c  |  2 ++
 src/test/test-unit-file.c   |  2 ++
 src/test/test-unit-name.c   |  3 +++
 11 files changed, 86 insertions(+), 14 deletions(-)
 create mode 100644 src/test/test-helper.c

diff --git a/Makefile.am b/Makefile.am
index 2164c95263..73058b6bc8 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1787,7 +1787,8 @@ test_device_nodes_LDADD = \
 	libsystemd-shared.la
 
 test_engine_SOURCES = \
-	src/test/test-engine.c
+	src/test/test-engine.c \
+	src/test/test-helper.c
 
 test_engine_CFLAGS = \
 	$(AM_CFLAGS) \
@@ -1850,7 +1851,8 @@ test_boot_timestamps_LDADD = \
 endif
 
 test_unit_name_SOURCES = \
-	src/test/test-unit-name.c
+	src/test/test-unit-name.c \
+	src/test/test-helper.c
 
 test_unit_name_CFLAGS = \
 	$(AM_CFLAGS) \
@@ -1861,7 +1863,8 @@ test_unit_name_LDADD = \
 	libcore.la
 
 test_unit_file_SOURCES = \
-	src/test/test-unit-file.c
+	src/test/test-unit-file.c \
+	src/test/test-helper.c
 
 test_unit_file_CFLAGS = \
 	$(AM_CFLAGS) \
@@ -2382,7 +2385,8 @@ test_cgroup_LDADD = \
 	libsystemd-shared.la
 
 test_cgroup_mask_SOURCES = \
-	src/test/test-cgroup-mask.c
+	src/test/test-cgroup-mask.c \
+	src/test/test-helper.c
 
 test_cgroup_mask_CPPFLAGS = \
 	$(AM_CPPFLAGS) \
@@ -2426,7 +2430,8 @@ test_path_util_LDADD = \
 	libsystemd-shared.la
 
 test_path_SOURCES = \
-	src/test/test-path.c
+	src/test/test-path.c \
+	src/test/test-helper.c
 
 test_path_CFLAGS = \
 	$(AM_CFLAGS) \
@@ -2436,7 +2441,8 @@ test_path_LDADD = \
 	libcore.la
 
 test_execute_SOURCES = \
-	src/test/test-execute.c
+	src/test/test-execute.c \
+	src/test/test-helper.c
 
 test_execute_CFLAGS = \
 	$(AM_CFLAGS) \
@@ -2471,7 +2477,8 @@ test_watchdog_LDADD = \
 	libsystemd-shared.la
 
 test_sched_prio_SOURCES = \
-	src/test/test-sched-prio.c
+	src/test/test-sched-prio.c \
+	src/test/test-helper.c
 
 test_sched_prio_CPPFLAGS = \
 	$(AM_CPPFLAGS) \
diff --git a/src/test/meson.build b/src/test/meson.build
index 7b493a4d05..20ba3a45ad 100644
--- a/src/test/meson.build
+++ b/src/test/meson.build
@@ -42,7 +42,8 @@ tests += [
          [],
          []],
 
-        [['src/test/test-engine.c'],
+        [['src/test/test-engine.c',
+          'src/test/test-helper.c'],
          [libcore,
           libudev,
           libsystemd_internal],
@@ -105,7 +106,8 @@ tests += [
          [],
          'ENABLE_EFI'],
 
-        [['src/test/test-unit-name.c'],
+        [['src/test/test-unit-name.c',
+          'src/test/test-helper.c'],
          [libcore,
           libshared],
          [threads,
@@ -115,7 +117,8 @@ tests += [
           libmount,
           libblkid]],
 
-        [['src/test/test-unit-file.c'],
+        [['src/test/test-unit-file.c',
+          'src/test/test-helper.c'],
          [libcore,
           libshared],
          [threads,
@@ -456,7 +459,8 @@ tests += [
          '', 'manual'],
 
 
-        [['src/test/test-cgroup-mask.c'],
+        [['src/test/test-cgroup-mask.c',
+          'src/test/test-helper.c'],
          [libcore,
           libshared],
          [threads,
@@ -486,7 +490,8 @@ tests += [
          [],
          []],
 
-        [['src/test/test-path.c'],
+        [['src/test/test-path.c',
+          'src/test/test-helper.c'],
          [libcore,
           libshared],
          [threads,
@@ -496,7 +501,8 @@ tests += [
           libmount,
           libblkid]],
 
-        [['src/test/test-execute.c'],
+        [['src/test/test-execute.c',
+          'src/test/test-helper.c'],
          [libcore,
           libshared],
          [threads,
@@ -524,7 +530,8 @@ tests += [
          [],
          []],
 
-        [['src/test/test-sched-prio.c'],
+        [['src/test/test-sched-prio.c',
+          'src/test/test-helper.c'],
          [libcore,
           libshared],
          [threads,
diff --git a/src/test/test-cgroup-mask.c b/src/test/test-cgroup-mask.c
index b42088c680..4a13d710dc 100644
--- a/src/test/test-cgroup-mask.c
+++ b/src/test/test-cgroup-mask.c
@@ -34,6 +34,8 @@ static int test_cgroup_mask(void) {
         FDSet *fdset = NULL;
         int r;
 
+        enter_cgroup_subroot();
+
         /* Prepare the manager. */
         assert_se(set_unit_path(get_testdata_dir("")) >= 0);
         assert_se(runtime_dir = setup_fake_runtime_dir());
diff --git a/src/test/test-engine.c b/src/test/test-engine.c
index 8133343fb3..b5d3da0d38 100644
--- a/src/test/test-engine.c
+++ b/src/test/test-engine.c
@@ -37,6 +37,8 @@ int main(int argc, char *argv[]) {
         Job *j;
         int r;
 
+        enter_cgroup_subroot();
+
         /* prepare the test */
         assert_se(set_unit_path(get_testdata_dir("")) >= 0);
         assert_se(runtime_dir = setup_fake_runtime_dir());
diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index 29c8fd613f..ac2cdd50ed 100644
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -526,6 +526,8 @@ int main(int argc, char *argv[]) {
                 return EXIT_TEST_SKIP;
         }
 
+        enter_cgroup_subroot();
+
         assert_se(setenv("XDG_RUNTIME_DIR", "/tmp/", 1) == 0);
         assert_se(set_unit_path(get_testdata_dir("/test-execute")) >= 0);
 
diff --git a/src/test/test-helper.c b/src/test/test-helper.c
new file mode 100644
index 0000000000..5b707c3276
--- /dev/null
+++ b/src/test/test-helper.c
@@ -0,0 +1,41 @@
+/***
+  This file is part of systemd.
+
+  Copyright 2017 Lennart Poettering
+
+  systemd is free software; you can redistribute it and/or modify it
+  under the terms of the GNU Lesser General Public License as published by
+  the Free Software Foundation; either version 2.1 of the License, or
+  (at your option) any later version.
+
+  systemd is distributed in the hope that it will be useful, but
+  WITHOUT ANY WARRANTY; without even the implied warranty of
+  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+  Lesser General Public License for more details.
+
+  You should have received a copy of the GNU Lesser General Public License
+  along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include "test-helper.h"
+#include "random-util.h"
+#include "alloc-util.h"
+#include "cgroup-util.h"
+
+void enter_cgroup_subroot(void) {
+        _cleanup_free_ char *cgroup_root = NULL, *cgroup_subroot = NULL;
+        CGroupMask supported;
+
+        assert_se(cg_pid_get_path(NULL, 0, &cgroup_root) >= 0);
+        assert_se(asprintf(&cgroup_subroot, "%s/%" PRIx64, cgroup_root, random_u64()) >= 0);
+        assert_se(cg_mask_supported(&supported) >= 0);
+
+        /* If this fails, then we don't mind as the later cgroup operations will fail too, and it's fine if we handle
+         * any errors at that point. */
+
+        if (cg_create_everywhere(supported, _CGROUP_MASK_ALL, cgroup_subroot) < 0)
+                return;
+
+        if (cg_attach_everywhere(supported, cgroup_subroot, 0, NULL, NULL) < 0)
+                return;
+}
diff --git a/src/test/test-helper.h b/src/test/test-helper.h
index ddb10f88fd..8af32c8744 100644
--- a/src/test/test-helper.h
+++ b/src/test/test-helper.h
@@ -39,3 +39,5 @@
                -ENOENT,                                         \
                -ENOMEDIUM /* cannot determine cgroup */         \
                )
+
+void enter_cgroup_subroot(void);
diff --git a/src/test/test-path.c b/src/test/test-path.c
index 70ac6b3df3..6fc3f79d80 100644
--- a/src/test/test-path.c
+++ b/src/test/test-path.c
@@ -45,6 +45,8 @@ static int setup_test(Manager **m) {
 
         assert_se(m);
 
+        enter_cgroup_subroot();
+
         r = manager_new(UNIT_FILE_USER, true, &tmp);
         if (MANAGER_SKIP_TEST(r)) {
                 log_notice_errno(r, "Skipping test: manager_new: %m");
diff --git a/src/test/test-sched-prio.c b/src/test/test-sched-prio.c
index 81d9abc2d5..062a02baf5 100644
--- a/src/test/test-sched-prio.c
+++ b/src/test/test-sched-prio.c
@@ -34,6 +34,8 @@ int main(int argc, char *argv[]) {
         FDSet *fdset = NULL;
         int r;
 
+        enter_cgroup_subroot();
+
         /* prepare the test */
         assert_se(set_unit_path(get_testdata_dir("")) >= 0);
         assert_se(runtime_dir = setup_fake_runtime_dir());
diff --git a/src/test/test-unit-file.c b/src/test/test-unit-file.c
index fd797b587e..d0c844dddb 100644
--- a/src/test/test-unit-file.c
+++ b/src/test/test-unit-file.c
@@ -848,6 +848,8 @@ int main(int argc, char *argv[]) {
         log_parse_environment();
         log_open();
 
+        enter_cgroup_subroot();
+
         assert_se(runtime_dir = setup_fake_runtime_dir());
 
         r = test_unit_file_get_set();
diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c
index 2fd83f321c..8ef7998fac 100644
--- a/src/test/test-unit-name.c
+++ b/src/test/test-unit-name.c
@@ -464,6 +464,9 @@ static void test_unit_name_path_unescape(void) {
 
 int main(int argc, char* argv[]) {
         int rc = 0;
+
+        enter_cgroup_subroot();
+
         test_unit_name_is_valid();
         test_unit_name_replace_instance();
         test_unit_name_from_path();
-- 
2.16.4

Places

File 0001-tests-when-running-a-manager-object-in-a-test-migrat.patch of Package systemd (Revision a47769d8fb254159c07462a5675e1695)

Places