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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/integration-tests/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mod tests {
pub mod libvirt_base_disks;
pub mod libvirt_ignition;
pub mod libvirt_port_forward;
pub mod libvirt_to_base_disk;
pub mod libvirt_upload_disk;
pub mod libvirt_verb;
pub mod mount_feature;
Expand Down
226 changes: 226 additions & 0 deletions crates/integration-tests/src/tests/libvirt_to_base_disk.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
//! Integration tests for libvirt to-base-disk functionality
//!
//! Tests the to-base-disk command which creates base disk images for libvirt VMs:
//! - Basic to-base-disk creation
//! - to-base-disk with different options
//! - to-base-disk reuse behavior
//! - Integration with libvirt base-disks list

use integration_tests::integration_test;
use itest::TestResult;
use xshell::cmd;

use regex::Regex;

use crate::{get_bck_command, get_test_image, shell};

/// Test basic to-base-disk command functionality
fn test_to_base_disk_basic() -> TestResult {
let sh = shell()?;
let bck = get_bck_command()?;
let test_image = get_test_image();

println!("Testing basic to-base-disk functionality");

// Run to-base-disk command
let stdout = cmd!(sh, "{bck} libvirt to-base-disk {test_image}").read()?;

println!("to-base-disk output: {}", stdout);

// Should indicate successful creation
assert!(
stdout.contains("Created base disk:") || stdout.contains("Using cached"),
"Should show creation or reuse of base disk, got: {}",
stdout
);

// Extract disk path from output
let disk_path_regex = Regex::new(r"Created base disk: (.+)").unwrap();
let disk_path = if let Some(captures) = disk_path_regex.captures(&stdout) {
captures.get(1).unwrap().as_str()
} else {
// If it's a cached disk, we need to check the list instead
println!("Base disk was cached, checking list...");
let list_output = cmd!(sh, "{bck} libvirt base-disks list").read()?;
assert!(
!list_output.contains("No base disk"),
"Should have at least one base disk after creation"
);
return Ok(());
};

println!("Created disk path: {}", disk_path);

// Verify the disk file exists
assert!(
std::path::Path::new(disk_path).exists(),
"Created disk file should exist at: {}",
disk_path
);

// Verify it shows up in base-disks list
let list_output = cmd!(sh, "{bck} libvirt base-disks list").read()?;
println!("base-disks list after creation:\n{}", list_output);

// Should not be empty and should contain our disk
assert!(
!list_output.contains("No base disk"),
"Should have base disks after creation"
);

println!("✓ Basic to-base-disk test passed");
Ok(())
}
integration_test!(test_to_base_disk_basic);

/// Test to-base-disk with filesystem option
fn test_to_base_disk_with_filesystem() -> TestResult {
let sh = shell()?;
let bck = get_bck_command()?;
let test_image = get_test_image();

println!("Testing to-base-disk with --filesystem option");

// Run to-base-disk command with ext4 filesystem
let stdout = cmd!(
sh,
"{bck} libvirt to-base-disk --filesystem ext4 {test_image}"
)
.read()?;

println!("to-base-disk --filesystem ext4 output: {}", stdout);

// Should indicate successful creation
assert!(
stdout.contains("Created base disk:") || stdout.contains("Using cached"),
"Should show creation or reuse of base disk with filesystem option, got: {}",
stdout
);

println!("✓ to-base-disk with filesystem option test passed");
Ok(())
}
integration_test!(test_to_base_disk_with_filesystem);

/// Test to-base-disk reuse behavior
fn test_to_base_disk_reuse() -> TestResult {
let sh = shell()?;
let bck = get_bck_command()?;
let test_image = get_test_image();

println!("Testing to-base-disk reuse behavior");

// First call - might create or reuse existing
let stdout1 = cmd!(sh, "{bck} libvirt to-base-disk {test_image}").read()?;
println!("First to-base-disk call: {}", stdout1);

// Second call with same image - should reuse
let stdout2 = cmd!(sh, "{bck} libvirt to-base-disk {test_image}").read()?;
println!("Second to-base-disk call: {}", stdout2);

// At least one should show base disk creation/usage
assert!(
stdout1.contains("Created base disk:")
|| stdout1.contains("Using cached")
|| stdout2.contains("Created base disk:")
|| stdout2.contains("Using cached"),
"Should show base disk creation or reuse"
);

println!("✓ to-base-disk reuse test passed");
Ok(())
}
integration_test!(test_to_base_disk_reuse);

/// Test to-base-disk with root-size option
fn test_to_base_disk_with_root_size() -> TestResult {
let sh = shell()?;
let bck = get_bck_command()?;
let test_image = get_test_image();

println!("Testing to-base-disk with --root-size option");

// Run to-base-disk command with custom root size
let stdout = cmd!(
sh,
"{bck} libvirt to-base-disk --root-size 15G {test_image}"
)
.read()?;

println!("to-base-disk --root-size 15G output: {}", stdout);

// Should indicate successful creation
assert!(
stdout.contains("Created base disk:") || stdout.contains("Using cached"),
"Should show creation or reuse of base disk with root-size option, got: {}",
stdout
);

println!("✓ to-base-disk with root-size option test passed");
Ok(())
}
integration_test!(test_to_base_disk_with_root_size);

/// Test that to-base-disk integrates properly with base-disks commands
fn test_to_base_disk_integration_with_list() -> TestResult {
let sh = shell()?;
let bck = get_bck_command()?;
let test_image = get_test_image();

println!("Testing to-base-disk integration with base-disks list");

// Get initial count
let initial_list = cmd!(sh, "{bck} libvirt base-disks list").read()?;
let initial_count = if initial_list.contains("No base disk") {
0
} else {
// Count lines with disk entries (skip header and summary)
initial_list
.lines()
.filter(|line| {
!line.contains("NAME") && !line.contains("Found") && !line.trim().is_empty()
})
.count()
};

println!("Initial base disk count: {}", initial_count);

// Create base disk
let stdout = cmd!(sh, "{bck} libvirt to-base-disk {test_image}").read()?;
println!("to-base-disk output: {}", stdout);

// Check final count
let final_list = cmd!(sh, "{bck} libvirt base-disks list").read()?;
println!("Final base-disks list:\n{}", final_list);

if stdout.contains("Created base disk:") {
// If we created a new disk, count should increase
let final_count = final_list
.lines()
.filter(|line| {
!line.contains("NAME") && !line.contains("Found") && !line.trim().is_empty()
})
.count();

assert!(
final_count > initial_count,
"Base disk count should increase after creation"
);
} else {
// If we reused existing, should still have disks listed
assert!(
!final_list.contains("No base disk"),
"Should still have base disks listed after reuse"
);
}

// Verify the list shows proper columns
assert!(
final_list.contains("NAME") && final_list.contains("SIZE"),
"List should show proper table headers"
);

println!("✓ to-base-disk integration with list test passed");
Ok(())
}
integration_test!(test_to_base_disk_integration_with_list);
36 changes: 33 additions & 3 deletions crates/kit/src/libvirt/base_disks_cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ use color_eyre::Result;
use comfy_table::{presets::UTF8_FULL, Table};
use serde_json;

use super::base_disks::{list_base_disks, prune_base_disks};
use super::base_disks::{find_or_create_base_disk, list_base_disks, prune_base_disks};
use super::OutputFormat;
use crate::images;
use crate::install_options::InstallOptions;

/// Options for base-disks command
#[derive(Debug, Parser)]
Expand All @@ -18,6 +20,14 @@ pub struct LibvirtBaseDisksOpts {
pub command: BaseDisksSubcommand,
}

/// Options for base-disks create command
#[derive(Debug, Parser)]
pub struct CreateBaseDiskOpts {
pub source_image: String,
#[clap(flatten)]
pub install_options: InstallOptions,
}
Comment thread
Johan-Liebert1 marked this conversation as resolved.

/// Base disk subcommands
#[derive(Debug, Subcommand)]
pub enum BaseDisksSubcommand {
Expand Down Expand Up @@ -53,6 +63,26 @@ pub fn run(global_opts: &crate::libvirt::LibvirtOptions, opts: LibvirtBaseDisksO
}
}

/// Execute the to-base-disk command (standalone)
pub fn run_create(
global_opts: &crate::libvirt::LibvirtOptions,
opts: CreateBaseDiskOpts,
) -> Result<()> {
let connect_uri = global_opts.connect.as_deref();
let inspect = images::inspect(&opts.source_image)?;
let image_digest = inspect.digest.to_string();

let path = find_or_create_base_disk(
&opts.source_image,
&image_digest,
&opts.install_options,
connect_uri,
)?;
println!("Created base disk: {path}");

Ok(())
}

/// Execute the list subcommand
fn run_list(connect_uri: Option<&str>, opts: ListOpts) -> Result<()> {
let base_disks = list_base_disks(connect_uri)?;
Expand Down Expand Up @@ -114,12 +144,12 @@ fn run_list(connect_uri: Option<&str>, opts: ListOpts) -> Result<()> {
OutputFormat::Yaml => {
return Err(color_eyre::eyre::eyre!(
"YAML format is not supported for base-disks list command"
))
));
}
OutputFormat::Xml => {
return Err(color_eyre::eyre::eyre!(
"XML format is not supported for base-disks list command"
))
));
}
}

