diff options
author | Daan De Meyer <daan.j.demeyer@gmail.com> | 2021-12-17 20:01:31 +0100 |
---|---|---|
committer | Daan De Meyer <daan.j.demeyer@gmail.com> | 2021-12-18 11:26:25 +0100 |
commit | aaae822b37aa3ca39aebb516fdc6bef36d730c25 (patch) | |
tree | 8fa911e2f2010960a22297ab0ba7d0ed0a2bbf42 | |
parent | 40f41f34d4af15d0147b5b2525f0b87ff62eae9a (diff) | |
download | systemd-aaae822b37aa3ca39aebb516fdc6bef36d730c25.tar.gz |
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.
-rw-r--r-- | src/core/path.c | 10 | ||||
-rw-r--r-- | src/core/path.h | 3 | ||||
-rw-r--r-- | test/testsuite-63.units/test63.service | 2 | ||||
-rw-r--r-- | test/units/testsuite-63.service | 21 |
4 files changed, 31 insertions, 5 deletions
diff --git a/src/core/path.c b/src/core/path.c index 1b0f115dd8..f89e35a001 100644 --- a/src/core/path.c +++ b/src/core/path.c @@ -265,6 +265,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) { @@ -494,6 +497,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."); @@ -836,6 +845,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 66ae857dc4..d835c24166 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 = -EINVAL, } PathResult; @@ -61,6 +62,8 @@ struct Path { mode_t directory_mode; PathResult result; + + RateLimit trigger_limit; }; void path_free_specs(Path *p); diff --git a/test/testsuite-63.units/test63.service b/test/testsuite-63.units/test63.service index 0253943f0c..1a8721d82c 100644 --- a/test/testsuite-63.units/test63.service +++ b/test/testsuite-63.units/test63.service @@ -1,6 +1,6 @@ # SPDX-License-Identifier: LGPL-2.1-or-later [Unit] -ConditionPathExists=!/tmp/nonexistent +ConditionPathExists=/tmp/nonexistent [Service] ExecStart=true diff --git a/test/units/testsuite-63.service b/test/units/testsuite-63.service index 0a8d143be9..40422127ff 100644 --- a/test/units/testsuite-63.service +++ b/test/units/testsuite-63.service @@ -5,13 +5,26 @@ 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 -P ActiveState)" = failed' -ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = start-limit-hit' +ExecStart=sh -x -c 'test "$(systemctl show test63.service -P ActiveState)" = inactive' +ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success' ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = failed' -ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = unit-start-limit-hit' +ExecStart=sh -x -c 'test "$(systemctl show test63.path -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 -P ActiveState)" = inactive' +ExecStart=sh -x -c 'test "$(systemctl show test63.service -P Result)" = success' +ExecStart=sh -x -c 'test "$(systemctl show test63.path -P ActiveState)" = active' +ExecStart=sh -x -c 'test "$(systemctl show test63.path -P Result)" = success' ExecStart=sh -x -c 'echo OK >/testok' |