From 34cde21810b7e7da9c3e367a9f8848236c8aa865 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 21 Nov 2025 12:56:55 +0530 Subject: [PATCH 1/2] composefs/update: Ensure idempotency on update Handle the following cases we can encounter on `bootc udpate` 1. The verity is the same as that of the currently booted deployment - Nothing to do here in case of update as we're currently booted. But if we're switching then we update the target imageref in the .origin file for the deployment 2. The verity is the same as that of the staged deployment - Nothing to do, as we only get a "staged" deployment if we have /run/composefs/staged-deployment which is the last thing we create while upgrading 3. The verity is the same as that of the rollback deployment or any other deployment we have already deployed - Nothing to do since this is a rollback deployment which means this was unstaged at some point 4. The verity is not found - The update/switch might've been canceled before /run/composefs/staged-deployment was created, or at any other point in time, or it's a new one. Any which way, we can overwrite everything. In this case we remove all the staged bootloader entries, if any, and remove the entire state directory, as it would most probably be in an inconsistent state. Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/state.rs | 49 +++- crates/lib/src/bootc_composefs/update.rs | 291 +++++++++++++++++------ 2 files changed, 272 insertions(+), 68 deletions(-) diff --git a/crates/lib/src/bootc_composefs/state.rs b/crates/lib/src/bootc_composefs/state.rs index 723d6ed19..196e83284 100644 --- a/crates/lib/src/bootc_composefs/state.rs +++ b/crates/lib/src/bootc_composefs/state.rs @@ -1,4 +1,7 @@ +use std::io::Write; +use std::ops::Deref; use std::os::unix::fs::symlink; +use std::path::Path; use std::{fs::create_dir_all, process::Command}; use anyhow::{Context, Result}; @@ -8,7 +11,7 @@ use bootc_mount::tempmount::TempMount; use bootc_utils::CommandRunExt; use camino::Utf8PathBuf; use cap_std_ext::cap_std::ambient_authority; -use cap_std_ext::cap_std::fs::Dir; +use cap_std_ext::cap_std::fs::{Dir, Permissions, PermissionsExt}; use cap_std_ext::dirext::CapStdExtDirExt; use composefs::fsverity::{FsVerityHashValue, Sha512HashValue}; use fn_error_context::context; @@ -23,6 +26,7 @@ use crate::bootc_composefs::boot::BootType; use crate::bootc_composefs::repo::get_imgref; use crate::bootc_composefs::status::get_sorted_type1_boot_entries; use crate::parsers::bls_config::BLSConfigType; +use crate::store::{BootedComposefs, Storage}; use crate::{ composefs_consts::{ COMPOSEFS_CMDLINE, COMPOSEFS_STAGED_DEPLOYMENT_FNAME, COMPOSEFS_TRANSIENT_STATE_DIR, @@ -104,6 +108,49 @@ pub(crate) fn copy_etc_to_state( cp_ret } +/// Updates the currently booted image's target imgref +pub(crate) fn update_target_imgref_in_origin( + storage: &Storage, + booted_cfs: &BootedComposefs, + imgref: &ImageReference, +) -> Result<()> { + let path = Path::new(STATE_DIR_RELATIVE).join(booted_cfs.cmdline.digest.deref()); + + let state_dir = storage + .physical_root + .open_dir(path) + .context("Opening state dir")?; + + let origin_filename = format!("{}.origin", booted_cfs.cmdline.digest.deref()); + + let origin_file = state_dir + .read_to_string(&origin_filename) + .context("Reading origin file")?; + + let mut ini = + tini::Ini::from_string(&origin_file).context("Failed to parse file origin file as ini")?; + + // Replace the origin + ini = ini.section("origin").item( + ORIGIN_CONTAINER, + format!("ostree-unverified-image:{imgref}"), + ); + + state_dir + .atomic_replace_with(origin_filename, move |f| -> std::io::Result<_> { + f.write_all(ini.to_string().as_bytes())?; + f.flush()?; + + let perms = Permissions::from_mode(0o644); + f.get_mut().as_file_mut().set_permissions(perms)?; + + Ok(()) + }) + .context("Writing to origin file")?; + + Ok(()) +} + /// Creates and populates /sysroot/state/deploy/image_id #[context("Writing composefs state")] pub(crate) fn write_composefs_state( diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index aa2d70ac8..870e90bca 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -2,9 +2,11 @@ use anyhow::{Context, Result}; use camino::Utf8PathBuf; use cap_std_ext::cap_std::fs::Dir; use composefs::{ - fsverity::FsVerityHashValue, + fsverity::{FsVerityHashValue, Sha512HashValue}, util::{parse_sha256, Sha256Digest}, }; +use composefs_boot::BootOps; +use composefs_oci::image::create_filesystem; use fn_error_context::context; use ostree_ext::oci_spec::image::{ImageConfiguration, ImageManifest}; @@ -13,11 +15,12 @@ use crate::{ boot::{setup_composefs_bls_boot, setup_composefs_uki_boot, BootSetupType, BootType}, repo::{get_imgref, pull_composefs_repo}, service::start_finalize_stated_svc, - state::write_composefs_state, - status::{get_composefs_status, get_container_manifest_and_config}, + state::{update_target_imgref_in_origin, write_composefs_state}, + status::{get_bootloader, get_composefs_status, get_container_manifest_and_config}, }, cli::UpgradeOpts, - spec::ImageReference, + composefs_consts::{STATE_DIR_RELATIVE, TYPE1_ENT_PATH_STAGED, USER_CFG_STAGED}, + spec::{Bootloader, Host, ImageReference}, store::{BootedComposefs, ComposefsRepository, Storage}, }; @@ -42,14 +45,14 @@ pub fn str_to_sha256digest(id: &str) -> Result { /// # Returns /// /// Returns a tuple containing: -/// * `true` if the image is pulled/available locally, `false` otherwise +/// * `Some` if the image is pulled/available locally, `None` otherwise /// * The container image manifest /// * The container image configuration #[context("Checking if image {} is pulled", imgref.image)] async fn is_image_pulled( repo: &ComposefsRepository, imgref: &ImageReference, -) -> Result<(bool, ImageManifest, ImageConfiguration)> { +) -> Result<(Option, ImageManifest, ImageConfiguration)> { let imgref_repr = get_imgref(&imgref.transport, &imgref.image); let (manifest, config) = get_container_manifest_and_config(&imgref_repr).await?; @@ -59,7 +62,169 @@ async fn is_image_pulled( // check_stream is expensive to run, but probably a good idea let container_pulled = repo.check_stream(&img_sha256).context("Checking stream")?; - Ok((container_pulled.is_some(), manifest, config)) + Ok((container_pulled, manifest, config)) +} + +fn rm_staged_type1_ent(boot_dir: &Dir) -> Result<()> { + if boot_dir.exists(TYPE1_ENT_PATH_STAGED) { + boot_dir + .remove_dir_all(TYPE1_ENT_PATH_STAGED) + .context("Removing staged bootloader entry")?; + } + + Ok(()) +} + +pub(crate) enum UpdateAction { + /// Skip the update. We probably have the update in our deployments + Skip, + /// Proceed with the update + Proceed, + /// Only update the target imgref in the .origin file + UpdateOrigin, +} + +/// Determines what action should be taken for the update +fn validate_update( + storage: &Storage, + booted_cfs: &BootedComposefs, + host: &Host, + img_digest: &str, + config_verity: &Sha512HashValue, +) -> Result { + // Cases + // + // 1. The verity is the same as that of the currently booted deployment + // - Nothing to do here as we're currently booted + // + // 2. The verity is the same as that of the staged deployment + // - Nothing to do, as we only get a "staged" deployment if we have + // /run/composefs/staged-deployment which is the last thing we create while upgrading + // + // 3. The verity is the same as that of the rollback deployment + // - Nothing to do since this is a rollback deployment which means this was unstaged at some + // point + // + // 4. The verity is not found + // - The update/switch might've been canceled before /run/composefs/staged-deployment + // was created, or at any other point in time, or it's a new one. + // Any which way, we can overwrite everything + + let repo = &*booted_cfs.repo; + + let mut fs = create_filesystem(repo, img_digest, Some(config_verity))?; + fs.transform_for_boot(&repo)?; + + let image_id = fs.compute_image_id(); + + // Case1 + // + // "update" image has the same verity as the one currently booted + // This could be someone trying to `bootc switch ` where + // remote_image is the exact same image as the one currently booted, but + // they are wanting to change the target + // + // We could simply update the image origin file here + if image_id.to_hex() == *booted_cfs.cmdline.digest { + // update_target_imgref_in_origin(storage, booted_cfs); + return Ok(UpdateAction::UpdateOrigin); + } + + let all_deployments = host.all_composefs_deployments()?; + + let found_depl = all_deployments + .iter() + .find(|d| d.deployment.verity == image_id.to_hex()); + + // We have this in our deployments somewhere, i.e. Case 2 or 3 + if found_depl.is_some() { + return Ok(UpdateAction::Skip); + } + + let booted = host.require_composefs_booted()?; + let boot_dir = storage.require_boot_dir()?; + + // Remove staged bootloader entries, if any + // GC should take care of the UKI PEs and other binaries + match get_bootloader()? { + Bootloader::Grub => match booted.boot_type { + BootType::Bls => rm_staged_type1_ent(boot_dir)?, + + BootType::Uki => { + let grub = boot_dir.open_dir("grub2").context("Opening grub dir")?; + + if grub.exists(USER_CFG_STAGED) { + grub.remove_file(USER_CFG_STAGED) + .context("Removing staged grub user config")?; + } + } + }, + + Bootloader::Systemd => rm_staged_type1_ent(boot_dir)?, + } + + // Remove state directory + let state_dir = storage + .physical_root + .open_dir(STATE_DIR_RELATIVE) + .context("Opening state dir")?; + + if state_dir.exists(image_id.to_hex()) { + state_dir + .remove_dir_all(image_id.to_hex()) + .context("Removing state")?; + } + + Ok(UpdateAction::Proceed) +} + +async fn do_upgrade(storage: &Storage, host: &Host, imgref: &ImageReference) -> Result<()> { + start_finalize_stated_svc()?; + + let (repo, entries, id, fs) = pull_composefs_repo(&imgref.transport, &imgref.image).await?; + + let Some(entry) = entries.iter().next() else { + anyhow::bail!("No boot entries!"); + }; + + let mounted_fs = Dir::reopen_dir( + &repo + .mount(&id.to_hex()) + .context("Failed to mount composefs image")?, + )?; + + let boot_type = BootType::from(entry); + let mut boot_digest = None; + + match boot_type { + BootType::Bls => { + boot_digest = Some(setup_composefs_bls_boot( + BootSetupType::Upgrade((storage, &fs, &host)), + repo, + &id, + entry, + &mounted_fs, + )?) + } + + BootType::Uki => setup_composefs_uki_boot( + BootSetupType::Upgrade((storage, &fs, &host)), + repo, + &id, + entries, + )?, + }; + + write_composefs_state( + &Utf8PathBuf::from("/sysroot"), + id, + imgref, + true, + boot_type, + boot_digest, + )?; + + Ok(()) } #[context("Upgrading composefs")] @@ -72,7 +237,7 @@ pub(crate) async fn upgrade_composefs( .await .context("Getting composefs deployment status")?; - let mut imgref = host + let mut booted_imgref = host .spec .image .as_ref() @@ -80,17 +245,11 @@ pub(crate) async fn upgrade_composefs( let repo = &*composefs.repo; - let (img_pulled, mut manifest, mut config) = is_image_pulled(&repo, imgref).await?; - let booted_img_digest = manifest.config().digest().digest(); - - // We already have this container config. No update available - if img_pulled { - println!("No changes in: {imgref:#}"); - // TODO(Johan-Liebert1): What if we have the config but we failed the previous update in the middle? - return Ok(()); - } + let (img_pulled, mut manifest, mut config) = is_image_pulled(&repo, booted_imgref).await?; + let booted_img_digest = manifest.config().digest().digest().to_owned(); // Check if we already have this update staged + // Or if we have another staged deployment with a different image let staged_image = host.status.staged.as_ref().and_then(|i| i.image.as_ref()); if let Some(staged_image) = staged_image { @@ -109,16 +268,57 @@ pub(crate) async fn upgrade_composefs( // We have a staged image but it's not the update image. // Maybe it's something we got by `bootc switch` // Switch takes precedence over update, so we change the imgref - imgref = &staged_image.image; + booted_imgref = &staged_image.image; - let (img_pulled, staged_manifest, staged_cfg) = is_image_pulled(&repo, imgref).await?; + let (img_pulled, staged_manifest, staged_cfg) = + is_image_pulled(&repo, booted_imgref).await?; manifest = staged_manifest; config = staged_cfg; - // We already have this container config. No update available - if img_pulled { - println!("No changes in staged image: {imgref:#}"); - return Ok(()); + if let Some(cfg_verity) = img_pulled { + let action = validate_update( + storage, + composefs, + &host, + manifest.config().digest().digest(), + &cfg_verity, + )?; + + match action { + UpdateAction::Skip => { + println!("No changes in staged image: {booted_imgref:#}"); + return Ok(()); + } + + UpdateAction::Proceed => { + return do_upgrade(storage, &host, booted_imgref).await; + } + + UpdateAction::UpdateOrigin => { + // The staged image will never be the current image's verity digest + anyhow::bail!("Staged image verity digest is the same as booted image") + } + } + } + } + + // We already have this container config + if let Some(cfg_verity) = img_pulled { + let action = validate_update(storage, composefs, &host, &booted_img_digest, &cfg_verity)?; + + match action { + UpdateAction::Skip => { + println!("No changes in: {booted_imgref:#}"); + return Ok(()); + } + + UpdateAction::Proceed => { + return do_upgrade(storage, &host, booted_imgref).await; + } + + UpdateAction::UpdateOrigin => { + return update_target_imgref_in_origin(storage, composefs, booted_imgref); + } } } @@ -150,50 +350,7 @@ pub(crate) async fn upgrade_composefs( return Ok(()); } - start_finalize_stated_svc()?; - - let (repo, entries, id, fs) = pull_composefs_repo(&imgref.transport, &imgref.image).await?; - - let Some(entry) = entries.iter().next() else { - anyhow::bail!("No boot entries!"); - }; - - let mounted_fs = Dir::reopen_dir( - &repo - .mount(&id.to_hex()) - .context("Failed to mount composefs image")?, - )?; - - let boot_type = BootType::from(entry); - let mut boot_digest = None; - - match boot_type { - BootType::Bls => { - boot_digest = Some(setup_composefs_bls_boot( - BootSetupType::Upgrade((storage, &fs, &host)), - repo, - &id, - entry, - &mounted_fs, - )?) - } - - BootType::Uki => setup_composefs_uki_boot( - BootSetupType::Upgrade((storage, &fs, &host)), - repo, - &id, - entries, - )?, - }; - - write_composefs_state( - &Utf8PathBuf::from("/sysroot"), - id, - imgref, - true, - boot_type, - boot_digest, - )?; + do_upgrade(storage, &host, booted_imgref).await?; if opts.apply { return crate::reboot::reboot(); From f5e2c4f4c720964d57278da8f8d3dcbe5f305655 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 21 Nov 2025 16:13:17 +0530 Subject: [PATCH 2/2] composefs: Ensure idempotency for switch Similar to how we handle bootc update Signed-off-by: Pragyan Poudyal --- crates/lib/src/bootc_composefs/switch.rs | 75 ++++++++----------- crates/lib/src/bootc_composefs/update.rs | 95 +++++++++++++++++------- 2 files changed, 96 insertions(+), 74 deletions(-) diff --git a/crates/lib/src/bootc_composefs/switch.rs b/crates/lib/src/bootc_composefs/switch.rs index 28ca67c48..8b8a232ec 100644 --- a/crates/lib/src/bootc_composefs/switch.rs +++ b/crates/lib/src/bootc_composefs/switch.rs @@ -1,16 +1,11 @@ use anyhow::{Context, Result}; -use camino::Utf8PathBuf; -use cap_std_ext::cap_std::fs::Dir; -use composefs::fsverity::FsVerityHashValue; use fn_error_context::context; use crate::{ bootc_composefs::{ - boot::{setup_composefs_bls_boot, setup_composefs_uki_boot, BootSetupType, BootType}, - repo::pull_composefs_repo, - service::start_finalize_stated_svc, - state::write_composefs_state, + state::update_target_imgref_in_origin, status::get_composefs_status, + update::{do_upgrade, is_image_pulled, validate_update, UpdateAction}, }, cli::{imgref_for_switch, SwitchOpts}, store::{BootedComposefs, Storage}, @@ -44,51 +39,39 @@ pub(crate) async fn switch_composefs( anyhow::bail!("Target image is undefined") }; - start_finalize_stated_svc()?; + let repo = &*booted_cfs.repo; + let (image, manifest, _) = is_image_pulled(repo, &target_imgref).await?; - let (repo, entries, id, fs) = - pull_composefs_repo(&target_imgref.transport, &target_imgref.image).await?; + if let Some(cfg_verity) = image { + let action = validate_update( + storage, + booted_cfs, + &host, + manifest.config().digest().digest(), + &cfg_verity, + true, + )?; - let Some(entry) = entries.iter().next() else { - anyhow::bail!("No boot entries!"); - }; - - let boot_type = BootType::from(entry); - let mut boot_digest = None; + match action { + UpdateAction::Skip => { + println!("No changes in image: {target_imgref:#}"); + return Ok(()); + } - let mounted_fs = Dir::reopen_dir( - &repo - .mount(&id.to_hex()) - .context("Failed to mount composefs image")?, - )?; + UpdateAction::Proceed => { + return do_upgrade(storage, &host, &target_imgref).await; + } - match boot_type { - BootType::Bls => { - boot_digest = Some(setup_composefs_bls_boot( - BootSetupType::Upgrade((storage, &fs, &host)), - repo, - &id, - entry, - &mounted_fs, - )?) + UpdateAction::UpdateOrigin => { + // The staged image will never be the current image's verity digest + println!("Image already in compoesfs repository"); + println!("Updating target image reference"); + return update_target_imgref_in_origin(storage, booted_cfs, &target_imgref); + } } - BootType::Uki => setup_composefs_uki_boot( - BootSetupType::Upgrade((storage, &fs, &host)), - repo, - &id, - entries, - )?, - }; + } - // TODO: Remove this hardcoded path when write_composefs_state accepts a Dir - write_composefs_state( - &Utf8PathBuf::from("/sysroot"), - id, - &target_imgref, - true, - boot_type, - boot_digest, - )?; + do_upgrade(storage, &host, &target_imgref).await?; Ok(()) } diff --git a/crates/lib/src/bootc_composefs/update.rs b/crates/lib/src/bootc_composefs/update.rs index 870e90bca..654b0c258 100644 --- a/crates/lib/src/bootc_composefs/update.rs +++ b/crates/lib/src/bootc_composefs/update.rs @@ -15,7 +15,7 @@ use crate::{ boot::{setup_composefs_bls_boot, setup_composefs_uki_boot, BootSetupType, BootType}, repo::{get_imgref, pull_composefs_repo}, service::start_finalize_stated_svc, - state::{update_target_imgref_in_origin, write_composefs_state}, + state::write_composefs_state, status::{get_bootloader, get_composefs_status, get_container_manifest_and_config}, }, cli::UpgradeOpts, @@ -49,7 +49,7 @@ pub fn str_to_sha256digest(id: &str) -> Result { /// * The container image manifest /// * The container image configuration #[context("Checking if image {} is pulled", imgref.image)] -async fn is_image_pulled( +pub(crate) async fn is_image_pulled( repo: &ComposefsRepository, imgref: &ImageReference, ) -> Result<(Option, ImageManifest, ImageConfiguration)> { @@ -75,41 +75,62 @@ fn rm_staged_type1_ent(boot_dir: &Dir) -> Result<()> { Ok(()) } +#[derive(Debug)] pub(crate) enum UpdateAction { /// Skip the update. We probably have the update in our deployments Skip, /// Proceed with the update Proceed, /// Only update the target imgref in the .origin file + /// Will only be returned if the Operation is update and not switch UpdateOrigin, } /// Determines what action should be taken for the update -fn validate_update( +/// +/// Cases: +/// +/// - The verity is the same as that of the currently booted deployment +/// +/// Nothing to do here as we're currently booted +/// +/// - The verity is the same as that of the staged deployment +/// +/// Nothing to do, as we only get a "staged" deployment if we have +/// /run/composefs/staged-deployment which is the last thing we create while upgrading +/// +/// - The verity is the same as that of the rollback deployment +/// +/// Nothing to do since this is a rollback deployment which means this was unstaged at some +/// point +/// +/// - The verity is not found +/// +/// The update/switch might've been canceled before /run/composefs/staged-deployment +/// was created, or at any other point in time, or it's a new one. +/// Any which way, we can overwrite everything +/// +/// # Arguments +/// +/// * `storage` - The global storage object +/// * `booted_cfs` - Reference to the booted composefs deployment +/// * `host` - Object returned by `get_composefs_status` +/// * `img_digest` - The SHA256 sum of the target image +/// * `config_verity` - The verity of the Image config splitstream +/// * `is_switch` - Whether this is an update operation or a switch operation +/// +/// # Returns +/// * UpdateAction::Skip - Skip the update/switch as we have it as a deployment +/// * UpdateAction::UpdateOrigin - Just update the target imgref in the origin file +/// * UpdateAction::Proceed - Proceed with the update +pub(crate) fn validate_update( storage: &Storage, booted_cfs: &BootedComposefs, host: &Host, img_digest: &str, config_verity: &Sha512HashValue, + is_switch: bool, ) -> Result { - // Cases - // - // 1. The verity is the same as that of the currently booted deployment - // - Nothing to do here as we're currently booted - // - // 2. The verity is the same as that of the staged deployment - // - Nothing to do, as we only get a "staged" deployment if we have - // /run/composefs/staged-deployment which is the last thing we create while upgrading - // - // 3. The verity is the same as that of the rollback deployment - // - Nothing to do since this is a rollback deployment which means this was unstaged at some - // point - // - // 4. The verity is not found - // - The update/switch might've been canceled before /run/composefs/staged-deployment - // was created, or at any other point in time, or it's a new one. - // Any which way, we can overwrite everything - let repo = &*booted_cfs.repo; let mut fs = create_filesystem(repo, img_digest, Some(config_verity))?; @@ -126,8 +147,13 @@ fn validate_update( // // We could simply update the image origin file here if image_id.to_hex() == *booted_cfs.cmdline.digest { - // update_target_imgref_in_origin(storage, booted_cfs); - return Ok(UpdateAction::UpdateOrigin); + let ret = if is_switch { + UpdateAction::UpdateOrigin + } else { + UpdateAction::Skip + }; + + return Ok(ret); } let all_deployments = host.all_composefs_deployments()?; @@ -178,7 +204,13 @@ fn validate_update( Ok(UpdateAction::Proceed) } -async fn do_upgrade(storage: &Storage, host: &Host, imgref: &ImageReference) -> Result<()> { +/// Performs the Update or Switch operation +#[context("Performing Upgrade Operation")] +pub(crate) async fn do_upgrade( + storage: &Storage, + host: &Host, + imgref: &ImageReference, +) -> Result<()> { start_finalize_stated_svc()?; let (repo, entries, id, fs) = pull_composefs_repo(&imgref.transport, &imgref.image).await?; @@ -282,6 +314,7 @@ pub(crate) async fn upgrade_composefs( &host, manifest.config().digest().digest(), &cfg_verity, + false, )?; match action { @@ -295,8 +328,7 @@ pub(crate) async fn upgrade_composefs( } UpdateAction::UpdateOrigin => { - // The staged image will never be the current image's verity digest - anyhow::bail!("Staged image verity digest is the same as booted image") + anyhow::bail!("Updating origin not supported for update operation") } } } @@ -304,7 +336,14 @@ pub(crate) async fn upgrade_composefs( // We already have this container config if let Some(cfg_verity) = img_pulled { - let action = validate_update(storage, composefs, &host, &booted_img_digest, &cfg_verity)?; + let action = validate_update( + storage, + composefs, + &host, + &booted_img_digest, + &cfg_verity, + false, + )?; match action { UpdateAction::Skip => { @@ -317,7 +356,7 @@ pub(crate) async fn upgrade_composefs( } UpdateAction::UpdateOrigin => { - return update_target_imgref_in_origin(storage, composefs, booted_imgref); + anyhow::bail!("Updating origin not supported for update operation") } } }