Skip to content

Commit cb6e88a

Browse files
EmilienMs2cube
andauthored
fix(vm): unpack registry images correctly and validate prepared disks (#3524)
* 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 <emacchi@redhat.com> * 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 <emacchi@redhat.com> --------- Signed-off-by: Emilien Macchi <emacchi@redhat.com> Co-authored-by: s2cube <26961336+s2cube@users.noreply.github.com>
1 parent 2493d41 commit cb6e88a

6 files changed

Lines changed: 172 additions & 23 deletions

File tree

‎crates/openshell-driver-vm/README.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,9 @@ payload to a temporary bootstrap VM, and guest init runs `umoci raw unpack` onto
199199
Linux-owned ext4 storage. The resulting disk is cached under
200200
`<state-dir>/images/<cache-id>/rootfs.ext4` and attached read-only to later
201201
sandboxes. Local Docker images are still exported as rootfs tar archives and
202-
prepared inside the bootstrap VM. Set `OPENSHELL_VM_IMAGE_PULL_CONCURRENCY` to
202+
prepared inside the bootstrap VM. The driver checks that a prepared disk
203+
contains the unpacked rootfs before caching it; on failure it caches nothing
204+
and reports the image-prep console tail. Set `OPENSHELL_VM_IMAGE_PULL_CONCURRENCY` to
203205
tune registry layer download parallelism (default `4`, maximum `16`).
204206
Both caches are scoped by source image identity and OpenShell version, so an
205207
OpenShell upgrade builds a fresh guest rootfs instead of reusing one with an old

‎crates/openshell-driver-vm/scripts/openshell-vm-sandbox-init.sh‎

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -162,37 +162,43 @@ prepare_guest_image_rootfs() {
162162

163163
rm -rf "$image_root" "$partial_root"
164164

165+
# Build the rootfs under $partial_root and rename it into place last. The
166+
# host only caches a prepared disk that has $image_root, and guest exit
167+
# codes do not reach the host, so $image_root must not exist until every
168+
# step has succeeded.
165169
case "$source" in
166170
local-docker)
167-
mkdir -p "$image_root"
168-
tar -xpf "$payload_dir/source-rootfs.tar" -C "$image_root"
171+
mkdir -p "$partial_root"
172+
tar -xpf "$payload_dir/source-rootfs.tar" -C "$partial_root"
169173
;;
170174
oci-layout)
171175
if [ ! -x /opt/openshell/bin/umoci ]; then
172176
ts "FATAL: umoci not found in VM bootstrap image"
173177
exit 1
174178
fi
179+
# `umoci raw unpack` extracts the image filesystem directly into
180+
# the target directory; unlike `umoci unpack`, it does not create
181+
# a bundle with a rootfs/ subdirectory.
175182
/opt/openshell/bin/umoci raw unpack \
176183
--image "$payload_dir/oci:openshell" \
177184
"$partial_root"
178-
if [ ! -d "$partial_root/rootfs" ]; then
179-
ts "FATAL: umoci unpack did not produce rootfs directory"
185+
if [ ! -d "$partial_root" ]; then
186+
ts "FATAL: umoci unpack did not produce a rootfs directory"
180187
exit 1
181188
fi
182-
mv "$partial_root/rootfs" "$image_root"
183-
rm -rf "$partial_root"
184189
;;
185190
*)
186191
ts "FATAL: unknown guest image payload source: ${source:-missing}"
187192
exit 1
188193
;;
189194
esac
190195

191-
ensure_target_runtime "$image_root"
196+
ensure_target_runtime "$partial_root"
192197
if [ -f "$payload_dir/identity" ]; then
193-
cp "$payload_dir/identity" "$image_root/.openshell-rootfs-variant"
198+
cp "$payload_dir/identity" "$partial_root/.openshell-rootfs-variant"
194199
fi
195200
rm -rf "$payload_dir"
201+
mv "$partial_root" "$image_root"
196202
}
197203

