Skip to content

Commit 5e020f7

Browse files
committed
test(microvm): the PSS fixture assumed sh -c execs, which dash does not
Two assertions in e11-density.test.sh failed on ubuntu-24.04 in CI while passing on macOS and on Amazon Linux 2023. One root cause, and it is the FIXTURE, not the driver: sh -c 'sleep 5' & # $! is the wrapper, matched by pattern "sleep 5" dash — Ubuntu's /bin/sh — does not exec the command in `sh -c CMD`; it FORKS a child. Reproduced in an ubuntu:24.04 container (dash, procps-ng 4.0.4, Python 3.12.3), `pgrep -f -- "sleep 5"` returns two pids: pid=2534 cmdline=[sh -c sleep 5] <- $!, the only pid the fake proc tree covered pid=2536 cmdline=[sleep 5] <- the forked child, uncovered pss_bytes_for_pids then found a LIVE pid with no readable smaps_rollup under the fabricated $PROC_ROOT and refused, exactly as spec §7.3's boxed warning requires ("refusing to fall back to RSS"). host_signals_snapshot propagated that, so `a nonzero PSS ... IS accepted with require_vmm=1` read "want 0, got 1" and the next assertion's json.load got an empty file — the traceback in the CI log is that symptom, not a second bug. bash, which is /bin/sh on macOS and on Amazon Linux 2023, execs instead: one pid, fixture covers it, both hosts pass. Nothing here involves yama.ptrace_scope: every file this block reads comes from the fabricated proc root, never /proc. Fixed by making the fixture deterministic rather than by touching the driver (shape 1, fully synthetic — the zero-PSS refusal is the whole point of the finding and is untouched): - spawn_marker_process starts ONE process, no shell wrapper, whose argv carries a token unique to this suite run, so no exec/fork choice and no host process can change what the pattern matches. - discovered_pids_for runs the REAL discover_pids, retrying until the process is visible, so a scheduling delay cannot silently make a block assert things about an empty pid set. - plant_rollups fabricates a rollup for EVERY pid discover_pids actually returns, so the fixture covers the real pid set instead of assuming it, and the expected total is derived from that count. - All three `sh -c 'sleep 5' &` sites move to markers. Two of them passed in CI for partly wrong reasons: killing the wrapper orphaned its `sleep` child into later blocks matching the same pattern, and the chmod-000 case could refuse because of an uncovered sibling rather than the unreadable rollup it is about. Non-vacuousness, as required before asserting a refusal does not fire: the block now proves the real discover_pids DOES find its marker before asserting anything about the pid set, and the CI failure mechanism itself is pinned as correct behaviour — a second live process the pattern matches, whose rollup the fixture does not cover, must make the whole snapshot refuse and print no JSON. That case skips visibly ("not run, not claimed verified") if the second process never becomes discoverable, so it can never read as a pass. Verified on the platform that failed, not only locally, in an ubuntu:24.04 container matching the runner (dash, procps-ng 4.0.4, Python 3.12.3, non-root uid 1001): - before: SUITE EXIT 2, the same two failures - after: SUITE EXIT 0, 159 ok, no FAIL, no skip - e10-lifecycle 114 ok, build-snapshot 190 ok, systemd-units 24 ok, all exit 0 - mutation check on the same container: disabling the driver's zero-PSS refusal turns "a matched VMM whose PSS sums to 0 is refused as well" red, so the rebuilt fixture still catches the regression it exists to catch - macOS: 160 ok, exit 0; make test-deploy PASS; shellcheck -S warning clean Assisted-By: Claude (Anthropic AI) <noreply@anthropic.com> Signed-off-by: Paolo Dettori <dettori@us.ibm.com>
1 parent e226bd3 commit 5e020f7

1 file changed

Lines changed: 140 additions & 22 deletions

File tree

deploy/microvm/tests/e11-density.test.sh

Lines changed: 140 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,86 @@ extract_fns() {
5858
done
5959
}
6060

