sysext: support EXTENSION_RELOAD_MANAGER metadata

tormath1 · poettering · commit 41712cd1c0d7 · 2023-08-21T11:13:54.000+02:00
This metadata (EXTENSION_RELOAD_MANAGER) can be set to "1" to reload the manager
when merging/refreshing/unmerging a system extension image. This can be useful in case the sysext
image provides systemd units that need to be loaded.

With `--no-reload`, one can deactivate the EXTENSION_RELOAD_MANAGER metadata interpretation.

Signed-off-by: Mathieu Tortuyaux &lt;mtortuyaux@microsoft.com&gt;
diff --git a/man/systemd-sysext.xml b/man/systemd-sysext.xml
@@ -139,6 +139,11 @@
     architecture reported by <citerefentry><refentrytitle>uname</refentrytitle><manvolnum>2</manvolnum></citerefentry>
     but the used architecture identifiers are the same as for <varname>ConditionArchitecture=</varname>
     described in <citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>.
+    <varname>EXTENSION_RELOAD_MANAGER=</varname> can be set to 1 if the extension requires a service manager reload after application
+    of the extension. Note that the for the reasons mentioned earlier:
+    <ulink url="https://systemd.io/PORTABLE_SERVICES">Portable Services</ulink> remain
+    the recommended way to ship system services.
+
     System extensions should not ship a <filename>/usr/lib/os-release</filename> file (as that would be merged
     into the host <filename>/usr/</filename> tree, overriding the host OS version data, which is not desirable).
     The <filename>extension-release</filename> file follows the same format and semantics, and carries the same
@@ -299,6 +304,17 @@
         it.</para></listitem>
       </varlistentry>
 
+      <varlistentry>
+        <term><option>--no-reload</option></term>
+
+        <listitem>
+          <para>When used with <command>merge</command>,
+          <command>unmerge</command> or <command>refresh</command>, do not reload daemon
+          after executing the changes even if an extension that is applied requires a reload via the
+          <varname>EXTENSION_RELOAD_MANAGER=</varname> set to 1.</para>
+        </listitem>
+      </varlistentry>
+
       <xi:include href="standard-options.xml" xpointer="no-pager" />
       <xi:include href="standard-options.xml" xpointer="no-legend" />
       <xi:include href="standard-options.xml" xpointer="json" />
diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c
@@ -7,9 +7,16 @@
 #include <sys/mount.h>
 #include <unistd.h>
 
+#include "sd-bus.h"
+
 #include "build.h"
+#include "bus-locator.h"
+#include "bus-error.h"
+#include "bus-unit-util.h"
+#include "bus-util.h"
 #include "capability-util.h"
 #include "chase.h"
+#include "constants.h"
 #include "devnum-util.h"
 #include "discover-image.h"
 #include "dissect-image.h"
@@ -45,6 +52,7 @@ static JsonFormatFlags arg_json_format_flags = JSON_FORMAT_OFF;
 static PagerFlags arg_pager_flags = 0;
 static bool arg_legend = true;
 static bool arg_force = false;
+static bool arg_no_reload = false;
 static int arg_noexec = -1;
 static ImagePolicy *arg_image_policy = NULL;
 
@@ -144,6 +152,87 @@ static int is_our_mount_point(const char *p) {
         return true;
 }
 
