diff --git a/crates/lib/src/bootc_composefs/finalize.rs b/crates/lib/src/bootc_composefs/finalize.rs index 8864ca660b..eec1825949 100644 --- a/crates/lib/src/bootc_composefs/finalize.rs +++ b/crates/lib/src/bootc_composefs/finalize.rs @@ -5,6 +5,7 @@ use crate::bootc_composefs::gc::{GCOpts, composefs_gc}; use crate::bootc_composefs::rollback::{rename_exchange_bls_entries, rename_exchange_user_cfg}; use crate::bootc_composefs::status::get_composefs_status; use crate::composefs_consts::STATE_DIR_ABS; +use crate::install::BOOT; use crate::spec::BootloaderKind; use crate::store::{BootedComposefs, Storage}; use anyhow::{Context, Result}; @@ -15,7 +16,7 @@ use cap_std_ext::dirext::CapStdExtDirExt; use composefs::generic_tree::{FileSystem, Stat}; use composefs_ctl::composefs; use etc_merge::{Diff, compute_diff, merge, traverse_etc}; -use rustix::fs::fsync; +use rustix::fs::{Mode, OFlags, fsync}; use fn_error_context::context; @@ -167,6 +168,33 @@ pub(crate) async fn composefs_backend_finalize( Ok(()) } +/// Keep /boot open until we're killed, which systemd does with SIGTERM +/// when `bootc-finalize-staged-hold.service` is stopped after +/// `bootc-finalize-staged.service`. +/// +/// When /boot is an automount (e.g. the ESP set up by +/// systemd-gpt-auto-generator), an idle expire breaks finalization in two +/// ways. If it races with shutdown, it deadlocks: the finalization (which +/// looks up /boot) blocks on the expire, while the unmount is ordered after +/// the finalization. If it completes before shutdown, systemd won't remount +/// /boot once shutdown has begun, so finalization fails to open it (EHOSTDOWN) +/// and the old deployment boots. An open file descriptor makes autofs treat +/// the mount as busy, so it never expires. Note this only works from the +/// root mount namespace. +pub(crate) fn hold_boot() -> Result<()> { + let path = format!("/{BOOT}"); + let _fd = rustix::fs::open( + path.as_str(), + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC, + Mode::empty(), + ) + .with_context(|| format!("Opening {path}"))?; + tracing::debug!("Holding {path} open until terminated"); + loop { + std::thread::park(); + } +} + #[context("Grub: Finalizing staged UKI")] fn finalize_staged_grub_uki(boot_fd: &Dir) -> Result<()> { let entries_dir = boot_fd.open_dir("grub2")?; diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index bc11e07e2d..9c34dd5fed 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -212,6 +212,19 @@ pub(crate) struct SwitchOpts { pub(crate) progress: ProgressOptions, } +/// Finalize a staged composefs deployment. +/// +/// This is invoked at shutdown by `bootc-finalize-staged.service`. +#[derive(Debug, Parser, PartialEq, Eq)] +pub(crate) struct ComposefsFinalizeStagedOpts { + /// Hold /boot open until terminated, instead of finalizing. + /// + /// This is used by `bootc-finalize-staged-hold.service` to keep an + /// automounted /boot from expiring while a deployment is staged. + #[clap(long)] + pub(crate) hold: bool, +} + /// Options controlling rollback #[derive(Debug, Parser, PartialEq, Eq)] pub(crate) struct RollbackOpts { @@ -1066,7 +1079,7 @@ pub(crate) enum Opt { #[clap(subcommand)] #[clap(hide = true)] Internals(InternalsOpts), - ComposefsFinalizeStaged, + ComposefsFinalizeStaged(ComposefsFinalizeStagedOpts), /// Diff current /etc configuration versus default #[clap(hide = true)] ConfigDiff, @@ -2714,14 +2727,23 @@ async fn run_from_opt(opt: Opt) -> Result { } }, - Opt::ComposefsFinalizeStaged => { - let storage = &get_storage().await?; - match storage.kind()? { - BootedStorageKind::Ostree(_) => { - anyhow::bail!("ComposefsFinalizeStaged is only supported for composefs backend") - } - BootedStorageKind::Composefs(booted_cfs) => { - composefs_backend_finalize(storage, &booted_cfs).await + Opt::ComposefsFinalizeStaged(opts) => { + if opts.hold { + // Must happen before loading storage: the hold has to stay in + // the root mount namespace, and must not depend on anything + // but /boot. + crate::bootc_composefs::finalize::hold_boot() + } else { + let storage = &get_storage().await?; + match storage.kind()? { + BootedStorageKind::Ostree(_) => { + anyhow::bail!( + "ComposefsFinalizeStaged is only supported for composefs backend" + ) + } + BootedStorageKind::Composefs(booted_cfs) => { + composefs_backend_finalize(storage, &booted_cfs).await + } } } } @@ -2870,6 +2892,17 @@ mod tests { Opt::parse_including_static(["bootc", "status", "-v"]), Opt::Status(StatusOpts { verbose: true, .. }) )); + + for (args, hold) in [ + (&["bootc", "composefs-finalize-staged"][..], false), + (&["bootc", "composefs-finalize-staged", "--hold"][..], true), + ] { + assert_eq!( + Opt::parse_including_static(args), + Opt::ComposefsFinalizeStaged(ComposefsFinalizeStagedOpts { hold }), + "{args:?}" + ); + } } #[test] diff --git a/docs/src/man/bootc-composefs-finalize-staged.8.md b/docs/src/man/bootc-composefs-finalize-staged.8.md index 569b883673..a8b054ab35 100644 --- a/docs/src/man/bootc-composefs-finalize-staged.8.md +++ b/docs/src/man/bootc-composefs-finalize-staged.8.md @@ -1,25 +1,52 @@ # NAME -bootc-composefs-finalize-staged - TODO: Add description +bootc-composefs-finalize-staged - Finalize a staged composefs deployment # SYNOPSIS -bootc composefs-finalize-staged +**bootc composefs-finalize-staged** \[*OPTIONS...*\] # DESCRIPTION -TODO: Add description +Finalize a staged composefs deployment. This is an internal command +invoked at shutdown by **bootc-finalize-staged.service**; it is not +intended to be run directly. + +When `bootc upgrade` or `bootc switch` stages a new deployment on a +composefs system, it starts **bootc-finalize-staged.service**, whose +`ExecStop` runs this command as the system shuts down. It merges the +current `/etc` into the staged deployment and updates the bootloader +configuration so that the next boot uses it. If no deployment is +staged, it does nothing. It fails on systems using the ostree +backend, where **ostree-finalize-staged.service** does this instead. + +The finalize service also pulls in +**bootc-finalize-staged-hold.service**, which runs this command with +`--hold` to keep `/boot` open while a deployment is staged. Otherwise +an automounted `/boot` (such as the ESP set up by +**systemd-gpt-auto-generator**(8)) could expire while idle, and then +either deadlock with shutdown or be unavailable when finalization +runs. + +# OPTIONS +**--hold** + + Hold /boot open until terminated, instead of finalizing + # EXAMPLES -TODO: Add practical examples showing how to use this command. +Check whether a staged deployment is waiting to be finalized at the +next shutdown: + + systemctl status bootc-finalize-staged.service bootc-finalize-staged-hold.service # SEE ALSO -**bootc**(8) +**bootc**(8), **bootc-upgrade**(8), **bootc-switch**(8) # VERSION diff --git a/docs/src/man/bootc.8.md b/docs/src/man/bootc.8.md index d543d04994..1b50497815 100644 --- a/docs/src/man/bootc.8.md +++ b/docs/src/man/bootc.8.md @@ -34,7 +34,7 @@ pulled and `bootc upgrade`. | **bootc install** | Install the running container to a target | | **bootc container** | Operations which can be executed as part of a container build | | **bootc loader-entries** | Operations on Boot Loader Specification (BLS) entries | -| **bootc composefs-finalize-staged** | | +| **bootc composefs-finalize-staged** | Finalize a staged composefs deployment | diff --git a/systemd/bootc-finalize-staged-hold.service b/systemd/bootc-finalize-staged-hold.service new file mode 100644 index 0000000000..86cf225ada --- /dev/null +++ b/systemd/bootc-finalize-staged-hold.service @@ -0,0 +1,25 @@ +# Keeps /boot busy while a deployment is staged, so that an automounted +# /boot (e.g. the ESP set up by systemd-gpt-auto-generator) can't idle-expire. +# An expire racing with shutdown deadlocks bootc-finalize-staged.service's +# ExecStop, and one that completes before shutdown leaves /boot unmounted +# for good, since systemd won't trigger the automount again during shutdown, +# so the ExecStop fails. This is a port of +# ostree-finalize-staged-hold.service; see +# https://github.com/ostreedev/ostree/pull/2543 for background. +[Unit] +Description=Hold /boot Open for Composefs Finalize Staged Deployment +Documentation=man:bootc(1) +DefaultDependencies=no + +RequiresMountsFor=/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/bootc composefs-finalize-staged --hold diff --git a/systemd/bootc-finalize-staged.service b/systemd/bootc-finalize-staged.service index 60e3f06833..57366a63f1 100644 --- a/systemd/bootc-finalize-staged.service +++ b/systemd/bootc-finalize-staged.service @@ -28,6 +28,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=bootc-finalize-staged-hold.service +After=bootc-finalize-staged-hold.service + [Service] Type=oneshot RemainAfterExit=yes diff --git a/tmt/tests/booted/test-44-shadow-fixup.nu b/tmt/tests/booted/test-44-shadow-fixup.nu index c4fa6fe8e7..34d088ec3d 100644 --- a/tmt/tests/booted/test-44-shadow-fixup.nu +++ b/tmt/tests/booted/test-44-shadow-fixup.nu @@ -62,6 +62,14 @@ def initial_build [] { podman build -t localhost/bootc-shadow-fixup-b . bootc switch --transport containers-storage localhost/bootc-shadow-fixup-b + + # On composefs, staging must also start the hold unit that keeps an + # automounted /boot from expiring before finalization at shutdown. + if (tap is_composefs) { + let hold_state = (do { ^systemctl is-active bootc-finalize-staged-hold.service } | complete | get stdout | str trim) + assert ($hold_state == "active") $"bootc-finalize-staged-hold.service not active after staging: ($hold_state)" + } + bootc_testlib reboot }