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
86 changes: 86 additions & 0 deletions crates/integration-tests/src/tests/to_disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
53 changes: 44 additions & 9 deletions crates/kit/src/cache_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>,

Expand All @@ -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<String>,

Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand All @@ -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<Result<(), ValidationError>> {
if !path.exists() {
Expand All @@ -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
Expand Down Expand Up @@ -242,28 +253,31 @@ 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!(
metadata1.compute_cache_hash(),
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(),
Expand All @@ -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()],
Expand All @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion crates/kit/src/libvirt/base_disks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ pub fn find_or_create_base_disk(
install_options: &InstallOptions,
connect_uri: Option<&str>,
) -> Result<Utf8PathBuf> {
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:")
Expand All @@ -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()
Expand Down Expand Up @@ -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")?;
Expand Down
36 changes: 30 additions & 6 deletions crates/kit/src/to_disk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ pub struct ToDiskAdditionalOpts {
help = "Add metadata to the container in key=value form"
)]
pub label: Vec<String>,

/// 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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 4 additions & 0 deletions docs/src/man/bcvk-to-disk.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

<!-- END GENERATED OPTIONS -->

# ARGUMENTS
Expand Down
Loading