diff --git a/crates/integration-tests/src/tests/libvirt_ignition.rs b/crates/integration-tests/src/tests/libvirt_ignition.rs index e13a238e1..02d08dca6 100644 --- a/crates/integration-tests/src/tests/libvirt_ignition.rs +++ b/crates/integration-tests/src/tests/libvirt_ignition.rs @@ -25,11 +25,23 @@ fn random_suffix() -> String { /// Fedora CoreOS image that supports Ignition const FCOS_IMAGE: &str = "quay.io/fedora/fedora-coreos:stable"; -/// Test that Ignition config injection mechanism works for libvirt +/// Test that Ignition config injection mechanism works end-to-end for libvirt. /// -/// This test verifies that the Ignition config injection mechanism is working -/// by checking that the VM can be created with --ignition flag and that the -/// config file is properly stored. +/// DISABLED: FCOS installed via `bootc install` drops into emergency mode +/// because the partition layout (BIOS boot + EFI + root) lacks the separate +/// "boot"-labeled partition that FCOS's `coreos-boot-mount-generator` expects. +/// The 90-second device wait for `/dev/disk/by-label/boot` times out, SSHD +/// never starts, and the root account is locked. Additionally, Ignition +/// itself is skipped — `bootc install` marks the disk as already provisioned, +/// so FCOS treats every boot as "subsequent." +/// +/// The upstream fix (extending `coreos-boot-mount-generator` to tolerate +/// missing boot partitions for the `bootc install` case, analogous to the +/// virtiofs fix in ) +/// has not landed yet. Re-enable this test once it does. +/// +/// See also: +#[allow(dead_code)] fn test_libvirt_ignition_works() -> TestResult { let sh = shell()?; let bck = get_bck_command()?; @@ -86,7 +98,8 @@ fn test_libvirt_ignition_works() -> TestResult { println!("Ignition config injection test passed"); Ok(()) } -integration_test!(test_libvirt_ignition_works); +// DISABLED: see doc comment above for rationale and upstream links. +// integration_test!(test_libvirt_ignition_works); /// Test that Ignition config validation rejects nonexistent files fn test_libvirt_ignition_invalid_path() -> TestResult { diff --git a/crates/kit/src/libvirt/run.rs b/crates/kit/src/libvirt/run.rs index 2885827c2..bf37aa185 100644 --- a/crates/kit/src/libvirt/run.rs +++ b/crates/kit/src/libvirt/run.rs @@ -359,9 +359,10 @@ impl LibvirtRunOpts { } } -/// Wait for SSH to become available on a libvirt domain +/// Wait for SSH to become available on a libvirt domain. /// -/// Polls SSH connectivity by attempting simple commands until successful or timeout. +/// Uses the same `wait_for_readiness` polling loop as the ephemeral path +/// and `run_ssh_impl`, just with a longer timeout for initial VM boot. fn wait_for_ssh_ready( global_opts: &crate::libvirt::LibvirtOptions, domain_name: &str, @@ -374,39 +375,36 @@ fn wait_for_ssh_ready( domain_name, timeout_secs ); - // Create progress bar - let pb = crate::boot_progress::create_boot_progress_bar(); - pb.set_message("Waiting for SSH to become available..."); - - // Clone values for closure - let global_opts_clone = global_opts.clone(); - let domain_name_clone = domain_name.to_string(); + // Do expensive setup once: verify domain, extract SSH config, create temp key. + let ssh_opts = crate::libvirt::ssh::LibvirtSshOpts { + domain_name: domain_name.to_string(), + user: "root".to_string(), + command: vec![], + strict_host_keys: false, + timeout: 5, + log_level: "ERROR".to_string(), + extra_options: vec![], + suppress_output: true, + }; + ssh_opts.verify_domain_running(global_opts)?; + let ssh_config = ssh_opts.extract_ssh_config(global_opts)?; + let (temp_key, parsed_extra_options) = ssh_opts.prepare_ssh_session(&ssh_config)?; - // Use shared polling function with libvirt-specific test + let pb = crate::boot_progress::create_boot_progress_bar(); let (_elapsed, pb) = crate::utils::wait_for_readiness( pb, "Waiting for SSH", || { - // Create a test SSH connection with short timeout - let ssh_opts = crate::libvirt::ssh::LibvirtSshOpts { - domain_name: domain_name_clone.clone(), - user: "root".to_string(), - command: vec!["true".to_string()], // Simple command to test connectivity - strict_host_keys: false, - timeout: 5, // Short timeout for each attempt - log_level: "ERROR".to_string(), - extra_options: vec![], - suppress_output: true, // Suppress error messages during connectivity testing - }; - - // Try to connect - match crate::libvirt::ssh::run_ssh_impl(&global_opts_clone, ssh_opts) { - Ok(_) => Ok(true), - Err(_) => Ok(false), + let mut test_cmd = + ssh_opts.build_ssh_command(&ssh_config, &temp_key, parsed_extra_options.clone()); + test_cmd.arg("--").arg("true"); + match test_cmd.output() { + Ok(output) if output.status.success() => Ok(true), + _ => Ok(false), } }, Duration::from_secs(timeout_secs), - Duration::from_secs(2), // Poll every 2 seconds + Duration::from_secs(2), )?; pb.finish_and_clear(); diff --git a/crates/kit/src/libvirt/ssh.rs b/crates/kit/src/libvirt/ssh.rs index 3d4261544..884e708f1 100644 --- a/crates/kit/src/libvirt/ssh.rs +++ b/crates/kit/src/libvirt/ssh.rs @@ -15,7 +15,7 @@ use std::io::Write; use std::os::unix::fs::PermissionsExt as _; use std::os::unix::process::CommandExt; use std::process::Command; -use std::time::{Duration, Instant}; +use std::time::Duration; use tempfile; use tracing::debug; @@ -60,7 +60,7 @@ pub struct LibvirtSshOpts { /// SSH configuration extracted from domain metadata #[derive(Debug)] -struct DomainSshConfig { +pub(crate) struct DomainSshConfig { private_key_content: String, ssh_port: u16, is_generated: bool, @@ -93,7 +93,7 @@ impl LibvirtSshOpts { } /// Extract SSH configuration from domain XML metadata - fn extract_ssh_config( + pub(crate) fn extract_ssh_config( &self, global_opts: &crate::libvirt::LibvirtOptions, ) -> Result { @@ -243,7 +243,7 @@ impl LibvirtSshOpts { } /// Build SSH command with configured options - fn build_ssh_command( + pub(crate) fn build_ssh_command( &self, ssh_config: &DomainSshConfig, temp_key: &tempfile::NamedTempFile, @@ -269,25 +269,34 @@ impl LibvirtSshOpts { ssh_cmd } - /// Execute SSH connection to domain with retries - fn connect_ssh( + /// Verify the domain exists and is running. + pub(crate) fn verify_domain_running( &self, - _global_opts: &crate::libvirt::LibvirtOptions, - ssh_config: &DomainSshConfig, + global_opts: &crate::libvirt::LibvirtOptions, ) -> Result<()> { - debug!( - "Connecting to domain '{}' via SSH on port {} (user: {})", - self.domain_name, ssh_config.ssh_port, self.user - ); - - if ssh_config.is_generated { - debug!("Using ephemeral SSH key from domain metadata"); + if !self.check_domain_exists(global_opts)? { + return Err(eyre!("Domain '{}' not found", self.domain_name)); } + let state = self.get_domain_state(global_opts)?; + if state != "running" { + return Err(eyre!( + "Domain '{}' is not running (current state: {}). Start it first with: virsh start {}", + self.domain_name, + state, + self.domain_name + )); + } + Ok(()) + } - // Create temporary SSH key file + /// Create temp key file and parse extra SSH options — shared setup for + /// both the retry path and single-attempt tests. + pub(crate) fn prepare_ssh_session( + &self, + ssh_config: &DomainSshConfig, + ) -> Result<(tempfile::NamedTempFile, Vec<(String, String)>)> { let temp_key = self.create_temp_ssh_key(ssh_config)?; - // Parse extra options let mut parsed_extra_options = Vec::new(); for option in &self.extra_options { if let Some((key, value)) = option.split_once('=') { @@ -299,65 +308,29 @@ impl LibvirtSshOpts { )); } } + Ok((temp_key, parsed_extra_options)) + } - let start_time = Instant::now(); - let timeout = Duration::from_secs(SSH_RETRY_TIMEOUT_SECS); - - // First, do connectivity check with retries (for both interactive and command) - debug!("Testing SSH connectivity before session"); - - // Create progress bar for user feedback (only shown in terminals) - let pb = crate::boot_progress::create_boot_progress_bar(); - pb.set_message("Waiting for SSH to be ready..."); - - loop { - let mut test_cmd = - self.build_ssh_command(ssh_config, &temp_key, parsed_extra_options.clone()); - test_cmd.arg("--").arg("true"); // Simple test command - - let output = test_cmd.output().context("Failed to spawn SSH command")?; - - if output.status.success() { - debug!( - "SSH connectivity confirmed after {:.1}s", - start_time.elapsed().as_secs_f64() - ); - pb.finish_and_clear(); - break; - } - - // Check if we've exceeded timeout - if start_time.elapsed() >= timeout { - pb.finish_and_clear(); - if !self.suppress_output { - let stderr_str = String::from_utf8_lossy(&output.stderr); - eprint!("{}", stderr_str); - eprintln!( - "\nSSH connection failed after {:.1}s. To see VM console output, run: virsh console {}", - start_time.elapsed().as_secs_f64(), - self.domain_name - ); - } - return Err(eyre!("SSH connection failed after timeout")); - } - - std::thread::sleep(Duration::from_secs(SSH_POLL_DELAY_SECS)); - } - - // SSH is ready - now do the actual operation (oneshot) + /// Execute the SSH session (interactive or command) after connectivity + /// has already been confirmed by the caller. + fn exec_ssh_session( + &self, + ssh_config: &DomainSshConfig, + temp_key: &tempfile::NamedTempFile, + parsed_extra_options: Vec<(String, String)>, + ) -> Result<()> { if self.command.is_empty() { - // Interactive: exec directly - debug!("SSH ready, launching interactive session"); - let mut ssh_cmd = self.build_ssh_command(ssh_config, &temp_key, parsed_extra_options); + // Interactive: exec directly (replaces current process) + debug!("Launching interactive SSH session"); + let mut ssh_cmd = self.build_ssh_command(ssh_config, temp_key, parsed_extra_options); let error = ssh_cmd.exec(); return Err(eyre!("Failed to exec SSH command: {}", error)); } - // Command execution: single attempt since we already confirmed connectivity - debug!("SSH ready, executing command"); - let mut ssh_cmd = self.build_ssh_command(ssh_config, &temp_key, parsed_extra_options); + // Command execution + debug!("Executing SSH command"); + let mut ssh_cmd = self.build_ssh_command(ssh_config, temp_key, parsed_extra_options); - // Add command ssh_cmd.arg("--"); if self.command.len() > 1 { let combined_command = crate::ssh::shell_escape_command(&self.command) @@ -367,7 +340,6 @@ impl LibvirtSshOpts { ssh_cmd.args(&self.command); } - // Execute command let output = ssh_cmd .output() .map_err(|e| eyre!("Failed to execute SSH command: {}", e))?; @@ -376,14 +348,9 @@ impl LibvirtSshOpts { if !output.stdout.is_empty() && !self.suppress_output { print!("{}", String::from_utf8_lossy(&output.stdout)); } - debug!( - "Command completed successfully after {:.1}s total", - start_time.elapsed().as_secs_f64() - ); return Ok(()); } - // Command failed if !self.suppress_output { let stderr_str = String::from_utf8_lossy(&output.stderr); eprint!("{}", stderr_str); @@ -400,36 +367,63 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtSshOpts) - run_ssh_impl(global_opts, opts) } -/// SSH implementation +/// SSH implementation — waits for connectivity then runs the session. pub fn run_ssh_impl( global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtSshOpts, ) -> Result<()> { debug!("Connecting to libvirt domain: {}", opts.domain_name); - // Check if domain exists - if !opts.check_domain_exists(global_opts)? { - return Err(eyre!("Domain '{}' not found", opts.domain_name)); - } + opts.verify_domain_running(global_opts)?; + + let ssh_config = opts.extract_ssh_config(global_opts)?; - // Check if domain is running - let state = opts.get_domain_state(global_opts)?; - if state != "running" { - return Err(eyre!( - "Domain '{}' is not running (current state: {}). Start it first with: virsh start {}", - opts.domain_name, - state, - opts.domain_name - )); + if ssh_config.is_generated { + debug!("Using ephemeral SSH key from domain metadata"); } - // Extract SSH configuration from domain metadata - let ssh_config = opts.extract_ssh_config(global_opts)?; + let (temp_key, parsed_extra_options) = opts.prepare_ssh_session(&ssh_config)?; - // Connect via SSH with retries - opts.connect_ssh(global_opts, &ssh_config)?; + // Wait for SSH connectivity using the shared polling loop — same + // pattern as the ephemeral path in run_ephemeral_ssh::wait_for_ssh_ready. + let mut last_stderr = String::new(); + let pb = crate::boot_progress::create_boot_progress_bar(); + let (_elapsed, pb) = crate::utils::wait_for_readiness( + pb, + "Waiting for SSH", + || { + let mut test_cmd = + opts.build_ssh_command(&ssh_config, &temp_key, parsed_extra_options.clone()); + test_cmd.arg("--").arg("true"); + + match test_cmd.output() { + Ok(output) if output.status.success() => Ok(true), + Ok(output) => { + last_stderr = String::from_utf8_lossy(&output.stderr).into_owned(); + Ok(false) + } + Err(_) => Ok(false), + } + }, + Duration::from_secs(SSH_RETRY_TIMEOUT_SECS), + Duration::from_secs(SSH_POLL_DELAY_SECS), + ) + .map_err(|_| { + if !opts.suppress_output { + if !last_stderr.is_empty() { + eprint!("{}", last_stderr); + } + eprintln!( + "\nSSH connection failed. To see VM console output, run: virsh console {}", + opts.domain_name + ); + } + eyre!("SSH connection failed after timeout") + })?; + pb.finish_and_clear(); - Ok(()) + // Connectivity confirmed — run the actual session + opts.exec_ssh_session(&ssh_config, &temp_key, parsed_extra_options) } #[cfg(test)]