+static int need_reload(void) {
+        /* Parse the mounted images to find out if we need
+        to reload the daemon. */
+        int r;
+
+        if (arg_no_reload)
+                return false;
+
+        STRV_FOREACH(p, arg_hierarchies) {
+                _cleanup_free_ char *f = NULL, *buf = NULL, *resolved = NULL;
+                _cleanup_strv_free_ char **mounted_extensions = NULL;
+
+                r = chase(*p, arg_root, CHASE_PREFIX_ROOT, &resolved, NULL);
+                if (r == -ENOENT) {
+                        log_debug_errno(r, "Hierarchy '%s%s' does not exist, ignoring.", strempty(arg_root), *p);
+                        continue;
+                }
+                if (r < 0) {
+                        log_warning_errno(r, "Failed to resolve path to hierarchy '%s%s': %m, ignoring.", strempty(arg_root), *p);
+                        continue;
+                }
+
+                r = is_our_mount_point(resolved);
+                if (r < 0)
+                        return r;
+                if (!r)
+                        continue;
+
+                f = path_join(*p, image_class_info[arg_image_class].dot_directory_name, image_class_info[arg_image_class].short_identifier_plural);
+                if (!f)
+                        return log_oom();
+
+                r = read_full_file(f, &buf, NULL);
+                if (r < 0)
+                        return log_error_errno(r, "Failed to open '%s': %m", f);
+
+                mounted_extensions = strv_split_newlines(buf);
+                if (!mounted_extensions)
+                        return log_oom();
+
+                STRV_FOREACH(extension, mounted_extensions) {
+                        _cleanup_strv_free_ char **extension_release = NULL;
+                        const char *extension_reload_manager = NULL;
+                        int b;
+
+                        r = load_extension_release_pairs(arg_root, arg_image_class, *extension, /* relax_extension_release_check */ true, &extension_release);
+                        if (r < 0) {
+                                log_debug_errno(r, "Failed to parse extension-release metadata of %s, ignoring: %m", *extension);
+                                continue;
+                        }
+
+                        extension_reload_manager = strv_env_pairs_get(extension_release, "EXTENSION_RELOAD_MANAGER");
+                        if (isempty(extension_reload_manager))
+                                continue;
+
+                        b = parse_boolean(extension_reload_manager);
+                        if (b < 0) {
+                                log_warning_errno(b, "Failed to parse the extension metadata to know if the manager needs to be reloaded, ignoring: %m");
+                                continue;
+                        }
+
+                        if (b)
+                                /* If at least one extension wants a reload, we reload. */
+                                return true;
+                }
+        }
+
+        return false;
+}
+
+static int daemon_reload(void) {
+        sd_bus *bus;
+        int r;
+
+        r = bus_connect_system_systemd(&bus);
+        if (r < 0)
+                return log_error_errno(r, "Failed to get D-Bus connection: %m");
+
+        return bus_service_manager_reload(bus);
+}
+
 static int unmerge_hierarchy(const char *p) {
         int r;
 
@@ -169,6 +258,12 @@ static int unmerge_hierarchy(const char *p) {
 
 static int unmerge(void) {
         int r, ret = 0;
+        bool need_to_reload;
+
+        r = need_reload();
+        if (r < 0)
+                return r;
+        need_to_reload = r > 0;
 
         STRV_FOREACH(p, arg_hierarchies) {
                 _cleanup_free_ char *resolved = NULL;
@@ -191,6 +286,12 @@ static int unmerge(void) {
                         ret = r;
         }
 
+        if (need_to_reload) {
+                r = daemon_reload();
+                if (r < 0)
+                        return r;
+        }
+
         return ret;
 }
 
@@ -784,7 +885,19 @@ static int merge(Hashmap *images) {
         if (r < 0)
                 return r;
 
-        return r != 123; /* exit code 123 means: didn't do anything */
+        if (r == 123) /* exit code 123 means: didn't do anything */
+                return 0;
+
+        r = need_reload();
+        if (r < 0)
+                return r;
+        if (r > 0) {
+                r = daemon_reload();
+                if (r < 0)
+                        return r;
+        }
+
+        return 1;
 }
 
 static int image_discover_and_read_metadata(Hashmap **ret_images) {
@@ -955,6 +1068,7 @@ static int verb_help(int argc, char **argv, void *userdata) {
                "     --json=pretty|short|off\n"
                "                          Generate JSON output\n"
                "     --force              Ignore version incompatibilities\n"
+               "     --no-reload          Do not reload the service manager\n"
                "     --image-policy=POLICY\n"
                "                          Specify disk image dissection policy\n"
                "     --noexec=BOOL        Whether to mount extension overlay with noexec\n"
@@ -980,6 +1094,7 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_FORCE,
                 ARG_IMAGE_POLICY,
                 ARG_NOEXEC,
+                ARG_NO_RELOAD,
         };
 
         static const struct option options[] = {
@@ -992,6 +1107,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "force",        no_argument,       NULL, ARG_FORCE        },
                 { "image-policy", required_argument, NULL, ARG_IMAGE_POLICY },
                 { "noexec",       required_argument, NULL, ARG_NOEXEC       },
+                { "no-reload",    no_argument,       NULL, ARG_NO_RELOAD    },
                 {}
         };
 
@@ -1049,6 +1165,10 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_noexec = r;
                         break;
 
+                case ARG_NO_RELOAD:
+                        arg_no_reload = true;
+                        break;
+
                 case '?':
                         return -EINVAL;
 
diff --git a/test/test-functions b/test/test-functions
@@ -846,6 +846,26 @@ EOF
           echo "ARCHITECTURE=_any" ) >"$initdir/etc/extension-release.d/extension-release.service-scoped-test"
         echo MARKER_CONFEXT_123 >"$initdir/etc/systemd/system/some_file"
         mksquashfs "$initdir" "$oldinitdir/etc/service-scoped-test.raw" -noappend
+
+        # We need to create a dedicated sysext image to test the reload mechanism. If we share an image to install the
+        # 'foo.service' it will be loaded from another test run, which will impact the targeted test.
+        export initdir="$TESTDIR/app-reload"
+        mkdir -p "$initdir/usr/lib/extension-release.d" "$initdir/usr/lib/systemd/system"
+        ( echo "ID=_any"
+          echo "ARCHITECTURE=_any"
+          echo "EXTENSION_RELOAD_MANAGER=1" ) >"$initdir/usr/lib/extension-release.d/extension-release.app-reload"
+        mkdir -p "$initdir/usr/lib/systemd/system/multi-user.target.d"
+        cat >"${initdir}/usr/lib/systemd/system/foo.service" <<EOF
+[Service]
+Type=oneshot
+RemainAfterExit=yes
+ExecStart=echo foo
+
+[Install]
+WantedBy=multi-user.target
+EOF
+        { echo "[Unit]"; echo "Upholds=foo.service"; } > "$initdir/usr/lib/systemd/system/multi-user.target.d/10-foo-service.conf"
+        mksquashfs "$initdir" "$oldinitdir/usr/share/app-reload.raw" -noappend
     )
 }
 