61+
# ---------------------------------------------------------------------------
62+
# Marker processes: how this suite gets a real, LIVE, DETERMINISTICALLY DISCOVERABLE pid
63+
# for the PSS sampler to be pointed at.
64+
#
65+
# Every block below used to spawn `sh -c 'sleep 5' &` and match it with the pattern
66+
# "sleep 5". That is where the ubuntu-24.04 CI failure came from, and it is a fixture bug,
67+
# not a driver bug:
68+
#
69+
# - Ubuntu's /bin/sh is dash, and dash does NOT exec the command in `sh -c CMD` -- it
70+
# FORKS a child. So `pgrep -f 'sleep 5'` returned TWO pids: the `sh -c sleep 5` wrapper
71+
# that `$!` names, and a `sleep 5` child. The fabricated proc tree covered only the
72+
# first, and a LIVE pid with no readable smaps_rollup is precisely what
73+
# pss_bytes_for_pids refuses to guess about (spec section 7.3's boxed warning) -- so the
74+
# snapshot refused, the acceptance assertion read "want 0, got 1", and json.load then
75+
# got an empty file. Confirmed by running this suite in an ubuntu:24.04 container:
76+
# `pid=2534 cmdline=[sh -c sleep 5]` alongside `pid=2536 cmdline=[sleep 5]`.
77+
# - bash, which is /bin/sh on macOS and on Amazon Linux 2023, execs instead, so there was
78+
# exactly one pid and the same fixture passed on both of those hosts.
79+
# - `kill "$!"` killed only the wrapper, orphaning the `sleep 5` child for the rest of its
80+
# five seconds -- where a later block matching the same pattern could pick it up.
81+
#
82+
# The replacement depends on no shell's exec/fork choice and on no host process: ONE
83+
# process, no shell wrapper, and a token in its argv that nothing else on any host shares.
84+
# Blocks then fabricate a rollup for EVERY pid discover_pids actually returns, so the
85+
# fixture covers the real pid set instead of assuming what it will be.
86+
marker_token() {
87+
printf '__e11_marker_%s_%s' "$$" "$1"
88+
}
89+
90+
# spawn_marker_process starts one marker process and sets MARKER_PID. A plain assignment
91+
# rather than a printed pid, so the process is this shell's own child and `wait` works.
92+
# The 60s lifetime bounds the leak if the suite dies before stop_marker_process.
93+
MARKER_PID=""
94+
spawn_marker_process() {
95+
python3 -c 'import sys, time; time.sleep(int(sys.argv[1]))' 60 "$1" &
96+
MARKER_PID=$!
97+
}
98+
99+
stop_marker_process() {
100+
[ -n "${1:-}" ] || return 0
101+
kill "$1" 2>/dev/null
102+
wait "$1" 2>/dev/null
103+
return 0
104+
}
105+
106+
# discovered_pids_for prints the pids the REAL discover_pids (sourced from $1) returns for
107+
# pattern $2, retrying until at least $3 of them appear. The retry is not politeness: a
108+
# process that has not been scheduled yet would silently make a block assert things about an
109+
# EMPTY pid set, which is the vacuous-test shape this suite exists to avoid.
110+
discovered_pids_for() {
111+
local snippet="$1" pattern="$2" want="${3:-1}" tries=0 pids="" n=0
112+
while [ "$tries" -lt 25 ]; do
113+
tries=$((tries + 1))
114+
pids="$(
115+
# shellcheck disable=SC1090
116+
. "$snippet"
117+
discover_pids "$pattern"
118+
)"
119+
n="$(printf '%s\n' "$pids" | grep -c '[0-9]')"
120+
[ "$n" -lt "$want" ] || break
121+
sleep 0.2
122+
done
123+
printf '%s' "$pids"
124+
}
125+
126+
# count_pids counts the pid lines in $1 (an empty list counts 0, not 1).
127+
count_pids() {
128+
printf '%s\n' "$1" | grep -c '[0-9]'
129+
}
130+
131+
# plant_rollups fabricates $3 kB of Pss for EVERY pid in $2, under fake proc root $1.
132+
plant_rollups() {
133+
local root="$1" kb="$3" pid
134+
while IFS= read -r pid; do
135+
[ -n "$pid" ] || continue
136+
mkdir -p "$root/$pid"
137+
printf 'Pss: %s kB\n' "$kb" >"$root/$pid/smaps_rollup"
138+
done <<<"$2"
139+
}
140+
61141
echo "== the script exists, is executable, and is shellcheck-clean"
62142
check "e11-density.sh present" "$([ -f "$SCRIPT" ] && echo yes || echo no)" "yes"
63143
check "e11-density.sh executable" "$([ -x "$SCRIPT" ] && echo yes || echo no)" "yes"
@@ -106,10 +186,12 @@ if [ -n "$pss_body" ]; then
106186
printf '%s\n' "$pss_body"
107187
} >"$pss_snippet"
108188

