diff --git a/Makefile-boot.am b/Makefile-boot.am index e42e5180..90f98048 100644 --- a/Makefile-boot.am +++ b/Makefile-boot.am @@ -41,6 +41,7 @@ systemdsystemunit_DATA = src/boot/ostree-prepare-root.service \ src/boot/ostree-boot-complete.service \ src/boot/ostree-finalize-staged.service \ src/boot/ostree-finalize-staged.path \ + src/boot/ostree-finalize-staged-hold.service \ $(NULL) systemdtmpfilesdir = $(prefix)/lib/tmpfiles.d dist_systemdtmpfiles_DATA = src/boot/ostree-tmpfiles.conf @@ -70,6 +71,7 @@ EXTRA_DIST += src/boot/dracut/module-setup.sh \ src/boot/ostree-finalize-staged.path \ src/boot/ostree-remount.service \ src/boot/ostree-finalize-staged.service \ + src/boot/ostree-finalize-staged-hold.service \ src/boot/grub2/grub2-15_ostree \ src/boot/grub2/ostree-grub-generator \ $(NULL) diff --git a/src/boot/ostree-finalize-staged-hold.service b/src/boot/ostree-finalize-staged-hold.service new file mode 100644 index 00000000..85b5d543 --- /dev/null +++ b/src/boot/ostree-finalize-staged-hold.service @@ -0,0 +1,35 @@ +# Copyright (C) 2018 Red Hat, Inc. +# Copyright (C) 2022 Endless OS Foundation LLC +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. If not, see . + +# See https://github.com/ostreedev/ostree/pull/2543 for background. +[Unit] +Description=Hold /boot Open for OSTree Finalize Staged Deployment +Documentation=man:ostree(1) +ConditionPathExists=/run/ostree-booted +DefaultDependencies=no + +RequiresMountsFor=/sysroot /boot +After=local-fs.target +Before=basic.target final.target + +[Service] +Type=exec + +# This is explicitly run in the root namespace to ensure an automounted +# /boot doesn't time out since autofs doesn't handle mount namespaces. +# +# https://bugzilla.redhat.com/show_bug.cgi?id=2056090 +ExecStart=+/usr/bin/ostree admin finalize-staged --hold diff --git a/src/boot/ostree-finalize-staged.service b/src/boot/ostree-finalize-staged.service index 2f28bbb7..63621ce1 100644 --- a/src/boot/ostree-finalize-staged.service +++ b/src/boot/ostree-finalize-staged.service @@ -29,6 +29,11 @@ Before=basic.target final.target After=systemd-journal-flush.service Conflicts=final.target +# Start the hold unit and ensure it stays active throughout this +# service. +Wants=ostree-finalize-staged-hold.service +After=ostree-finalize-staged-hold.service + [Service] Type=oneshot RemainAfterExit=yes diff --git a/src/ostree/ot-admin-builtin-finalize-staged.c b/src/ostree/ot-admin-builtin-finalize-staged.c index eedffdde..ed4f20d7 100644 --- a/src/ostree/ot-admin-builtin-finalize-staged.c +++ b/src/ostree/ot-admin-builtin-finalize-staged.c @@ -1,5 +1,6 @@ /* * Copyright (C) 2018 Red Hat, Inc. + * Copyright (C) 2022 Endless OS Foundation LLC * * SPDX-License-Identifier: LGPL-2.0+ * @@ -19,8 +20,9 @@ #include "config.h" -#include "config.h" - +#include +#include +#include #include #include "ot-main.h" @@ -32,10 +34,23 @@ #include "ostree-cmd-private.h" #include "ostree.h" +static gboolean opt_hold; + static GOptionEntry options[] = { + { "hold", 0, 0, G_OPTION_ARG_NONE, &opt_hold, "Hold /boot open during finalization", NULL }, { NULL } }; +static gboolean +sigterm_cb (gpointer user_data) +{ + gboolean *running = user_data; + g_print ("Received SIGTERM, exiting\n"); + *running = FALSE; + g_main_context_wakeup (NULL); + return G_SOURCE_REMOVE; +} + /* Called by ostree-finalize-staged.service, and in turn * invokes a cmdprivate function inside the shared library. */ @@ -50,13 +65,56 @@ ot_admin_builtin_finalize_staged (int argc, char **argv, OstreeCommandInvocation g_autoptr(GOptionContext) context = g_option_context_new (""); g_autoptr(OstreeSysroot) sysroot = NULL; + + /* First parse the args without loading the sysroot to see what options are + * set. */ if (!ostree_admin_option_context_parse (context, options, &argc, &argv, - OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, + OSTREE_ADMIN_BUILTIN_FLAG_NO_LOAD, invocation, &sysroot, cancellable, error)) return FALSE; - if (!ostree_cmd__private__()->ostree_finalize_staged (sysroot, cancellable, error)) - return FALSE; + if (opt_hold) + { + /* Load the sysroot unlocked so that a separate namespace isn't + * created. */ + if (!ostree_admin_sysroot_load (sysroot, + OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER | OSTREE_ADMIN_BUILTIN_FLAG_UNLOCKED, + cancellable, error)) + return FALSE; + + /* In case it's an automount, open /boot so that the automount doesn't + * expire until before this process exits. If it did expire and got + * unmounted, the service would be stopped and the deployment would be + * finalized earlier than expected. + */ + int sysroot_fd = ostree_sysroot_get_fd (sysroot); + glnx_autofd int boot_fd = -1; + g_debug ("Opening /boot directory"); + if (!glnx_opendirat (sysroot_fd, "boot", TRUE, &boot_fd, error)) + return FALSE; + + /* We want to keep /boot open until the deployment is finalized during + * system shutdown, so block on SIGTERM under the assumption that it will + * be received when systemd stops the unit. + */ + gboolean running = TRUE; + g_unix_signal_add (SIGTERM, sigterm_cb, &running); + g_print ("Waiting for SIGTERM\n"); + while (running) + g_main_context_iteration (NULL, TRUE); + } + else + { + /* Load the sysroot with the normal flags and actually finalize the + * deployment. */ + if (!ostree_admin_sysroot_load (sysroot, + OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, + cancellable, error)) + return FALSE; + + if (!ostree_cmd__private__()->ostree_finalize_staged (sysroot, cancellable, error)) + return FALSE; + } return TRUE; } diff --git a/src/ostree/ot-main.c b/src/ostree/ot-main.c index 7a4405a5..770962a4 100644 --- a/src/ostree/ot-main.c +++ b/src/ostree/ot-main.c @@ -579,41 +579,11 @@ on_sysroot_journal_msg (OstreeSysroot *sysroot, } gboolean -ostree_admin_option_context_parse (GOptionContext *context, - const GOptionEntry *main_entries, - int *argc, - char ***argv, - OstreeAdminBuiltinFlags flags, - OstreeCommandInvocation *invocation, - OstreeSysroot **out_sysroot, - GCancellable *cancellable, - GError **error) +ostree_admin_sysroot_load (OstreeSysroot *sysroot, + OstreeAdminBuiltinFlags flags, + GCancellable *cancellable, + GError **error) { - /* Entries are listed in --help output in the order added. We add the - * main entries ourselves so that we can add the --sysroot entry first. */ - - g_option_context_add_main_entries (context, global_admin_entries, NULL); - - if (!ostree_option_context_parse (context, main_entries, argc, argv, - invocation, NULL, cancellable, error)) - return FALSE; - - if (!opt_print_current_dir && (flags & OSTREE_ADMIN_BUILTIN_FLAG_NO_SYSROOT)) - { - g_assert_null (out_sysroot); - /* Early return if no sysroot is requested */ - return TRUE; - } - - g_autoptr(GFile) sysroot_path = NULL; - if (opt_sysroot != NULL) - sysroot_path = g_file_new_for_path (opt_sysroot); - - g_autoptr(OstreeSysroot) sysroot = ostree_sysroot_new (sysroot_path); - if (!ostree_sysroot_initialize (sysroot, error)) - return FALSE; - g_signal_connect (sysroot, "journal-msg", G_CALLBACK (on_sysroot_journal_msg), NULL); - if ((flags & OSTREE_ADMIN_BUILTIN_FLAG_UNLOCKED) == 0) { /* If we're requested to lock the sysroot, first check if we're operating @@ -657,6 +627,51 @@ ostree_admin_option_context_parse (GOptionContext *context, } } + return TRUE; +} + +gboolean +ostree_admin_option_context_parse (GOptionContext *context, + const GOptionEntry *main_entries, + int *argc, + char ***argv, + OstreeAdminBuiltinFlags flags, + OstreeCommandInvocation *invocation, + OstreeSysroot **out_sysroot, + GCancellable *cancellable, + GError **error) +{ + /* Entries are listed in --help output in the order added. We add the + * main entries ourselves so that we can add the --sysroot entry first. */ + + g_option_context_add_main_entries (context, global_admin_entries, NULL); + + if (!ostree_option_context_parse (context, main_entries, argc, argv, + invocation, NULL, cancellable, error)) + return FALSE; + + if (!opt_print_current_dir && (flags & OSTREE_ADMIN_BUILTIN_FLAG_NO_SYSROOT)) + { + g_assert_null (out_sysroot); + /* Early return if no sysroot is requested */ + return TRUE; + } + + g_autoptr(GFile) sysroot_path = NULL; + if (opt_sysroot != NULL) + sysroot_path = g_file_new_for_path (opt_sysroot); + + g_autoptr(OstreeSysroot) sysroot = ostree_sysroot_new (sysroot_path); + if (!ostree_sysroot_initialize (sysroot, error)) + return FALSE; + g_signal_connect (sysroot, "journal-msg", G_CALLBACK (on_sysroot_journal_msg), NULL); + + if (opt_print_current_dir || (flags & OSTREE_ADMIN_BUILTIN_FLAG_NO_LOAD) == 0) + { + if (!ostree_admin_sysroot_load (sysroot, flags, cancellable, error)) + return FALSE; + } + if (opt_print_current_dir) { g_autoptr(GPtrArray) deployments = NULL; diff --git a/src/ostree/ot-main.h b/src/ostree/ot-main.h index b369deb8..e296501a 100644 --- a/src/ostree/ot-main.h +++ b/src/ostree/ot-main.h @@ -36,6 +36,7 @@ typedef enum { OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER = (1 << 0), OSTREE_ADMIN_BUILTIN_FLAG_UNLOCKED = (1 << 1), OSTREE_ADMIN_BUILTIN_FLAG_NO_SYSROOT = (1 << 2), + OSTREE_ADMIN_BUILTIN_FLAG_NO_LOAD = (1 << 3), } OstreeAdminBuiltinFlags; @@ -91,6 +92,11 @@ gboolean ostree_admin_option_context_parse (GOptionContext *context, OstreeSysroot **out_sysroot, GCancellable *cancellable, GError **error); +gboolean ostree_admin_sysroot_load (OstreeSysroot *sysroot, + OstreeAdminBuiltinFlags flags, + GCancellable *cancellable, + GError **error); + gboolean ostree_ensure_repo_writable (OstreeRepo *repo, GError **error); void ostree_print_gpg_verify_result (OstreeGpgVerifyResult *result); diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs index da188bcb..d0c9c9dc 100644 --- a/tests/inst/src/destructive.rs +++ b/tests/inst/src/destructive.rs @@ -463,6 +463,7 @@ fn impl_transaction_test>( " systemctl stop rpm-ostreed systemctl stop ostree-finalize-staged + systemctl stop ostree-finalize-staged-hold ostree reset testrepo:${testref} ${booted_commit} rpm-ostree cleanup -pbrm ", @@ -504,7 +505,8 @@ fn impl_transaction_test>( InterruptStrategy::Force(ForceInterruptStrategy::Kill9) => { bash!( "systemctl kill -s KILL rpm-ostreed || true - systemctl kill -s KILL ostree-finalize-staged || true" + systemctl kill -s KILL ostree-finalize-staged || true + systemctl kill -s KILL ostree-finalize-staged-hold || true" )?; live_strategy = Some(strategy); } @@ -528,7 +530,8 @@ fn impl_transaction_test>( InterruptStrategy::Polite(PoliteInterruptStrategy::Stop) => { bash!( "systemctl stop rpm-ostreed || true - systemctl stop ostree-finalize-staged || true" + systemctl stop ostree-finalize-staged || true + systemctl stop ostree-finalize-staged-hold || true" )?; live_strategy = Some(strategy); } diff --git a/tests/kolainst/destructive/boot-automount.sh b/tests/kolainst/destructive/boot-automount.sh new file mode 100755 index 00000000..d6d1732e --- /dev/null +++ b/tests/kolainst/destructive/boot-automount.sh @@ -0,0 +1,83 @@ +#!/bin/bash +# https://github.com/ostreedev/ostree/issues/2543 +set -xeuo pipefail + +. ${KOLA_EXT_DATA}/libinsttest.sh + +case "${AUTOPKGTEST_REBOOT_MARK:-}" in + "") + # Ensure boot is mount point + mountpoint /boot + + # Create an automount unit with an extremely short timeout + cat > /etc/systemd/system/boot.automount <<"EOF" +[Automount] +Where=/boot +TimeoutIdleSec=1 + +[Install] +WantedBy=local-fs.target +EOF + systemctl daemon-reload + systemctl enable boot.automount + + # Unmount /boot, start the automount unit, and ensure the units are + # in the correct states. + umount /boot + systemctl start boot.automount + boot_state=$(systemctl show -P ActiveState boot.mount) + boot_auto_state=$(systemctl show -P ActiveState boot.automount) + assert_streq "${boot_state}" inactive + assert_streq "${boot_auto_state}" active + + # Trigger a new staged deployment and check that the relevant units + # are enabled. + ostree admin deploy --stage --karg-append=somedummykarg=1 "${host_commit}" + rpm-ostree status --json + deployment_staged=$(rpmostree_query_json '.deployments[0].staged') + assert_streq "${deployment_staged}" true + test -f /run/ostree/staged-deployment + finalize_staged_state=$(systemctl show -P ActiveState ostree-finalize-staged.service) + finalize_staged_hold_state=$(systemctl show -P ActiveState ostree-finalize-staged-hold.service) + assert_streq "${finalize_staged_state}" active + assert_streq "${finalize_staged_hold_state}" active + + # Sleep 1 second to ensure the boot automount idle timeout has + # passed and then check that /boot is still mounted. + sleep 1 + boot_state=$(systemctl show -P ActiveState boot.mount) + assert_streq "${boot_state}" active + + /tmp/autopkgtest-reboot "2" + ;; + "2") + rpm-ostree status --json + deployment_staged=$(rpmostree_query_json '.deployments[0].staged') + assert_streq "${deployment_staged}" false + test ! -f /run/ostree/staged-deployment + assert_file_has_content_literal /proc/cmdline somedummykarg=1 + + # Check that the finalize and hold services succeeded in the + # previous boot. Dump them to the test log to help debugging. + prepare_tmpdir + journalctl -b -1 -o short-monotonic \ + -u ostree-finalize-staged.service \ + -u ostree-finalize-staged-hold.service \ + -u boot.mount \ + -u boot.automount \ + > logs.txt + cat logs.txt + assert_file_has_content logs.txt 'ostree-finalize-staged.service: \(Succeeded\|Deactivated successfully\)' + assert_file_has_content logs.txt 'ostree-finalize-staged-hold.service: \(Succeeded\|Deactivated successfully\)' + + # Check that the hold service remained active and kept /boot mounted until + # the finalize service completed. + finalize_stopped=$(journalctl -b -1 -o json -g Stopped -u ostree-finalize-staged.service | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + hold_stopping=$(journalctl -b -1 -o json -g Stopping -u ostree-finalize-staged-hold.service | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + hold_stopped=$(journalctl -b -1 -o json -g Stopped -u ostree-finalize-staged-hold.service | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + boot_unmounting=$(journalctl -b -1 -o json -g Unmounting -u boot.mount | tail -n1 | jq -r .__MONOTONIC_TIMESTAMP) + test "${finalize_stopped}" -lt "${hold_stopping}" + test "${hold_stopped}" -lt "${boot_unmounting}" + ;; + *) fatal "Unexpected AUTOPKGTEST_REBOOT_MARK=${AUTOPKGTEST_REBOOT_MARK}" ;; +esac