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

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 15be6f12358f4ccc2211a9e0709d5e5467572cf3..154a7a2e66a06aa67b24e6bded5d8063bfc2970c 100644 (file)
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -1021,12 +1021,15 @@
 
       <varlistentry>
         <term><varname>RestartForceExitStatus=</varname></term>
-        <listitem><para>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 <varname>Restart=</varname>. The argument format is
-        similar to
-        <varname>RestartPreventExitStatus=</varname>.</para>
+
+        <listitem><para>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
+        <varname>Restart=</varname>. The argument format is similar to <varname>RestartPreventExitStatus=</varname>.
+        </para>
+
+        <para>Note that for <varname>Type=oneshot</varname> 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.</para>
 
         <xi:include href="version-info.xml" xpointer="v215"/></listitem>
       </varlistentry>

diff --git a/src/core/service.c b/src/core/service.c
index ae3f9ed266ef1ac3b226317ad14c380197c6a82f..d2b8c18af1687068f880b36dd3f4818930ba5bdc 100644 (file)
--- 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 (executable)
index 0000000..4c9e10b
--- /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 433cd698187351b4064e52e423474a3bc49b399d..bb4d664945e12622c36ca2ab492492d21281b737 100755 (executable)
--- 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" <<EOF
+[Unit]
+OnSuccess=$ONSUCCESS_UNIT_NAME
+
+[Service]
+Type=oneshot
+RestartForceExitStatus=0 2
+ExecStart=/usr/lib/systemd/tests/testdata/testsuite-23.units/testsuite-23-oneshot-restartforce.sh "$TMP_FILE"
+
+[Install]
+WantedBy=multi-user.target
+EOF
+
+cat >"/run/systemd/system/$ONSUCCESS_UNIT_NAME" <<EOF
+[Service]
+Type=oneshot
+ExecStart=bash -c 'echo finished >$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) <<EOF
+Result=success
+NRestarts=1
+SubState=dead
+EOF
+
+systemctl disable "$UNIT_NAME"
+rm "$TMP_FILE" /run/systemd/system/{"$UNIT_NAME","$ONSUCCESS_UNIT_NAME"} "$FIFO_FILE"
 
-systemd-analyze log-level info
+systemctl log-level info