From 47dc7ec73424343f762b6602f10d4d16d9d543e9 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Tue, 28 Apr 2026 10:20:21 +0530 Subject: [PATCH 1/7] tmt: Test in parallel Signed-off-by: Pragyan Poudyal --- crates/xtask/src/tmt.rs | 484 ++++++++++++++++++++++++++-------------- 1 file changed, 313 insertions(+), 171 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index 795830343..33cd48981 100644 --- a/crates/xtask/src/tmt.rs +++ b/crates/xtask/src/tmt.rs @@ -1,3 +1,5 @@ +use std::thread::JoinHandle; + use anyhow::{Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; use fn_error_context::context; @@ -294,6 +296,203 @@ fn parse_plan_metadata( Ok(plan_metadata) } +struct RunPlanResult { + plan_name: String, + passed: bool, + run_id: Option, +} + +impl RunPlanResult { + fn new(plan_name: String, passed: bool, run_id: Option) -> Self { + Self { + plan_name, + passed, + run_id, + } + } +} + +fn run_plan( + plan: String, + vm_name: String, + image: String, + plan_bcvk_opts: Vec, + firmware_args: Vec, + context: Vec, + tmt_env_vars: Vec, + arg_env: Vec, + preserve_vm: bool, + vm_cpu: String, + vm_mem_mb: String, +) -> RunPlanResult { + let sh = match Shell::new() { + Ok(sh) => sh, + Err(err) => { + eprintln!("Failed to create new shell instance: {err:?}"); + return RunPlanResult::new(plan, false, None); + } + }; + + // Launch VM with bcvk + let firmware_args_slice = firmware_args.as_slice(); + let launch_result = cmd!( + sh, + "bcvk libvirt run --name {vm_name} --memory {vm_mem_mb} --cpus {vm_cpu} --detach {firmware_args_slice...} {COMMON_INST_ARGS...} {plan_bcvk_opts...} {image}" + ) + .run() + .context("Launching VM with bcvk"); + + if let Err(e) = launch_result { + eprintln!("Failed to launch VM for plan {}: {:#}", plan, e); + return RunPlanResult::new(plan, false, None); + } + + // Ensure VM cleanup happens even on error (unless --preserve-vm is set) + let cleanup_vm = || { + if preserve_vm { + return; + } + if let Err(e) = cmd!(sh, "bcvk libvirt rm --stop --force {vm_name}") + .ignore_stderr() + .ignore_status() + .run() + { + eprintln!("Warning: Failed to cleanup VM {}: {}", vm_name, e); + } + }; + + // Wait for VM to be ready and get SSH info + let vm_info = wait_for_vm_ready(&sh, &vm_name); + let (ssh_port, ssh_key) = match vm_info { + Ok((port, key)) => (port, key), + Err(e) => { + eprintln!("Failed to get VM info for plan {}: {:#}", plan, e); + cleanup_vm(); + return RunPlanResult::new(plan, false, None); + } + }; + + println!("VM ready, SSH port: {}", ssh_port); + + // Save SSH private key to a temporary file + let key_file = tempfile::NamedTempFile::new().context("Creating temporary SSH key file"); + + let key_file = match key_file { + Ok(f) => f, + Err(e) => { + eprintln!("Failed to create SSH key file for plan {}: {:#}", plan, e); + cleanup_vm(); + return RunPlanResult::new(plan, false, None); + } + }; + + let key_path = Utf8PathBuf::try_from(key_file.path().to_path_buf()) + .context("Converting key path to UTF-8"); + + let key_path = match key_path { + Ok(p) => p, + Err(e) => { + eprintln!("Failed to convert key path for plan {}: {:#}", plan, e); + cleanup_vm(); + return RunPlanResult::new(plan, false, None); + } + }; + + if let Err(e) = std::fs::write(&key_path, ssh_key) { + eprintln!("Failed to write SSH key for plan {}: {:#}", plan, e); + cleanup_vm(); + return RunPlanResult::new(plan, false, None); + } + + // Set proper permissions on the key file (SSH requires 0600) + { + use std::os::unix::fs::PermissionsExt; + let perms = std::fs::Permissions::from_mode(0o600); + if let Err(e) = std::fs::set_permissions(&key_path, perms) { + eprintln!("Failed to set key permissions for plan {}: {:#}", plan, e); + cleanup_vm(); + return RunPlanResult::new(plan, false, None); + } + } + + // Verify SSH connectivity + println!("Verifying SSH connectivity..."); + if let Err(e) = verify_ssh_connectivity(&sh, ssh_port, &key_path) { + eprintln!("SSH verification failed for plan {}: {:#}", plan, e); + cleanup_vm(); + return RunPlanResult::new(plan, false, None); + } + + println!("SSH connectivity verified"); + + let ssh_port_str = ssh_port.to_string(); + + // Run tmt for this specific plan using connect provisioner + println!("Running tmt tests for plan {}...", plan); + + // Generate a unique run ID for this test + // Use the VM name which already contains a random suffix for uniqueness + let run_id = vm_name.clone(); + + // Run tmt for this specific plan + // Note: provision must come before plan for connect to work properly + let how = ["--how=connect", "--guest=localhost", "--user=root"]; + let env = ["TMT_SCRIPTS_DIR=/var/lib/tmt/scripts", "BCVK_EXPORT=1"] + .into_iter() + .chain(arg_env.iter().map(|v| v.as_str())) + .chain(tmt_env_vars.iter().map(|v| v.as_str())) + .flat_map(|v| ["--environment", v]); + let test_result = cmd!( + sh, + "tmt {context...} run --id {run_id} --all {env...} provision {how...} --port {ssh_port_str} --key {key_path} plan --name {plan}" + ) + .run(); + + // Log disk usage after each test run to help diagnose "no space left on device" failures + println!("Disk usage after plan {}:", plan); + let _ = cmd!(sh, "df -h").run(); + + // Clean up VM regardless of test result (unless --preserve-vm is set) + cleanup_vm(); + + let plan_result = match test_result { + Ok(_) => { + println!("Plan {} completed successfully", plan); + RunPlanResult::new(plan, true, Some(run_id)) + } + Err(e) => { + eprintln!("Plan {} failed: {:#}", plan, e); + RunPlanResult::new(plan, false, Some(run_id)) + } + }; + + // Print VM connection details if preserving + if preserve_vm { + // Copy SSH key to a persistent location + let persistent_key_path = Utf8Path::new("target").join(format!("{}.ssh-key", vm_name)); + if let Err(e) = std::fs::copy(&key_path, &persistent_key_path) { + eprintln!("Warning: Failed to save persistent SSH key: {}", e); + } else { + println!("\n========================================"); + println!("VM preserved for debugging:"); + println!("========================================"); + println!("VM name: {}", vm_name); + println!("SSH port: {}", ssh_port_str); + println!("SSH key: {}", persistent_key_path); + println!("\nTo connect via SSH:"); + println!( + " ssh -i {} -p {} -o IdentitiesOnly=yes root@localhost", + persistent_key_path, ssh_port_str + ); + println!("\nTo cleanup:"); + println!(" bcvk libvirt rm --stop --force {}", vm_name); + println!("========================================\n"); + } + } + + plan_result +} + /// Run TMT tests using bcvk for VM management /// This spawns a separate VM per test plan to avoid state leakage between tests. #[context("Running TMT tests")] @@ -419,16 +618,67 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { println!("Found {} test plan(s): {:?}", plans.len(), plans); + let mut install_opts = Vec::new(); + + // Add --filesystem=xfs by default on fedora-coreos + if variant_id == "coreos" { + if distro.starts_with("fedora") { + install_opts.push("--filesystem=xfs".to_string()); + } + } + + if args.composefs_backend { + let filesystem = args.filesystem.as_deref().unwrap_or("ext4"); + install_opts.push(format!("--filesystem={}", filesystem)); + install_opts.push("--composefs-backend".into()); + + if let Some(b) = &args.bootloader { + install_opts.push(format!("--bootloader={b}")); + } + } + + for k in &args.karg { + install_opts.push(format!("--karg={k}")); + } + + println!("Creating base disk..."); + let opts = install_opts.clone(); + cmd!(sh, "bcvk libvirt to-base-disk {opts...} localhost/bootc").run()?; + + // println!( + // "Created base disk {}", + // std::str::from_utf8(&created_disk.stdout).unwrap_or("bcvk output was not valid UTF-8") + // ); + // Generate a random suffix for VM names let random_suffix = generate_random_suffix(); // Track overall success/failure let mut all_passed = true; - let mut test_results: Vec<(String, bool, Option)> = Vec::new(); + let mut test_results: Vec = Vec::new(); // Environment variables to pass to tmt (in addition to args.env) let mut tmt_env_vars = Vec::new(); + let mut handles: Vec> = vec![]; + + let num_cpu = std::thread::available_parallelism() + .map(|c| c.get()) + .unwrap_or(1); + + println!("num_cpu: {num_cpu}"); + + let (vm_cpu, vm_mem) = (1, 2048); + + // Leave 1 cpu for the host + // If there's only 1 cpu (unlikely), then we only run 1 VM + let avail_cpu = (num_cpu - 1).max(1); + + // More than this and we bottleneck on IO + let parallel_vms = (avail_cpu / vm_cpu).min(6); + + println!("parallel_vms: {parallel_vms}"); + // Run each plan in its own VM for plan in plans { let plan_name = sanitize_plan_name(plan); @@ -471,188 +721,65 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { distro ); } - // Add --filesystem=xfs by default on fedora-coreos - if variant_id == "coreos" { - if distro.starts_with("fedora") { - opts.push("--filesystem=xfs".to_string()); - } - } opts.extend(bcvk_opts.install_args()); opts }; - // Launch VM with bcvk - let firmware_args_slice = firmware_args.as_slice(); - let launch_result = cmd!( - sh, - "bcvk libvirt run --name {vm_name} --detach {firmware_args_slice...} {COMMON_INST_ARGS...} {plan_bcvk_opts...} {image}" - ) - .run() - .context("Launching VM with bcvk"); - - if let Err(e) = launch_result { - eprintln!("Failed to launch VM for plan {}: {:#}", plan, e); - all_passed = false; - test_results.push((plan.to_string(), false, None)); - continue; - } - - // Ensure VM cleanup happens even on error (unless --preserve-vm is set) - let cleanup_vm = || { - if preserve_vm { - return; - } - if let Err(e) = cmd!(sh, "bcvk libvirt rm --stop --force {vm_name}") - .ignore_stderr() - .ignore_status() - .run() - { - eprintln!("Warning: Failed to cleanup VM {}: {}", vm_name, e); - } - }; - - // Wait for VM to be ready and get SSH info - let vm_info = wait_for_vm_ready(sh, &vm_name); - let (ssh_port, ssh_key) = match vm_info { - Ok((port, key)) => (port, key), - Err(e) => { - eprintln!("Failed to get VM info for plan {}: {:#}", plan, e); - cleanup_vm(); - all_passed = false; - test_results.push((plan.to_string(), false, None)); - continue; - } - }; - - println!("VM ready, SSH port: {}", ssh_port); - - // Save SSH private key to a temporary file - let key_file = tempfile::NamedTempFile::new().context("Creating temporary SSH key file"); + let firmware_args = firmware_args.clone(); + let context = context.clone(); + let tmt_env_vars = tmt_env_vars.clone(); + let env = args.env.clone(); + let cloned_plan = plan.to_string(); + let cloned_vm_name = vm_name.to_string(); + let image = image.to_string(); + let vm_mem = vm_mem.to_string(); + let vm_cpu = vm_cpu.to_string(); + + let handle = std::thread::spawn(move || { + run_plan( + cloned_plan, + cloned_vm_name, + image, + plan_bcvk_opts, + firmware_args, + context, + tmt_env_vars, + env, + preserve_vm, + vm_cpu, + vm_mem, + ) + }); - let key_file = match key_file { - Ok(f) => f, - Err(e) => { - eprintln!("Failed to create SSH key file for plan {}: {:#}", plan, e); - cleanup_vm(); - all_passed = false; - test_results.push((plan.to_string(), false, None)); - continue; - } - }; + handles.push(handle); - let key_path = Utf8PathBuf::try_from(key_file.path().to_path_buf()) - .context("Converting key path to UTF-8"); + if handles.len() >= parallel_vms { + let e = handles.remove(0).join(); - let key_path = match key_path { - Ok(p) => p, - Err(e) => { - eprintln!("Failed to convert key path for plan {}: {:#}", plan, e); - cleanup_vm(); - all_passed = false; - test_results.push((plan.to_string(), false, None)); - continue; - } - }; - - if let Err(e) = std::fs::write(&key_path, ssh_key) { - eprintln!("Failed to write SSH key for plan {}: {:#}", plan, e); - cleanup_vm(); - all_passed = false; - test_results.push((plan.to_string(), false, None)); - continue; - } + match e { + Ok(plan_result) => { + test_results.push(plan_result); + } - // Set proper permissions on the key file (SSH requires 0600) - { - use std::os::unix::fs::PermissionsExt; - let perms = std::fs::Permissions::from_mode(0o600); - if let Err(e) = std::fs::set_permissions(&key_path, perms) { - eprintln!("Failed to set key permissions for plan {}: {:#}", plan, e); - cleanup_vm(); - all_passed = false; - test_results.push((plan.to_string(), false, None)); - continue; + Err(e) => { + eprintln!("Join failed: {e:?}"); + } } } + } - // Verify SSH connectivity - println!("Verifying SSH connectivity..."); - if let Err(e) = verify_ssh_connectivity(sh, ssh_port, &key_path) { - eprintln!("SSH verification failed for plan {}: {:#}", plan, e); - cleanup_vm(); - all_passed = false; - test_results.push((plan.to_string(), false, None)); - continue; - } - - println!("SSH connectivity verified"); - - let ssh_port_str = ssh_port.to_string(); - - // Run tmt for this specific plan using connect provisioner - println!("Running tmt tests for plan {}...", plan); - - // Generate a unique run ID for this test - // Use the VM name which already contains a random suffix for uniqueness - let run_id = vm_name.clone(); - - // Run tmt for this specific plan - // Note: provision must come before plan for connect to work properly - let context = context.clone(); - let how = ["--how=connect", "--guest=localhost", "--user=root"]; - let env = ["TMT_SCRIPTS_DIR=/var/lib/tmt/scripts", "BCVK_EXPORT=1"] - .into_iter() - .chain(args.env.iter().map(|v| v.as_str())) - .chain(tmt_env_vars.iter().map(|v| v.as_str())) - .flat_map(|v| ["--environment", v]); - let test_result = cmd!( - sh, - "tmt {context...} run --id {run_id} --all {env...} provision {how...} --port {ssh_port_str} --key {key_path} plan --name {plan}" - ) - .run(); - - // Log disk usage after each test run to help diagnose "no space left on device" failures - println!("Disk usage after plan {}:", plan); - let _ = cmd!(sh, "df -h").run(); - - // Clean up VM regardless of test result (unless --preserve-vm is set) - cleanup_vm(); + for h in handles { + let e = h.join(); - match test_result { - Ok(_) => { - println!("Plan {} completed successfully", plan); - test_results.push((plan.to_string(), true, Some(run_id))); + match e { + Ok(plan_result) => { + test_results.push(plan_result); } - Err(e) => { - eprintln!("Plan {} failed: {:#}", plan, e); - all_passed = false; - test_results.push((plan.to_string(), false, Some(run_id))); - } - } - // Print VM connection details if preserving - if preserve_vm { - // Copy SSH key to a persistent location - let persistent_key_path = Utf8Path::new("target").join(format!("{}.ssh-key", vm_name)); - if let Err(e) = std::fs::copy(&key_path, &persistent_key_path) { - eprintln!("Warning: Failed to save persistent SSH key: {}", e); - } else { - println!("\n========================================"); - println!("VM preserved for debugging:"); - println!("========================================"); - println!("VM name: {}", vm_name); - println!("SSH port: {}", ssh_port_str); - println!("SSH key: {}", persistent_key_path); - println!("\nTo connect via SSH:"); - println!( - " ssh -i {} -p {} -o IdentitiesOnly=yes root@localhost", - persistent_key_path, ssh_port_str - ); - println!("\nTo cleanup:"); - println!(" bcvk libvirt rm --stop --force {}", vm_name); - println!("========================================\n"); + Err(e) => { + eprintln!("Join failed: {e:?}"); } } } @@ -661,8 +788,18 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { println!("\n========================================"); println!("Test Summary"); println!("========================================"); - for (plan, passed, _) in &test_results { - let status = if *passed { "PASSED" } else { "FAILED" }; + for RunPlanResult { + plan_name: plan, + passed, + .. + } in &test_results + { + let status = if *passed { + "PASSED" + } else { + all_passed = false; + "FAILED" + }; println!("{}: {}", plan, status); } println!("========================================\n"); @@ -670,7 +807,7 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { // Print detailed error reports for failed tests let failed_tests: Vec<_> = test_results .iter() - .filter(|(_, passed, _)| !passed) + .filter(|plan_res| !plan_res.passed) .collect(); if !failed_tests.is_empty() { @@ -678,7 +815,12 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { println!("Detailed Error Reports"); println!("========================================\n"); - for (plan, _, run_id) in failed_tests { + for RunPlanResult { + plan_name: plan, + run_id, + .. + } in failed_tests + { println!("----------------------------------------"); println!("Plan: {}", plan); println!("----------------------------------------"); From 659bb99ce7500c91a347a59cc6879a6e1c7a8067 Mon Sep 17 00:00:00 2001 From: Johan-Liebert1 Date: Sat, 9 May 2026 11:27:22 +0530 Subject: [PATCH 2/7] xtask: Compute time taken by each test Signed-off-by: Johan-Liebert1 --- crates/xtask/src/tmt.rs | 52 ++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index 33cd48981..d4dd7a900 100644 --- a/crates/xtask/src/tmt.rs +++ b/crates/xtask/src/tmt.rs @@ -1,4 +1,5 @@ use std::thread::JoinHandle; +use std::time::Duration; use anyhow::{Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; @@ -299,14 +300,21 @@ fn parse_plan_metadata( struct RunPlanResult { plan_name: String, passed: bool, + time_taken: Option, run_id: Option, } impl RunPlanResult { - fn new(plan_name: String, passed: bool, run_id: Option) -> Self { + fn new( + plan_name: String, + passed: bool, + time_taken: Option, + run_id: Option, + ) -> Self { Self { plan_name, passed, + time_taken, run_id, } } @@ -329,7 +337,7 @@ fn run_plan( Ok(sh) => sh, Err(err) => { eprintln!("Failed to create new shell instance: {err:?}"); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } }; @@ -344,7 +352,7 @@ fn run_plan( if let Err(e) = launch_result { eprintln!("Failed to launch VM for plan {}: {:#}", plan, e); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } // Ensure VM cleanup happens even on error (unless --preserve-vm is set) @@ -368,7 +376,7 @@ fn run_plan( Err(e) => { eprintln!("Failed to get VM info for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } }; @@ -382,7 +390,7 @@ fn run_plan( Err(e) => { eprintln!("Failed to create SSH key file for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } }; @@ -394,14 +402,14 @@ fn run_plan( Err(e) => { eprintln!("Failed to convert key path for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } }; if let Err(e) = std::fs::write(&key_path, ssh_key) { eprintln!("Failed to write SSH key for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } // Set proper permissions on the key file (SSH requires 0600) @@ -411,7 +419,7 @@ fn run_plan( if let Err(e) = std::fs::set_permissions(&key_path, perms) { eprintln!("Failed to set key permissions for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } } @@ -420,13 +428,15 @@ fn run_plan( if let Err(e) = verify_ssh_connectivity(&sh, ssh_port, &key_path) { eprintln!("SSH verification failed for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } println!("SSH connectivity verified"); let ssh_port_str = ssh_port.to_string(); + let time_start = std::time::Instant::now(); + // Run tmt for this specific plan using connect provisioner println!("Running tmt tests for plan {}...", plan); @@ -448,6 +458,8 @@ fn run_plan( ) .run(); + let elapsed = time_start.elapsed(); + // Log disk usage after each test run to help diagnose "no space left on device" failures println!("Disk usage after plan {}:", plan); let _ = cmd!(sh, "df -h").run(); @@ -458,11 +470,11 @@ fn run_plan( let plan_result = match test_result { Ok(_) => { println!("Plan {} completed successfully", plan); - RunPlanResult::new(plan, true, Some(run_id)) + RunPlanResult::new(plan, true, Some(elapsed), Some(run_id)) } Err(e) => { eprintln!("Plan {} failed: {:#}", plan, e); - RunPlanResult::new(plan, false, Some(run_id)) + RunPlanResult::new(plan, false, Some(elapsed), Some(run_id)) } }; @@ -641,14 +653,13 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { install_opts.push(format!("--karg={k}")); } + let start = std::time::Instant::now(); + println!("Creating base disk..."); let opts = install_opts.clone(); cmd!(sh, "bcvk libvirt to-base-disk {opts...} localhost/bootc").run()?; - // println!( - // "Created base disk {}", - // std::str::from_utf8(&created_disk.stdout).unwrap_or("bcvk output was not valid UTF-8") - // ); + println!("Creating base disk took: {:#?}", start.elapsed()); // Generate a random suffix for VM names let random_suffix = generate_random_suffix(); @@ -788,9 +799,13 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { println!("\n========================================"); println!("Test Summary"); println!("========================================"); + + test_results.sort_by(|a, b| b.time_taken.cmp(&a.time_taken)); + for RunPlanResult { plan_name: plan, passed, + time_taken, .. } in &test_results { @@ -800,7 +815,12 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { all_passed = false; "FAILED" }; - println!("{}: {}", plan, status); + println!( + "{}: {} ({:?})", + plan, + status, + time_taken.unwrap_or(Duration::from_secs(0)) + ); } println!("========================================\n"); From 521dd1486ab59fdfe49d174a804773a32238e02d Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Mon, 11 May 2026 15:17:27 +0530 Subject: [PATCH 3/7] tmt: Introduce `skip_for_ostree` So we don't spawn VMs for tests like "composefs-gc" just to do nothing and exit Signed-off-by: Pragyan Poudyal --- crates/xtask/src/tmt.rs | 56 ++++++++++++++++++--- tmt/plans/integration.fmf | 4 ++ tmt/tests/booted/test-composefs-gc-uki.nu | 2 + tmt/tests/booted/test-composefs-gc.nu | 3 ++ tmt/tests/booted/test-switch-same-digest.nu | 2 + 5 files changed, 60 insertions(+), 7 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index d4dd7a900..beb5e9c6f 100644 --- a/crates/xtask/src/tmt.rs +++ b/crates/xtask/src/tmt.rs @@ -28,6 +28,10 @@ const FIELD_ADJUST: &str = "adjust"; const FIELD_FIXME_SKIP_IF_COMPOSEFS: &str = "fixme_skip_if_composefs"; const FIELD_FIXME_SKIP_IF_UKI: &str = "fixme_skip_if_uki"; +/// For tests that should only run for composefs systems +/// Ex. composefs-gc +const FIELD_SKIP_IF_OSTREE: &str = "skip_if_ostree"; + // bcvk options const BCVK_OPT_BIND_STORAGE_RO: &str = "--bind-storage-ro"; const ENV_BOOTC_UPGRADE_IMAGE: &str = "BOOTC_upgrade_image"; @@ -210,10 +214,11 @@ fn verify_ssh_connectivity(sh: &Shell, port: u16, key_path: &Utf8Path) -> Result ) } -#[derive(Debug)] +#[derive(Debug, Default)] struct PlanMetadata { try_bind_storage: bool, skip_if_composefs: bool, + skip_if_ostree: bool, skip_if_uki: bool, } @@ -255,8 +260,7 @@ fn parse_plan_metadata( .and_modify(|m| m.try_bind_storage = b) .or_insert(PlanMetadata { try_bind_storage: b, - skip_if_uki: false, - skip_if_composefs: false, + ..Default::default() }); } } @@ -271,8 +275,7 @@ fn parse_plan_metadata( .and_modify(|m| m.skip_if_composefs = b) .or_insert(PlanMetadata { skip_if_composefs: b, - skip_if_uki: false, - try_bind_storage: false, + ..Default::default() }); } } @@ -287,8 +290,22 @@ fn parse_plan_metadata( .and_modify(|m| m.skip_if_uki = b) .or_insert(PlanMetadata { skip_if_uki: b, - skip_if_composefs: false, - try_bind_storage: false, + ..Default::default() + }); + } + } + + if let Some(skip_if_ostree) = plan_data.get(&serde_yaml::Value::String(format!( + "extra-{}", + FIELD_SKIP_IF_OSTREE + ))) { + if let Some(b) = skip_if_ostree.as_bool() { + plan_metadata + .entry(plan_name.to_string()) + .and_modify(|m| m.skip_if_ostree = b) + .or_insert(PlanMetadata { + skip_if_ostree: b, + ..Default::default() }); } } @@ -602,6 +619,14 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { .map(|(_, v)| v.skip_if_composefs) .unwrap_or(false) }); + } else { + plans.retain(|plan| { + !plan_metadata + .iter() + .find(|(key, _)| plan.ends_with(key.as_str())) + .map(|(_, v)| v.skip_if_ostree) + .unwrap_or(false) + }); } if matches!(args.boot_type, crate::BootType::Uki) { @@ -1068,6 +1093,8 @@ struct TestDef { try_bind_storage: bool, /// Whether to skip this test for composefs backend skip_if_composefs: bool, + /// Whether to skip this test for ostree backend + skip_if_ostree: bool, /// Whether to skip this test for images with UKI skip_if_uki: bool, /// TMT fmf attributes to pass through (summary, duration, adjust, etc.) @@ -1233,6 +1260,13 @@ fn generate_integration() -> Result<(String, String)> { .and_then(|v| v.as_bool()) .unwrap_or(false); + let skip_if_ostree = metadata + .extra + .as_mapping() + .and_then(|m| m.get(&serde_yaml::Value::String(FIELD_SKIP_IF_OSTREE.to_string()))) + .and_then(|v| v.as_bool()) + .unwrap_or(false); + let skip_if_uki = metadata .extra .as_mapping() @@ -1250,6 +1284,7 @@ fn generate_integration() -> Result<(String, String)> { test_command, try_bind_storage, skip_if_composefs, + skip_if_ostree, skip_if_uki, tmt: metadata.tmt, }); @@ -1373,6 +1408,13 @@ fn generate_integration() -> Result<(String, String)> { ); } + if test.skip_if_ostree { + plan_value.insert( + serde_yaml::Value::String(format!("extra-{}", FIELD_SKIP_IF_OSTREE)), + serde_yaml::Value::Bool(true), + ); + } + if test.skip_if_uki { plan_value.insert( serde_yaml::Value::String(format!("extra-{}", FIELD_FIXME_SKIP_IF_UKI)), diff --git a/tmt/plans/integration.fmf b/tmt/plans/integration.fmf index b3366648b..d27a77058 100644 --- a/tmt/plans/integration.fmf +++ b/tmt/plans/integration.fmf @@ -196,6 +196,8 @@ execute: how: fmf test: - /tmt/tests/tests/test-35-composefs-gc + extra-skip_if_ostree: true + extra-fixme_skip_if_uki: true /plan-35-upgrade-preflight-disk-check: summary: Verify pre-flight disk space check rejects images with inflated layer sizes @@ -254,6 +256,7 @@ execute: how: fmf test: - /tmt/tests/tests/test-41-composefs-gc-uki + extra-skip_if_ostree: true /plan-42-loader-entries-source: summary: Test bootc loader-entries set-options-for-source @@ -269,6 +272,7 @@ execute: how: fmf test: - /tmt/tests/tests/test-43-switch-same-digest + extra-skip_if_ostree: true /plan-44-shadow-fixup: summary: Test bootc-sysusers-shadow-sync removes orphaned gshadow entries before sysusers diff --git a/tmt/tests/booted/test-composefs-gc-uki.nu b/tmt/tests/booted/test-composefs-gc-uki.nu index e2efd561c..cd08c2fb2 100644 --- a/tmt/tests/booted/test-composefs-gc-uki.nu +++ b/tmt/tests/booted/test-composefs-gc-uki.nu @@ -2,6 +2,8 @@ # tmt: # summary: Test composefs garbage collection for UKI # duration: 30m +# extra: +# skip_if_ostree: true use std assert use tap.nu diff --git a/tmt/tests/booted/test-composefs-gc.nu b/tmt/tests/booted/test-composefs-gc.nu index d19a01efe..a6b8b9e27 100644 --- a/tmt/tests/booted/test-composefs-gc.nu +++ b/tmt/tests/booted/test-composefs-gc.nu @@ -2,6 +2,9 @@ # tmt: # summary: Test composefs garbage collection with same and different kernel+initrd # duration: 30m +# extra: +# skip_if_ostree: true +# fixme_skip_if_uki: true use std assert use tap.nu diff --git a/tmt/tests/booted/test-switch-same-digest.nu b/tmt/tests/booted/test-switch-same-digest.nu index eda56d467..033fc2b69 100644 --- a/tmt/tests/booted/test-switch-same-digest.nu +++ b/tmt/tests/booted/test-switch-same-digest.nu @@ -2,6 +2,8 @@ # tmt: # summary: Error on bootc switch to image with identical fs-verity digest # duration: 10m +# extra: +# skip_if_ostree: true # # Verify that `bootc switch` errors out when the target image produces the # same composefs fs-verity digest as an existing deployment. The simplest From 9501d10675308a6fa31323301b3bf3eaee39507b Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Mon, 11 May 2026 16:09:47 +0530 Subject: [PATCH 4/7] tmt/test: Sort tests by time taken, use mpsc channels Sort tests in descending order of time taken for completion so longer tests get scheduled together. Also, update to use mpsc channels for communication between threads Signed-off-by: Pragyan Poudyal --- crates/xtask/src/tmt.rs | 83 ++++++++++++++++++++++++++++++++--------- 1 file changed, 66 insertions(+), 17 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index beb5e9c6f..231be2bbe 100644 --- a/crates/xtask/src/tmt.rs +++ b/crates/xtask/src/tmt.rs @@ -1,5 +1,6 @@ -use std::thread::JoinHandle; +use std::sync::mpsc; use std::time::Duration; +use std::usize; use anyhow::{Context, Result}; use camino::{Utf8Path, Utf8PathBuf}; @@ -39,6 +40,36 @@ const ENV_BOOTC_UPGRADE_IMAGE: &str = "BOOTC_upgrade_image"; // Distro identifiers const DISTRO_CENTOS_9: &str = "centos-9"; +// Tests sorted by time taken (descending) +const TESTS_SORTED_BY_TIME: [&str; 23] = [ + // 10+ mins + "multi-device-esp", + "composefs-gc-uki", + "composefs-gc", + // 5+ mins + "loader-entries-source", + "download-only-upgrade", + "bib-build", + "rollback", + "logically-bound-switch", + "soft-reboot", + "switch-to-unified", + "image-pushpull-upgrade", + "install-no-boot-dir", + "upgrade-tag", + "custom-selinux-policy", + "factory-reset", + // 3+ mins + "upgrade-check-status", + "soft-reboot-selinux-policy", + "install-bootloader-none", + "install-outside-container", + "install-unified-flag", + "usroverlay", + "image-upgrade-reboot", + "install-karg-delete", +]; + // Import the argument types from xtask.rs use crate::bcvk::BcvkInstallOpts; use crate::{RunTmtArgs, SealState, TmtProvisionArgs, out_of_sync_error}; @@ -653,6 +684,13 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { return Ok(()); } + plans.sort_by_key(|full_plan_name| { + TESTS_SORTED_BY_TIME + .iter() + .position(|test_time| full_plan_name.contains(test_time)) + .unwrap_or(usize::MAX) + }); + println!("Found {} test plan(s): {:?}", plans.len(), plans); let mut install_opts = Vec::new(); @@ -696,7 +734,7 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { // Environment variables to pass to tmt (in addition to args.env) let mut tmt_env_vars = Vec::new(); - let mut handles: Vec> = vec![]; + let mut active_threads = 0; let num_cpu = std::thread::available_parallelism() .map(|c| c.get()) @@ -715,6 +753,8 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { println!("parallel_vms: {parallel_vms}"); + let (tx, rx) = mpsc::channel::(); + // Run each plan in its own VM for plan in plans { let plan_name = sanitize_plan_name(plan); @@ -773,8 +813,9 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { let vm_mem = vm_mem.to_string(); let vm_cpu = vm_cpu.to_string(); - let handle = std::thread::spawn(move || { - run_plan( + let tx_clone = tx.clone(); + std::thread::spawn(move || { + let result = run_plan( cloned_plan, cloned_vm_name, image, @@ -786,36 +827,44 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { preserve_vm, vm_cpu, vm_mem, - ) - }); + ); - handles.push(handle); + if let Err(e) = tx_clone.send(result) { + eprintln!("Failed to send result through channel: {}", e); + } + }); - if handles.len() >= parallel_vms { - let e = handles.remove(0).join(); + active_threads += 1; - match e { + // wait for a thread to complete if we've reached the parallel limit + if active_threads >= parallel_vms { + match rx.recv() { Ok(plan_result) => { test_results.push(plan_result); + active_threads -= 1; } - Err(e) => { - eprintln!("Join failed: {e:?}"); + eprintln!("Failed to receive result from channel: {}", e); + // still decrement to avoid infinite loop + // in theory this shouldn't happen as we loop over plans, but + // for sanity + active_threads -= 1; } } } } - for h in handles { - let e = h.join(); + // drop the sender to signal no more messages + drop(tx); - match e { + // remaining results from channel + for _ in 0..active_threads { + match rx.recv() { Ok(plan_result) => { test_results.push(plan_result); } - Err(e) => { - eprintln!("Join failed: {e:?}"); + eprintln!("Failed to receive remaining result from channel: {}", e); } } } From 0b150cd73bbb9693ddff11ecf0f63ed523b418ba Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Fri, 15 May 2026 16:28:14 +0530 Subject: [PATCH 5/7] tmt/test/bib-build: Save disk image in /var/output Signed-off-by: Pragyan Poudyal --- tmt/tests/booted/test-bib-build.nu | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tmt/tests/booted/test-bib-build.nu b/tmt/tests/booted/test-bib-build.nu index 765698204..18d7c4af5 100644 --- a/tmt/tests/booted/test-bib-build.nu +++ b/tmt/tests/booted/test-bib-build.nu @@ -26,6 +26,8 @@ const BIB_IMAGE = "quay.io/centos-bootc/bootc-image-builder:latest" def main [] { tap begin "bootc-image-builder qcow2 build test" + print ">>>>>>>>>>>>>>>>>> the current working directory <<<<<<<<<<<<<<<<<" ($env.PWD) + let td = mktemp -d cd $td @@ -87,8 +89,10 @@ DISKEOF ' | save Dockerfile podman build -t localhost/bootc-bib-test . + let output_dir = "/var/output" + # Create output directory for bib - mkdir output + mkdir $output_dir # Run bootc-image-builder to create a qcow2 # We use --local to pull from local containers-storage @@ -97,11 +101,11 @@ DISKEOF let bib_image = $BIB_IMAGE # Note: we disable SELinux labeling since we're running in a test VM # and use unconfined_t to avoid permission issues - podman run --rm --privileged -v /var/lib/containers/storage:/var/lib/containers/storage --security-opt label=type:unconfined_t -v ./output:/output $bib_image --type qcow2 --rootfs xfs localhost/bootc-bib-test + podman run --rm --privileged -v /var/lib/containers/storage:/var/lib/containers/storage --security-opt label=type:unconfined_t -v $"($output_dir):/output" $bib_image --type qcow2 --rootfs xfs localhost/bootc-bib-test # Verify output was created print "=== Verifying output ===" - let disk_path = "output/qcow2/disk.qcow2" + let disk_path = $"($output_dir)/qcow2/disk.qcow2" assert ($disk_path | path exists) $"Expected disk image at ($disk_path)" # Check the disk has reasonable virtual size (at least 4GB as per disk.yaml) From dc2b1351bb2174e6b1297e4080def20a58a68080 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Mon, 18 May 2026 15:01:28 +0530 Subject: [PATCH 6/7] tmt: Treat VM creation as critical section Use a Mutex guard for VM creation as CI sometimes is failing with ``` Error: 0: Failed to create libvirt domain 1: Failed to start libvirt domain: error: Failed to start domain 'bootc-..' error: failed to create directory '/run/user/1001/libvirt/qemu/run/swtpm': File exists ``` Signed-off-by: Pragyan Poudyal --- crates/xtask/src/tmt.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index 231be2bbe..cfbaeac9f 100644 --- a/crates/xtask/src/tmt.rs +++ b/crates/xtask/src/tmt.rs @@ -1,4 +1,4 @@ -use std::sync::mpsc; +use std::sync::{Arc, Mutex, mpsc}; use std::time::Duration; use std::usize; @@ -380,6 +380,7 @@ fn run_plan( preserve_vm: bool, vm_cpu: String, vm_mem_mb: String, + libvirt_lock: Arc>, ) -> RunPlanResult { let sh = match Shell::new() { Ok(sh) => sh, @@ -391,6 +392,15 @@ fn run_plan( // Launch VM with bcvk let firmware_args_slice = firmware_args.as_slice(); + + let guard = match libvirt_lock.lock() { + Ok(g) => g, + Err(e) => { + eprintln!("Mutex lock failed for plan {plan}: {e:#}"); + return RunPlanResult::new(plan, false, None, None); + } + }; + let launch_result = cmd!( sh, "bcvk libvirt run --name {vm_name} --memory {vm_mem_mb} --cpus {vm_cpu} --detach {firmware_args_slice...} {COMMON_INST_ARGS...} {plan_bcvk_opts...} {image}" @@ -398,6 +408,8 @@ fn run_plan( .run() .context("Launching VM with bcvk"); + drop(guard); + if let Err(e) = launch_result { eprintln!("Failed to launch VM for plan {}: {:#}", plan, e); return RunPlanResult::new(plan, false, None, None); @@ -755,6 +767,8 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { let (tx, rx) = mpsc::channel::(); + let libvirt_lock = Arc::new(Mutex::new(())); + // Run each plan in its own VM for plan in plans { let plan_name = sanitize_plan_name(plan); @@ -812,6 +826,7 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { let image = image.to_string(); let vm_mem = vm_mem.to_string(); let vm_cpu = vm_cpu.to_string(); + let libvirt_lock = libvirt_lock.clone(); let tx_clone = tx.clone(); std::thread::spawn(move || { @@ -827,6 +842,7 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { preserve_vm, vm_cpu, vm_mem, + libvirt_lock, ); if let Err(e) = tx_clone.send(result) { From 8ebd2af731f7bec4f5cc0ac87266cda4871b1d56 Mon Sep 17 00:00:00 2001 From: Pragyan Poudyal Date: Mon, 1 Jun 2026 13:52:15 +0530 Subject: [PATCH 7/7] tmt: Rerun failed tests To reduce CI failures due to flakes, rerun failed tests Signed-off-by: Pragyan Poudyal --- crates/xtask/src/tmt.rs | 452 +++++++++++++++++++++++++--------------- 1 file changed, 281 insertions(+), 171 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index cfbaeac9f..cd6ea815f 100644 --- a/crates/xtask/src/tmt.rs +++ b/crates/xtask/src/tmt.rs @@ -352,6 +352,26 @@ struct RunPlanResult { run_id: Option, } +struct VmConfig { + cpu: String, + mem: String, + parallel_count: usize, + preserve: bool, +} + +struct TestContext<'a> { + random_suffix: &'a str, + distro: &'a str, + image: &'a str, + upgrade_image: Option<&'a str>, +} + +struct TmtConfig<'a> { + context: &'a [String], + env_vars: &'a [String], + user_env: &'a [String], +} + impl RunPlanResult { fn new( plan_name: String, @@ -369,24 +389,24 @@ impl RunPlanResult { } fn run_plan( - plan: String, - vm_name: String, - image: String, + plan: &str, + vm_name: &str, + image: &str, plan_bcvk_opts: Vec, firmware_args: Vec, context: Vec, tmt_env_vars: Vec, arg_env: Vec, preserve_vm: bool, - vm_cpu: String, - vm_mem_mb: String, + vm_cpu: &str, + vm_mem_mb: &str, libvirt_lock: Arc>, ) -> RunPlanResult { let sh = match Shell::new() { Ok(sh) => sh, Err(err) => { eprintln!("Failed to create new shell instance: {err:?}"); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } }; @@ -397,7 +417,7 @@ fn run_plan( Ok(g) => g, Err(e) => { eprintln!("Mutex lock failed for plan {plan}: {e:#}"); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } }; @@ -412,7 +432,7 @@ fn run_plan( if let Err(e) = launch_result { eprintln!("Failed to launch VM for plan {}: {:#}", plan, e); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } // Ensure VM cleanup happens even on error (unless --preserve-vm is set) @@ -436,7 +456,7 @@ fn run_plan( Err(e) => { eprintln!("Failed to get VM info for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } }; @@ -450,7 +470,7 @@ fn run_plan( Err(e) => { eprintln!("Failed to create SSH key file for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } }; @@ -462,14 +482,14 @@ fn run_plan( Err(e) => { eprintln!("Failed to convert key path for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } }; if let Err(e) = std::fs::write(&key_path, ssh_key) { eprintln!("Failed to write SSH key for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } // Set proper permissions on the key file (SSH requires 0600) @@ -479,7 +499,7 @@ fn run_plan( if let Err(e) = std::fs::set_permissions(&key_path, perms) { eprintln!("Failed to set key permissions for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } } @@ -488,7 +508,7 @@ fn run_plan( if let Err(e) = verify_ssh_connectivity(&sh, ssh_port, &key_path) { eprintln!("SSH verification failed for plan {}: {:#}", plan, e); cleanup_vm(); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } println!("SSH connectivity verified"); @@ -502,7 +522,7 @@ fn run_plan( // Generate a unique run ID for this test // Use the VM name which already contains a random suffix for uniqueness - let run_id = vm_name.clone(); + let run_id = vm_name; // Run tmt for this specific plan // Note: provision must come before plan for connect to work properly @@ -530,11 +550,21 @@ fn run_plan( let plan_result = match test_result { Ok(_) => { println!("Plan {} completed successfully", plan); - RunPlanResult::new(plan, true, Some(elapsed), Some(run_id)) + RunPlanResult::new( + plan.to_string(), + true, + Some(elapsed), + Some(run_id.to_string()), + ) } Err(e) => { eprintln!("Plan {} failed: {:#}", plan, e); - RunPlanResult::new(plan, false, Some(elapsed), Some(run_id)) + RunPlanResult::new( + plan.to_string(), + false, + Some(elapsed), + Some(run_id.to_string()), + ) } }; @@ -565,6 +595,122 @@ fn run_plan( plan_result } +fn run_plans( + plans: Vec<&str>, + plan_metadata: &std::collections::HashMap, + bcvk_opts: &crate::bcvk::BcvkInstallOpts, + firmware_args: &[String], + vm_config: VmConfig, + test_context: TestContext, + tmt_config: &TmtConfig, + libvirt_lock: &Arc>, +) -> Result> { + let mut test_results: Vec = Vec::new(); + let mut active_threads = 0; + let (tx, rx) = mpsc::channel::(); + + for plan in plans { + let plan_name = sanitize_plan_name(plan); + let vm_name = format!("bootc-tmt-{}-{}", test_context.random_suffix, plan_name); + + println!("\n========================================"); + println!("Running plan: {}", plan); + println!("VM name: {}", vm_name); + println!("========================================\n"); + + // Get bcvk-opts based on plan metadata and distro support + let plan_bcvk_opts = { + let supports_bind_storage_ro = distro_supports_bind_storage_ro(test_context.distro); + let try_bind_storage = plan_metadata + .iter() + .find(|(key, _)| plan.ends_with(key.as_str())) + .map(|(_, v)| v.try_bind_storage) + .unwrap_or(false); + + let mut opts = Vec::new(); + let mut env_vars = tmt_config.env_vars.to_vec(); + + if try_bind_storage && supports_bind_storage_ro { + opts.push(BCVK_OPT_BIND_STORAGE_RO.to_string()); + if let Some(upgrade_img) = test_context.upgrade_image { + env_vars.push(format!("{}={}", ENV_BOOTC_UPGRADE_IMAGE, upgrade_img)); + } + } else if try_bind_storage && !supports_bind_storage_ro { + println!( + "Note: Test wants bind storage but skipping on {} (missing systemd.extra-unit.* support)", + test_context.distro + ); + } + + opts.extend(bcvk_opts.install_args()); + opts + }; + + let firmware_args = firmware_args.to_vec(); + let context = tmt_config.context.to_vec(); + let env_vars = tmt_config.env_vars.to_vec(); + let user_env = tmt_config.user_env.to_vec(); + let cloned_plan = plan.to_string(); + let cloned_vm_name = vm_name.to_string(); + let image = test_context.image.to_string(); + let vm_mem = vm_config.mem.clone(); + let vm_cpu = vm_config.cpu.clone(); + let libvirt_lock = libvirt_lock.clone(); + + let tx_clone = tx.clone(); + std::thread::spawn(move || { + let result = run_plan( + &cloned_plan, + &cloned_vm_name, + &image, + plan_bcvk_opts, + firmware_args, + context, + env_vars, + user_env, + vm_config.preserve, + &vm_cpu, + &vm_mem, + libvirt_lock, + ); + + if let Err(e) = tx_clone.send(result) { + eprintln!("Failed to send result through channel: {}", e); + } + }); + + active_threads += 1; + + if active_threads >= vm_config.parallel_count { + match rx.recv() { + Ok(plan_result) => { + test_results.push(plan_result); + active_threads -= 1; + } + Err(e) => { + eprintln!("Failed to receive result from channel: {}", e); + active_threads -= 1; + } + } + } + } + + drop(tx); + + for _ in 0..active_threads { + match rx.recv() { + Ok(plan_result) => { + test_results.push(plan_result); + } + Err(e) => { + eprintln!("Failed to receive remaining result from channel: {}", e); + } + } + } + + Ok(test_results) +} + /// Run TMT tests using bcvk for VM management /// This spawns a separate VM per test plan to avoid state leakage between tests. #[context("Running TMT tests")] @@ -739,15 +885,6 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { // Generate a random suffix for VM names let random_suffix = generate_random_suffix(); - // Track overall success/failure - let mut all_passed = true; - let mut test_results: Vec = Vec::new(); - - // Environment variables to pass to tmt (in addition to args.env) - let mut tmt_env_vars = Vec::new(); - - let mut active_threads = 0; - let num_cpu = std::thread::available_parallelism() .map(|c| c.get()) .unwrap_or(1); @@ -755,135 +892,44 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { println!("num_cpu: {num_cpu}"); let (vm_cpu, vm_mem) = (1, 2048); - - // Leave 1 cpu for the host - // If there's only 1 cpu (unlikely), then we only run 1 VM let avail_cpu = (num_cpu - 1).max(1); - - // More than this and we bottleneck on IO let parallel_vms = (avail_cpu / vm_cpu).min(6); println!("parallel_vms: {parallel_vms}"); - let (tx, rx) = mpsc::channel::(); - let libvirt_lock = Arc::new(Mutex::new(())); - // Run each plan in its own VM - for plan in plans { - let plan_name = sanitize_plan_name(plan); - let vm_name = format!("bootc-tmt-{}-{}", random_suffix, plan_name); - - println!("\n========================================"); - println!("Running plan: {}", plan); - println!("VM name: {}", vm_name); - println!("========================================\n"); - - // Reset plan-specific environment variables - tmt_env_vars.clear(); - - // Get bcvk-opts based on plan metadata and distro support - let plan_bcvk_opts = { - let supports_bind_storage_ro = distro_supports_bind_storage_ro(&distro); - - // Plan names from tmt are like /tmt/plans/integration/plan-01-readonly - // but metadata keys are like /plan-01-readonly, so match on suffix - let try_bind_storage = plan_metadata - .iter() - .find(|(key, _)| plan.ends_with(key.as_str())) - .map(|(_, v)| v.try_bind_storage) - .unwrap_or(false); - - let mut opts = Vec::new(); - - // If test wants bind storage and distro supports it, add --bind-storage-ro - if try_bind_storage && supports_bind_storage_ro { - opts.push(BCVK_OPT_BIND_STORAGE_RO.to_string()); - - // If upgrade image is provided, set it as an environment variable for tmt - // (not bcvk, as bcvk doesn't support --env) - if let Some(ref upgrade_img) = args.upgrade_image { - tmt_env_vars.push(format!("{}={}", ENV_BOOTC_UPGRADE_IMAGE, upgrade_img)); - } - } else if try_bind_storage && !supports_bind_storage_ro { - println!( - "Note: Test wants bind storage but skipping on {} (missing systemd.extra-unit.* support)", - distro - ); - } - - opts.extend(bcvk_opts.install_args()); - - opts - }; - - let firmware_args = firmware_args.clone(); - let context = context.clone(); - let tmt_env_vars = tmt_env_vars.clone(); - let env = args.env.clone(); - let cloned_plan = plan.to_string(); - let cloned_vm_name = vm_name.to_string(); - let image = image.to_string(); - let vm_mem = vm_mem.to_string(); - let vm_cpu = vm_cpu.to_string(); - let libvirt_lock = libvirt_lock.clone(); - - let tx_clone = tx.clone(); - std::thread::spawn(move || { - let result = run_plan( - cloned_plan, - cloned_vm_name, - image, - plan_bcvk_opts, - firmware_args, - context, - tmt_env_vars, - env, - preserve_vm, - vm_cpu, - vm_mem, - libvirt_lock, - ); - - if let Err(e) = tx_clone.send(result) { - eprintln!("Failed to send result through channel: {}", e); - } - }); - - active_threads += 1; + let vm_config = VmConfig { + cpu: vm_cpu.to_string(), + mem: vm_mem.to_string(), + parallel_count: parallel_vms, + preserve: preserve_vm, + }; - // wait for a thread to complete if we've reached the parallel limit - if active_threads >= parallel_vms { - match rx.recv() { - Ok(plan_result) => { - test_results.push(plan_result); - active_threads -= 1; - } - Err(e) => { - eprintln!("Failed to receive result from channel: {}", e); - // still decrement to avoid infinite loop - // in theory this shouldn't happen as we loop over plans, but - // for sanity - active_threads -= 1; - } - } - } - } + let test_context = TestContext { + random_suffix: &random_suffix, + distro: &distro, + image, + upgrade_image: args.upgrade_image.as_deref(), + }; - // drop the sender to signal no more messages - drop(tx); + let tmt_config = TmtConfig { + context: &context, + env_vars: &Vec::new(), // Will be populated per plan + user_env: &args.env, + }; - // remaining results from channel - for _ in 0..active_threads { - match rx.recv() { - Ok(plan_result) => { - test_results.push(plan_result); - } - Err(e) => { - eprintln!("Failed to receive remaining result from channel: {}", e); - } - } - } + // Run all plans + let mut test_results = run_plans( + plans, + &plan_metadata, + &bcvk_opts, + &firmware_args, + vm_config, + test_context, + &tmt_config, + &libvirt_lock, + )?; // Print summary println!("\n========================================"); @@ -899,12 +945,7 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { .. } in &test_results { - let status = if *passed { - "PASSED" - } else { - all_passed = false; - "FAILED" - }; + let status = if *passed { "PASSED" } else { "FAILED" }; println!( "{}: {} ({:?})", plan, @@ -920,22 +961,27 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { .filter(|plan_res| !plan_res.passed) .collect(); - if !failed_tests.is_empty() { - println!("\n========================================"); - println!("Detailed Error Reports"); - println!("========================================\n"); + if failed_tests.is_empty() { + println!("All tests passed"); + return Ok(()); + } - for RunPlanResult { - plan_name: plan, - run_id, - .. - } in failed_tests - { - println!("----------------------------------------"); - println!("Plan: {}", plan); - println!("----------------------------------------"); + println!("\n========================================"); + println!("Detailed Error Reports"); + println!("========================================\n"); - if let Some(id) = run_id { + for RunPlanResult { + plan_name: plan, + run_id, + .. + } in &failed_tests + { + println!("----------------------------------------"); + println!("Plan: {}", plan); + println!("----------------------------------------"); + + match run_id { + Some(id) => { println!("Run ID: {}\n", id); // Run tmt with the specific run ID and generate verbose report @@ -952,18 +998,82 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { ); } } - } else { - println!("Run ID not available - cannot generate detailed report"); } - println!("\n"); + None => { + println!("Run ID not available - cannot generate detailed report"); + } } - println!("========================================\n"); + println!("\n"); + } + + println!("========================================\n"); + + // Rerun failed plans + println!("Rerunning failed plans...\n"); + + let failed_plan_names: Vec<&str> = failed_tests + .iter() + .map(|result| result.plan_name.as_str()) + .collect(); + + let rerun_suffix = format!("{}-rerun", random_suffix); + let test_context_rerun = TestContext { + random_suffix: &rerun_suffix, + distro: &distro, + image, + upgrade_image: args.upgrade_image.as_deref(), + }; + + let vm_config_rerun = VmConfig { + cpu: vm_cpu.to_string(), + mem: vm_mem.to_string(), + parallel_count: parallel_vms, + preserve: preserve_vm, + }; + + let rerun_results = run_plans( + failed_plan_names, + &plan_metadata, + &bcvk_opts, + &firmware_args, + vm_config_rerun, + test_context_rerun, + &tmt_config, + &libvirt_lock, + )?; + + // Print rerun summary + println!("\n========================================"); + println!("Rerun Summary"); + println!("========================================"); + + let mut rerun_all_passed = true; + for RunPlanResult { + plan_name: plan, + passed, + time_taken, + .. + } in &rerun_results + { + let status = if *passed { + "PASSED" + } else { + rerun_all_passed = false; + "FAILED" + }; + println!( + "{}: {} ({:?})", + plan, + status, + time_taken.unwrap_or(Duration::from_secs(0)) + ); } + println!("========================================\n"); - if !all_passed { - anyhow::bail!("Some test plans failed"); + if !rerun_all_passed { + anyhow::bail!("Some test plans still failed after rerun"); } Ok(())