diff --git a/crates/lib/src/bootc_composefs/switch.rs b/crates/lib/src/bootc_composefs/switch.rs index 0cb0b9ebe..1be15da8e 100644 --- a/crates/lib/src/bootc_composefs/switch.rs +++ b/crates/lib/src/bootc_composefs/switch.rs @@ -11,6 +11,7 @@ use crate::{ }, cli::{SwitchOpts, imgref_for_switch}, progress_jsonl::ProgressWriter, + spec::{Host, ImageReference}, store::{BootedComposefs, Storage}, }; @@ -27,7 +28,7 @@ pub(crate) async fn switch_composefs( let prog: ProgressWriter = opts.progress.clone().try_into()?; - let mut do_upgrade_opts = DoUpgradeOpts { + let do_upgrade_opts = DoUpgradeOpts { soft_reboot: opts.soft_reboot, apply: opts.apply, download_only: opts.download_opts.download_only, @@ -42,6 +43,29 @@ pub(crate) async fn switch_composefs( let target = imgref_for_switch(&opts)?; + switch_composefs_to( + storage, + booted_cfs, + &host, + target, + do_upgrade_opts, + opts.unified_storage_exp, + ) + .await +} + +/// Switch the booted composefs system to `target`, staging a new deployment. +/// +/// `unified_storage_exp` forces the use of unified storage; otherwise it is +/// used when either the booted or the target image is already there. +pub(crate) async fn switch_composefs_to( + storage: &Storage, + booted_cfs: &BootedComposefs, + host: &Host, + target: ImageReference, + mut do_upgrade_opts: DoUpgradeOpts, + unified_storage_exp: bool, +) -> Result<()> { let new_spec = { let mut new_spec = host.spec.clone(); new_spec.image = Some(target.clone()); @@ -50,7 +74,7 @@ pub(crate) async fn switch_composefs( if new_spec == host.spec { println!("Image specification is unchanged."); - if opts.apply && host.status.staged.is_some() { + if do_upgrade_opts.apply && host.status.staged.is_some() { crate::reboot::reboot()?; } return Ok(()); @@ -66,9 +90,8 @@ pub(crate) async fn switch_composefs( message_id = COMPOSEFS_SWITCH_JOURNAL_ID, bootc.operation = "switch", bootc.target_image = target_imgref.to_string(), - bootc.apply_mode = opts.apply, - bootc.download_only = opts.download_opts.download_only, - bootc.from_downloaded = opts.download_opts.from_downloaded, + bootc.apply_mode = do_upgrade_opts.apply, + bootc.download_only = do_upgrade_opts.download_only, "Starting composefs switch operation", ); @@ -78,7 +101,7 @@ pub(crate) async fn switch_composefs( // target image is already in bootc-owned containers-storage, OR the booted // image is — which means the user has opted into unified storage and all // subsequent operations (including switch to a new image) should use it. - do_upgrade_opts.use_unified = if opts.unified_storage_exp { + do_upgrade_opts.use_unified = if unified_storage_exp { true } else { let booted_imgref = host.spec.image.as_ref(); @@ -98,7 +121,7 @@ pub(crate) async fn switch_composefs( let action = validate_update( storage, booted_cfs, - &host, + host, img_config.manifest.config().digest().as_ref(), &cfg_verity, true, @@ -114,7 +137,7 @@ pub(crate) async fn switch_composefs( return do_upgrade( storage, booted_cfs, - &host, + host, &target_imgref, &do_upgrade_opts, &img_config.manifest, @@ -127,7 +150,7 @@ pub(crate) async fn switch_composefs( do_upgrade( storage, booted_cfs, - &host, + host, &target_imgref, &do_upgrade_opts, &img_config.manifest, diff --git a/crates/lib/src/cli.rs b/crates/lib/src/cli.rs index 9c34dd5fe..e064c2044 100644 --- a/crates/lib/src/cli.rs +++ b/crates/lib/src/cli.rs @@ -47,17 +47,18 @@ use crate::bootc_composefs::{ finalize::{composefs_backend_finalize, get_etc_diff}, rollback::composefs_rollback, state::composefs_usr_overlay, - switch::switch_composefs, - update::upgrade_composefs, + status::get_composefs_status, + switch::{switch_composefs, switch_composefs_to}, + update::{DoUpgradeOpts, upgrade_composefs}, }; use crate::deploy::{MergeState, RequiredHostSpec}; use crate::podstorage::set_additional_image_store; use crate::progress_jsonl::{ProgressWriter, RawProgressFd}; use crate::spec::FilesystemOverlayAccessMode; -use crate::spec::Host; use crate::spec::ImageReference; +use crate::spec::{Host, HostSpec}; use crate::status::get_host; -use crate::store::{BootedOstree, Storage}; +use crate::store::{BootedComposefs, BootedOstree, Storage}; use crate::store::{BootedStorage, BootedStorageKind}; use crate::utils::sigpolicy_from_opt; use crate::{bootc_composefs, lints}; @@ -1763,6 +1764,39 @@ async fn rollback(opts: &RollbackOpts) -> Result<()> { } } +/// Read the edited host definition for `bootc edit`, either from `filename` or +/// by spawning an editor on the current one, and validate it with +/// [`validate_edited_spec`]. +fn edited_host_spec(filename: Option<&str>, host: &Host) -> Result> { + let new_host: Host = if let Some(filename) = filename { + let f = std::fs::File::open(filename).with_context(|| format!("Opening {filename}"))?; + serde_yaml::from_reader(std::io::BufReader::new(f)) + .with_context(|| format!("Parsing {filename}"))? + } else { + let tmpf = tempfile::NamedTempFile::with_suffix(".yaml")?; + serde_yaml::to_writer(std::io::BufWriter::new(tmpf.as_file()), host)?; + crate::utils::spawn_editor(&tmpf)?; + tmpf.as_file().seek(std::io::SeekFrom::Start(0))?; + serde_yaml::from_reader(&mut tmpf.as_file()).context("Parsing edited host")? + }; + + let r = validate_edited_spec(&host.spec, new_host.spec)?; + if r.is_none() { + println!("Edit cancelled, no changes made."); + } + Ok(r) +} + +/// Returns `None` if the edited spec is unchanged; otherwise the new spec, +/// after checking that it is a supported transition from the current one. +fn validate_edited_spec(current: &HostSpec, new: HostSpec) -> Result> { + if &new == current { + return Ok(None); + } + current.verify_transition(&new)?; + Ok(Some(new)) +} + /// Implementation of the `bootc edit` CLI command for ostree backend. #[context("Editing spec (ostree)")] async fn edit_ostree( @@ -1773,32 +1807,19 @@ async fn edit_ostree( let repo = &booted_ostree.repo(); let (_, host) = crate::status::get_status(booted_ostree)?; - let new_host: Host = if let Some(filename) = opts.filename { - let mut r = std::io::BufReader::new(std::fs::File::open(filename)?); - serde_yaml::from_reader(&mut r)? - } else { - let tmpf = tempfile::NamedTempFile::with_suffix(".yaml")?; - serde_yaml::to_writer(std::io::BufWriter::new(tmpf.as_file()), &host)?; - crate::utils::spawn_editor(&tmpf)?; - tmpf.as_file().seek(std::io::SeekFrom::Start(0))?; - serde_yaml::from_reader(&mut tmpf.as_file())? - }; - - if new_host.spec == host.spec { - println!("Edit cancelled, no changes made."); + let Some(new_spec) = edited_host_spec(opts.filename.as_deref(), &host)? else { return Ok(()); - } - host.spec.verify_transition(&new_host.spec)?; - let new_spec = RequiredHostSpec::from_spec(&new_host.spec)?; - - let prog = ProgressWriter::default(); + }; // We only support two state transitions right now; switching the image, // or flipping the bootloader ordering. - if host.spec.boot_order != new_host.spec.boot_order { + if host.spec.boot_order != new_spec.boot_order { return crate::deploy::rollback(storage).await; } + let new_spec = RequiredHostSpec::from_spec(&new_spec)?; + let prog = ProgressWriter::default(); + let fetched = crate::deploy::pull( repo, new_spec.image, @@ -1820,6 +1841,47 @@ async fn edit_ostree( Ok(()) } +/// Implementation of the `bootc edit` CLI command for composefs backend. +#[context("Editing spec (composefs)")] +async fn edit_composefs( + opts: EditOpts, + storage: &Storage, + booted_cfs: &BootedComposefs, +) -> Result<()> { + let host = get_composefs_status(storage, booted_cfs) + .await + .context("Getting composefs deployment status")?; + + let Some(new_spec) = edited_host_spec(opts.filename.as_deref(), &host)? else { + return Ok(()); + }; + + // As for ostree, the supported transitions are flipping the boot order + // (a rollback) or changing the image; verify_transition rejected doing both. + if host.spec.boot_order != new_spec.boot_order { + return composefs_rollback(storage, booted_cfs).await; + } + + let new_spec = RequiredHostSpec::from_spec(&new_spec)?; + let do_upgrade_opts = DoUpgradeOpts { + apply: false, + soft_reboot: None, + download_only: false, + use_unified: false, + quiet: opts.quiet, + prog: ProgressWriter::default(), + }; + switch_composefs_to( + storage, + booted_cfs, + &host, + new_spec.image.clone(), + do_upgrade_opts, + false, + ) + .await +} + /// Implementation of the `bootc edit` CLI command. #[context("Editing spec")] async fn edit(opts: EditOpts) -> Result<()> { @@ -1828,8 +1890,8 @@ async fn edit(opts: EditOpts) -> Result<()> { BootedStorageKind::Ostree(booted_ostree) => { edit_ostree(opts, storage, &booted_ostree).await } - BootedStorageKind::Composefs(_) => { - anyhow::bail!("Edit is not yet supported for composefs backend") + BootedStorageKind::Composefs(booted_cfs) => { + edit_composefs(opts, storage, &booted_cfs).await } } } @@ -2781,6 +2843,43 @@ async fn run_from_opt(opt: Opt) -> Result { mod tests { use super::*; + #[test] + fn test_validate_edited_spec() { + use crate::spec::BootOrder; + + let host: Host = + serde_yaml::from_str(include_str!("fixtures/spec-staged-rollback.yaml")).unwrap(); + let current = &host.spec; + let other_image = || ImageReference { + image: "quay.io/example/other:latest".into(), + transport: "registry".into(), + signature: None, + }; + let spec = |image: Option, boot_order| HostSpec { image, boot_order }; + + // (edited spec, whether a change is expected, or None for an error) + let cases = [ + (current.clone(), Some(false)), + (spec(Some(other_image()), BootOrder::Default), Some(true)), + (spec(current.image.clone(), BootOrder::Rollback), Some(true)), + (spec(None, BootOrder::Default), Some(true)), + (spec(Some(other_image()), BootOrder::Rollback), None), + ]; + for (new, expected) in cases { + let r = validate_edited_spec(current, new.clone()); + match expected { + Some(changed) => { + let r = r.unwrap(); + assert_eq!(r.is_some(), changed, "{new:?}"); + if let Some(r) = r { + assert_eq!(r, new); + } + } + None => assert!(r.is_err(), "{new:?}"), + } + } + } + #[test] fn test_callname() { use std::os::unix::ffi::OsStrExt; diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index 06dadb2cf..beb4d9ece 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -331,4 +331,11 @@ execute: enabled: true extra-try_bind_storage: true extra-skip_if_ostree: true + +/plan-50-edit: + summary: Test bootc edit for image changes and rollback + discover: + how: fmf + test: + - /tmt/tests/tests/test-50-edit # END GENERATED PLANS diff --git a/tmt/tests/booted/test-edit.nu b/tmt/tests/booted/test-edit.nu new file mode 100644 index 000000000..5ab0b9990 --- /dev/null +++ b/tmt/tests/booted/test-edit.nu @@ -0,0 +1,95 @@ +# number: 50 +# tmt: +# summary: Test bootc edit for image changes and rollback +# duration: 30m +# +# This test verifies `bootc edit --filename` on both backends: +# 1. An unchanged spec is a no-op, and an edit changing both the image and +# the boot order is rejected +# 2. Changing spec.image stages that image, and we boot into it +# 3. Flipping spec.bootOrder queues a rollback, and we boot back +use std assert +use tap.nu + +const target_image = "localhost/bootc-edit-target" +const marker = "/usr/share/bootc-edit-marker" +const initial_state = "/var/bootc-edit-initial-state.json" +const spec_file = "/var/tmp/bootc-edit-host.yaml" + +def status_json [] { + bootc status --json | from json +} + +# Run `bootc edit` with the given host definition +def bootc_edit [host: record] { + $host | to yaml | save -f $spec_file + bootc edit --filename $spec_file +} + +def first_boot [] { + tap begin "bootc edit" + + let initial = status_json + $initial.status.booted.image | to json | save -f $initial_state + + let out = bootc_edit $initial + assert str contains $out "Edit cancelled" + assert ((status_json).status.staged | is-empty) + + bootc image copy-to-storage + let dockerfile = $"FROM localhost/bootc as base +RUN echo 'bootc edit target' > ($marker) +" + (tap make_uki_containerfile $dockerfile) | podman build -t $target_image -f - . + + let target = $initial + | update spec.image.image $target_image + | update spec.image.transport "containers-storage" + + # Changing the image and the boot order at once is not a valid transition + let r = do { bootc_edit ($target | update spec.bootOrder "rollback") } | complete + assert ($r.exit_code != 0) "Expected image change + rollback to be rejected" + assert str contains $r.stderr "Invalid state transition" + assert ((status_json).status.staged | is-empty) + + bootc_edit $target + let staged = (status_json).status.staged + assert equal $staged.image.image.image $target_image + assert equal $staged.image.image.transport "containers-storage" + + tmt-reboot +} + +def second_boot [] { + let st = status_json + assert equal $st.status.booted.image.image.image $target_image + assert ($marker | path exists) + assert equal $st.spec.bootOrder "default" + + bootc_edit ($st | update spec.bootOrder "rollback") + let st = status_json + assert equal $st.spec.bootOrder "rollback" + assert equal $st.status.rollbackQueued true + + tmt-reboot +} + +def third_boot [] { + let st = status_json + let initial = open $initial_state + assert equal $st.status.booted.image $initial + assert (not ($marker | path exists)) + assert equal $st.status.rollbackQueued false + + tap ok +} + +def main [] { + # See https://tmt.readthedocs.io/en/stable/stories/features.html#reboot-during-test + match $env.TMT_REBOOT_COUNT? { + null | "0" => first_boot, + "1" => second_boot, + "2" => third_boot, + $o => { error make { msg: $"Invalid TMT_REBOOT_COUNT ($o)" } }, + } +} diff --git a/tmt/tests/tests.fmf b/tmt/tests/tests.fmf index f27e9addd..89f14261f 100644 --- a/tmt/tests/tests.fmf +++ b/tmt/tests/tests.fmf @@ -206,3 +206,8 @@ check: - when: composefs_bridge == true enabled: true test: nu booted/test-49-composefs-1-16-bridge.nu + +/test-50-edit: + summary: Test bootc edit for image changes and rollback + duration: 30m + test: nu booted/test-edit.nu