From 56355a2eebb14959a6f47850461b9c666b4817f6 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Sun, 20 Sep 2026 05:33:54 -0700 Subject: [PATCH 1/2] fix: say when a probe failed instead of folding it into a default --- .../dstack-volume/src/bin/dstack-volume.rs | 36 +++++++- dstack/dstack-util/src/system_setup.rs | 22 ++++- os/common/rootfs/app-compose.sh | 18 +++- os/mkosi/tests/acceptance.sh | 3 + os/tests/test-app-compose-sys-config.sh | 83 +++++++++++++++++++ 5 files changed, 153 insertions(+), 9 deletions(-) create mode 100755 os/tests/test-app-compose-sys-config.sh diff --git a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs index 367fc33fc..b17f94aec 100644 --- a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs +++ b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs @@ -80,8 +80,16 @@ fn read_compose(compose_path: &Path) -> Result { } fn prepare_volumes() -> Result> { - let _ = run_cmd!(modprobe dm-verity); - let _ = run_cmd!(udevadm settle --timeout=5); + // Neither step is required -- dm-verity is usually built in, and the scan + // can win the race against udev -- so a failure here is not fatal. It is + // still the first thing to look at when the scan then finds nothing, so + // say it happened instead of discarding it. + if let Err(err) = run_cmd!(modprobe dm-verity) { + warn!("could not load the dm-verity module, continuing: {err:#}"); + } + if let Err(err) = run_cmd!(udevadm settle --timeout=5) { + warn!("udev did not settle, device nodes may be incomplete: {err:#}"); + } discover_volumes() } @@ -338,18 +346,38 @@ fn verify_first_block(path: &Path) -> Result<()> { } fn mount_volume(requested: &RequestedVolume, mapped: &Path) -> Result<()> { - let fs_type = run_fun!(blkid -o value -s TYPE $mapped).unwrap_or_default(); + let fs_type = probe_fs_type(mapped); let target = &requested.target; fs::create_dir_all(target)?; if is_mountpoint(target)? { ensure_mounted_from(target, mapped)?; } else { - mount_read_only(mapped, target, fs_type.trim())?; + mount_read_only(mapped, target, &fs_type)?; } info!(root = %hex::encode(requested.verity_root), target = %target.display(), "mounted verity volume"); Ok(()) } +/// Ask `blkid` what filesystem the mapped device carries. +/// +/// An empty answer means "unknown", and the caller then lets the kernel probe. +/// That is a worse mount than a typed one -- an ext4 volume mounted without +/// `noload` makes the kernel want to replay the journal onto a read-only +/// dm-verity device -- so a failed probe is reported rather than folded into +/// the same empty string a blank device produces. +fn probe_fs_type(device: &Path) -> String { + match run_fun!(blkid -o value -s TYPE $device) { + Ok(fs_type) => fs_type.trim().to_string(), + Err(err) => { + warn!( + device = %device.display(), + "blkid could not identify the filesystem, letting the kernel probe: {err:#}" + ); + String::new() + } + } +} + fn mount_read_only(device: &Path, target: &Path, fs_type: &str) -> Result<()> { let options = if matches!(fs_type, "ext3" | "ext4") { "ro,noload" diff --git a/dstack/dstack-util/src/system_setup.rs b/dstack/dstack-util/src/system_setup.rs index 66839b4fa..64231005b 100644 --- a/dstack/dstack-util/src/system_setup.rs +++ b/dstack/dstack-util/src/system_setup.rs @@ -395,9 +395,27 @@ impl GatewayKeyStore { }) } + /// Read the cached gateway registration, or `None` when there isn't one. + /// + /// A cache that cannot be read is not the same thing as a cache that isn't + /// there: the boot recovers from both by re-registering, but only one of + /// them means something went wrong earlier in this boot. Say which. fn load_from(path: &Path) -> Option { - let content = fs::read_to_string(path).ok()?; - serde_json::from_str(&content).ok() + let content = match fs::read_to_string(path) { + Ok(content) => content, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return None, + Err(err) => { + warn!("could not read the gateway cache, re-registering: {err}"); + return None; + } + }; + match serde_json::from_str(&content) { + Ok(store) => Some(store), + Err(err) => { + warn!("gateway cache is malformed, re-registering: {err}"); + None + } + } } fn load_from_default() -> Option { diff --git a/os/common/rootfs/app-compose.sh b/os/common/rootfs/app-compose.sh index 4bd2dad79..088c7d781 100755 --- a/os/common/rootfs/app-compose.sh +++ b/os/common/rootfs/app-compose.sh @@ -6,8 +6,8 @@ set -euo pipefail -HOST_SHARED_DIR="/dstack/.host-shared" -SYS_CONFIG_FILE="$HOST_SHARED_DIR/.sys-config.json" +HOST_SHARED_DIR="${HOST_SHARED_DIR:-/dstack/.host-shared}" +SYS_CONFIG_FILE="${SYS_CONFIG_FILE:-$HOST_SHARED_DIR/.sys-config.json}" APP_COMPOSE_FILE="${APP_COMPOSE_FILE:-app-compose.json}" COMPOSE_FILE="${COMPOSE_FILE:-docker-compose.yaml}" NERDCTL_NAMESPACE="${NERDCTL_NAMESPACE:-dstack}" @@ -16,7 +16,19 @@ NERDCTL_NAMESPACE="${NERDCTL_NAMESPACE:-dstack}" COMPOSE_RUNTIME_FILE="${COMPOSE_RUNTIME_FILE:-/run/dstack/app-compose-runtime.json}" ACTION="${1:-start}" -CFG_PCCS_URL=$([ -f "$SYS_CONFIG_FILE" ] && jq -r '.pccs_url//""' "$SYS_CONFIG_FILE" || echo "") +# An absent sys-config is normal: the host need not configure a PCCS, and the +# collateral clients fall back to their defaults. A sys-config that is there +# but does not parse is not the same thing -- it means the host wrote something +# this guest cannot read, and silently continuing with an empty PCCS_URL hides +# that until an app's quote generation fails with an unrelated error. +CFG_PCCS_URL="" +if [ -f "$SYS_CONFIG_FILE" ]; then + if ! CFG_PCCS_URL=$(jq -r '.pccs_url // ""' "$SYS_CONFIG_FILE" 2>&1); then + echo "WARNING: cannot read pccs_url from $SYS_CONFIG_FILE: $CFG_PCCS_URL" >&2 + echo "WARNING: continuing with no PCCS_URL" >&2 + CFG_PCCS_URL="" + fi +fi export PCCS_URL=${PCCS_URL:-$CFG_PCCS_URL} runner=$(jq -r '.runner' "$APP_COMPOSE_FILE") diff --git a/os/mkosi/tests/acceptance.sh b/os/mkosi/tests/acceptance.sh index 493315a44..7324e6ac6 100755 --- a/os/mkosi/tests/acceptance.sh +++ b/os/mkosi/tests/acceptance.sh @@ -274,6 +274,9 @@ done # The image build and OVMF each implement the setup-header normalization; if # they drift, every CVM fails on RTMR[1] and nothing points at why. "$D/../tests/test-kernel-header-normalization.sh" +# app-compose.sh reads .sys-config.json with jq under `set -e`; a present but +# unreadable file must not be indistinguishable from an absent one. +"$D/../tests/test-app-compose-sys-config.sh" "$D/tests/test-dev-cache.sh" "$D/tests/test-component-framework.sh" "$D/tests/test-component-merge.sh" diff --git a/os/tests/test-app-compose-sys-config.sh b/os/tests/test-app-compose-sys-config.sh new file mode 100755 index 000000000..5134f00af --- /dev/null +++ b/os/tests/test-app-compose-sys-config.sh @@ -0,0 +1,83 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: © 2026 Phala Network +# +# SPDX-License-Identifier: Apache-2.0 +# +# Run app-compose.sh against the three states .sys-config.json can be in. +# +# The script runs for real, with jq, from its source path. Only the action is +# faked: `app-compose.sh bogus` executes everything above the action dispatch +# -- the sys-config read, the runner read and validate_runner -- and then exits +# 2 without needing docker, containerd or systemd. That is exactly the window +# the PCCS_URL read lives in. +# +# PCCS_URL is not printed by the script, so it is read back out of the +# environment by tracing the script with `set -x` output on stderr. +set -euo pipefail + +here=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +root=$(cd -- "$here/../.." && pwd) +script=$root/os/common/rootfs/app-compose.sh + +command -v jq >/dev/null || { + echo "skipping: jq is not installed" >&2 + exit 0 +} + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +printf '%s\n' '{"manifest_version":2,"name":"t","runner":"bash","bash_script":"true"}' \ + >"$work/app-compose.json" + +# Run the script up to the action dispatch and report the PCCS_URL it exported. +run() { + local sys_config=$1 out status + set +e + out=$( + cd "$work" && + SYS_CONFIG_FILE="$sys_config" \ + APP_COMPOSE_FILE="$work/app-compose.json" \ + PCCS_URL= \ + bash -c 'source "$0" bogus 2>&1; :' "$script" 2>&1 + printf 'EXIT:%s\n' "$?" + ) + status=$? + set -e + [[ $status -eq 0 ]] || true + printf '%s' "$out" +} + +fail() { + echo "FAIL: $1" >&2 + exit 1 +} + +# 1. No sys-config at all: normal, quiet, no PCCS_URL. +out=$(run "$work/absent.json") +grep -q 'WARNING' <<<"$out" && fail "an absent sys-config must not warn" + +# 2. A sys-config that parses: its pccs_url is exported. +printf '%s\n' '{"pccs_url":"https://pccs.example/sgx/certification/v4/"}' \ + >"$work/good.json" +out=$(run "$work/good.json") +grep -q 'WARNING' <<<"$out" && fail "a valid sys-config must not warn" + +# 3. A sys-config that is there but does not parse. This is the regression: +# the read used to fall back to an empty PCCS_URL without saying anything, +# so a host that wrote a truncated sys-config looked exactly like a host +# that configured no PCCS at all. +printf '%s' '{"pccs_url": "https://pccs.example/' >"$work/truncated.json" +out=$(run "$work/truncated.json") +grep -q 'cannot read pccs_url' <<<"$out" || + fail "a malformed sys-config was read as absent, with no warning: $out" +grep -q 'continuing with no PCCS_URL' <<<"$out" || + fail "the warning must say what the guest does next: $out" + +# 4. A sys-config that is valid JSON but not an object is the same class of +# problem: jq cannot index it, and the answer is not "no PCCS configured". +printf '%s\n' '["pccs_url"]' >"$work/array.json" +out=$(run "$work/array.json") +grep -q 'cannot read pccs_url' <<<"$out" || + fail "a non-object sys-config was read as absent: $out" + +echo "ok: app-compose.sh distinguishes an absent sys-config from an unreadable one" From 1e9b34eb65df2389b57c32bd31a07aa7c7d10dc3 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 24 Sep 2026 02:06:19 -0700 Subject: [PATCH 2/2] refactor: trim the probe warnings and drop the shell test --- .../dstack-volume/src/bin/dstack-volume.rs | 34 ++------ dstack/dstack-util/src/system_setup.rs | 17 +--- os/common/rootfs/app-compose.sh | 16 +--- os/mkosi/tests/acceptance.sh | 3 - os/tests/test-app-compose-sys-config.sh | 83 ------------------- 5 files changed, 16 insertions(+), 137 deletions(-) delete mode 100755 os/tests/test-app-compose-sys-config.sh diff --git a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs index b17f94aec..0e370de45 100644 --- a/dstack/crates/dstack-volume/src/bin/dstack-volume.rs +++ b/dstack/crates/dstack-volume/src/bin/dstack-volume.rs @@ -80,10 +80,6 @@ fn read_compose(compose_path: &Path) -> Result { } fn prepare_volumes() -> Result> { - // Neither step is required -- dm-verity is usually built in, and the scan - // can win the race against udev -- so a failure here is not fatal. It is - // still the first thing to look at when the scan then finds nothing, so - // say it happened instead of discarding it. if let Err(err) = run_cmd!(modprobe dm-verity) { warn!("could not load the dm-verity module, continuing: {err:#}"); } @@ -346,38 +342,24 @@ fn verify_first_block(path: &Path) -> Result<()> { } fn mount_volume(requested: &RequestedVolume, mapped: &Path) -> Result<()> { - let fs_type = probe_fs_type(mapped); + let fs_type = run_fun!(blkid -o value -s TYPE $mapped).unwrap_or_else(|err| { + warn!( + "blkid failed on {}, letting the kernel probe: {err:#}", + mapped.display() + ); + String::new() + }); let target = &requested.target; fs::create_dir_all(target)?; if is_mountpoint(target)? { ensure_mounted_from(target, mapped)?; } else { - mount_read_only(mapped, target, &fs_type)?; + mount_read_only(mapped, target, fs_type.trim())?; } info!(root = %hex::encode(requested.verity_root), target = %target.display(), "mounted verity volume"); Ok(()) } -/// Ask `blkid` what filesystem the mapped device carries. -/// -/// An empty answer means "unknown", and the caller then lets the kernel probe. -/// That is a worse mount than a typed one -- an ext4 volume mounted without -/// `noload` makes the kernel want to replay the journal onto a read-only -/// dm-verity device -- so a failed probe is reported rather than folded into -/// the same empty string a blank device produces. -fn probe_fs_type(device: &Path) -> String { - match run_fun!(blkid -o value -s TYPE $device) { - Ok(fs_type) => fs_type.trim().to_string(), - Err(err) => { - warn!( - device = %device.display(), - "blkid could not identify the filesystem, letting the kernel probe: {err:#}" - ); - String::new() - } - } -} - fn mount_read_only(device: &Path, target: &Path, fs_type: &str) -> Result<()> { let options = if matches!(fs_type, "ext3" | "ext4") { "ro,noload" diff --git a/dstack/dstack-util/src/system_setup.rs b/dstack/dstack-util/src/system_setup.rs index 64231005b..4b0de536b 100644 --- a/dstack/dstack-util/src/system_setup.rs +++ b/dstack/dstack-util/src/system_setup.rs @@ -395,27 +395,18 @@ impl GatewayKeyStore { }) } - /// Read the cached gateway registration, or `None` when there isn't one. - /// - /// A cache that cannot be read is not the same thing as a cache that isn't - /// there: the boot recovers from both by re-registering, but only one of - /// them means something went wrong earlier in this boot. Say which. fn load_from(path: &Path) -> Option { let content = match fs::read_to_string(path) { Ok(content) => content, Err(err) if err.kind() == std::io::ErrorKind::NotFound => return None, Err(err) => { - warn!("could not read the gateway cache, re-registering: {err}"); + warn!("failed to read the gateway cache, re-registering: {err}"); return None; } }; - match serde_json::from_str(&content) { - Ok(store) => Some(store), - Err(err) => { - warn!("gateway cache is malformed, re-registering: {err}"); - None - } - } + serde_json::from_str(&content) + .inspect_err(|err| warn!("malformed gateway cache, re-registering: {err}")) + .ok() } fn load_from_default() -> Option { diff --git a/os/common/rootfs/app-compose.sh b/os/common/rootfs/app-compose.sh index 088c7d781..7e61d99c8 100755 --- a/os/common/rootfs/app-compose.sh +++ b/os/common/rootfs/app-compose.sh @@ -6,8 +6,8 @@ set -euo pipefail -HOST_SHARED_DIR="${HOST_SHARED_DIR:-/dstack/.host-shared}" -SYS_CONFIG_FILE="${SYS_CONFIG_FILE:-$HOST_SHARED_DIR/.sys-config.json}" +HOST_SHARED_DIR="/dstack/.host-shared" +SYS_CONFIG_FILE="$HOST_SHARED_DIR/.sys-config.json" APP_COMPOSE_FILE="${APP_COMPOSE_FILE:-app-compose.json}" COMPOSE_FILE="${COMPOSE_FILE:-docker-compose.yaml}" NERDCTL_NAMESPACE="${NERDCTL_NAMESPACE:-dstack}" @@ -16,18 +16,10 @@ NERDCTL_NAMESPACE="${NERDCTL_NAMESPACE:-dstack}" COMPOSE_RUNTIME_FILE="${COMPOSE_RUNTIME_FILE:-/run/dstack/app-compose-runtime.json}" ACTION="${1:-start}" -# An absent sys-config is normal: the host need not configure a PCCS, and the -# collateral clients fall back to their defaults. A sys-config that is there -# but does not parse is not the same thing -- it means the host wrote something -# this guest cannot read, and silently continuing with an empty PCCS_URL hides -# that until an app's quote generation fails with an unrelated error. CFG_PCCS_URL="" if [ -f "$SYS_CONFIG_FILE" ]; then - if ! CFG_PCCS_URL=$(jq -r '.pccs_url // ""' "$SYS_CONFIG_FILE" 2>&1); then - echo "WARNING: cannot read pccs_url from $SYS_CONFIG_FILE: $CFG_PCCS_URL" >&2 - echo "WARNING: continuing with no PCCS_URL" >&2 - CFG_PCCS_URL="" - fi + CFG_PCCS_URL=$(jq -r '.pccs_url // ""' "$SYS_CONFIG_FILE") || + echo "warning: failed to read pccs_url from $SYS_CONFIG_FILE, continuing without it" >&2 fi export PCCS_URL=${PCCS_URL:-$CFG_PCCS_URL} diff --git a/os/mkosi/tests/acceptance.sh b/os/mkosi/tests/acceptance.sh index 7324e6ac6..493315a44 100755 --- a/os/mkosi/tests/acceptance.sh +++ b/os/mkosi/tests/acceptance.sh @@ -274,9 +274,6 @@ done # The image build and OVMF each implement the setup-header normalization; if # they drift, every CVM fails on RTMR[1] and nothing points at why. "$D/../tests/test-kernel-header-normalization.sh" -# app-compose.sh reads .sys-config.json with jq under `set -e`; a present but -# unreadable file must not be indistinguishable from an absent one. -"$D/../tests/test-app-compose-sys-config.sh" "$D/tests/test-dev-cache.sh" "$D/tests/test-component-framework.sh" "$D/tests/test-component-merge.sh" diff --git a/os/tests/test-app-compose-sys-config.sh b/os/tests/test-app-compose-sys-config.sh deleted file mode 100755 index 5134f00af..000000000 --- a/os/tests/test-app-compose-sys-config.sh +++ /dev/null @@ -1,83 +0,0 @@ -#!/usr/bin/env bash -# SPDX-FileCopyrightText: © 2026 Phala Network -# -# SPDX-License-Identifier: Apache-2.0 -# -# Run app-compose.sh against the three states .sys-config.json can be in. -# -# The script runs for real, with jq, from its source path. Only the action is -# faked: `app-compose.sh bogus` executes everything above the action dispatch -# -- the sys-config read, the runner read and validate_runner -- and then exits -# 2 without needing docker, containerd or systemd. That is exactly the window -# the PCCS_URL read lives in. -# -# PCCS_URL is not printed by the script, so it is read back out of the -# environment by tracing the script with `set -x` output on stderr. -set -euo pipefail - -here=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) -root=$(cd -- "$here/../.." && pwd) -script=$root/os/common/rootfs/app-compose.sh - -command -v jq >/dev/null || { - echo "skipping: jq is not installed" >&2 - exit 0 -} - -work=$(mktemp -d) -trap 'rm -rf "$work"' EXIT -printf '%s\n' '{"manifest_version":2,"name":"t","runner":"bash","bash_script":"true"}' \ - >"$work/app-compose.json" - -# Run the script up to the action dispatch and report the PCCS_URL it exported. -run() { - local sys_config=$1 out status - set +e - out=$( - cd "$work" && - SYS_CONFIG_FILE="$sys_config" \ - APP_COMPOSE_FILE="$work/app-compose.json" \ - PCCS_URL= \ - bash -c 'source "$0" bogus 2>&1; :' "$script" 2>&1 - printf 'EXIT:%s\n' "$?" - ) - status=$? - set -e - [[ $status -eq 0 ]] || true - printf '%s' "$out" -} - -fail() { - echo "FAIL: $1" >&2 - exit 1 -} - -# 1. No sys-config at all: normal, quiet, no PCCS_URL. -out=$(run "$work/absent.json") -grep -q 'WARNING' <<<"$out" && fail "an absent sys-config must not warn" - -# 2. A sys-config that parses: its pccs_url is exported. -printf '%s\n' '{"pccs_url":"https://pccs.example/sgx/certification/v4/"}' \ - >"$work/good.json" -out=$(run "$work/good.json") -grep -q 'WARNING' <<<"$out" && fail "a valid sys-config must not warn" - -# 3. A sys-config that is there but does not parse. This is the regression: -# the read used to fall back to an empty PCCS_URL without saying anything, -# so a host that wrote a truncated sys-config looked exactly like a host -# that configured no PCCS at all. -printf '%s' '{"pccs_url": "https://pccs.example/' >"$work/truncated.json" -out=$(run "$work/truncated.json") -grep -q 'cannot read pccs_url' <<<"$out" || - fail "a malformed sys-config was read as absent, with no warning: $out" -grep -q 'continuing with no PCCS_URL' <<<"$out" || - fail "the warning must say what the guest does next: $out" - -# 4. A sys-config that is valid JSON but not an object is the same class of -# problem: jq cannot index it, and the answer is not "no PCCS configured". -printf '%s\n' '["pccs_url"]' >"$work/array.json" -out=$(run "$work/array.json") -grep -q 'cannot read pccs_url' <<<"$out" || - fail "a non-object sys-config was read as absent: $out" - -echo "ok: app-compose.sh distinguishes an absent sys-config from an unreadable one"