From 3aea4c9f1beb164d2f8dacb02679f29ea2e22cab Mon Sep 17 00:00:00 2001 From: Piotr Mlocek Date: Wed, 22 Jul 2026 10:46:03 -0700 Subject: [PATCH] fix(vm): reduce registry rootfs staging pressure Remove temporary registry layer data before formatting the merged rootfs, preserve formatter execution failures over missing fallbacks, and validate required ext4 tools in rust-vm CI. Closes #2423 Signed-off-by: Piotr Mlocek --- .github/workflows/e2e-test.yml | 6 ++ crates/openshell-driver-vm/src/driver.rs | 35 +++++++ crates/openshell-driver-vm/src/rootfs.rs | 120 +++++++++++++++++++---- 3 files changed, 143 insertions(+), 18 deletions(-) diff --git a/.github/workflows/e2e-test.yml b/.github/workflows/e2e-test.yml index eae744f252..d8f33f7016 100644 --- a/.github/workflows/e2e-test.yml +++ b/.github/workflows/e2e-test.yml @@ -310,6 +310,12 @@ jobs: socat \ zstd + - name: Validate VM host tools + run: | + command -v mke2fs + command -v mkfs.ext4 + command -v debugfs + - name: Install mise run: | curl https://mise.run | MISE_VERSION=v2026.4.25 sh diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 6c203f7a9d..7af0ddc389 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -3538,6 +3538,8 @@ impl VmDriver { apply_registry_layer_blob(image_ref, rootfs, layer).await?; } + remove_registry_layer_staging(staging_dir).await?; + Ok(()) } @@ -3688,6 +3690,16 @@ async fn apply_registry_layer_blob( }) } +async fn remove_registry_layer_staging(staging_dir: &Path) -> Result<(), Status> { + let layers_dir = staging_dir.join("layers"); + tokio::fs::remove_dir_all(&layers_dir).await.map_err(|err| { + Status::internal(format!( + "remove registry layer staging dir '{}' failed: {err}", + layers_dir.display() + )) + }) +} + async fn download_registry_descriptor_blob_file( client: &OciClient, reference: &Reference, @@ -6615,6 +6627,29 @@ mod tests { ); } + #[tokio::test] + async fn remove_registry_layer_staging_preserves_merged_rootfs() { + let base = unique_temp_dir(); + let layers_dir = base.join("layers"); + let rootfs_dir = base.join("rootfs"); + fs::create_dir_all(&layers_dir).unwrap(); + fs::create_dir_all(&rootfs_dir).unwrap(); + fs::write(layers_dir.join("layer.blob"), b"compressed layer").unwrap(); + fs::write(rootfs_dir.join("merged.txt"), b"merged rootfs").unwrap(); + + remove_registry_layer_staging(&base) + .await + .expect("remove layer staging"); + + assert!(!layers_dir.exists()); + assert_eq!( + fs::read(rootfs_dir.join("merged.txt")).unwrap(), + b"merged rootfs" + ); + + let _ = fs::remove_dir_all(base); + } + #[test] fn sanitize_image_identity_rewrites_path_separators() { assert_eq!( diff --git a/crates/openshell-driver-vm/src/rootfs.rs b/crates/openshell-driver-vm/src/rootfs.rs index 536b359e38..c71ebe6884 100644 --- a/crates/openshell-driver-vm/src/rootfs.rs +++ b/crates/openshell-driver-vm/src/rootfs.rs @@ -461,12 +461,19 @@ fn round_up_to_mib(bytes: u64) -> u64 { bytes.div_ceil(MIB) * MIB } +enum FormatterAttempt { + Succeeded, + Failed(String), + Unavailable(String), +} + fn format_ext4_image_from_dir(source: &Path, image_path: &Path) -> Result<(), String> { - let mut last_error = None; - for tool in ["mke2fs", "mkfs.ext4"] { - for candidate in e2fs_tool_candidates(tool) { + let candidates = ["mke2fs", "mkfs.ext4"] + .into_iter() + .flat_map(e2fs_tool_candidates); + run_ext4_formatter_candidates(candidates, |candidate| { let label = candidate.display().to_string(); - let output = Command::new(&candidate) + let output = Command::new(candidate) .arg("-q") .arg("-F") .arg("-t") @@ -478,29 +485,51 @@ fn format_ext4_image_from_dir(source: &Path, image_path: &Path) -> Result<(), St .arg(image_path) .output(); match output { - Ok(output) if output.status.success() => return Ok(()), - Ok(output) => { - last_error = Some(format!( + Ok(output) if output.status.success() => FormatterAttempt::Succeeded, + Ok(output) => FormatterAttempt::Failed(format!( "{label} failed with status {}\nstdout: {}\nstderr: {}", output.status, String::from_utf8_lossy(&output.stdout), String::from_utf8_lossy(&output.stderr) - )); - } + )), Err(err) if err.kind() == std::io::ErrorKind::NotFound => { - last_error = Some(format!("{label} not found")); - } - Err(err) => { - last_error = Some(format!("run {label}: {err}")); + FormatterAttempt::Unavailable(format!("{label} not found")) } + Err(err) => FormatterAttempt::Failed(format!("run {label}: {err}")), } + }) + .map_err(|details| { + format!( + "failed to create ext4 rootfs image from {}: {details}. Install e2fsprogs (mke2fs/mkfs.ext4) and retry", + source.display() + ) + }) +} + +fn run_ext4_formatter_candidates( + candidates: impl IntoIterator, + mut run: impl FnMut(&Path) -> FormatterAttempt, +) -> Result<(), String> { + let mut failures = Vec::new(); + let mut unavailable = Vec::new(); + + for candidate in candidates { + match run(&candidate) { + FormatterAttempt::Succeeded => return Ok(()), + FormatterAttempt::Failed(error) => failures.push(error), + FormatterAttempt::Unavailable(error) => unavailable.push(error), } } - Err(format!( - "failed to create ext4 rootfs image from {}: {}. Install e2fsprogs (mke2fs/mkfs.ext4) and retry", - source.display(), - last_error.unwrap_or_else(|| "no ext4 formatter found".to_string()) - )) + + if failures.is_empty() { + Err(if unavailable.is_empty() { + "no ext4 formatter candidates configured".to_string() + } else { + unavailable.join("\n") + }) + } else { + Err(failures.join("\n")) + } } fn ensure_rootfs_image_parent_dirs(image_path: &Path, guest_path: &str) { @@ -1131,6 +1160,61 @@ mod tests { assert_eq!(debugfs_quote_argument("/tmp/bad\npath"), None); } + #[test] + fn formatter_candidates_preserve_executed_failure_over_missing_fallback() { + let candidates = vec![PathBuf::from("mke2fs"), PathBuf::from("missing")]; + + let err = run_ext4_formatter_candidates(candidates, |candidate| { + if candidate == Path::new("mke2fs") { + FormatterAttempt::Failed( + "mke2fs failed with status 1\nstdout: formatter output\nstderr: no space left" + .to_string(), + ) + } else { + FormatterAttempt::Unavailable("missing not found".to_string()) + } + }) + .expect_err("formatter should fail"); + + assert!(err.contains("mke2fs failed with status 1")); + assert!(err.contains("no space left")); + assert!(!err.contains("missing not found")); + } + + #[test] + fn formatter_candidates_report_all_missing_tools() { + let candidates = vec![PathBuf::from("mke2fs"), PathBuf::from("mkfs.ext4")]; + + let err = run_ext4_formatter_candidates(candidates, |candidate| { + FormatterAttempt::Unavailable(format!("{} not found", candidate.display())) + }) + .expect_err("formatter should be unavailable"); + + assert!(err.contains("mke2fs not found")); + assert!(err.contains("mkfs.ext4 not found")); + } + + #[test] + fn formatter_candidates_accept_successful_fallback() { + let candidates = vec![PathBuf::from("first"), PathBuf::from("second")]; + let mut attempted = Vec::new(); + + run_ext4_formatter_candidates(candidates, |candidate| { + attempted.push(candidate.to_path_buf()); + if candidate == Path::new("second") { + FormatterAttempt::Succeeded + } else { + FormatterAttempt::Failed("first failed".to_string()) + } + }) + .expect("fallback should succeed"); + + assert_eq!( + attempted, + vec![PathBuf::from("first"), PathBuf::from("second")] + ); + } + fn unique_temp_dir() -> PathBuf { static COUNTER: AtomicU64 = AtomicU64::new(0); let nanos = SystemTime::now()