109-
# A real, still-alive process (this shell's own subshell sleeper) whose fake
110-
# PROC_ROOT/<pid>/smaps_rollup we control directly.
111-
sh -c 'sleep 5' &
112-
live_pid=$!
189+
# A real, still-alive process whose fake PROC_ROOT/<pid>/smaps_rollup we control directly.
190+
# One process, not an `sh -c` wrapper: see spawn_marker_process for why that distinction
191+
# cost a CI run. This block passes the pid EXPLICITLY, so it never depended on pgrep --
192+
# but killing the wrapper used to orphan its `sleep` child into later blocks that do.
193+
spawn_marker_process "$(marker_token pss_unit)"
194+
live_pid="$MARKER_PID"
113195

114196
fake_proc="$pss_tmpdir/proc"
115197
mkdir -p "$fake_proc/$live_pid"
@@ -163,8 +245,7 @@ if [ -n "$pss_body" ]; then
163245
check "an already-exited pid contributes 0, is not an unreadable-file failure" "$rc_dead" "0"
164246
check "an already-exited pid's contribution is exactly 0 bytes" "$out_dead" "0"
165247

166-
kill "$live_pid" 2>/dev/null
167-
wait "$live_pid" 2>/dev/null
248+
stop_marker_process "$live_pid"
168249
rm -rf "$pss_tmpdir"
169250
fi
170251

@@ -271,8 +352,9 @@ if [ -n "$signals_body" ]; then
271352

