Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -152,14 +152,26 @@ jobs:
strategy:
fail-fast: false
matrix:
# No fedora-44 due to https://bugzilla.redhat.com/show_bug.cgi?id=2429501
test_os: [fedora-42, fedora-43, centos-9, centos-10]
variant: [ostree, composefs-sealeduki-sdboot]
test_os: [fedora-42]
variant: [ostree]
gating: [true]
exclude:
# centos-9 UKI is experimental/broken (https://github.com/bootc-dev/bootc/issues/1812)
- test_os: centos-9
variant: composefs-sealeduki-sdboot

- test_os: fedora-44
gating: true
# include:
# # fedora-44 non-gating due to grub2 regression
# # https://bugzilla.redhat.com/show_bug.cgi?id=2429501
# - test_os: fedora-44
# gating: false
# variant: ostree
# - test_os: fedora-44
# gating: false
# variant: composefs-sealeduki-sdboot
# Non-gating jobs are allowed to fail without blocking the PR
continue-on-error: ${{ !matrix.gating }}
runs-on: ubuntu-24.04

steps:
Expand Down Expand Up @@ -200,11 +212,12 @@ jobs:

- name: Run TMT integration tests
run: |
if [ "${{ matrix.variant }}" = "composefs-sealeduki-sdboot" ]; then
just test-composefs
if [ "${{ matrix.variant }}" = "composefs" ]; then
just test-tmt
else
just test-tmt integration
fi

just clean-local-images

- name: Archive TMT logs
Expand Down
3 changes: 2 additions & 1 deletion Justfile
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ package:

# Build+test using the `composefs-sealeduki-sdboot` variant.
test-composefs:
just variant=composefs-sealeduki-sdboot test-tmt readonly local-upgrade-reboot
just variant=composefs-sealeduki-sdboot test-tmt readonly soft-reboot

