From b37da9867e6665ce140589d3cad65b00a1c8b552 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Wed, 16 Sep 2026 21:04:26 +0200 Subject: [PATCH 1/2] usb-ready: wait for a queued unit, not just a running one (#147) ConditionResult is "no" both for a unit systemd has skipped and for one it has not evaluated yet, so the fallback written for the skipped case also fired in the window before ssh-keygen-boot started. Reflash took that as permission to mount, and mounted /dev/sda2 while expand-usb's mkfs was still running inside that unit: "bad superblock", storage FAILED, no host keys, no sshd. The board served HTTP fine and looked, from outside, like a board that would not boot. The Job property is what separates the two. Measured on systemd 257 (the board's own) and 237: case ActiveState ConditionResult Job skipped by its condition inactive no (empty) queued, not started yet inactive no 12561 ExecStart running activating yes 12561 finished active yes (empty) Everything else is identical in the first two rows, including ConditionTimestamp and ExecMainStartTimestamp, which are 0 for both - so the discriminator this needed was never in the properties it was reading. The existing test for this window stubbed ConditionResult as "yes", which is not what systemd reports there, so it passed while the real case failed. It now stubs what a board actually reports, and fails against the old script. Co-Authored-By: Claude Opus 5 --- bin/prod/usb-ready | 30 +++++++++++++++++++++++++++++- test/bats/getters.bats | 23 +++++++++++++++++++---- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/bin/prod/usb-ready b/bin/prod/usb-ready index 54cc410..b70fb67 100755 --- a/bin/prod/usb-ready +++ b/bin/prod/usb-ready @@ -36,8 +36,36 @@ fi state=$(systemctl show -p ActiveState --value "$UNIT" 2>/dev/null || echo "unknown") cond=$(systemctl show -p ConditionResult --value "$UNIT" 2>/dev/null || echo "unknown") +# The job, if systemd still has one for this unit. This is the only property +# that separates "queued, about to run" from "dispatched and skipped": +# +# case ActiveState ConditionResult Job +# skipped by its condition inactive no (empty) +# queued, not started yet inactive no 12561 +# ExecStart running activating yes 12561 +# finished active yes (empty) +# +# Measured on systemd 257 (Debian 13, the board's own) and 237 - the first two +# rows are identical in every other property, including ConditionTimestamp and +# ExecMainStartTimestamp, which are 0 for both. +# +# That mattered: reading only ActiveState and ConditionResult, this said "true" +# in the queued window, Reflash mounted /dev/sda2 while expand-usb's mkfs was +# still running inside ssh-keygen-boot, and the mount failed with "bad +# superblock" - the filesystem did not exist yet. Storage was marked FAILED and +# no host keys were written, so the board came up with no sshd (#147). +job=$(systemctl show -p Job --value "$UNIT" 2>/dev/null | awk '{print $1}') case "$state" in active|failed) echo "true" ;; # ran to completion - *) [ "$cond" = "no" ] && echo "true" || echo "false" ;; # skipped, or not yet + activating) echo "false" ;; # running right now + *) + if [ -n "$job" ]; then + echo "false" # queued: it has not run yet + elif [ "$cond" = "no" ]; then + echo "true" # dispatched and skipped + else + echo "false" + fi + ;; esac diff --git a/test/bats/getters.bats b/test/bats/getters.bats index f8d687f..ba6b959 100644 --- a/test/bats/getters.bats +++ b/test/bats/getters.bats @@ -45,7 +45,10 @@ teardown() { teardown_sandbox; } # Reflash holds /mnt/usb for the life of the process, so it waits for whatever # needs the drive read-write to finish rather than racing it. -# systemctl show -p X --value is called twice: ActiveState then ConditionResult. +# systemctl show -p X --value, for the three properties usb-ready reads: +# ActiveState, ConditionResult and Job. Job is what tells a queued unit from a +# skipped one - see the table in usb-ready - so a stub without it cannot +# reproduce the window that #147 was about. stub_unit() { cat > "$SHIMDIR/systemctl" <> "$CALLS" case "\$*" in *ConditionResult*) echo "${2:-yes}" ;; *ActiveState*) echo "${1:-inactive}" ;; + *Job*) echo "${3:-}" ;; esac EOF chmod +x "$SHIMDIR/systemctl" @@ -74,10 +78,20 @@ with_partition() { } # The race this closes: a oneshot reads "inactive" before it starts as well as -# after a Condition skips it. Mounting in that gap let the unit steal the mount. +# after a Condition skips it, and ConditionResult is "no" in BOTH - this test +# used to stub it as "yes", which is not what systemd reports in that window, so +# it passed while the real case failed (#147). Only the queued job tells them +# apart. @test "usb-ready: false while the owning unit has not started yet" { with_partition - stub_unit inactive yes + stub_unit inactive no 12561 + run "$PROD_BIN/usb-ready" + [ "$output" = "false" ] +} + +@test "usb-ready: false while the owning unit is queued behind something else" { + with_partition + stub_unit inactive yes 12561 run "$PROD_BIN/usb-ready" [ "$output" = "false" ] } @@ -98,9 +112,10 @@ with_partition() { [ "$output" = "true" ] } +# Skipped means dispatched and decided: no job is left for it. @test "usb-ready: true when the owning unit was skipped by its condition" { with_partition - stub_unit inactive no + stub_unit inactive no "" run "$PROD_BIN/usb-ready" [ "$output" = "true" ] } From 23a2bb665b38c3975d354b07f278759e016101e7 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Wed, 16 Sep 2026 21:08:42 +0200 Subject: [PATCH 2/2] Record what systemd actually reports, next to the stub that fakes it The helper's job is to imitate systemd, and the last version of it was written from what seemed reasonable rather than from what a board says: it stubbed ConditionResult=yes for a unit that had not started, where systemd says "no". The test passed, the real case failed, and #147 went unnoticed until a board came up without sshd. The table is measured, with the probe's shape and the systemd versions it was taken on, so the next stub can be written from it. Co-Authored-By: Claude Opus 5 --- test/bats/getters.bats | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/test/bats/getters.bats b/test/bats/getters.bats index ba6b959..277325f 100644 --- a/test/bats/getters.bats +++ b/test/bats/getters.bats @@ -46,9 +46,25 @@ teardown() { teardown_sandbox; } # needs the drive read-write to finish rather than racing it. # systemctl show -p X --value, for the three properties usb-ready reads: -# ActiveState, ConditionResult and Job. Job is what tells a queued unit from a -# skipped one - see the table in usb-ready - so a stub without it cannot -# reproduce the window that #147 was about. +# ActiveState, ConditionResult and Job. +# +# WHAT A BOARD ACTUALLY REPORTS. Stub these from the table, not from what seems +# reasonable - the previous version of this helper guessed ConditionResult=yes +# for a unit that had not started, systemd says "no", and the test passed for +# years while the real case failed (#147): +# +# case ActiveState ConditionResult Job +# skipped by its condition inactive no (empty) +# queued, not started yet inactive no 12561 +# ExecStart running activating yes 12561 +# finished active yes (empty) +# +# Measured on systemd 257 (Debian 13, what the board runs) and 237, with a +# probe unit of the same shape as ssh-keygen-boot: Type=oneshot, +# RemainAfterExit=yes, a ConditionPathExists that passes or fails. The first two +# rows differ in Job and in nothing else - ConditionTimestamp and +# ExecMainStartTimestamp are 0 for both - which is why Job is the property +# usb-ready has to read. stub_unit() { cat > "$SHIMDIR/systemctl" <