diff --git a/crates/integration-tests/src/tests/to_disk.rs b/crates/integration-tests/src/tests/to_disk.rs index ad73057..54dbb3f 100644 --- a/crates/integration-tests/src/tests/to_disk.rs +++ b/crates/integration-tests/src/tests/to_disk.rs @@ -214,3 +214,89 @@ fn test_to_disk_caching() -> Result<()> { Ok(()) } integration_test!(test_to_disk_caching); + +/// Test that different image references with the same digest create separate cached disks +fn test_to_disk_different_imgref_same_digest() -> Result<()> { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + + // First, pull the test image + let test_image = get_test_image(); + let output = Command::new("podman") + .args(["pull", &test_image]) + .output() + .expect("Failed to run podman pull"); + assert!( + output.status.success(), + "Failed to pull test image: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Create a second tag pointing to the same digest + let second_tag = format!("{}-alias", test_image); + let output = Command::new("podman") + .args(["tag", &test_image, &second_tag]) + .output() + .expect("Failed to run podman tag"); + assert!( + output.status.success(), + "Failed to tag image: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Create first disk with original image reference + let disk_path = Utf8PathBuf::try_from(temp_dir.path().join("test-disk.img")) + .expect("temp path is not UTF-8"); + + let output1 = run_bcvk(&[ + "to-disk", + "--label", + INTEGRATION_TEST_LABEL, + &test_image, + disk_path.as_str(), + ])?; + + assert!( + output1.success(), + "First to-disk run failed with exit code: {:?}. stdout: {}, stderr: {}", + output1.exit_code(), + output1.stdout, + output1.stderr + ); + + let metadata1 = + std::fs::metadata(&disk_path).expect("Failed to get disk metadata after first run"); + assert!(metadata1.len() > 0, "Disk image is empty"); + + // Use --dry-run with the aliased image reference (same digest, different imgref) + // to verify it would regenerate instead of reusing the cache + let output2 = run_bcvk(&[ + "to-disk", + "--dry-run", + "--label", + INTEGRATION_TEST_LABEL, + &second_tag, + disk_path.as_str(), + ])?; + + assert!( + output2.success(), + "Dry-run with different imgref failed with exit code: {:?}. stdout: {}, stderr: {}", + output2.exit_code(), + output2.stdout, + output2.stderr + ); + + // The dry-run should report it would regenerate because the source imgref is different + assert!( + output2.stdout.contains("would-regenerate"), + "Dry-run should report 'would-regenerate' for different imgref. stdout: {}, stderr: {}", + output2.stdout, + output2.stderr + ); + + // Clean up: remove the aliased tag + let _ = Command::new("podman").args(["rmi", &second_tag]).output(); + + Ok(()) +} +integration_test!(test_to_disk_different_imgref_same_digest); diff --git a/crates/kit/src/cache_metadata.rs b/crates/kit/src/cache_metadata.rs index 61e297b..45e8841 100644 --- a/crates/kit/src/cache_metadata.rs +++ b/crates/kit/src/cache_metadata.rs @@ -30,6 +30,10 @@ struct CacheInputs { /// SHA256 digest of the source container image image_digest: String, + /// Source image reference (e.g., "quay.io/centos-bootc/centos-bootc:stream9") + /// This is crucial because it determines the upgrade source for the installed system + source_imgref: String, + /// Filesystem type used for installation (e.g., "ext4", "xfs", "btrfs") filesystem: Option, @@ -52,6 +56,10 @@ pub struct DiskImageMetadata { /// SHA256 digest of the source container image pub digest: String, + /// Source image reference (e.g., "quay.io/centos-bootc/centos-bootc:stream9") + /// This is crucial because it determines the upgrade source for the installed system + pub source_imgref: String, + /// Filesystem type used for installation (e.g., "ext4", "xfs", "btrfs") pub filesystem: Option, @@ -73,6 +81,7 @@ impl DiskImageMetadata { pub fn compute_cache_hash(&self) -> String { let inputs = CacheInputs { image_digest: self.digest.clone(), + source_imgref: self.source_imgref.clone(), filesystem: self.filesystem.clone(), root_size: self.root_size.clone(), composefs_backend: self.composefs_backend, @@ -152,11 +161,12 @@ impl DiskImageMetadata { } impl DiskImageMetadata { - /// Create new metadata from InstallOptions and image digest - pub fn from(options: &InstallOptions, image: &str) -> Self { + /// Create new metadata from InstallOptions, image digest, and source imgref + pub fn from(options: &InstallOptions, image_digest: &str, source_imgref: &str) -> Self { Self { version: 1, - digest: image.to_owned(), + digest: image_digest.to_owned(), + source_imgref: source_imgref.to_owned(), filesystem: options.filesystem.clone(), root_size: options.root_size.clone(), kernel_args: options.karg.clone(), @@ -179,6 +189,7 @@ pub(crate) enum ValidationError { pub fn check_cached_disk( path: &Path, image_digest: &str, + source_imgref: &str, install_options: &InstallOptions, ) -> Result> { if !path.exists() { @@ -187,7 +198,7 @@ pub fn check_cached_disk( } // Create metadata for the current request to compute expected hash - let expected_meta = DiskImageMetadata::from(install_options, image_digest); + let expected_meta = DiskImageMetadata::from(install_options, image_digest, source_imgref); let expected_hash = expected_meta.compute_cache_hash(); // Read the cache hash from the disk image @@ -242,14 +253,16 @@ mod tests { root_size: Some("20G".to_string()), ..Default::default() }; - let metadata1 = DiskImageMetadata::from(&install_options1, "sha256:abc123"); + let metadata1 = + DiskImageMetadata::from(&install_options1, "sha256:abc123", "quay.io/test/image:v1"); let install_options2 = InstallOptions { filesystem: Some("ext4".to_string()), root_size: Some("20G".to_string()), ..Default::default() }; - let metadata2 = DiskImageMetadata::from(&install_options2, "sha256:abc123"); + let metadata2 = + DiskImageMetadata::from(&install_options2, "sha256:abc123", "quay.io/test/image:v1"); // Same inputs should generate same hash assert_eq!( @@ -257,13 +270,14 @@ mod tests { metadata2.compute_cache_hash() ); - // Different inputs should generate different hashes + // Different digest should generate different hashes let install_options3 = InstallOptions { filesystem: Some("ext4".to_string()), root_size: Some("20G".to_string()), ..Default::default() }; - let metadata3 = DiskImageMetadata::from(&install_options3, "sha256:xyz789"); + let metadata3 = + DiskImageMetadata::from(&install_options3, "sha256:xyz789", "quay.io/test/image:v1"); assert_ne!( metadata1.compute_cache_hash(), @@ -276,18 +290,38 @@ mod tests { root_size: Some("20G".to_string()), ..Default::default() }; - let metadata4 = DiskImageMetadata::from(&install_options4, "sha256:abc123"); + let metadata4 = + DiskImageMetadata::from(&install_options4, "sha256:abc123", "quay.io/test/image:v1"); assert_ne!( metadata1.compute_cache_hash(), metadata4.compute_cache_hash() ); + + // Different source imgref should generate different hash even with same digest + let install_options5 = InstallOptions { + filesystem: Some("ext4".to_string()), + root_size: Some("20G".to_string()), + ..Default::default() + }; + let metadata5 = DiskImageMetadata::from( + &install_options5, + "sha256:abc123", + "quay.io/different/image:latest", + ); + + assert_ne!( + metadata1.compute_cache_hash(), + metadata5.compute_cache_hash(), + "Different source imgrefs with same digest should generate different cache hashes" + ); } #[test] fn test_cache_inputs_serialization() -> Result<()> { let inputs = CacheInputs { image_digest: "sha256:abc123".to_string(), + source_imgref: "quay.io/test/image:v1".to_string(), filesystem: Some("ext4".to_string()), root_size: Some("20G".to_string()), kernel_args: vec!["console=ttyS0".to_string()], @@ -299,6 +333,7 @@ mod tests { let deserialized: CacheInputs = serde_json::from_str(&json)?; assert_eq!(inputs.image_digest, deserialized.image_digest); + assert_eq!(inputs.source_imgref, deserialized.source_imgref); assert_eq!(inputs.filesystem, deserialized.filesystem); assert_eq!(inputs.root_size, deserialized.root_size); assert_eq!(inputs.kernel_args, deserialized.kernel_args); diff --git a/crates/kit/src/libvirt/base_disks.rs b/crates/kit/src/libvirt/base_disks.rs index 73e7f01..6aafd7d 100644 --- a/crates/kit/src/libvirt/base_disks.rs +++ b/crates/kit/src/libvirt/base_disks.rs @@ -19,7 +19,7 @@ pub fn find_or_create_base_disk( install_options: &InstallOptions, connect_uri: Option<&str>, ) -> Result { - let metadata = DiskImageMetadata::from(install_options, image_digest); + let metadata = DiskImageMetadata::from(install_options, image_digest, source_image); let cache_hash = metadata.compute_cache_hash(); // Extract short hash for filename (first 16 chars after "sha256:") @@ -43,6 +43,7 @@ pub fn find_or_create_base_disk( if crate::cache_metadata::check_cached_disk( base_disk_path.as_std_path(), image_digest, + source_image, install_options, )? .is_ok() @@ -130,6 +131,7 @@ fn create_base_disk( let metadata_valid = crate::cache_metadata::check_cached_disk( temp_disk_path.as_std_path(), image_digest, + source_image, install_options, ) .context("Querying cached disk")?; diff --git a/crates/kit/src/to_disk.rs b/crates/kit/src/to_disk.rs index 3d85e67..4c1d577 100644 --- a/crates/kit/src/to_disk.rs +++ b/crates/kit/src/to_disk.rs @@ -138,6 +138,10 @@ pub struct ToDiskAdditionalOpts { help = "Add metadata to the container in key=value form" )] pub label: Vec, + + /// Check if the disk would be regenerated without actually creating it + #[clap(long)] + pub dry_run: bool, } /// Configuration options for installing a bootc container image to disk @@ -302,7 +306,7 @@ impl ToDiskOpts { /// for details on the installation workflow and architecture. pub fn run(opts: ToDiskOpts) -> Result<()> { // Phase 0: Check for existing cached disk image - if opts.target_disk.exists() { + let would_reuse = if opts.target_disk.exists() { debug!( "Target disk {} already exists, checking cache metadata", opts.target_disk @@ -316,9 +320,14 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { match crate::cache_metadata::check_cached_disk( opts.target_disk.as_std_path(), &image_digest, + &opts.source_image, &opts.install, )? { Ok(()) => { + if opts.additional.dry_run { + println!("would-reuse"); + return Ok(()); + } println!( "Reusing existing cached disk image (digest {image_digest}) at: {}", opts.target_disk @@ -327,12 +336,27 @@ pub fn run(opts: ToDiskOpts) -> Result<()> { } 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) - })?; + if !opts.additional.dry_run { + // 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) + })?; + } + false } } + } else { + false + }; + + // In dry-run mode, report whether we would regenerate + if opts.additional.dry_run { + if would_reuse { + println!("would-reuse"); + } else { + println!("would-regenerate"); + } + return Ok(()); } // Phase 1: Validation and preparation @@ -512,7 +536,7 @@ fn write_disk_metadata( let digest = inspect.digest.to_string(); // Prepare metadata using the new helper method - let metadata = DiskImageMetadata::from(install_options, &digest); + let metadata = DiskImageMetadata::from(install_options, &digest, source_image); // Write metadata using rustix fsetxattr let file = std::fs::OpenOptions::new() diff --git a/docs/src/man/bcvk-to-disk.md b/docs/src/man/bcvk-to-disk.md index aee5e6f..c082e83 100644 --- a/docs/src/man/bcvk-to-disk.md +++ b/docs/src/man/bcvk-to-disk.md @@ -111,6 +111,10 @@ The installation process: Add metadata to the container in key=value form +**--dry-run** + + Check if the disk would be regenerated without actually creating it + # ARGUMENTS