From 41712cd1c0d774d5eac592964f25d798df44a190 Mon Sep 17 00:00:00 2001 From: Mathieu Tortuyaux Date: Tue, 25 Apr 2023 14:06:59 +0200 Subject: [PATCH] sysext: support EXTENSION_RELOAD_MANAGER metadata 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 --- man/systemd-sysext.xml | 16 +++++ src/sysext/sysext.c | 122 ++++++++++++++++++++++++++++++++++++- test/test-functions | 20 ++++++ test/units/testsuite-50.sh | 28 +++++++++ 4 files changed, 185 insertions(+), 1 deletion(-) diff --git a/man/systemd-sysext.xml b/man/systemd-sysext.xml index db6b2235551..08540feab28 100644 --- a/man/systemd-sysext.xml +++ b/man/systemd-sysext.xml @@ -139,6 +139,11 @@ architecture reported by uname2 but the used architecture identifiers are the same as for ConditionArchitecture= described in systemd.unit5. + EXTENSION_RELOAD_MANAGER= 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: + Portable Services remain + the recommended way to ship system services. + System extensions should not ship a /usr/lib/os-release file (as that would be merged into the host /usr/ tree, overriding the host OS version data, which is not desirable). The extension-release file follows the same format and semantics, and carries the same @@ -299,6 +304,17 @@ it. + + + + + When used with merge, + unmerge or refresh, do not reload daemon + after executing the changes even if an extension that is applied requires a reload via the + EXTENSION_RELOAD_MANAGER= set to 1. + + + diff --git a/src/sysext/sysext.c b/src/sysext/sysext.c index b043eac562c..98640c026b3 100644 --- a/src/sysext/sysext.c +++ b/src/sysext/sysext.c @@ -7,9 +7,16 @@ #include #include +#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 index 9971ced7de7..8b234668776 100644 --- 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" < "$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 index 728674e14e5..c02dfc862c7 100755 --- 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 -- 2.47.3