libvirt: Add support for --label

This parallels what we do with `ephemeral`, where it's super
natural because containers have labels.

With libvirt we already have custom stuff in the XML, so
add support for reading/writing labels there, and filtering
by them in `bcvk libvirt list --label=foo` etc.

This makes it easier to use bcvk in a per-project setting.

Signed-off-by: Colin Walters <[email protected]>

Author: cgwalters

crates/integration-tests/src/bin/cleanup.rs

@@ -1,7 +1,7 @@
 use std::process::Command;
 
-/// Label used to identify containers created by integration tests
-const INTEGRATION_TEST_LABEL: &str = "bcvk.integration-test=1";
+// Import shared constants from the library
+use integration_tests::{INTEGRATION_TEST_LABEL, LIBVIRT_INTEGRATION_TEST_LABEL};
 
 fn cleanup_integration_test_containers() -> Result<(), Box<dyn std::error::Error>> {
     println!("Cleaning up integration test containers...");
@@ -59,9 +59,68 @@ fn cleanup_integration_test_containers() -> Result<(), Box<dyn std::error::Error
     Ok(())
 }
 
+fn cleanup_libvirt_integration_test_vms() -> Result<(), Box<dyn std::error::Error>> {
+    println!("Cleaning up integration test libvirt VMs...");
+
+    // Get path to bcvk binary (should be in the same directory as this cleanup binary)
+    let current_exe = std::env::current_exe()?;
+    let bcvk_path = current_exe
+        .parent()
+        .ok_or("Failed to get parent directory")?
+        .join("bcvk");
+
+    if !bcvk_path.exists() {
+        println!(
+            "bcvk binary not found at {:?}, skipping libvirt cleanup",
+            bcvk_path
+        );
+        return Ok(());
+    }
+
+    // Use bcvk libvirt rm-all with label filter
+    let rm_output = Command::new(&bcvk_path)
+        .args([
+            "libvirt",
+            "rm-all",
+            "--label",
+            LIBVIRT_INTEGRATION_TEST_LABEL,
+            "--force",
+            "--stop",
+        ])
+        .output()?;
+
+    if !rm_output.status.success() {
+        let stderr = String::from_utf8_lossy(&rm_output.stderr);
+        // Don't fail if no VMs found
+        if stderr.contains("No VMs found") {
+            println!("No integration test libvirt VMs found to clean up");
+            return Ok(());
+        }
+        eprintln!("Warning: Failed to clean up libvirt VMs: {}", stderr);
+        return Ok(());
+    }
+
+    let stdout = String::from_utf8_lossy(&rm_output.stdout);
+    println!("{}", stdout);
+
+    Ok(())
+}
+
 fn main() {
+    let mut errors = Vec::new();
+
     if let Err(e) = cleanup_integration_test_containers() {
-        eprintln!("Error during cleanup: {}", e);
+        eprintln!("Error during container cleanup: {}", e);
+        errors.push(format!("containers: {}", e));
+    }
+
+    if let Err(e) = cleanup_libvirt_integration_test_vms() {
+        eprintln!("Error during libvirt VM cleanup: {}", e);
+        errors.push(format!("libvirt: {}", e));
+    }
+
+    if !errors.is_empty() {
+        eprintln!("Cleanup completed with errors: {}", errors.join(", "));
         std::process::exit(1);
     }
 }

crates/integration-tests/src/lib.rs

@@ -0,0 +1,10 @@
+//! Shared library code for integration tests
+//!
+//! This module contains constants and utilities that are shared between
+//! the main test binary and helper binaries like cleanup.
+
+/// Label used to identify containers created by integration tests
+pub const INTEGRATION_TEST_LABEL: &str = "bcvk.integration-test=1";
+
+/// Label used to identify libvirt VMs created by integration tests
+pub const LIBVIRT_INTEGRATION_TEST_LABEL: &str = "bcvk-integration";

crates/integration-tests/src/main.rs

@@ -6,8 +6,8 @@ use libtest_mimic::{Arguments, Trial};
 use serde_json::Value;
 use xshell::{cmd, Shell};
 
-/// Label used to identify containers created by integration tests
-pub(crate) const INTEGRATION_TEST_LABEL: &str = "bcvk.integration-test=1";
+// Re-export constants from lib for internal use
+pub(crate) use integration_tests::{INTEGRATION_TEST_LABEL, LIBVIRT_INTEGRATION_TEST_LABEL};
 
 mod tests {
     pub mod libvirt_base_disks;
@@ -205,6 +205,10 @@ fn main() {
             tests::libvirt_verb::test_libvirt_vm_lifecycle();
             Ok(())
         }),
