Clean up disk caching query, add more info

This should make it easier to debug why the GHA runs are failing.

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

Author: cgwalters

crates/kit/src/cache_metadata.rs

@@ -165,16 +165,26 @@ impl DiskImageMetadata {
     }
 }
 
+#[derive(Debug, thiserror::Error)]
+pub(crate) enum ValidationError {
+    #[error("file is missing")]
+    MissingFile,
+    #[error("Missing extended attribute metadata")]
+    MissingXattr,
+    #[error("Hash mismatch")]
+    HashMismatch,
+}
+
 /// Check if a cached disk image can be reused by comparing cache hashes
 pub fn check_cached_disk(
     path: &Path,
     image_digest: &str,
     install_options: &InstallOptions,
     kernel_args: &[String],
-) -> Result<bool> {
+) -> Result<Result<(), ValidationError>> {
     if !path.exists() {
         tracing::debug!("Disk image {:?} does not exist", path);
-        return Ok(false);
+        return Ok(Err(ValidationError::MissingFile));
     }
 
     // Create metadata for the current request to compute expected hash
@@ -198,7 +208,7 @@ pub fn check_cached_disk(
             .to_string(),
         None => {
             tracing::debug!("No cache hash xattr found on {:?}", path);
-            return Ok(false);
+            return Ok(Err(ValidationError::MissingXattr));
         }
     };
 
@@ -209,6 +219,7 @@ pub fn check_cached_disk(
             path,
             expected_hash
         );
+        Ok(Ok(()))
     } else {
         tracing::debug!(
             "Cached disk at {:?} does not match requirements. \
@@ -217,9 +228,8 @@ pub fn check_cached_disk(
             expected_hash,
             cached_hash
         );
+        Ok(Err(ValidationError::HashMismatch))
     }
-
-    Ok(matches)
 }
 
 #[cfg(test)]

crates/kit/src/libvirt/base_disks.rs

@@ -7,7 +7,8 @@
 use crate::cache_metadata::DiskImageMetadata;
 use crate::install_options::InstallOptions;
 use camino::{Utf8Path, Utf8PathBuf};
-use color_eyre::{eyre::Context, Result};
+use color_eyre::eyre::{eyre, Context};
+use color_eyre::Result;
 use std::fs;
 use tracing::{debug, info};
 
@@ -45,7 +46,9 @@ pub fn find_or_create_base_disk(
             image_digest,
             install_options,
             kernel_args,
-        )? {
+        )?
+        .is_ok()
+        {
             info!("Found cached base disk: {:?}", base_disk_path);
             return Ok(base_disk_path);
         } else {
@@ -130,10 +133,11 @@ fn create_base_disk(
         image_digest,
         install_options,
         kernel_args,
-    );
+    )
+    .context("Querying cached disk")?;
 
     match metadata_valid {
-        Ok(true) => {
+        Ok(()) => {
             // All validations passed - move to final location
             if let Err(e) = fs::rename(&temp_disk_path, base_disk_path) {
                 cleanup_temp_disk();
@@ -166,15 +170,9 @@ fn create_base_disk(
             );
             Ok(())
         }
-        Ok(false) => {
-            cleanup_temp_disk();
-            Err(color_eyre::eyre::eyre!(
-                "Base disk created but metadata verification failed"
-            ))
-        }
         Err(e) => {
             cleanup_temp_disk();
-            Err(e).with_context(|| "Failed to check disk metadata")
+            Err(eyre!("Generated disk metadata validation failed: {e}"))
         }
     }
 }

crates/kit/src/to_disk.rs

@@ -293,24 +293,26 @@ pub fn run(opts: ToDiskOpts) -> Result<()> {
         let image_digest = inspect.digest.to_string();
 
         // Check if cached disk matches our requirements
-        let matches = crate::cache_metadata::check_cached_disk(
+        match crate::cache_metadata::check_cached_disk(
             opts.target_disk.as_std_path(),
             &image_digest,
             &opts.install,
             &opts.additional.common.kernel_args,
-        )?;
-
-        if matches {
-            println!(
-                "Reusing existing cached disk image (digest {image_digest}) at: {}",
-                opts.target_disk
-            );
-            return Ok(());
-        } else {
-            debug!("Existing disk does not match requirements, recreating");
-            // Remove the existing disk so we can recreate it
-            std::fs::remove_file(&opts.target_disk)
-                .with_context(|| format!("Failed to remove existing disk {}", opts.target_disk))?;
+        )? {
+            Ok(()) => {
+                println!(
+                    "Reusing existing cached disk image (digest {image_digest}) at: {}",
+                    opts.target_disk
+                );
+                return Ok(());
+            }
+            Err(e) => {
+                debug!("Existing disk does not match requirements, recreating: {e}");
+                // Remove the existing disk so we can recreate it
+                std::fs::remove_file(&opts.target_disk).with_context(|| {
+                    format!("Failed to remove existing disk {}", opts.target_disk)
+                })?;
+            }
         }
     }