198204
exec_supervisor_in_newroot() {

‎crates/openshell-driver-vm/src/driver.rs‎

Lines changed: 40 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@ use crate::lifecycle::{
1212
};
1313
use crate::rootfs::{
1414
clone_or_copy_sparse_file, create_ext4_image_from_dir_with_size, create_rootfs_image_from_dir,
15-
extract_host_supervisor, extract_rootfs_archive_to, prepare_sandbox_rootfs_from_image_root,
16-
recover_rootfs_image, remove_rootfs_image_file, sandbox_guest_init_path,
17-
sandbox_guest_runtime_identity, sandbox_guest_user_ids_from_image,
15+
ext4_image_has_directory, extract_host_supervisor, extract_rootfs_archive_to,
16+
prepare_sandbox_rootfs_from_image_root, recover_rootfs_image, remove_rootfs_image_file,
17+
sandbox_guest_init_path, sandbox_guest_runtime_identity, sandbox_guest_user_ids_from_image,
1818
sandbox_guest_user_ids_from_overlay_image, set_rootfs_image_file_mode,
1919
validate_host_supervisor, write_rootfs_image_file,
2020
};
@@ -202,6 +202,10 @@ const PREPARED_IMAGE_CACHE_LAYOUT_VERSION: &str = "sandbox-prepared-rootfs-ext4-
202202
const IMAGE_IDENTITY_FILE: &str = "image-identity";
203203
const IMAGE_REFERENCE_FILE: &str = "image-reference";
204204
const IMAGE_PREP_INIT_MODE: &str = "image-prep";
205+
const IMAGE_PREP_CONSOLE_LOG: &str = "image-prep-console.log";
206+
/// Directory the guest image-prep init writes at the root of the prepared disk
207+
/// once preparation succeeds (`image_root` in `openshell-vm-sandbox-init.sh`).
208+
const PREPARED_IMAGE_ROOTFS_DIR: &str = "/image-rootfs";
205209
static IMAGE_CACHE_BUILD_COUNTER: AtomicU64 = AtomicU64::new(0);
206210
static OWNER_STATE_WRITE_COUNTER: AtomicU64 = AtomicU64::new(0);
207211

@@ -3586,6 +3590,33 @@ impl VmDriver {
35863590
return Err(err);
35873591
}
35883592

3593+
// The prep VM exits successfully even when guest init fails, so check
3594+
// the disk itself. Caching a disk without the rootfs would break every
3595+
// later sandbox that uses this image.
3596+
let prepared_image_for_check = prepared_image.clone();
3597+
let has_rootfs = tokio::task::spawn_blocking(move || {
3598+
ext4_image_has_directory(&prepared_image_for_check, PREPARED_IMAGE_ROOTFS_DIR)
3599+
})
3600+
.await
3601+
.map_err(|err| Status::internal(format!("prepared image validation panicked: {err}")))?;
3602+
if !matches!(has_rootfs, Ok(true)) {
3603+
let mut message = format!(
3604+
"image-prep for \"{image_ref}\" did not produce {PREPARED_IMAGE_ROOTFS_DIR}"
3605+
);
3606+
if let Err(err) = &has_rootfs {
3607+
write!(message, ": {err}").expect("writing to String cannot fail");
3608+
}
3609+
if let Some(console) = read_vm_console_tail(
3610+
&staging_dir.join(IMAGE_PREP_CONSOLE_LOG),
3611+
VM_CONSOLE_DIAGNOSTIC_BYTES,
3612+
) {
3613+
write!(message, "; guest console tail:\n{console}")
3614+
.expect("writing to String cannot fail");
3615+
}
3616+
let _ = tokio::fs::remove_dir_all(staging_dir).await;
3617+
return Err(Status::failed_precondition(message));
3618+
}
3619+
35893620
if tokio::fs::metadata(&image_path).await.is_ok() {
35903621
let _ = tokio::fs::remove_dir_all(staging_dir).await;
35913622
return Ok(());
@@ -3604,7 +3635,7 @@ impl VmDriver {
36043635
prep_disk: &Path,
36053636
run_dir: &Path,
36063637
) -> Result<(), Status> {
3607-
let console_output = run_dir.join("image-prep-console.log");
3638+
let console_output = run_dir.join(IMAGE_PREP_CONSOLE_LOG);
36083639
let mut command = Command::new(&self.launcher_bin);
36093640
command.kill_on_drop(true);
36103641
command.stdin(Stdio::null());
@@ -6622,9 +6653,12 @@ fn prepared_image_disk_size_bytes(
66226653
.map_err(|err| format!("stat {}: {err}", rootfs_archive.display()))?
66236654
.len(),
66246655
};
6656+
// The payload and the unpacked rootfs coexist until the guest deletes the
6657+
// payload, and compressed layers commonly expand 2.5-3x. The disk file is
6658+
// sparse, so extra headroom costs no host disk space.
66256659
let requested = payload_size
6626-
.saturating_mul(3)
6627-
.saturating_add(512 * 1024 * 1024);
6660+
.saturating_mul(4)
6661+
.saturating_add(1024 * 1024 * 1024);
66286662
Ok(minimum_size_bytes.max(requested))
66296663
}
66306664

‎crates/openshell-driver-vm/src/rootfs.rs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -879,6 +879,60 @@ pub fn sandbox_guest_user_ids_from_overlay_image(
879879
sandbox_guest_user_ids_from_image_path(image_path, "/upper/etc/passwd")
880880
}
881881

882+
/// Check, without mounting, whether an ext4 image has a directory at
883+
/// `guest_path`. Symlinks are not followed.
884+
pub fn ext4_image_has_directory(image_path: &Path, guest_path: &str) -> Result<bool, String> {
885+
let quoted_path = debugfs_quote_absolute_path(guest_path)
886+
.ok_or_else(|| format!("invalid debugfs guest path '{guest_path}'"))?;
887+
let command = format!("stat {quoted_path}");
888+
let mut last_error = None;
889+
890+
for candidate in e2fs_tool_candidates("debugfs") {
891+
let label = candidate.display().to_string();
892+
match Command::new(&candidate)
893+
.arg("-R")
894+
.arg(&command)
895+
.arg(image_path)
896+
.output()
897+
{
898+
Ok(output) if output.status.success() => {
899+
// debugfs exits 0 whether or not the path exists; the answer
900+
// is only in its output.
901+
let stdout = String::from_utf8_lossy(&output.stdout);
902+
let stderr = String::from_utf8_lossy(&output.stderr);
903+
if stdout.contains("Type: directory") {
904+
return Ok(true);
905+
}
906+
if stdout.contains("Type: ") || stderr.contains("File not found") {
907+
return Ok(false);
908+
}
909+
return Err(format!(
910+
"debugfs command '{command}' produced unrecognized output for {}\nstdout: {stdout}\nstderr: {stderr}",
911+
image_path.display()
912+
));
913+
}
914+
Ok(output) => {
915+
last_error = Some(format!(
916+
"{label} failed with status {}\nstdout: {}\nstderr: {}",
917+
output.status,
918+
String::from_utf8_lossy(&output.stdout),
919+
String::from_utf8_lossy(&output.stderr)
920+
));
921+
}
922+
Err(error) if error.kind() == std::io::ErrorKind::NotFound => {
923+
last_error = Some(format!("{label} not found"));
924+
}
925+
Err(error) => last_error = Some(format!("run {label}: {error}")),
926+
}
927+
}
928+
929+
Err(format!(
930+
"debugfs command '{command}' failed for {}: {}. Install e2fsprogs (debugfs) and retry",
931+
image_path.display(),
932+
last_error.unwrap_or_else(|| "debugfs not found".to_string())
933+
))
934+
}
935+
882936
fn sandbox_guest_user_ids_from_image_path(
883937
image_path: &Path,
884938
guest_path: &str,
@@ -1524,6 +1578,35 @@ mod tests {
15241578
let _ = fs::remove_dir_all(&dir);
15251579
}
15261580

1581+
#[test]
1582+
fn ext4_image_has_directory_distinguishes_directories_files_and_missing_paths() {
1583+
if !e2fs_tool_candidates("debugfs")
1584+
.iter()
1585+
.any(|candidate| Command::new(candidate).arg("-V").output().is_ok())
1586+
{
1587+
return;
1588+
}
1589+
1590+
let dir = unique_temp_dir();
1591+
let source = dir.join("source");
1592+
let image = dir.join("prepared.ext4");
1593+
fs::create_dir_all(source.join("image-rootfs/bin")).expect("create image rootfs");
1594+
fs::write(source.join("regular"), "file\n").expect("write regular file");
1595+
create_ext4_image_from_dir_with_size(&source, &image, 64 * 1024 * 1024)
1596+
.expect("create ext4 image");
1597+
1598+
assert!(ext4_image_has_directory(&image, "/image-rootfs").expect("stat directory"));
1599+
assert!(!ext4_image_has_directory(&image, "/regular").expect("stat regular file"));
1600+
assert!(!ext4_image_has_directory(&image, "/missing").expect("stat missing path"));
1601+
assert!(ext4_image_has_directory(&image, "relative").is_err());
1602+
1603+
let not_ext4 = dir.join("not-ext4.img");
1604+
fs::write(&not_ext4, vec![0_u8; 64 * 1024]).expect("write non-ext4 image");
1605+
assert!(ext4_image_has_directory(&not_ext4, "/image-rootfs").is_err());
1606+
1607+
let _ = fs::remove_dir_all(&dir);
1608+
}
1609+
15271610
#[test]
15281611
fn sandbox_guest_user_ids_reads_existing_sandbox_user() {
15291612
let dir = unique_temp_dir();

‎e2e/rust/tests/host_gateway_alias.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,11 @@ async fn sandbox_reaches_host_openshell_internal_via_host_gateway_alias() {
315315
.expect("temp policy path should be utf-8")
316316
.to_string();
317317

318+
// The workload needs curl, which minimal default images such as the VM
319+
// driver's nvcr.io/nvidia/base/ubuntu do not ship.
318320
let guard = SandboxGuard::create(&[
321+
"--from",
322+
"base",
319323
"--policy",
320324
&policy_path,
321325
"--",
@@ -404,6 +408,8 @@ async fn static_provider_credentials_are_bound_to_profile_endpoints() {
404408
server.port, server.port, server.port
405409
);
406410
let mut guard = SandboxGuard::create(&[
411+
"--from",
412+
"base",
407413
"--policy",
408414
&policy_path,
409415
"--provider",

‎e2e/rust/tests/vm_corporate_proxy.rs‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -738,10 +738,20 @@ async fn vm_corporate_proxy_routes_approved_tls_egress() {
738738
// ── Run the workload ──────────────────────────────────────────────
739739
let (_policy, policy_path) = temp_file_with(&policy_yaml(&ports), "policy file");
740740
let script = workload_script(&ports);
741-
let mut sandbox =
742-
SandboxGuard::create(&["--policy", &policy_path, "--", "python3", "-c", &script])
743-
.await
744-
.expect("create VM sandbox behind the corporate proxy");
741+
// The workload runs python3, which the VM driver's default
742+
// nvcr.io/nvidia/base/ubuntu image does not ship.
743+
let mut sandbox = SandboxGuard::create(&[
744+
"--from",
745+
"base",
746+
"--policy",
747+
&policy_path,
748+
"--",
749+
"python3",
750+
"-c",
751+
&script,
752+
])
753+
.await
754+
.expect("create VM sandbox behind the corporate proxy");
745755

746756
assert_proxied_egress(
747757
&sandbox.create_output,
@@ -805,10 +815,18 @@ async fn vm_corporate_proxy_trusts_ca_bundle_for_https_proxy() {
805815

806816
let (_policy, policy_path) = temp_file_with(&policy_yaml(&ports), "policy file");
807817
let script = workload_script(&ports);
808-
let mut sandbox =
809-
SandboxGuard::create(&["--policy", &policy_path, "--", "python3", "-c", &script])
810-
.await
811-
.expect("create VM sandbox behind the https corporate proxy");
818+
let mut sandbox = SandboxGuard::create(&[
819+
"--from",
820+
"base",
821+
"--policy",
822+
&policy_path,
823+
"--",
824+
"python3",
825+
"-c",
826+
&script,
827+
])
828+
.await
829+
.expect("create VM sandbox behind the https corporate proxy");
812830

813831
let proxy_logs = proxy.logs().expect("read https proxy logs");
814832
assert!(

0 commit comments

Comments
 (0)