+        Trial::test("libvirt_label_functionality", || {
+            tests::libvirt_verb::test_libvirt_label_functionality();
+            Ok(())
+        }),
         Trial::test("libvirt_error_handling", || {
             tests::libvirt_verb::test_libvirt_error_handling();
             Ok(())

crates/integration-tests/src/tests/libvirt_verb.rs

@@ -9,7 +9,7 @@
 
 use std::process::Command;
 
-use crate::{get_bck_command, get_test_image};
+use crate::{get_bck_command, get_test_image, LIBVIRT_INTEGRATION_TEST_LABEL};
 
 /// Test libvirt list functionality (lists domains)
 pub fn test_libvirt_list_functionality() {
@@ -224,6 +224,8 @@ pub fn test_libvirt_run_ssh_full_workflow() {
             "run",
             "--name",
             &domain_name,
+            "--label",
+            LIBVIRT_INTEGRATION_TEST_LABEL,
             "--filesystem",
             "ext4",
             &test_image,
@@ -416,6 +418,8 @@ pub fn test_libvirt_vm_lifecycle() {
             "ext4",
             "--name",
             &domain_name,
+            "--label",
+            INTEGRATION_TEST_LABEL,
             test_image,
         ])
         .output()
@@ -526,6 +530,8 @@ pub fn test_libvirt_bind_storage_ro() {
             "run",
             "--name",
             &domain_name,
+            "--label",
+            INTEGRATION_TEST_LABEL,
             "--bind-storage-ro",
             "--filesystem",
             "ext4",
@@ -699,6 +705,144 @@ pub fn test_libvirt_bind_storage_ro() {
     println!("✓ --bind-storage-ro end-to-end test passed");
 }
 
+/// Test libvirt label functionality
+pub fn test_libvirt_label_functionality() {
+    let bck = get_bck_command().unwrap();
+    let test_image = get_test_image();
+
+    // Generate unique domain name for this test
+    let domain_name = format!(
+        "test-label-{}",
+        std::time::SystemTime::now()
+            .duration_since(std::time::UNIX_EPOCH)
+            .unwrap()
+            .as_secs()
+    );
+
+    println!(
+        "Testing libvirt label functionality with domain: {}",
+        domain_name
+    );
+
+    // Cleanup any existing domain with this name
+    let _ = Command::new("virsh")
+        .args(&["destroy", &domain_name])
+        .output();
+    let _ = Command::new(&bck)
+        .args(&["libvirt", "rm", &domain_name, "--force", "--stop"])
+        .output();
+
+    // Create domain with multiple labels
+    println!("Creating libvirt domain with multiple labels...");
+    let create_output = Command::new("timeout")
+        .args([
+            "300s",
+            &bck,
+            "libvirt",
+            "run",
+            "--name",
+            &domain_name,
+            "--label",
+            INTEGRATION_TEST_LABEL,
+            "--label",
+            "test-env",
+            "--label",
+            "temporary",
+            "--filesystem",
+            "ext4",
+            &test_image,
+        ])
+        .output()
+        .expect("Failed to run libvirt run with labels");
+
+    let create_stdout = String::from_utf8_lossy(&create_output.stdout);
+    let create_stderr = String::from_utf8_lossy(&create_output.stderr);
+
+    println!("Create stdout: {}", create_stdout);
+    println!("Create stderr: {}", create_stderr);
+
+    if !create_output.status.success() {
+        cleanup_domain(&domain_name);
+        panic!("Failed to create domain with labels: {}", create_stderr);
+    }
+
+    println!("Successfully created domain with labels: {}", domain_name);
+
+    // Verify labels are stored in domain XML
+    println!("Checking domain XML for labels...");
+    let dumpxml_output = Command::new("virsh")
+        .args(&["dumpxml", &domain_name])
+        .output()
+        .expect("Failed to dump domain XML");
+
+    let domain_xml = String::from_utf8_lossy(&dumpxml_output.stdout);
+
+    // Check that labels are in the XML
+    assert!(
+        domain_xml.contains("bootc:label") || domain_xml.contains("<label>"),
+        "Domain XML should contain label metadata"
+    );
+    assert!(
+        domain_xml.contains(LIBVIRT_INTEGRATION_TEST_LABEL),
+        "Domain XML should contain bcvk-integration label"
+    );
+
+    // Test filtering by label
+    println!("Testing label filtering with libvirt list...");
+    let list_output = Command::new(&bck)
+        .args([
+            "libvirt",
+            "list",
+            "--label",
+            LIBVIRT_INTEGRATION_TEST_LABEL,
+            "-a",
+        ])
+        .output()
+        .expect("Failed to run libvirt list with label filter");
+
+    let list_stdout = String::from_utf8_lossy(&list_output.stdout);
+    println!("List output: {}", list_stdout);
+
+    assert!(
+        list_output.status.success(),
+        "libvirt list with label filter should succeed"
+    );
+    assert!(
+        list_stdout.contains(&domain_name),
+        "Domain should appear in filtered list. Output: {}",
+        list_stdout
+    );
+
+    // Test filtering by a label that should match
+    let list_test_env = Command::new(&bck)
+        .args(["libvirt", "list", "--label", "test-env", "-a"])
+        .output()
+        .expect("Failed to run libvirt list with test-env label");
+
+    let list_test_env_stdout = String::from_utf8_lossy(&list_test_env.stdout);
+    assert!(
+        list_test_env_stdout.contains(&domain_name),
+        "Domain should appear when filtering by test-env label"
+    );
+
+    // Test filtering by a label that should NOT match
+    let list_nomatch = Command::new(&bck)
+        .args(["libvirt", "list", "--label", "nonexistent-label", "-a"])
+        .output()
+        .expect("Failed to run libvirt list with nonexistent label");
+
+    let list_nomatch_stdout = String::from_utf8_lossy(&list_nomatch.stdout);
+    assert!(
+        !list_nomatch_stdout.contains(&domain_name),
+        "Domain should NOT appear when filtering by nonexistent label"
+    );
+
+    // Cleanup domain
+    cleanup_domain(&domain_name);
+
+    println!("✓ Label functionality test passed");
+}
+
 /// Test error handling for invalid configurations
 pub fn test_libvirt_error_handling() {
     let bck = get_bck_command().unwrap();

crates/kit/src/domain_list.rs

@@ -22,10 +22,12 @@ pub struct PodmanBootcDomain {
     pub created: Option<SystemTime>,
     /// Memory allocation in MB
     pub memory_mb: Option<u32>,
-    /// Number of virtual CPUs  
+    /// Number of virtual CPUs
     pub vcpus: Option<u32>,
     /// Disk path
     pub disk_path: Option<String>,
+    /// User-defined labels for organizing domains
+    pub labels: Vec<String>,
 }
 
 impl PodmanBootcDomain {
@@ -176,6 +178,19 @@ impl DomainLister {
             .or_else(|| dom.find("created"))
             .map(|node| node.text_content().to_string());
 
+        // Extract labels (comma-separated)
+        let labels = dom
+            .find("bootc:label")
+            .or_else(|| dom.find("label"))
+            .map(|node| {
+                node.text_content()
+                    .split(',')
+                    .map(|s| s.trim().to_string())
+                    .filter(|s| !s.is_empty())
+                    .collect::<Vec<_>>()
+            })
+            .unwrap_or_default();
+
         // Extract memory and vcpu from domain XML
         let memory_mb = dom.find("memory").and_then(|node| {
             // Memory might have unit attribute, but we'll try to parse the value
@@ -195,6 +210,7 @@ impl DomainLister {
             memory_mb,
             vcpus,
             disk_path,
+            labels,
         }))
     }
 
@@ -223,6 +239,10 @@ impl DomainLister {
             memory_mb: metadata.as_ref().and_then(|m| m.memory_mb),
             vcpus: metadata.as_ref().and_then(|m| m.vcpus),
             disk_path: metadata.as_ref().and_then(|m| m.disk_path.clone()),
+            labels: metadata
+                .as_ref()
+                .map(|m| m.labels.clone())
+                .unwrap_or_default(),
         })
     }
 
@@ -284,6 +304,7 @@ struct PodmanBootcDomainMetadata {
     memory_mb: Option<u32>,
     vcpus: Option<u32>,
     disk_path: Option<String>,
+    labels: Vec<String>,
 }
 
 /// Extract disk path from domain XML using DOM parser
@@ -384,6 +405,7 @@ mod tests {
             memory_mb: None,
             vcpus: None,
             disk_path: None,
+            labels: vec![],
         };
 
         assert!(domain.is_running());
@@ -398,6 +420,7 @@ mod tests {
             memory_mb: None,
             vcpus: None,
             disk_path: None,
+            labels: vec![],
         };
 
         assert!(!stopped_domain.is_running());