272353
# --- The whole assembly, end to end, parsed by its REAL consumer (json.load), with a
273354
# live pid whose smaps_rollup exists so no other field can fail for its own reasons.
274-
sh -c 'sleep 5' &
275-
sig_live=$!
355+
sig_marker="$(marker_token signals)"
356+
spawn_marker_process "$sig_marker"
357+
sig_live="$MARKER_PID"
276358
mkdir -p "$sig_proc/$sig_live"
277359
printf 'Pss: 512 kB\n' >"$sig_proc/$sig_live/smaps_rollup"
278360
sig_json=$(
@@ -302,7 +384,10 @@ if [ -n "$signals_body" ]; then
302384
PROC_ROOT="$sig_proc"
303385
export PROC_ROOT
304386
# shellcheck disable=SC2034 # read by host_signals_snapshot, sourced below
305-
VMM_PROC_PATTERN="sleep 5"
387+
# The marker token, not "sleep 5": under dash that pattern also matched a forked child
388+
# this fixture never covered, so the refusal below could fire for the WRONG reason (an
389+
# uncovered sibling) rather than the chmod 000 rollup this case is about.
390+
VMM_PROC_PATTERN="$sig_marker"
306391
# shellcheck disable=SC2034
307392
VIRTIOFSD_PROC_PATTERN="__e11_no_such_process__"
308393
# shellcheck disable=SC1090
@@ -318,8 +403,7 @@ if [ -n "$signals_body" ]; then
318403
fi
319404
chmod 644 "$sig_proc/$sig_live/smaps_rollup" 2>/dev/null || true
320405

321-
kill "$sig_live" 2>/dev/null
322-
wait "$sig_live" 2>/dev/null
406+
stop_marker_process "$sig_live"
323407
rm -rf "$sig_tmpdir"
324408
fi
325409

@@ -636,25 +720,59 @@ if [ -n "$vmm_body" ]; then
636720

637721
# --- And a matched process whose rollups sum to 0 is refused too: pids present is not
638722
# the same claim as memory measured.
639-
sh -c 'sleep 5' &
640-
zero_pid=$!
641-
mkdir -p "$vmm_proc/$zero_pid"
642-
printf 'Pss: 0 kB\n' >"$vmm_proc/$zero_pid/smaps_rollup"
723+
#
724+
# The pid set comes from the REAL discover_pids and the fabricated rollups cover ALL of
725+
# it, so this depends on no host process and on no shell's exec/fork choice -- see
726+
# spawn_marker_process for the ubuntu-24.04 failure that shape caused.
727+
vmm_marker="$(marker_token pss)"
728+
spawn_marker_process "$vmm_marker"
729+
vmm_marker_pid="$MARKER_PID"
730+
vmm_pids="$(discovered_pids_for "$vmm_snippet" "$vmm_marker" 1)"
731+
vmm_pid_count="$(count_pids "$vmm_pids")"
732+
# Non-vacuousness: everything below is about a NON-EMPTY matched pid set, so prove the
733+
# real discover_pids found one before asserting anything about what is done with it.
734+
check "non-vacuousness: the real discover_pids finds this block's marker process" \
735+
"$([ "$vmm_pid_count" -ge 1 ] && echo yes || echo no)" "yes"
736+
[ "$vmm_pid_count" -le 1 ] ||
737+
echo " (note: the marker matched $vmm_pid_count processes; every one of them gets a fabricated rollup, so the totals below still add up)"
738+
739+
plant_rollups "$vmm_proc" "$vmm_pids" 0
643740
zero_rc=0
644-
zero_out="$(snapshot_with "sleep 5" 1 2>/dev/null)" || zero_rc=$?
741+
zero_out="$(snapshot_with "$vmm_marker" 1 2>/dev/null)" || zero_rc=$?
645742
check "a matched VMM whose PSS sums to 0 is refused as well" \
646743
"$([ "$zero_rc" -ne 0 ] && echo yes || echo no)" "yes"
647744
check " ...printing no JSON" "$zero_out" ""
648-
# ...while a real, nonzero reading for that same pid is accepted, so the check above is
745+
746+
# ...while a real, nonzero reading for the SAME pids is accepted, so the check above is
649747
# about the VALUE being zero and not about the pattern matching at all.
650-
printf 'Pss: 512 kB\n' >"$vmm_proc/$zero_pid/smaps_rollup"
748+
plant_rollups "$vmm_proc" "$vmm_pids" 512
749+
nz_expected=$((512 * 1024 * vmm_pid_count))
651750
nz_rc=0
652-
nz_json="$(snapshot_with "sleep 5" 1 2>/dev/null)" || nz_rc=$?
751+
nz_json="$(snapshot_with "$vmm_marker" 1 2>/dev/null)" || nz_rc=$?
653752
check "a nonzero PSS for the same matched pid IS accepted with require_vmm=1" "$nz_rc" "0"
654753
check " ...and reports the real total" \
655-
"$(printf '%s' "$nz_json" | python3 -c 'import json,sys; print(json.load(sys.stdin)["pssBytes"])' 2>&1)" "524288"
656-
kill "$zero_pid" 2>/dev/null
657-
wait "$zero_pid" 2>/dev/null
754+
"$(printf '%s' "$nz_json" | python3 -c 'import json,sys; print(json.load(sys.stdin)["pssBytes"])' 2>&1)" "$nz_expected"
755+
756+
# --- And the CI failure mode itself, pinned as CORRECT behaviour rather than something to
757+
# be loosened away: a SECOND live process the pattern matches, whose rollup the fabricated
758+
# tree does not cover, makes the whole snapshot refuse. That is spec section 7.3's
759+
# never-guess-a-pid's-memory guarantee, and it is exactly what dash's forked `sleep`
760+
# child did to this block on ubuntu-24.04.
761+
spawn_marker_process "$vmm_marker"
762+
sibling_pid="$MARKER_PID"
763+
sib_pids="$(discovered_pids_for "$vmm_snippet" "$vmm_marker" $((vmm_pid_count + 1)))"
764+
if [ "$(count_pids "$sib_pids")" -le "$vmm_pid_count" ]; then
765+
echo " (skip: the second marker process never became discoverable -- the uncovered-sibling refusal was not exercised, not claimed verified)"
766+
else
767+
sib_rc=0
768+
sib_out="$(snapshot_with "$vmm_marker" 1 2>/dev/null)" || sib_rc=$?
769+
check "an uncovered sibling pid makes the whole snapshot REFUSE (the ubuntu-24.04 symptom)" \
770+
"$([ "$sib_rc" -ne 0 ] && echo yes || echo no)" "yes"
771+
check " ...printing no JSON, which is what left json.load with an empty file in CI" \
772+
"$sib_out" ""
773+
fi
774+
stop_marker_process "$sibling_pid"
775+
stop_marker_process "$vmm_marker_pid"
658776

659777
rm -rf "$vmm_tmpdir"
660778
fi

0 commit comments

Comments
 (0)