diff --git a/crates/lib/src/bootc_composefs/switch.rs b/crates/lib/src/bootc_composefs/switch.rs index 0cb0b9ebe..042065192 100644 --- a/crates/lib/src/bootc_composefs/switch.rs +++ b/crates/lib/src/bootc_composefs/switch.rs @@ -34,21 +34,31 @@ pub(crate) async fn switch_composefs( use_unified: false, quiet: opts.quiet, prog, + origin_override: None, }; if opts.download_opts.from_downloaded { return apply_upgrade_from_downloaded(storage, booted_cfs, &host, &do_upgrade_opts).await; } - let target = imgref_for_switch(&opts)?; + // The source we fetch the image from now. + let source = imgref_for_switch(&opts)?; + // Optional decoupled reference to persist as the origin for future upgrades + // (`--target-imgref`, issue #2464). `None` means source and origin coincide. + let origin_override = + crate::cli::target_imgref_for_switch(&opts)?.map(crate::spec::ImageReference::from); let new_spec = { let mut new_spec = host.spec.clone(); - new_spec.image = Some(target.clone()); + new_spec.image = Some(origin_override.clone().unwrap_or_else(|| source.clone())); new_spec }; - if new_spec == host.spec { + // Only take the unchanged fast path when the pull source is also the origin. + // With `--target-imgref` the source is decoupled from the persisted origin, so a + // switch from a different source keeping the same origin (issue #2464) must still + // run the pull even though `new_spec == host.spec`. + if origin_override.is_none() && new_spec == host.spec { println!("Image specification is unchanged."); if opts.apply && host.status.staged.is_some() { crate::reboot::reboot()?; @@ -56,16 +66,25 @@ pub(crate) async fn switch_composefs( return Ok(()); } - let Some(target_imgref) = new_spec.image else { - anyhow::bail!("Target image is undefined") - }; + // Persist the decoupled origin (if any); everything below pulls and validates + // against the source image. + do_upgrade_opts.origin_override = origin_override; + let target_imgref = source; const COMPOSEFS_SWITCH_JOURNAL_ID: &str = "7a6b5c4d3e2f1a0b9c8d7e6f5a4b3c2d1"; + // With `--target-imgref` the persisted origin is decoupled from the pull + // source, so record both (mirroring the ostree path's journal fields). + let origin_image = do_upgrade_opts + .origin_override + .as_ref() + .unwrap_or(&target_imgref); + tracing::info!( message_id = COMPOSEFS_SWITCH_JOURNAL_ID, bootc.operation = "switch", - bootc.target_image = target_imgref.to_string(), + bootc.source_image = target_imgref.to_string(), + bootc.target_image = origin_image.to_string(), bootc.apply_mode = opts.apply, bootc.download_only = opts.download_opts.download_only, bootc.from_downloaded = opts.download_opts.from_downloaded, diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index c2b43c8b8..bf3813845 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -220,6 +220,10 @@ pub(crate) struct DoUpgradeOpts { pub(crate) quiet: bool, /// Structured (JSON-Lines) progress sink; see `--progress-fd`. pub(crate) prog: ProgressWriter, + /// Origin to persist for future upgrades when it is decoupled from the pull + /// source (i.e. `switch --target-imgref`, issue #2464). `None` means the pull + /// source is also the origin, which is the default for plain upgrades. + pub(crate) origin_override: Option, } async fn apply_upgrade( @@ -364,10 +368,15 @@ pub(crate) async fn do_upgrade( finalization_locked: opts.download_only, }; + // The image is fetched from `imgref`, but the origin persisted for future + // upgrades may be decoupled from that pull source via `switch --target-imgref` + // (issue #2464); fall back to the source when they coincide. + let origin = opts.origin_override.as_ref().unwrap_or(imgref); + write_composefs_state( &Utf8PathBuf::from("/sysroot"), &id, - imgref, + origin, Some(staged_state), boot_type, boot_digest, @@ -490,6 +499,8 @@ pub(crate) async fn upgrade_composefs( use_unified: false, quiet: opts.quiet, prog, + // Plain upgrade never decouples the origin from the pull source. + origin_override: None, }; if opts.download_opts.from_downloaded { diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 4a8ccea63..a38cd859f 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -165,6 +165,22 @@ pub(crate) struct SwitchOpts { #[clap(long, default_value = "registry")] pub(crate) transport: String, + /// Specify the image to record as the origin for subsequent updates. + /// + /// By default the image is fetched from `--transport`/`` and that same + /// reference is also persisted as the origin used for future `bootc upgrade`s. + /// Use this to decouple the two: the image is still fetched now from + /// `--transport`/`` (e.g. a local `containers-storage` copy loaded via + /// `podman load`), but the origin recorded for subsequent updates is this + /// reference instead (e.g. the normal registry pull spec). + #[clap(long, conflicts_with_all = ["from_downloaded", "mutate_in_place"])] + pub(crate) target_imgref: Option, + + /// The transport for `--target-imgref`; e.g. registry, oci, oci-archive, + /// docker-daemon, containers-storage. Defaults to `registry`. + #[clap(long, default_value = "registry")] + pub(crate) target_transport: String, + #[clap(flatten)] pub(crate) download_opts: DownloadOnlyOpts, @@ -1495,6 +1511,27 @@ pub(crate) fn imgref_for_switch(opts: &SwitchOpts) -> Result { return Ok(target); } +/// Build the optional `--target-imgref` for `switch`; this is the reference that will +/// be recorded as the origin for subsequent updates, decoupled from the source the +/// image is fetched from now. Returns `None` when `--target-imgref` was not provided. +pub(crate) fn target_imgref_for_switch( + opts: &SwitchOpts, +) -> Result> { + let Some(name) = opts.target_imgref.as_deref() else { + return Ok(None); + }; + let transport = ostree_container::Transport::try_from(opts.target_transport.as_str())?; + let imgref = ostree_container::ImageReference { + transport, + name: name.to_string(), + }; + let sigverify = sigpolicy_from_opt(opts.enforce_container_sigpolicy); + Ok(Some(ostree_container::OstreeImageReference { + sigverify, + imgref, + })) +} + /// Implementation of the `bootc switch` CLI command for ostree backend. #[context("Switching (ostree)")] async fn switch_ostree( @@ -1517,7 +1554,16 @@ async fn switch_ostree( .await; } - let target = imgref_for_switch(&opts)?; + // The source we fetch the image from right now. + let source = imgref_for_switch(&opts)?; + // Optional decoupled reference to persist as the origin for subsequent updates. + let target_imgref = target_imgref_for_switch(&opts)?; + // What we record in the spec/origin for future upgrades: the `--target-imgref` + // if given, otherwise the same reference we're fetching from. + let origin_ref = match target_imgref.as_ref() { + Some(t) => ImageReference::from(t.clone()), + None => source.clone(), + }; let prog: ProgressWriter = opts.progress.try_into()?; let cancellable = gio::Cancellable::NONE; @@ -1525,11 +1571,15 @@ async fn switch_ostree( let new_spec = { let mut new_spec = host.spec.clone(); - new_spec.image = Some(target.clone()); + new_spec.image = Some(origin_ref.clone()); new_spec }; - if new_spec == host.spec { + // Only take the unchanged fast path when the pull source is also the origin. + // With `--target-imgref` the source is decoupled from the persisted origin, so a + // switch from a different source keeping the same origin (issue #2464) must still + // run the pull even though `new_spec == host.spec`. + if target_imgref.is_none() && new_spec == host.spec { println!("Image specification is unchanged."); if opts.apply && host.status.staged.is_some() { crate::reboot::reboot()?; @@ -1549,11 +1599,13 @@ async fn switch_ostree( tracing::info!( message_id = SWITCH_JOURNAL_ID, bootc.old_image_reference = old_image, - bootc.new_image_reference = &target.image, - bootc.new_image_transport = &target.transport, + bootc.new_image_reference = &origin_ref.image, + bootc.new_image_transport = &origin_ref.transport, + bootc.source_image_reference = &source.image, + bootc.source_image_transport = &source.transport, "Switching from image {} to {}", old_image, - target.image + origin_ref.image ); let new_spec = RequiredHostSpec::from_spec(&new_spec)?; @@ -1564,14 +1616,14 @@ async fn switch_ostree( let use_unified = if opts.unified_storage_exp { true } else { - crate::deploy::image_exists_in_unified_storage(storage, &target).await? + crate::deploy::image_exists_in_unified_storage(storage, &source).await? }; let fetched = if use_unified { crate::deploy::pull_unified( repo, - &target, - None, + &source, + target_imgref.as_ref(), opts.quiet, prog.clone(), storage, @@ -1581,8 +1633,8 @@ async fn switch_ostree( } else { crate::deploy::pull( repo, - &target, - None, + &source, + target_imgref.as_ref(), opts.quiet, prog.clone(), Some(&booted_ostree.deployment), @@ -2722,6 +2774,74 @@ mod tests { )); } + #[test] + fn test_parse_switch_target_imgref() { + // Without --target-imgref, source and origin are the same reference. + let o = Opt::try_parse_from([ + "bootc", + "switch", + "--transport", + "containers-storage", + "localhost/someimage", + ]) + .unwrap(); + let opts = match o { + Opt::Switch(opts) => opts, + o => panic!("Expected switch opts, not {o:?}"), + }; + assert_eq!(opts.transport, "containers-storage"); + assert!(opts.target_imgref.is_none()); + assert!(target_imgref_for_switch(&opts).unwrap().is_none()); + let source = imgref_for_switch(&opts).unwrap(); + assert_eq!(source.transport, "containers-storage"); + assert_eq!(source.image, "localhost/someimage"); + + // With --target-imgref, the image is fetched from --transport/ but the + // origin recorded for upgrades is the (registry, by default) target imgref. + let o = Opt::try_parse_from([ + "bootc", + "switch", + "--transport", + "containers-storage", + "--target-imgref", + "quay.io/example/os:latest", + "localhost/someimage", + ]) + .unwrap(); + let opts = match o { + Opt::Switch(opts) => opts, + o => panic!("Expected switch opts, not {o:?}"), + }; + assert_eq!( + opts.target_imgref.as_deref(), + Some("quay.io/example/os:latest") + ); + assert_eq!(opts.target_transport, "registry"); + let source = imgref_for_switch(&opts).unwrap(); + assert_eq!(source.transport, "containers-storage"); + assert_eq!(source.image, "localhost/someimage"); + let target = target_imgref_for_switch(&opts).unwrap().unwrap(); + assert_eq!( + target.imgref.transport, + ostree_container::Transport::Registry + ); + assert_eq!(target.imgref.name, "quay.io/example/os:latest"); + + // --target-imgref performs a real pull; --mutate-in-place performs none, so the + // combination is rejected rather than silently ignoring --target-imgref. + assert!( + Opt::try_parse_from([ + "bootc", + "switch", + "--mutate-in-place", + "--target-imgref", + "quay.io/example/os:latest", + "localhost/someimage", + ]) + .is_err() + ); + } + #[test] fn test_parse_selinux_is_unlabeled() { let opt = Opt::try_parse_from([ diff --git a/docs/src/man/bootc-switch.8.md b/docs/src/man/bootc-switch.8.md index 407aa0bc0..2e5a8618e 100644 --- a/docs/src/man/bootc-switch.8.md +++ b/docs/src/man/bootc-switch.8.md @@ -63,6 +63,16 @@ Soft reboot allows faster system restart by avoiding full hardware reboot when p Default: registry +**--target-imgref**=*TARGET_IMGREF* + + Specify the image to record as the origin for subsequent updates + +**--target-transport**=*TARGET_TRANSPORT* + + The transport for `--target-imgref`; e.g. registry, oci, oci-archive, docker-daemon, containers-storage. Defaults to `registry` + + Default: registry + **--download-only** Download and stage the update without applying it diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 7b52f4f02..7dbc7f6fc 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -318,4 +318,13 @@ execute: test: - /tmt/tests/tests/test-48-composefs-uki-dumpfile extra-skip_if_ostree: true + +/plan-49-switch-target-imgref: + summary: switch --target-imgref records a decoupled origin (composefs) + discover: + how: fmf + test: + - /tmt/tests/tests/test-49-switch-target-imgref + extra-skip_if_ostree: true + extra-fixme_skip_if_uki: true # END GENERATED PLANS diff --git a/tmt/tests/booted/test-switch-target-imgref.nu b/tmt/tests/booted/test-switch-target-imgref.nu new file mode 100644 index 000000000..0433feb48 --- /dev/null +++ b/tmt/tests/booted/test-switch-target-imgref.nu @@ -0,0 +1,49 @@ +# number: 49 +# tmt: +# summary: switch --target-imgref records a decoupled origin (composefs) +# duration: 30m +# extra: +# skip_if_ostree: true +# fixme_skip_if_uki: true +# +# Verify that `bootc switch --target-imgref` fetches the image from the given +# source (here a local containers-storage copy) but persists the *decoupled* +# reference as the origin for subsequent upgrades (issue #2464), on the +# composefs backend. +# +# Skipped on UKI: the test switches to a freshly derived image, whose composefs +# digest won't match the one sealed into the booted UKI, so the switch is +# rejected before the origin is ever recorded (same limitation as +# test-composefs-gc). The decoupling logic is boot-type agnostic and is covered +# on the BLS composefs matrix. +use std assert +use tap.nu + +tap begin "bootc switch --target-imgref decouples pull source from origin" + +# Make the booted image available in podman storage as our pull-source base. +bootc image copy-to-storage + +# Derive a new image so its content — and therefore its composefs fs-verity +# digest — differs from the booted deployment; otherwise the same-digest guard +# (see test-43) would refuse the switch. This derived image is our pull source. +("FROM localhost/bootc\n" + + "RUN touch /usr/share/testing-target-imgref\n") | podman build -t localhost/bootc-source -f - . + +# The reference we want recorded as the origin for future upgrades. It is never +# pulled (so it needs no network / need not exist): switch fetches from the +# local source above and only *records* this as the origin. +let origin = "quay.io/example/os:latest" + +# Fetch from the local containers-storage copy, but decouple the persisted origin. +bootc switch --transport containers-storage --target-imgref $origin localhost/bootc-source + +# The staged deployment's origin must be the --target-imgref value, not the +# containers-storage source we actually pulled from. +let st = bootc status --json | from json +assert ($st.status.staged != null) "Expected a staged deployment after switch" +let staged = $st.status.staged.image +assert equal $staged.image.transport "registry" +assert equal $staged.image.image $origin + +tap ok diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index c2c91422d..c622c0368 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -197,3 +197,8 @@ check: summary: Test composefs UKI dumpfile diff print duration: 30m test: nu booted/test-composefs-uki-dumpfile.nu + +/test-49-switch-target-imgref: + summary: switch --target-imgref records a decoupled origin (composefs) + duration: 30m + test: nu booted/test-switch-target-imgref.nu