From: Mike Yuan <me@yhndnzj.com>
Date: Fri, 16 Feb 2024 12:00:06 +0000 (+0800)
Subject: core/exec-invoke: record correct exit status when failed to locate executable
X-Git-Tag: v256-rc1~809^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=80b18d217a0d2ef1f3b6793f8038a40ac0b5f719;p=thirdparty%2Fsystemd.git

core/exec-invoke: record correct exit status when failed to locate executable

Follow-up for 4d8b0f0f7aeadc401ac02f67576ccb1de8cf79e6

After the mentioned commit, when the ExecCommand executable is missing,
and failure will be ignored by manager, we exit with EXIT_SUCCESS at executor
side too. The behavior however contradicts systemd.service(5), which states:

> If the executable path is prefixed with "-", an exit code of the command
> normally considered a failure (i.e. non-zero exit status or abnormal exit
> due to signal is _recorded_, but has no further effect and is considered
> equivalent to success.

and thus makes debugging unexpected failures harder. Therefore, let's still
exit with EXIT_EXEC, but just skip LOG_ERR level log.
---

diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c
index 023ceef699e..ec765031edc 100644
--- a/src/core/exec-invoke.c
+++ b/src/core/exec-invoke.c
@@ -4792,26 +4792,17 @@ int exec_invoke(
         _cleanup_close_ int executable_fd = -EBADF;
         r = find_executable_full(command->path, /* root= */ NULL, context->exec_search_path, false, &executable, &executable_fd);
         if (r < 0) {
-                if (r != -ENOMEM && (command->flags & EXEC_COMMAND_IGNORE_FAILURE)) {
-                        log_exec_struct_errno(context, params, LOG_INFO, r,
-                                              "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR,
-                                              LOG_EXEC_INVOCATION_ID(params),
-                                              LOG_EXEC_MESSAGE(params,
-                                                               "Executable %s missing, skipping: %m",
-                                                               command->path),
-                                              "EXECUTABLE=%s", command->path);
-                        *exit_status = EXIT_SUCCESS;
-                        return 0;
-                }
-
                 *exit_status = EXIT_EXEC;
-                return log_exec_struct_errno(context, params, LOG_INFO, r,
-                                             "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR,
-                                             LOG_EXEC_INVOCATION_ID(params),
-                                             LOG_EXEC_MESSAGE(params,
-                                                              "Failed to locate executable %s: %m",
-                                                              command->path),
-                                             "EXECUTABLE=%s", command->path);
+                log_exec_struct_errno(context, params, LOG_INFO, r,
+                                      "MESSAGE_ID=" SD_MESSAGE_SPAWN_FAILED_STR,
+                                      LOG_EXEC_INVOCATION_ID(params),
+                                      LOG_EXEC_MESSAGE(params,
+                                                       "Unable to locate executable '%s': %m",
+                                                       command->path),
+                                      "EXECUTABLE=%s", command->path);
+                /* If the error will be ignored by manager, tune down the log level here. Missing executable
+                 * is very much expected in this case. */
+                return r != -ENOMEM && FLAGS_SET(command->flags, EXEC_COMMAND_IGNORE_FAILURE) ? 1 : r;
         }
 
         r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, &executable_fd);
diff --git a/src/core/executor.c b/src/core/executor.c
index b2716efeeaf..162561e9d43 100644
--- a/src/core/executor.c
+++ b/src/core/executor.c
@@ -250,7 +250,9 @@ static int run(int argc, char *argv[]) {
                                                        status, command.path),
                                       "EXECUTABLE=%s", command.path);
         } else
-                assert(exit_status == EXIT_SUCCESS); /* When 'skip' is chosen in the confirm spawn prompt */
+                /* r == 0: 'skip' is chosen in the confirm spawn prompt
+                 * r > 0:  expected/ignored failure, do not log at error level */
+                assert((r == 0) == (exit_status == EXIT_SUCCESS));
 
         return exit_status;
 }
diff --git a/test/units/testsuite-07.pr-31351.sh b/test/units/testsuite-07.pr-31351.sh
new file mode 100755
index 00000000000..f6911f28007
--- /dev/null
+++ b/test/units/testsuite-07.pr-31351.sh
@@ -0,0 +1,22 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+set -eux
+set -o pipefail
+
+# shellcheck source=test/units/util.sh
+. "$(dirname "$0")"/util.sh
+
+cat >/run/systemd/system/nonexistent-execstart-exit-status.service <<EOF
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=-/foo/bar/not-exist
+EOF
+
+systemctl start nonexistent-execstart-exit-status.service
+systemctl is-active nonexistent-execstart-exit-status.service
+assert_eq "$(systemctl show nonexistent-execstart-exit-status.service -P Result)" "success"
+(( $(systemctl show nonexistent-execstart-exit-status.service -P ExecMainStatus) > 0 ))
+
+systemctl stop nonexistent-execstart-exit-status.service
+rm /run/systemd/system/nonexistent-execstart-exit-status.service