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(); 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!(