# Only used by ci.yml right now
build-install-test-image: build
Expand Down Expand Up @@ -162,6 +162,7 @@ _build-upgrade-image:
# Assume the localhost/bootc image is up to date, and just run tests.
# Useful for iterating on tests quickly.
test-tmt-nobuild *ARGS:
echo {{ARGS}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This echo statement appears to be for debugging purposes. It should be removed to keep the build output clean.

cargo xtask run-tmt --env=BOOTC_variant={{variant}} --upgrade-image={{upgrade_img}} {{base_img}} {{ARGS}}

# Build test container image for testing on coreos with SKIP_CONFIGS=1,
Expand Down
2 changes: 1 addition & 1 deletion crates/lib/src/bootc_composefs/boot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,7 @@ pub(crate) async fn setup_composefs_boot(
&root_setup.physical_root_path,
&id,
&crate::spec::ImageReference::from(state.target_imgref.clone()),
false,
None,
boot_type,
boot_digest,
&get_container_manifest_and_config(&get_imgref(
Expand Down
5 changes: 3 additions & 2 deletions crates/lib/src/bootc_composefs/export.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@ pub async fn export_repo_to_image(

let imginfo = get_imginfo(storage, &depl_verity, None).await?;

let config_digest = imginfo.manifest.config().digest().digest();
// We want the digest in the form of "sha256:abc123"
let config_digest = format!("{}", imginfo.manifest.config().digest());

let var_tmp =
Dir::open_ambient_dir("/var/tmp", ambient_authority()).context("Opening /var/tmp")?;
Expand All @@ -60,7 +61,7 @@ pub async fn export_repo_to_image(

// Use composefs_oci::open_config to get the config and layer map
let (config, layer_map) =
open_config(&*booted_cfs.repo, config_digest, None).context("Opening config")?;
open_config(&*booted_cfs.repo, &config_digest, None).context("Opening config")?;

// We can't guarantee that we'll get the same tar stream as the container image
// So we create new config and manifest
Expand Down
5 changes: 5 additions & 0 deletions crates/lib/src/bootc_composefs/finalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,11 @@ pub(crate) async fn composefs_backend_finalize(
return Ok(());
};

if staged_depl.download_only {
tracing::debug!("Staged deployment is marked download only. Won't finalize");
return Ok(());
}

let staged_composefs = staged_depl.composefs.as_ref().ok_or(anyhow::anyhow!(
"Staged deployment is not a composefs deployment"
))?;
Expand Down
52 changes: 50 additions & 2 deletions crates/lib/src/bootc_composefs/soft_reboot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use crate::{
bootc_composefs::{
service::start_finalize_stated_svc, status::composefs_deployment_status_from,
},
cli::SoftRebootMode,
composefs_consts::COMPOSEFS_CMDLINE,
store::{BootedComposefs, Storage},
};
Expand All @@ -10,25 +11,66 @@ use bootc_initramfs_setup::setup_root;
use bootc_kernel_cmdline::utf8::Cmdline;
use bootc_mount::{PID1, bind_mount_from_pidns};
use camino::Utf8Path;
use cap_std_ext::cap_std::ambient_authority;
use cap_std_ext::cap_std::fs::Dir;
use cap_std_ext::dirext::CapStdExtDirExt;
use fn_error_context::context;
use ostree_ext::systemd_has_soft_reboot;
use rustix::mount::{UnmountFlags, unmount};
use std::{fs::create_dir_all, os::unix::process::CommandExt, path::PathBuf, process::Command};

const NEXTROOT: &str = "/run/nextroot";

#[context("Resetting soft reboot state")]
pub(crate) fn reset_soft_reboot() -> Result<()> {
let run = Utf8Path::new("/run");
bind_mount_from_pidns(PID1, &run, &run, true).context("Bind mounting /run")?;

let run_dir = Dir::open_ambient_dir("/run", ambient_authority()).context("Opening run")?;

let nextroot = run_dir
.open_dir_optional("nextroot")
.context("Opening nextroot")?;

let Some(nextroot) = nextroot else {
tracing::debug!("Nextroot is not a directory");
println!("No deployment staged for soft rebooting");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The messages printed to the console for user feedback should ideally be directed to stderr instead of stdout. This is a common practice for diagnostic messages, allowing users to separate program output from informational messages. For example, eprintln!("No deployment staged for soft rebooting");.

Suggested change
println!("No deployment staged for soft rebooting");
eprintln!("No deployment staged for soft rebooting");

return Ok(());
};

let nextroot_mounted = nextroot
.is_mountpoint(".")?
.ok_or_else(|| anyhow::anyhow!("Failed to get mount info"))?;

if !nextroot_mounted {
tracing::debug!("Nextroot is not a mountpoint");
println!("No deployment staged for soft rebooting");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the previous comment, this user-facing message should be printed to stderr rather than stdout.

Suggested change
println!("No deployment staged for soft rebooting");
eprintln!("No deployment staged for soft rebooting");

return Ok(());
}

unmount(NEXTROOT, UnmountFlags::DETACH).context("Unmounting nextroot")?;

println!("Soft reboot state cleared successfully");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This success message should also be printed to stderr to maintain consistency with other diagnostic messages.

Suggested change
println!("Soft reboot state cleared successfully");
eprintln!("Soft reboot state cleared successfully");


Ok(())
}

/// Checks if the provided deployment is soft reboot capable, and soft reboots the system if
/// argument `reboot` is true
#[context("Soft rebooting")]
pub(crate) async fn prepare_soft_reboot_composefs(
storage: &Storage,
booted_cfs: &BootedComposefs,
deployment_id: &String,
deployment_id: Option<&String>,
soft_reboot_mode: SoftRebootMode,
reboot: bool,
) -> Result<()> {
if !systemd_has_soft_reboot() {
anyhow::bail!("System does not support soft reboots")
}

let deployment_id = deployment_id.ok_or_else(|| anyhow::anyhow!("Expected deployment id"))?;

if *deployment_id == *booted_cfs.cmdline.digest {
anyhow::bail!("Cannot soft-reboot to currently booted deployment");
}
Expand All @@ -44,7 +86,13 @@ pub(crate) async fn prepare_soft_reboot_composefs(
.ok_or_else(|| anyhow::anyhow!("Deployment '{deployment_id}' not found"))?;

if !requred_deployment.soft_reboot_capable {
anyhow::bail!("Cannot soft-reboot to deployment with a different kernel state");
match soft_reboot_mode {
SoftRebootMode::Required => {
anyhow::bail!("Cannot soft-reboot to deployment with a different kernel state")
}

SoftRebootMode::Auto => return Ok(()),
}
}

start_finalize_stated_svc()?;
Expand Down
20 changes: 15 additions & 5 deletions crates/lib/src/bootc_composefs/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use bootc_kernel_cmdline::utf8::Cmdline;
use bootc_mount::tempmount::TempMount;
use bootc_utils::CommandRunExt;
use camino::Utf8PathBuf;
use canon_json::CanonJsonSerialize;
use cap_std_ext::cap_std::ambient_authority;
use cap_std_ext::cap_std::fs::{Dir, Permissions, PermissionsExt};
use cap_std_ext::dirext::CapStdExtDirExt;
Expand All @@ -23,7 +24,9 @@ use rustix::{

use crate::bootc_composefs::boot::BootType;
use crate::bootc_composefs::repo::get_imgref;
use crate::bootc_composefs::status::{ImgConfigManifest, get_sorted_type1_boot_entries};
use crate::bootc_composefs::status::{
ImgConfigManifest, StagedDeployment, get_sorted_type1_boot_entries,
};
use crate::parsers::bls_config::BLSConfigType;
use crate::store::{BootedComposefs, Storage};
use crate::{
Expand Down Expand Up @@ -227,7 +230,7 @@ pub(crate) async fn write_composefs_state(
root_path: &Utf8PathBuf,
deployment_id: &Sha512HashValue,
target_imgref: &ImageReference,
staged: bool,
staged: Option<StagedDeployment>,
boot_type: BootType,
boot_digest: String,
container_details: &ImgConfigManifest,
Expand All @@ -248,7 +251,12 @@ pub(crate) async fn write_composefs_state(
)
.context("Failed to create symlink for /var")?;

initialize_state(&root_path, &deployment_id.to_hex(), &state_path, !staged)?;
initialize_state(
&root_path,
&deployment_id.to_hex(),
&state_path,
staged.is_none(),
)?;

let ImageReference {
image: image_name,
Expand Down Expand Up @@ -291,7 +299,7 @@ pub(crate) async fn write_composefs_state(
)
.context("Failed to write to .origin file")?;

if staged {
if let Some(staged) = staged {
std::fs::create_dir_all(COMPOSEFS_TRANSIENT_STATE_DIR)
.with_context(|| format!("Creating {COMPOSEFS_TRANSIENT_STATE_DIR}"))?;

Expand All @@ -302,7 +310,9 @@ pub(crate) async fn write_composefs_state(
staged_depl_dir
.atomic_write(
COMPOSEFS_STAGED_DEPLOYMENT_FNAME,
deployment_id.to_hex().as_bytes(),
staged
.to_canon_json_vec()
.context("Failed to serialize staged deployment JSON")?,
)
.with_context(|| format!("Writing to {COMPOSEFS_STAGED_DEPLOYMENT_FNAME}"))?;
}
Expand Down
23 changes: 17 additions & 6 deletions crates/lib/src/bootc_composefs/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ impl std::fmt::Display for ComposefsCmdline {
}
}

#[derive(Debug, Serialize, Deserialize)]
pub(crate) struct StagedDeployment {
pub(crate) depl_id: String,
pub(crate) finalization_locked: bool,
}

/// Detect if we have composefs=<digest> in /proc/cmdline
pub(crate) fn composefs_booted() -> Result<Option<&'static ComposefsCmdline>> {
static CACHED_DIGEST_VALUE: OnceLock<Option<ComposefsCmdline>> = OnceLock::new();
Expand Down Expand Up @@ -448,8 +454,10 @@ fn set_reboot_capable_type1_deployments(
.chain(host.status.rollback.iter_mut())
.chain(host.status.other_deployments.iter_mut())
{
let entry = find_bls_entry(&depl.require_composefs()?.verity, &bls_entries)?
.ok_or_else(|| anyhow::anyhow!("Entry not found"))?;
let v = &depl.require_composefs()?.verity;

let entry = find_bls_entry(v, &bls_entries)?
.ok_or_else(|| anyhow::anyhow!("Entry {v} not found"))?;

let depl_cmdline = entry.get_cmdline()?;

Expand Down Expand Up @@ -554,7 +562,7 @@ pub(crate) async fn composefs_deployment_status_from(

let mut host = Host::new(host_spec);

let staged_deployment_id = match std::fs::File::open(format!(
let staged_deployment = match std::fs::File::open(format!(
"{COMPOSEFS_TRANSIENT_STATE_DIR}/{COMPOSEFS_STAGED_DEPLOYMENT_FNAME}"
)) {
Ok(mut f) => {
Expand Down Expand Up @@ -590,7 +598,7 @@ pub(crate) async fn composefs_deployment_status_from(
let ini = tini::Ini::from_string(&config)
.with_context(|| format!("Failed to parse file {depl_file_name}.origin as ini"))?;

let boot_entry =
let mut boot_entry =
boot_entry_from_composefs_deployment(storage, ini, depl_file_name.to_string()).await?;

// SAFETY: boot_entry.composefs will always be present
Expand All @@ -614,8 +622,11 @@ pub(crate) async fn composefs_deployment_status_from(
continue;
}

if let Some(staged_deployment_id) = &staged_deployment_id {
if depl_file_name == staged_deployment_id.trim() {
if let Some(staged_deployment) = &staged_deployment {
let staged_depl = serde_json::from_str::<StagedDeployment>(&staged_deployment)?;

if depl_file_name == staged_depl.depl_id {
boot_entry.download_only = staged_depl.finalization_locked;
host.status.staged = Some(boot_entry);
continue;
}
Expand Down
1 change: 1 addition & 0 deletions crates/lib/src/bootc_composefs/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub(crate) async fn switch_composefs(
let do_upgrade_opts = DoUpgradeOpts {
soft_reboot: opts.soft_reboot,
apply: opts.apply,
download_only: false,
};

if let Some(cfg_verity) = image {
Expand Down
Loading
Loading