From 189a08e83d59b912723eef73bbc0cedf06670a88 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Thu, 1 Feb 2024 01:25:49 +0800 Subject: [PATCH] core/service: allow RestartForceExitStatus= for oneshot services I think this was just overlooked in #13754, which removed the restriction of Restart= on Type=oneshot services. There's no reason to prevent RestartForceExitStatus= now that Restart= has been allowed. Closes #31148 --- man/systemd.service.xml | 15 +++-- src/core/service.c | 11 +++- .../testsuite-23-oneshot-restartforce.sh | 11 ++++ test/units/testsuite-23.oneshot-restart.sh | 57 +++++++++++++++++-- 4 files changed, 81 insertions(+), 13 deletions(-) create mode 100755 test/testsuite-23.units/testsuite-23-oneshot-restartforce.sh diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 15be6f12358..154a7a2e66a 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -1021,12 +1021,15 @@ RestartForceExitStatus= - Takes a list of exit status definitions that, - when returned by the main service process, will force automatic - service restarts, regardless of the restart setting configured - with Restart=. The argument format is - similar to - RestartPreventExitStatus=. + + Takes a list of exit status definitions that, when returned by the main service + process, will force automatic service restarts, regardless of the restart setting configured with + Restart=. The argument format is similar to RestartPreventExitStatus=. + + + Note that for Type=oneshot services, a success exit status will prevent + them from auto-restarting, no matter whether the corresponding exit statuses are listed in this + option or not. diff --git a/src/core/service.c b/src/core/service.c index ae3f9ed266e..d2b8c18af16 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -665,9 +665,6 @@ static int service_verify(Service *s) { if (s->type == SERVICE_ONESHOT && IN_SET(s->restart, SERVICE_RESTART_ALWAYS, SERVICE_RESTART_ON_SUCCESS)) return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Service has Restart= set to either always or on-success, which isn't allowed for Type=oneshot services. Refusing."); - if (s->type == SERVICE_ONESHOT && !exit_status_set_is_empty(&s->restart_force_status)) - return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Service has RestartForceExitStatus= set, which isn't allowed for Type=oneshot services. Refusing."); - if (s->type == SERVICE_ONESHOT && s->exit_type == SERVICE_EXIT_CGROUP) return log_unit_error_errno(UNIT(s), SYNTHETIC_ERRNO(ENOEXEC), "Service has ExitType=cgroup set, which isn't allowed for Type=oneshot services. Refusing."); @@ -1900,6 +1897,7 @@ static int cgroup_good(Service *s) { static bool service_shall_restart(Service *s, const char **reason) { assert(s); + assert(reason); /* Don't restart after manual stops */ if (s->forbid_restart) { @@ -1915,6 +1913,13 @@ static bool service_shall_restart(Service *s, const char **reason) { /* Restart if the exit code/status are configured as restart triggers */ if (exit_status_set_test(&s->restart_force_status, s->main_exec_status.code, s->main_exec_status.status)) { + /* Don't allow Type=oneshot services to restart on success. Note that Restart=always/on-success + * is already rejected in service_verify. */ + if (s->type == SERVICE_ONESHOT && s->result == SERVICE_SUCCESS) { + *reason = "service type and exit status"; + return false; + } + *reason = "forced by exit status"; return true; } diff --git a/test/testsuite-23.units/testsuite-23-oneshot-restartforce.sh b/test/testsuite-23.units/testsuite-23-oneshot-restartforce.sh new file mode 100755 index 00000000000..4c9e10b2cbc --- /dev/null +++ b/test/testsuite-23.units/testsuite-23-oneshot-restartforce.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +# SPDX-License-Identifier: LGPL-2.1-or-later +set -eux +set -o pipefail + +if [[ -f "$1" ]]; then + exit 0 +fi + +touch "$1" +exit 2 diff --git a/test/units/testsuite-23.oneshot-restart.sh b/test/units/testsuite-23.oneshot-restart.sh index 433cd698187..bb4d664945e 100755 --- a/test/units/testsuite-23.oneshot-restart.sh +++ b/test/units/testsuite-23.oneshot-restart.sh @@ -3,12 +3,15 @@ set -eux set -o pipefail +# shellcheck source=test/units/util.sh +. "$(dirname "$0")"/util.sh + # Test oneshot unit restart on failure # wait this many secs for each test service to succeed in what is being tested MAX_SECS=60 -systemd-analyze log-level debug +systemctl log-level debug # test one: Restart=on-failure should restart the service (! systemd-run --unit=oneshot-restart-one -p Type=oneshot -p Restart=on-failure /bin/bash -c "exit 1") @@ -21,7 +24,7 @@ if [[ "$(systemctl show oneshot-restart-one.service -P NRestarts)" -le 0 ]]; the exit 1 fi -TMP_FILE="/tmp/test-41-oneshot-restart-test" +TMP_FILE="/tmp/test-23-oneshot-restart-test$RANDOM" : >$TMP_FILE @@ -32,7 +35,7 @@ TMP_FILE="/tmp/test-41-oneshot-restart-test" -p StartLimitBurst=3 \ -p Type=oneshot \ -p Restart=on-failure \ - -p ExecStart="/bin/bash -c \"printf a >>$TMP_FILE\"" /bin/bash -c "exit 1") + -p ExecStart="/bin/bash -c 'printf a >>$TMP_FILE'" /bin/bash -c "exit 1") # wait for at least 3 restarts for ((secs = 0; secs < MAX_SECS; secs++)); do @@ -48,5 +51,51 @@ sleep 5 if [[ $(cat $TMP_FILE) != "aaa" ]]; then exit 1 fi +rm "$TMP_FILE" + +# Test RestartForceExitStatus=. Note that success exit statuses are meant to be skipped + +TMP_FILE="/tmp/test-23-oneshot-restart-test$RANDOM" +UNIT_NAME="testsuite-23-oneshot-restartforce.service" +ONSUCCESS_UNIT_NAME="testsuite-23-oneshot-restartforce-onsuccess.service" +FIFO_FILE="/tmp/test-23-oneshot-restart-test-fifo" + +cat >"/run/systemd/system/$UNIT_NAME" <"/run/systemd/system/$ONSUCCESS_UNIT_NAME" <$FIFO_FILE' +EOF + +mkfifo "$FIFO_FILE" + +# Pin the unit in memory +systemctl enable "$UNIT_NAME" +# Initial run should fail +(! systemctl start "$UNIT_NAME") +# Wait for OnSuccess= +read -r x <"$FIFO_FILE" +assert_eq "$x" "finished" + +cmp -b <(systemctl show "$UNIT_NAME" -p Result -p NRestarts -p SubState) <