From a98093c009c5dbd985f1670aa0a139277a1e589e 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 | 519 +++++++++++++++++++++++++--------------- 1 file changed, 332 insertions(+), 187 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index 728eec97f..7b64e76d9 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,225 @@ 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, + base_log_dir: Utf8PathBuf, + bcvk_has_log_dir: bool, +) -> 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); + } + }; + + // Set up per-VM log directory for journal + console capture (if bcvk supports it) + let vm_log_dir = base_log_dir.join(&vm_name); + let log_dir_args: Vec = if bcvk_has_log_dir { + if let Err(e) = std::fs::create_dir_all(&vm_log_dir) { + eprintln!("Creating VM log directory {}: {e:?}", vm_log_dir); + return RunPlanResult::new(plan, false, None); + } + + println!("VM logs will be written to: {}", vm_log_dir); + vec![format!("--log-dir=journal,console={}", vm_log_dir)] + } else { + vec![] + }; + + // 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...} {log_dir_args...} {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); + if bcvk_has_log_dir { + eprintln!( + "VM logs (journal + console) may be available at: {}", + vm_log_dir + ); + } + 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")] @@ -438,16 +659,62 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { .map(|help| help.contains("--log-dir")) .unwrap_or(false); + 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()?; + // 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); @@ -490,205 +757,68 @@ 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 }; - // Set up per-VM log directory for journal + console capture (if bcvk supports it) - let vm_log_dir = base_log_dir.join(&vm_name); - let log_dir_args: Vec = if bcvk_has_log_dir { - std::fs::create_dir_all(&vm_log_dir) - .with_context(|| format!("Creating VM log directory {}", vm_log_dir))?; - println!("VM logs will be written to: {}", vm_log_dir); - vec![format!("--log-dir=journal,console={}", vm_log_dir)] - } else { - vec![] - }; - - // 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...} {log_dir_args...} {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); + 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 base_log_dir = base_log_dir.clone(); + + 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, + base_log_dir, + bcvk_has_log_dir, + ) + }); - // Save SSH private key to a temporary file - let key_file = tempfile::NamedTempFile::new().context("Creating temporary SSH key file"); + handles.push(handle); - 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; - } - }; + if handles.len() >= parallel_vms { + let e = handles.remove(0).join(); - let key_path = Utf8PathBuf::try_from(key_file.path().to_path_buf()) - .context("Converting key path to UTF-8"); + match e { + Ok(plan_result) => { + test_results.push(plan_result); + } - 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; + Err(e) => { + eprintln!("Join failed: {e:?}"); + } } - }; - - 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; } + } - // 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; - } - } + for h in handles { + let e = h.join(); - // 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); - if bcvk_has_log_dir { - eprintln!( - "VM logs (journal + console) may be available at: {}", - vm_log_dir - ); + match e { + Ok(plan_result) => { + test_results.push(plan_result); } - 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(); - - match test_result { - Ok(_) => { - println!("Plan {} completed successfully", plan); - test_results.push((plan.to_string(), true, Some(run_id))); - } 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"); + eprintln!("Join failed: {e:?}"); } } } @@ -697,8 +827,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"); @@ -706,7 +846,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() { @@ -714,7 +854,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 9eb867503d80afdde092f29ce057f73c915c35b2 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 | 51 ++++++++++++++++++++++++++++++----------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index 7b64e76d9..bbc834c28 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, } } @@ -331,7 +339,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); } }; @@ -340,7 +348,7 @@ fn run_plan( let log_dir_args: Vec = if bcvk_has_log_dir { if let Err(e) = std::fs::create_dir_all(&vm_log_dir) { eprintln!("Creating VM log directory {}: {e:?}", vm_log_dir); - return RunPlanResult::new(plan, false, None); + return RunPlanResult::new(plan, false, None, None); } println!("VM logs will be written to: {}", vm_log_dir); @@ -360,7 +368,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) @@ -384,7 +392,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); } }; @@ -398,7 +406,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); } }; @@ -410,14 +418,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) @@ -427,7 +435,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); } } @@ -442,13 +450,15 @@ fn run_plan( ); } 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); @@ -470,6 +480,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(); @@ -480,11 +492,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)) } }; @@ -682,10 +694,14 @@ 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!("Creating base disk took: {:#?}", start.elapsed()); + // Generate a random suffix for VM names let random_suffix = generate_random_suffix(); @@ -827,9 +843,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 { @@ -839,7 +859,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 bfe14e6f0e917e889065a9cee9f903171125edb7 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 bbc834c28..6c18e1f92 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() }); } } @@ -624,6 +641,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) { @@ -1112,6 +1137,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.) @@ -1277,6 +1304,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() @@ -1294,6 +1328,7 @@ fn generate_integration() -> Result<(String, String)> { test_command, try_bind_storage, skip_if_composefs, + skip_if_ostree, skip_if_uki, tmt: metadata.tmt, }); @@ -1417,6 +1452,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 43e3f2cbc..b7e1648a7 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 @@ -256,6 +258,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 @@ -271,6 +274,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 e5742e6546c9926af5f45704ef451d7990968618 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 6c18e1f92..8f16c2b29 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}; @@ -675,6 +706,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); // Determine base log directory: CLI flag > TMT_LOG_DIR env var > default. @@ -737,7 +775,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()) @@ -756,6 +794,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); @@ -815,8 +855,9 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { let vm_cpu = vm_cpu.to_string(); let base_log_dir = base_log_dir.clone(); - 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, @@ -830,36 +871,44 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { vm_mem, base_log_dir, bcvk_has_log_dir, - ) - }); + ); - 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 78bf20a17250965b811b1c47354f2fae7cde2364 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 d4a923558698301516e5f589f5c1693ca7557b18 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 8f16c2b29..a95c4de78 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; @@ -382,6 +382,7 @@ fn run_plan( vm_mem_mb: String, base_log_dir: Utf8PathBuf, bcvk_has_log_dir: bool, + libvirt_lock: Arc>, ) -> RunPlanResult { let sh = match Shell::new() { Ok(sh) => sh, @@ -407,6 +408,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...} {log_dir_args...} {image}" @@ -414,6 +424,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); @@ -796,6 +808,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); @@ -854,6 +868,7 @@ 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 base_log_dir = base_log_dir.clone(); + let libvirt_lock = libvirt_lock.clone(); let tx_clone = tx.clone(); std::thread::spawn(move || { @@ -871,6 +886,7 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { vm_mem, base_log_dir, bcvk_has_log_dir, + libvirt_lock, ); if let Err(e) = tx_clone.send(result) { From bdb74ec3680b2db328152c9054dbdadacf21c135 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 | 544 ++++++++++++++++++++++++---------------- 1 file changed, 334 insertions(+), 210 deletions(-) diff --git a/crates/xtask/src/tmt.rs b/crates/xtask/src/tmt.rs index a95c4de78..6f01f3cdc 100644 --- a/crates/xtask/src/tmt.rs +++ b/crates/xtask/src/tmt.rs @@ -352,6 +352,25 @@ 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], + user_env: &'a [String], +} + impl RunPlanResult { fn new( plan_name: String, @@ -369,17 +388,17 @@ 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, base_log_dir: Utf8PathBuf, bcvk_has_log_dir: bool, libvirt_lock: Arc>, @@ -388,7 +407,7 @@ fn run_plan( 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 +416,7 @@ fn run_plan( let log_dir_args: Vec = if bcvk_has_log_dir { if let Err(e) = std::fs::create_dir_all(&vm_log_dir) { eprintln!("Creating VM log directory {}: {e:?}", vm_log_dir); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } println!("VM logs will be written to: {}", vm_log_dir); @@ -413,7 +432,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); } }; @@ -428,7 +447,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) @@ -452,7 +471,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); } }; @@ -466,7 +485,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); } }; @@ -478,14 +497,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) @@ -495,7 +514,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); } } @@ -510,7 +529,7 @@ fn run_plan( ); } cleanup_vm(); - return RunPlanResult::new(plan, false, None, None); + return RunPlanResult::new(plan.to_string(), false, None, None); } println!("SSH connectivity verified"); @@ -524,7 +543,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 @@ -552,11 +571,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()), + ) } }; @@ -587,6 +616,192 @@ fn run_plan( plan_result } +fn run_plans( + sh: &Shell, + args: &RunTmtArgs, + 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> { + // Determine base log directory: CLI flag > TMT_LOG_DIR env var > default. + // Filter out empty TMT_LOG_DIR (e.g. TMT_LOG_DIR="") to avoid creating + // log subdirectories in the current working directory. + let base_log_dir: Utf8PathBuf = if let Some(ref d) = args.log_dir { + d.clone() + } else if let Some(env_dir) = std::env::var("TMT_LOG_DIR").ok().filter(|s| !s.is_empty()) { + Utf8PathBuf::from(env_dir) + } else { + Utf8PathBuf::from("/var/tmp/tmt") + }; + + // Probe whether this bcvk supports --log-dir (added in bcvk 0.17). + // Older installs silently lack it; we skip the flag rather than hard-failing. + let bcvk_has_log_dir = cmd!(sh, "bcvk libvirt run --help") + .ignore_stderr() + .read() + .map(|help| help.contains("--log-dir")) + .unwrap_or(false); + + 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, env_vars) = { + 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 = Vec::new(); + + 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, env_vars) + }; + + let firmware_args = firmware_args.to_vec(); + let context = tmt_config.context.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 base_log_dir = base_log_dir.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, + base_log_dir, + bcvk_has_log_dir, + 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) +} + +fn print_error_reports(sh: &Shell, failed_tests: &Vec<&RunPlanResult>) { + println!("\n========================================"); + println!("Detailed Error Reports"); + println!("========================================\n"); + + 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 + let report_result = cmd!(sh, "tmt run -i {id} report -vvv") + .ignore_status() + .run(); + + match report_result { + Ok(_) => {} + Err(e) => { + eprintln!( + "Warning: Failed to generate detailed report for {}: {:#}", + plan, e + ); + } + } + } + + None => { + println!("Run ID not available - cannot generate detailed report"); + } + } + + println!("\n"); + } + + println!("========================================\n"); +} + /// 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")] @@ -727,25 +942,6 @@ pub(crate) fn run_tmt(sh: &Shell, args: &RunTmtArgs) -> Result<()> { println!("Found {} test plan(s): {:?}", plans.len(), plans); - // Determine base log directory: CLI flag > TMT_LOG_DIR env var > default. - // Filter out empty TMT_LOG_DIR (e.g. TMT_LOG_DIR="") to avoid creating - // log subdirectories in the current working directory. - let base_log_dir: Utf8PathBuf = if let Some(ref d) = args.log_dir { - d.clone() - } else if let Some(env_dir) = std::env::var("TMT_LOG_DIR").ok().filter(|s| !s.is_empty()) { - Utf8PathBuf::from(env_dir) - } else { - Utf8PathBuf::from("/var/tmp/tmt") - }; - - // Probe whether this bcvk supports --log-dir (added in bcvk 0.17). - // Older installs silently lack it; we skip the flag rather than hard-failing. - let bcvk_has_log_dir = cmd!(sh, "bcvk libvirt run --help") - .ignore_stderr() - .read() - .map(|help| help.contains("--log-dir")) - .unwrap_or(false); - let mut install_opts = Vec::new(); // Add --filesystem=xfs by default on fedora-coreos @@ -780,15 +976,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); @@ -796,138 +983,45 @@ 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 base_log_dir = base_log_dir.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, - tmt_env_vars, - env, - preserve_vm, - vm_cpu, - vm_mem, - base_log_dir, - bcvk_has_log_dir, - 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, + 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( + sh, + args, + plans, + &plan_metadata, + &bcvk_opts, + &firmware_args, + vm_config, + test_context, + &tmt_config, + &libvirt_lock, + )?; // Print summary println!("\n========================================"); @@ -943,12 +1037,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, @@ -964,50 +1053,85 @@ 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!("----------------------------------------"); + print_error_reports(sh, &failed_tests); - if let Some(id) = run_id { - println!("Run ID: {}\n", id); + // Rerun failed plans + println!("Rerunning failed plans...\n"); - // Run tmt with the specific run ID and generate verbose report - let report_result = cmd!(sh, "tmt run -i {id} report -vvv") - .ignore_status() - .run(); + let failed_plan_names: Vec<&str> = failed_tests + .iter() + .map(|result| result.plan_name.as_str()) + .collect(); - match report_result { - Ok(_) => {} - Err(e) => { - eprintln!( - "Warning: Failed to generate detailed report for {}: {:#}", - plan, e - ); - } - } - } else { - println!("Run ID not available - cannot generate detailed report"); - } + 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(), + }; - println!("\n"); - } + let vm_config_rerun = VmConfig { + cpu: vm_cpu.to_string(), + mem: vm_mem.to_string(), + parallel_count: parallel_vms, + preserve: preserve_vm, + }; - println!("========================================\n"); + let rerun_results = run_plans( + sh, + args, + 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 !rerun_all_passed { + let failed_reran_tests: Vec<_> = rerun_results + .iter() + .filter(|plan_res| !plan_res.passed) + .collect(); - if !all_passed { - anyhow::bail!("Some test plans failed"); + print_error_reports(sh, &failed_reran_tests); + anyhow::bail!("Some test plans still failed after rerun"); } Ok(())