Expand Down
4 changes: 4 additions & 0 deletions crates/kit/src/libvirt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,10 @@ pub enum LibvirtSubcommands {
#[clap(name = "base-disks")]
BaseDisks(base_disks_cli::LibvirtBaseDisksOpts),

/// Create a base disk image for libvirt VMs
#[clap(name = "to-base-disk")]
ToBaseDisk(base_disks_cli::CreateBaseDiskOpts),

/// Print detected firmware paths and configuration
#[clap(name = "print-firmware", hide = true)]
PrintFirmware(print_firmware::LibvirtPrintFirmwareOpts),
Expand Down
10 changes: 9 additions & 1 deletion crates/kit/src/libvirt/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,14 @@ fn ensure_default_pool(connect_uri: Option<&str>) -> Result<()> {
if !output.status.success() {
let stderr = String::from_utf8_lossy(&output.stderr);
let _ = std::fs::remove_file(xml_path);

// Check if the error is because the pool already exists
if stderr.contains("already exists") {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hmm, this is hacky. I guess it's hard to do better without actually using the libvirt API right now. We could try to create a global lockfile that serializes separate processes, but that's messy.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Or, we could also expose the pool creation API?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

bcvk libvirt init? Yeah, still needs idempotency though in general

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Going through the function, it feels like we can just turn it into a cli command.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

bcvk libvirt init? Yeah, still needs idempotency though in general

yeah, how about, before creating the pool, we check if it already exists? We wouldn't need an API in that case

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We should really switch to the libvirt API bindings (and same for podman really).

// Pool was created by race between our tests
info!("Default storage pool already exists, continuing");
return Ok(());
}

return Err(color_eyre::eyre::eyre!(
"Failed to define default pool: {}",
stderr
Expand Down Expand Up @@ -951,7 +959,7 @@ fn check_libvirt_readonly_support() -> Result<()> {
"Could not parse libvirt version. \
The --bind-storage-ro flag requires libvirt 11.0+ with rust-based virtiofsd support. \
Please ensure you have a compatible libvirt version installed."
))
)),
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions crates/kit/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,9 @@ fn main() -> Result<(), Report> {
libvirt::LibvirtSubcommands::BaseDisks(opts) => {
libvirt::base_disks_cli::run(&options, opts)?
}
libvirt::LibvirtSubcommands::ToBaseDisk(opts) => {
libvirt::base_disks_cli::run_create(&options, opts)?
}
libvirt::LibvirtSubcommands::PrintFirmware(opts) => {
libvirt::print_firmware::run(opts)?
}
Expand Down
8 changes: 8 additions & 0 deletions docs/src/man/bcvk-libvirt-run.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ Run a bootable container as a persistent VM

Disable TPM 2.0 support (enabled by default)

**--firmware-log**

Enable firmware debug log (captures OVMF/EDK2 DEBUG output via isa-debugcon)

**--secure-boot-keys**=*SECURE_BOOT_KEYS*

Directory containing secure boot keys (required for uefi-secure)
Expand All @@ -150,6 +154,10 @@ Run a bootable container as a persistent VM

Create a transient VM that disappears on shutdown/reboot

**--ignition**=*IGNITION_CONFIG*

Path to Ignition config file (JSON format) for first-boot provisioning

<!-- END GENERATED OPTIONS -->

# EXAMPLES
Expand Down
Loading
Loading