-
Notifications
You must be signed in to change notification settings - Fork 149
Various composefs changes #1715
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,33 +1,153 @@ | ||
| use anyhow::{Context, Result}; | ||
| use camino::Utf8PathBuf; | ||
| use composefs::util::{parse_sha256, Sha256Digest}; | ||
| use fn_error_context::context; | ||
| use ostree_ext::oci_spec::image::{ImageConfiguration, ImageManifest}; | ||
|
|
||
| use crate::{ | ||
| bootc_composefs::{ | ||
| boot::{setup_composefs_bls_boot, setup_composefs_uki_boot, BootSetupType, BootType}, | ||
| repo::pull_composefs_repo, | ||
| repo::{get_imgref, open_composefs_repo, pull_composefs_repo}, | ||
| service::start_finalize_stated_svc, | ||
| state::write_composefs_state, | ||
| status::composefs_deployment_status, | ||
| status::{composefs_deployment_status, get_container_manifest_and_config}, | ||
| }, | ||
| cli::UpgradeOpts, | ||
| spec::ImageReference, | ||
| store::ComposefsRepository, | ||
| }; | ||
|
|
||
| use cap_std_ext::cap_std::{ambient_authority, fs::Dir}; | ||
|
|
||
| #[context("Getting SHA256 Digest for {id}")] | ||
| pub fn str_to_sha256digest(id: &str) -> Result<Sha256Digest> { | ||
| let id = id.strip_prefix("sha256:").unwrap_or(id); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm it feels like being "loose" in our inputs is not a good idea; we should either strictly require a digest prefix or not.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see. This was just me being careful. I think the layer ids always start with |
||
| Ok(parse_sha256(&id)?) | ||
| } | ||
|
|
||
| /// Checks if a container image has been pulled to the local composefs repository. | ||
| /// | ||
| /// This function verifies whether the specified container image exists in the local | ||
| /// composefs repository by checking if the image's configuration digest stream is | ||
| /// available. It retrieves the image manifest and configuration from the container | ||
| /// registry and uses the configuration digest to perform the local availability check. | ||
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `repo` - The composefs repository | ||
| /// * `imgref` - Reference to the container image to check | ||
| /// | ||
| /// # Returns | ||
| /// | ||
| /// Returns a tuple containing: | ||
| /// * `true` if the image is pulled/available locally, `false` 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)> { | ||
| let imgref_repr = get_imgref(&imgref.transport, &imgref.image); | ||
| let (manifest, config) = get_container_manifest_and_config(&imgref_repr).await?; | ||
|
|
||
| let img_digest = manifest.config().digest().digest(); | ||
| let img_sha256 = str_to_sha256digest(&img_digest)?; | ||
|
|
||
| // 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)) | ||
| } | ||
|
|
||
| #[context("Upgrading composefs")] | ||
| pub(crate) async fn upgrade_composefs(_opts: UpgradeOpts) -> Result<()> { | ||
| pub(crate) async fn upgrade_composefs(opts: UpgradeOpts) -> Result<()> { | ||
| let host = composefs_deployment_status() | ||
| .await | ||
| .context("Getting composefs deployment status")?; | ||
|
|
||
| start_finalize_stated_svc()?; | ||
|
|
||
| // TODO: IMPORTANT We need to check if any deployment is staged and get the image from that | ||
| let imgref = host | ||
| let mut imgref = host | ||
| .spec | ||
| .image | ||
| .as_ref() | ||
| .ok_or_else(|| anyhow::anyhow!("No image source specified"))?; | ||
|
|
||
| let sysroot = | ||
| Dir::open_ambient_dir("/sysroot", ambient_authority()).context("Opening sysroot")?; | ||
| let repo = open_composefs_repo(&sysroot)?; | ||
|
|
||
| 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? | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think we need to only write the config object after all of its dependent layers.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's what we do. This comment was for the cases where we pull the entire image and create an erofs image, but fail to create some files in the state dir, or maybe something else. The bootloader entries will be overwritten, but we will fail the update if the state directory already exists. |
||
| return Ok(()); | ||
| } | ||
|
|
||
| // Check if we already have this update staged | ||
| let staged_image = host.status.staged.as_ref().and_then(|i| i.image.as_ref()); | ||
|
|
||
| if let Some(staged_image) = staged_image { | ||
| // We have a staged image and it has the same digest as the currently booted image's latest | ||
| // digest | ||
| if staged_image.image_digest == booted_img_digest { | ||
| if opts.apply { | ||
| return crate::reboot::reboot(); | ||
| } | ||
|
|
||
| println!("Update already staged. To apply update run `bootc update --apply`"); | ||
|
|
||
| return Ok(()); | ||
| } | ||
|
|
||
| // 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; | ||
|
|
||
| let (img_pulled, staged_manifest, staged_cfg) = is_image_pulled(&repo, 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 opts.check { | ||
| // TODO(Johan-Liebert1): If we have the previous, i.e. the current manifest with us then we can replace the | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And yes we need to store the manifest. |
||
| // following with [`ostree_container::ManifestDiff::new`] which will be much cleaner | ||
| for (idx, diff_id) in config.rootfs().diff_ids().iter().enumerate() { | ||
| let diff_id = str_to_sha256digest(diff_id)?; | ||
|
|
||
| // we could use `check_stream` here but that will most probably take forever as it | ||
| // usually takes ~3s to verify one single layer | ||
| let have_layer = repo.has_stream(&diff_id)?; | ||
|
|
||
| if have_layer.is_none() { | ||
| if idx >= manifest.layers().len() { | ||
| anyhow::bail!("Length mismatch between rootfs diff layers and manifest layers"); | ||
| } | ||
|
|
||
| let layer = &manifest.layers()[idx]; | ||
|
|
||
| println!( | ||
| "Added layer: {}\tSize: {}", | ||
| layer.digest(), | ||
| layer.size().to_string() | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| 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 { | ||
|
|
@@ -61,5 +181,9 @@ pub(crate) async fn upgrade_composefs(_opts: UpgradeOpts) -> Result<()> { | |
| boot_digest, | ||
| )?; | ||
|
|
||
| if opts.apply { | ||
| return crate::reboot::reboot(); | ||
| } | ||
|
|
||
| Ok(()) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -36,6 +36,10 @@ use bootc_mount::is_mounted_in_pid1_mountns; | |
| // This ensures we end up under 512 to be small-sized. | ||
| pub(crate) const BOOTPN_SIZE_MB: u32 = 510; | ||
| pub(crate) const EFIPN_SIZE_MB: u32 = 512; | ||
| /// EFI Partition size for composefs installations | ||
| /// We need more space than ostree as we have UKIs and UKI addons | ||
| /// We might also need to store UKIs for pinned deployments | ||
| pub(crate) const CFS_EFIPN_SIZE_MB: u32 = 1024; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Ultimately of course we want this to be more OS controllable, and that thread is adding support for repart.d in to-disk) |
||
| /// The GPT type for "linux" | ||
| pub(crate) const LINUX_PARTTYPE: &str = "0FC63DAF-8483-4772-8E79-3D69D8477DE4"; | ||
| #[cfg(feature = "install-to-disk")] | ||
|
|
@@ -277,9 +281,16 @@ pub(crate) fn install_create_rootfs( | |
| let esp_partno = if super::ARCH_USES_EFI { | ||
| let esp_guid = crate::discoverable_partition_specification::ESP; | ||
| partno += 1; | ||
|
|
||
| let esp_size = if state.composefs_options.composefs_backend { | ||
| CFS_EFIPN_SIZE_MB | ||
| } else { | ||
| EFIPN_SIZE_MB | ||
| }; | ||
|
|
||
| writeln!( | ||
| &mut partitioning_buf, | ||
| r#"size={EFIPN_SIZE_MB}MiB, type={esp_guid}, name="EFI-SYSTEM""# | ||
| r#"size={esp_size}MiB, type={esp_guid}, name="EFI-SYSTEM""# | ||
| )?; | ||
| Some(partno) | ||
| } else { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes...though we should also have exactly this same data in
/usr/lib/os-releasein the target image right? My inclination would be to use that over parsing the UKI, but in the end it doesn't really matter.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I think we could've gone either way as the data from
/usr/lib/os-releasewill be embedded in the UKI anyway