128 lines
5.3 KiB
Diff
128 lines
5.3 KiB
Diff
From d61bd956e599dd747490d36ff793b63fb6a9fedc Mon Sep 17 00:00:00 2001
|
|
From: Daan De Meyer <daan.j.demeyer@gmail.com>
|
|
Date: Fri, 17 Dec 2021 20:01:31 +0100
|
|
Subject: [PATCH] core: Add trigger limit for path units
|
|
|
|
When conditions fail on a service unit, a path unit can cause
|
|
PID 1 to busy loop as it keeps trying to activate the service unit.
|
|
To avoid this from happening, add a trigger limit to the path unit,
|
|
identical to the trigger limit we have for socket units.
|
|
|
|
Initially, let's start with a high limit and not make it configurable.
|
|
If needed, we can add properties to configure the rate limit similar
|
|
to the ones we have for socket units.
|
|
|
|
(cherry picked from commit aaae822b37aa3ca39aebb516fdc6bef36d730c25)
|
|
|
|
Resolves: #2123801
|
|
---
|
|
src/core/path.c | 10 ++++++++++
|
|
src/core/path.h | 3 +++
|
|
test/TEST-63-ISSUE-17433/test63.service | 2 +-
|
|
test/TEST-63-ISSUE-17433/testsuite.service | 21 +++++++++++++++++----
|
|
4 files changed, 31 insertions(+), 5 deletions(-)
|
|
|
|
diff --git a/src/core/path.c b/src/core/path.c
|
|
index c2facf0b16..b899bde0de 100644
|
|
--- a/src/core/path.c
|
|
+++ b/src/core/path.c
|
|
@@ -238,6 +238,9 @@ static void path_init(Unit *u) {
|
|
assert(u->load_state == UNIT_STUB);
|
|
|
|
p->directory_mode = 0755;
|
|
+
|
|
+ p->trigger_limit.interval = 2 * USEC_PER_SEC;
|
|
+ p->trigger_limit.burst = 200;
|
|
}
|
|
|
|
void path_free_specs(Path *p) {
|
|
@@ -467,6 +470,12 @@ static void path_enter_running(Path *p) {
|
|
if (unit_stop_pending(UNIT(p)))
|
|
return;
|
|
|
|
+ if (!ratelimit_below(&p->trigger_limit)) {
|
|
+ log_unit_warning(UNIT(p), "Trigger limit hit, refusing further activation.");
|
|
+ path_enter_dead(p, PATH_FAILURE_TRIGGER_LIMIT_HIT);
|
|
+ return;
|
|
+ }
|
|
+
|
|
trigger = UNIT_TRIGGER(UNIT(p));
|
|
if (!trigger) {
|
|
log_unit_error(UNIT(p), "Unit to trigger vanished.");
|
|
@@ -767,6 +776,7 @@ static const char* const path_result_table[_PATH_RESULT_MAX] = {
|
|
[PATH_FAILURE_RESOURCES] = "resources",
|
|
[PATH_FAILURE_START_LIMIT_HIT] = "start-limit-hit",
|
|
[PATH_FAILURE_UNIT_START_LIMIT_HIT] = "unit-start-limit-hit",
|
|
+ [PATH_FAILURE_TRIGGER_LIMIT_HIT] = "trigger-limit-hit",
|
|
};
|
|
|
|
DEFINE_STRING_TABLE_LOOKUP(path_result, PathResult);
|
|
diff --git a/src/core/path.h b/src/core/path.h
|
|
index 8a69f06c13..12fd13fbe3 100644
|
|
--- a/src/core/path.h
|
|
+++ b/src/core/path.h
|
|
@@ -46,6 +46,7 @@ typedef enum PathResult {
|
|
PATH_FAILURE_RESOURCES,
|
|
PATH_FAILURE_START_LIMIT_HIT,
|
|
PATH_FAILURE_UNIT_START_LIMIT_HIT,
|
|
+ PATH_FAILURE_TRIGGER_LIMIT_HIT,
|
|
_PATH_RESULT_MAX,
|
|
_PATH_RESULT_INVALID = -1
|
|
} PathResult;
|
|
@@ -63,6 +64,8 @@ struct Path {
|
|
mode_t directory_mode;
|
|
|
|
PathResult result;
|
|
+
|
|
+ RateLimit trigger_limit;
|
|
};
|
|
|
|
void path_free_specs(Path *p);
|
|
diff --git a/test/TEST-63-ISSUE-17433/test63.service b/test/TEST-63-ISSUE-17433/test63.service
|
|
index c83801874d..6292434c5c 100644
|
|
--- a/test/TEST-63-ISSUE-17433/test63.service
|
|
+++ b/test/TEST-63-ISSUE-17433/test63.service
|
|
@@ -1,5 +1,5 @@
|
|
[Unit]
|
|
-ConditionPathExists=!/tmp/nonexistent
|
|
+ConditionPathExists=/tmp/nonexistent
|
|
|
|
[Service]
|
|
ExecStart=true
|
|
diff --git a/test/TEST-63-ISSUE-17433/testsuite.service b/test/TEST-63-ISSUE-17433/testsuite.service
|
|
index d3ca5b002b..39f9643890 100644
|
|
--- a/test/TEST-63-ISSUE-17433/testsuite.service
|
|
+++ b/test/TEST-63-ISSUE-17433/testsuite.service
|
|
@@ -4,14 +4,27 @@ Description=TEST-63-ISSUE-17433
|
|
[Service]
|
|
ExecStartPre=rm -f /failed /testok
|
|
Type=oneshot
|
|
+
|
|
+# Test that a path unit continuously triggering a service that fails condition checks eventually fails with
|
|
+# the trigger-limit-hit error.
|
|
ExecStart=rm -f /tmp/nonexistent
|
|
ExecStart=systemctl start test63.path
|
|
ExecStart=touch /tmp/test63
|
|
-# Make sure systemd has sufficient time to hit the start limit for test63.service.
|
|
+# Make sure systemd has sufficient time to hit the trigger limit for test63.path.
|
|
ExecStart=sleep 2
|
|
-ExecStart=sh -x -c 'test "$(systemctl show test63.service --value -p ActiveState)" = failed'
|
|
-ExecStart=sh -x -c 'test "$(systemctl show test63.service --value -p Result)" = start-limit-hit'
|
|
+ExecStart=sh -x -c 'test "$(systemctl show test63.service --value -p ActiveState)" = inactive'
|
|
+ExecStart=sh -x -c 'test "$(systemctl show test63.service --value -p Result)" = success'
|
|
# FIXME: The path remains active, which it should not
|
|
# ExecStart=sh -x -c 'test "$(systemctl show test63.path --value -p ActiveState)" = failed'
|
|
-# ExecStart=sh -x -c 'test "$(systemctl show test63.path --value -p Result)" = unit-start-limit-hit'
|
|
+# ExecStart=sh -x -c 'test "$(systemctl show test63.path --value -p Result)" = trigger-limit-hit'
|
|
+
|
|
+# Test that starting the service manually doesn't affect the path unit.
|
|
+ExecStart=rm -f /tmp/test63
|
|
+ExecStart=systemctl reset-failed
|
|
+ExecStart=systemctl start test63.path
|
|
+ExecStart=systemctl start test63.service
|
|
+ExecStart=sh -x -c 'test "$(systemctl show test63.service --value -p ActiveState)" = inactive'
|
|
+ExecStart=sh -x -c 'test "$(systemctl show test63.service --value -p Result)" = success'
|
|
+ExecStart=sh -x -c 'test "$(systemctl show test63.path --value -p ActiveState)" = active'
|
|
+ExecStart=sh -x -c 'test "$(systemctl show test63.path --value -p Result)" = success'
|
|
ExecStart=sh -x -c 'echo OK >/testok'
|