Consolidate partition derivation on lsblk-by-PARTLABEL — where early boot allows it - #958
Merged
Merged
Conversation
Three shapes of the same boot-disk/partition derivation existed: pithead-mount-generator's awk/sed suffix arithmetic, pithead-data-reset's own copy of that arithmetic, and pithead-install's simpler `lsblk -lnpo NAME,PARTLABEL` scoped to the resolved disk. pithead-data-reset now resolves its data/ESP partitions the same way pithead-install already does: find the boot disk via `lsblk -no PKNAME` (sysfs-only, no udev needed), then look up the PARTLABEL on that one already-resolved disk. Behavior is unchanged — same partitions, same fail-safe handling when a label isn't there yet (first boot, pre-repart). pithead-mount-generator is deliberately left on its own derivation. It is a systemd generator, which per systemd.generator(7) runs before any other unit — including systemd-udevd — and "cannot rely on any external services". lsblk's PARTLABEL column is sourced from the udev database first, falling back to a direct blkid probe only once that database is unavailable, and lsblk(8) itself recommends a `udevadm settle` first when timing might be an issue — a settle a generator has no daemon left to run against. Moving the generator onto lsblk would trade a self-contained derivation (touches only /proc, does its own arithmetic) for one with an unprovable early-boot dependency, which is the opposite of what this refactor is for. Both files now carry the reasoning inline. lsblk itself is not newly required: it ships via util-linux, an Essential Debian package present in every debian:trixie-slim build regardless of the image's explicit apt list, and pithead-install already calls it on the appliance today. Adds tier-1 coverage for pithead-data-reset's boot_disk_part() with stubbed findmnt/lsblk (tests/stack/run.sh), covering the data and ESP lookups, an absent label (first boot, before repart creates the partition), and a non-/dev root (container/unexpected boot source). make lint: all surfaces clean (shellcheck, ruff, biome, yamllint, markdownlint, docs-voice, operator-strings, buf, taplo). make test: pithead tests: 2190 passed, 0 failed (tests/stack/run.sh, includes the new boot_disk_part coverage); dashboard 1763 passed (96.72% coverage); frontend 312/312; test-compose all green; selftest 154 passed, 0 failed; test-fakes 23 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…idation - repart.d's 10-esp.conf declared Label=ESP while every path that actually CREATES an ESP labels it lowercase (mkimage sgdisk, pithead-install) — the rule only ever matches the existing ESP, but a second casing is a trap; normalized at the source with the reasoning. - boot_disk_part returned rc 0 with empty output when no partition carried the label (awk exits 0 either way) — now fails closed. - the stub test hand-typed lowercase labels that happened to match the code — it now DERIVES them from mkimage.sh's sgdisk line and repart.d's Label=, so a future casing/name drift fails the test instead of green-lighting a dead lookup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #926, with the honest partial scope the issue asked for:
pithead-data-reset's boot_disk_part() andpithead-installnow share the lsblk-by-PARTLABEL shape;pithead-mount-generatordeliberately keeps its positional arithmetic — lsblk's PARTLABEL lookup is udev-database-first and generators run before systemd-udevd, an unprovable early-boot dependency the commit documents in place.Review round closed three findings (this is the reformat path — a wrong device here formats the wrong disk):
Label=ESP(uppercase) contradicted every path that actually creates an ESP (mkimage/installer sgdisk: lowercaseesp) — a latent trap normalized at the source. The verifier flagged this as critical against the lookup; ground truth showed live systems are all-lowercase, so the fix is the config file, not the lookup.Solo suite 2192/0. Battery boot/install phases are the live proof (queued). Ponytail: net-deletion consolidation.
🤖 Generated with Claude Code