diff --git a/test/units/testsuite-50.sh b/test/units/testsuite-50.sh
@@ -596,4 +596,32 @@ rm -rf /run/confexts/ testjob/
 
 systemd-run -P -p RootImage="${image}.raw" cat /run/host/os-release | cmp "${os_release}"
 
+# Test that systemd-sysext reloads the daemon.
+mkdir -p /var/lib/extensions/
+ln -s /usr/share/app-reload.raw /var/lib/extensions/app-reload.raw
+systemd-sysext merge --no-reload
+# the service should not be running
+if systemctl --quiet is-active foo.service; then
+    echo "foo.service should not be active"
+    exit 1
+fi
+systemd-sysext unmerge --no-reload
+systemd-sysext merge
+for RETRY in $(seq 60) LAST; do
+  if journalctl --boot --unit foo.service | grep -q -P 'echo\[[0-9]+\]: foo'; then
+    break
+  fi
+  if [ "${RETRY}" = LAST ]; then
+    echo "Output of foo.service not found"
+    exit 1
+  fi
+  sleep 0.5
+done
+systemd-sysext unmerge --no-reload
+# Grep on the Warning to find the warning helper mentioning the daemon reload.
+systemctl status foo.service 2>&1 | grep -q -F "Warning"
+systemd-sysext merge
+systemd-sysext unmerge
+systemctl status foo.service 2>&1 | grep -v -q -F "Warning"
+
 touch /testok
