From af3c7f33ef2e852e2525ad13d4822bc4ffed09b2 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Mon, 21 Sep 2026 14:31:10 -0400 Subject: [PATCH 1/2] fix(vm): unpack registry images correctly and validate prepared disks The registry image-prep path expected `umoci raw unpack` to produce a bundle-style rootfs/ subdirectory, but it extracts the image filesystem directly into the target. Every registry prep therefore failed after a successful unpack. Guest init exit codes do not survive the libkrun boundary, so the failure looked like success and the broken disk was cached, making every later sandbox for that image fail with "prepared image disk missing /image-rootfs". VM E2E started hitting this after the bootstrap image moved to nvcr.io/nvidia/base/ubuntu:24.04: `--from base` no longer matches the bootstrap image, so it now goes through registry prep. - Accept umoci's direct extraction layout in the guest prep script. - Build the image rootfs under a partial directory and rename it to /image-rootfs only after every prep step succeeds. - Check the prepared disk for /image-rootfs before caching it. On failure, leave the cache untouched and report the prep console tail. - Size the prep disk to hold the payload and the unpacked rootfs at the same time. The community base image needs 1.40 GB + 3.32 GB, which did not fit in the old payload*3 + 512 MiB. Fixes #2358 Co-authored-by: s2cube <26961336+s2cube@users.noreply.github.com> Signed-off-by: Emilien Macchi --- crates/openshell-driver-vm/README.md | 4 +- .../scripts/openshell-vm-sandbox-init.sh | 22 +++-- crates/openshell-driver-vm/src/driver.rs | 46 ++++++++-- crates/openshell-driver-vm/src/rootfs.rs | 83 +++++++++++++++++++ 4 files changed, 140 insertions(+), 15 deletions(-) diff --git a/crates/openshell-driver-vm/README.md b/crates/openshell-driver-vm/README.md index 2cce3b1c04..f38fe901da 100644 --- a/crates/openshell-driver-vm/README.md +++ b/crates/openshell-driver-vm/README.md @@ -199,7 +199,9 @@ payload to a temporary bootstrap VM, and guest init runs `umoci raw unpack` onto Linux-owned ext4 storage. The resulting disk is cached under `/images//rootfs.ext4` and attached read-only to later sandboxes. Local Docker images are still exported as rootfs tar archives and -prepared inside the bootstrap VM. Set `OPENSHELL_VM_IMAGE_PULL_CONCURRENCY` to +prepared inside the bootstrap VM. The driver checks that a prepared disk +contains the unpacked rootfs before caching it; on failure it caches nothing +and reports the image-prep console tail. Set `OPENSHELL_VM_IMAGE_PULL_CONCURRENCY` to tune registry layer download parallelism (default `4`, maximum `16`). Both caches are scoped by source image identity and OpenShell version, so an OpenShell upgrade builds a fresh guest rootfs instead of reusing one with an old diff --git a/crates/openshell-driver-vm/scripts/openshell-vm-sandbox-init.sh b/crates/openshell-driver-vm/scripts/openshell-vm-sandbox-init.sh index 0f63316195..7975f60cdb 100644 --- a/crates/openshell-driver-vm/scripts/openshell-vm-sandbox-init.sh +++ b/crates/openshell-driver-vm/scripts/openshell-vm-sandbox-init.sh @@ -162,25 +162,30 @@ prepare_guest_image_rootfs() { rm -rf "$image_root" "$partial_root" + # Build the rootfs under $partial_root and rename it into place last. The + # host only caches a prepared disk that has $image_root, and guest exit + # codes do not reach the host, so $image_root must not exist until every + # step has succeeded. case "$source" in local-docker) - mkdir -p "$image_root" - tar -xpf "$payload_dir/source-rootfs.tar" -C "$image_root" + mkdir -p "$partial_root" + tar -xpf "$payload_dir/source-rootfs.tar" -C "$partial_root" ;; oci-layout) if [ ! -x /opt/openshell/bin/umoci ]; then ts "FATAL: umoci not found in VM bootstrap image" exit 1 fi + # `umoci raw unpack` extracts the image filesystem directly into + # the target directory; unlike `umoci unpack`, it does not create + # a bundle with a rootfs/ subdirectory. /opt/openshell/bin/umoci raw unpack \ --image "$payload_dir/oci:openshell" \ "$partial_root" - if [ ! -d "$partial_root/rootfs" ]; then - ts "FATAL: umoci unpack did not produce rootfs directory" + if [ ! -d "$partial_root" ]; then + ts "FATAL: umoci unpack did not produce a rootfs directory" exit 1 fi - mv "$partial_root/rootfs" "$image_root" - rm -rf "$partial_root" ;; *) ts "FATAL: unknown guest image payload source: ${source:-missing}" @@ -188,11 +193,12 @@ prepare_guest_image_rootfs() { ;; esac - ensure_target_runtime "$image_root" + ensure_target_runtime "$partial_root" if [ -f "$payload_dir/identity" ]; then - cp "$payload_dir/identity" "$image_root/.openshell-rootfs-variant" + cp "$payload_dir/identity" "$partial_root/.openshell-rootfs-variant" fi rm -rf "$payload_dir" + mv "$partial_root" "$image_root" } exec_supervisor_in_newroot() { diff --git a/crates/openshell-driver-vm/src/driver.rs b/crates/openshell-driver-vm/src/driver.rs index 14f1f9e45b..b56742ad78 100644 --- a/crates/openshell-driver-vm/src/driver.rs +++ b/crates/openshell-driver-vm/src/driver.rs @@ -12,9 +12,9 @@ use crate::lifecycle::{ }; use crate::rootfs::{ clone_or_copy_sparse_file, create_ext4_image_from_dir_with_size, create_rootfs_image_from_dir, - extract_host_supervisor, extract_rootfs_archive_to, prepare_sandbox_rootfs_from_image_root, - recover_rootfs_image, remove_rootfs_image_file, sandbox_guest_init_path, - sandbox_guest_runtime_identity, sandbox_guest_user_ids_from_image, + ext4_image_has_directory, extract_host_supervisor, extract_rootfs_archive_to, + prepare_sandbox_rootfs_from_image_root, recover_rootfs_image, remove_rootfs_image_file, + sandbox_guest_init_path, sandbox_guest_runtime_identity, sandbox_guest_user_ids_from_image, sandbox_guest_user_ids_from_overlay_image, set_rootfs_image_file_mode, validate_host_supervisor, write_rootfs_image_file, }; @@ -202,6 +202,10 @@ const PREPARED_IMAGE_CACHE_LAYOUT_VERSION: &str = "sandbox-prepared-rootfs-ext4- const IMAGE_IDENTITY_FILE: &str = "image-identity"; const IMAGE_REFERENCE_FILE: &str = "image-reference"; const IMAGE_PREP_INIT_MODE: &str = "image-prep"; +const IMAGE_PREP_CONSOLE_LOG: &str = "image-prep-console.log"; +/// Directory the guest image-prep init writes at the root of the prepared disk +/// once preparation succeeds (`image_root` in `openshell-vm-sandbox-init.sh`). +const PREPARED_IMAGE_ROOTFS_DIR: &str = "/image-rootfs"; static IMAGE_CACHE_BUILD_COUNTER: AtomicU64 = AtomicU64::new(0); static OWNER_STATE_WRITE_COUNTER: AtomicU64 = AtomicU64::new(0); @@ -3580,6 +3584,33 @@ impl VmDriver { return Err(err); } + // The prep VM exits successfully even when guest init fails, so check + // the disk itself. Caching a disk without the rootfs would break every + // later sandbox that uses this image. + let prepared_image_for_check = prepared_image.clone(); + let has_rootfs = tokio::task::spawn_blocking(move || { + ext4_image_has_directory(&prepared_image_for_check, PREPARED_IMAGE_ROOTFS_DIR) + }) + .await + .map_err(|err| Status::internal(format!("prepared image validation panicked: {err}")))?; + if !matches!(has_rootfs, Ok(true)) { + let mut message = format!( + "image-prep for \"{image_ref}\" did not produce {PREPARED_IMAGE_ROOTFS_DIR}" + ); + if let Err(err) = &has_rootfs { + write!(message, ": {err}").expect("writing to String cannot fail"); + } + if let Some(console) = read_vm_console_tail( + &staging_dir.join(IMAGE_PREP_CONSOLE_LOG), + VM_CONSOLE_DIAGNOSTIC_BYTES, + ) { + write!(message, "; guest console tail:\n{console}") + .expect("writing to String cannot fail"); + } + let _ = tokio::fs::remove_dir_all(staging_dir).await; + return Err(Status::failed_precondition(message)); + } + if tokio::fs::metadata(&image_path).await.is_ok() { let _ = tokio::fs::remove_dir_all(staging_dir).await; return Ok(()); @@ -3598,7 +3629,7 @@ impl VmDriver { prep_disk: &Path, run_dir: &Path, ) -> Result<(), Status> { - let console_output = run_dir.join("image-prep-console.log"); + let console_output = run_dir.join(IMAGE_PREP_CONSOLE_LOG); let mut command = Command::new(&self.launcher_bin); command.kill_on_drop(true); command.stdin(Stdio::null()); @@ -6608,9 +6639,12 @@ fn prepared_image_disk_size_bytes( .map_err(|err| format!("stat {}: {err}", rootfs_archive.display()))? .len(), }; + // The payload and the unpacked rootfs coexist until the guest deletes the + // payload, and compressed layers commonly expand 2.5-3x. The disk file is + // sparse, so extra headroom costs no host disk space. let requested = payload_size - .saturating_mul(3) - .saturating_add(512 * 1024 * 1024); + .saturating_mul(4) + .saturating_add(1024 * 1024 * 1024); Ok(minimum_size_bytes.max(requested)) } diff --git a/crates/openshell-driver-vm/src/rootfs.rs b/crates/openshell-driver-vm/src/rootfs.rs index c868049343..e1a09bcd98 100644 --- a/crates/openshell-driver-vm/src/rootfs.rs +++ b/crates/openshell-driver-vm/src/rootfs.rs @@ -879,6 +879,60 @@ pub fn sandbox_guest_user_ids_from_overlay_image( sandbox_guest_user_ids_from_image_path(image_path, "/upper/etc/passwd") } +/// Check, without mounting, whether an ext4 image has a directory at +/// `guest_path`. Symlinks are not followed. +pub fn ext4_image_has_directory(image_path: &Path, guest_path: &str) -> Result { + let quoted_path = debugfs_quote_absolute_path(guest_path) + .ok_or_else(|| format!("invalid debugfs guest path '{guest_path}'"))?; + let command = format!("stat {quoted_path}"); + let mut last_error = None; + + for candidate in e2fs_tool_candidates("debugfs") { + let label = candidate.display().to_string(); + match Command::new(&candidate) + .arg("-R") + .arg(&command) + .arg(image_path) + .output() + { + Ok(output) if output.status.success() => { + // debugfs exits 0 whether or not the path exists; the answer + // is only in its output. + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + if stdout.contains("Type: directory") { + return Ok(true); + } + if stdout.contains("Type: ") || stderr.contains("File not found") { + return Ok(false); + } + return Err(format!( + "debugfs command '{command}' produced unrecognized output for {}\nstdout: {stdout}\nstderr: {stderr}", + image_path.display() + )); + } + Ok(output) => { + last_error = Some(format!( + "{label} failed with status {}\nstdout: {}\nstderr: {}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + )); + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + last_error = Some(format!("{label} not found")); + } + Err(error) => last_error = Some(format!("run {label}: {error}")), + } + } + + Err(format!( + "debugfs command '{command}' failed for {}: {}. Install e2fsprogs (debugfs) and retry", + image_path.display(), + last_error.unwrap_or_else(|| "debugfs not found".to_string()) + )) +} + fn sandbox_guest_user_ids_from_image_path( image_path: &Path, guest_path: &str, @@ -1524,6 +1578,35 @@ mod tests { let _ = fs::remove_dir_all(&dir); } + #[test] + fn ext4_image_has_directory_distinguishes_directories_files_and_missing_paths() { + if !e2fs_tool_candidates("debugfs") + .iter() + .any(|candidate| Command::new(candidate).arg("-V").output().is_ok()) + { + return; + } + + let dir = unique_temp_dir(); + let source = dir.join("source"); + let image = dir.join("prepared.ext4"); + fs::create_dir_all(source.join("image-rootfs/bin")).expect("create image rootfs"); + fs::write(source.join("regular"), "file\n").expect("write regular file"); + create_ext4_image_from_dir_with_size(&source, &image, 64 * 1024 * 1024) + .expect("create ext4 image"); + + assert!(ext4_image_has_directory(&image, "/image-rootfs").expect("stat directory")); + assert!(!ext4_image_has_directory(&image, "/regular").expect("stat regular file")); + assert!(!ext4_image_has_directory(&image, "/missing").expect("stat missing path")); + assert!(ext4_image_has_directory(&image, "relative").is_err()); + + let not_ext4 = dir.join("not-ext4.img"); + fs::write(¬_ext4, vec![0_u8; 64 * 1024]).expect("write non-ext4 image"); + assert!(ext4_image_has_directory(¬_ext4, "/image-rootfs").is_err()); + + let _ = fs::remove_dir_all(&dir); + } + #[test] fn sandbox_guest_user_ids_reads_existing_sandbox_user() { let dir = unique_temp_dir(); From 3cd2ddfa1ebf90d3c10356d5218bfad3b6d90149 Mon Sep 17 00:00:00 2001 From: Emilien Macchi Date: Mon, 21 Sep 2026 15:18:56 -0400 Subject: [PATCH 2/2] test(e2e): run tool-dependent VM tests from the community base image The host_gateway_alias and vm_corporate_proxy workloads run curl and python3. The VM driver now defaults to nvcr.io/nvidia/base/ubuntu:24.04, which ships neither, so these tests fail in VM E2E with "command not found". Request the community base image explicitly with `--from base`. Docker, Podman, and Kubernetes E2E already default to that image, so their behavior is unchanged. Signed-off-by: Emilien Macchi --- e2e/rust/tests/host_gateway_alias.rs | 6 +++++ e2e/rust/tests/vm_corporate_proxy.rs | 34 +++++++++++++++++++++------- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/e2e/rust/tests/host_gateway_alias.rs b/e2e/rust/tests/host_gateway_alias.rs index bc26068afc..e0df5d25cf 100644 --- a/e2e/rust/tests/host_gateway_alias.rs +++ b/e2e/rust/tests/host_gateway_alias.rs @@ -315,7 +315,11 @@ async fn sandbox_reaches_host_openshell_internal_via_host_gateway_alias() { .expect("temp policy path should be utf-8") .to_string(); + // The workload needs curl, which minimal default images such as the VM + // driver's nvcr.io/nvidia/base/ubuntu do not ship. let guard = SandboxGuard::create(&[ + "--from", + "base", "--policy", &policy_path, "--", @@ -404,6 +408,8 @@ async fn static_provider_credentials_are_bound_to_profile_endpoints() { server.port, server.port, server.port ); let mut guard = SandboxGuard::create(&[ + "--from", + "base", "--policy", &policy_path, "--provider", diff --git a/e2e/rust/tests/vm_corporate_proxy.rs b/e2e/rust/tests/vm_corporate_proxy.rs index 1e34e71d0d..1b7b588026 100644 --- a/e2e/rust/tests/vm_corporate_proxy.rs +++ b/e2e/rust/tests/vm_corporate_proxy.rs @@ -738,10 +738,20 @@ async fn vm_corporate_proxy_routes_approved_tls_egress() { // ── Run the workload ────────────────────────────────────────────── let (_policy, policy_path) = temp_file_with(&policy_yaml(&ports), "policy file"); let script = workload_script(&ports); - let mut sandbox = - SandboxGuard::create(&["--policy", &policy_path, "--", "python3", "-c", &script]) - .await - .expect("create VM sandbox behind the corporate proxy"); + // The workload runs python3, which the VM driver's default + // nvcr.io/nvidia/base/ubuntu image does not ship. + let mut sandbox = SandboxGuard::create(&[ + "--from", + "base", + "--policy", + &policy_path, + "--", + "python3", + "-c", + &script, + ]) + .await + .expect("create VM sandbox behind the corporate proxy"); assert_proxied_egress( &sandbox.create_output, @@ -805,10 +815,18 @@ async fn vm_corporate_proxy_trusts_ca_bundle_for_https_proxy() { let (_policy, policy_path) = temp_file_with(&policy_yaml(&ports), "policy file"); let script = workload_script(&ports); - let mut sandbox = - SandboxGuard::create(&["--policy", &policy_path, "--", "python3", "-c", &script]) - .await - .expect("create VM sandbox behind the https corporate proxy"); + let mut sandbox = SandboxGuard::create(&[ + "--from", + "base", + "--policy", + &policy_path, + "--", + "python3", + "-c", + &script, + ]) + .await + .expect("create VM sandbox behind the https corporate proxy"); let proxy_logs = proxy.logs().expect("read https proxy logs"); assert!(