From 5f3a18424ac11bbbbd59a320a6570f020aa3fb15 Mon Sep 17 00:00:00 2001 From: Doug Baggett Date: Sun, 13 Sep 2026 16:43:15 +0000 Subject: [PATCH] fix(systemd): drop local-fs.target ordering from rechunker-group-fix.service rechunker-group-fix.service is ordered Before=systemd-sysusers.service (since #530) but also Wants=/After=local-fs.target. systemd-sysusers is ordered before systemd-tmpfiles-setup-dev.service < local-fs-pre.target < local-fs.target, so the unit closes an ordering cycle on every boot: Found ordering cycle on systemd-sysusers.service/start; has dependency on rechunker-group-fix.service/start, local-fs.target/start, local-fs-pre.target/start, systemd-tmpfiles-setup-dev.service/start systemd breaks it by deleting whichever job it reaches first, so the outcome is per-boot nondeterministic: systemd-udevd (90 s device timeouts, /var never mounted), systemd-sysusers, local-fs-pre.target, or systemd-ask-password-console.path (LUKS volumes never unlocked after the initrd hands over). Measured on ghcr.io/projectbluefin/bluefin-lts:stable with `systemd-analyze verify --generators=yes` over default, multi-user, graphical, cryptsetup and the unit itself: 16 cycle lines before, 0 after. Order the unit exactly like systemd-sysusers.service and bootc-sysusers-shadow-sync.service instead: After=systemd-remount-fs.service (so /etc is writable), After=bootc-sysusers-shadow-sync.service, Before=systemd-sysusers.service. Drop the trailing systemd-tmpfiles pass: the unit now runs before local-fs-pre.target, where /var and /tmp are not mounted and the pass exits 65, and systemd-tmpfiles-setup.service runs the same command after local-fs.target and sysusers anyway. Tests: a static ordering-contract check plus a `systemd-analyze verify --root=` run over a minimal fixture of the stock early-boot units, which reports the cycle for the previous unit and nothing for this one. Closes #918 Refs projectbluefin/bluefin-lts#628, projectbluefin/bluefin-lts#585, projectbluefin/bluefin-lts#466, projectbluefin/bluefin-lts#391 Assisted-by: Claude Opus 5 (Claude Code) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_0114TUzBMEH13zPa2VVYwLdW --- docs/TESTING.md | 2 +- docs/skills/submodule-boundary.md | 19 +++- .../shared/usr/bin/rechunker-group-fix | 8 +- .../system/rechunker-group-fix.service | 20 ++++- tests/test_rechunker_group_fix.bats | 89 +++++++++++++++++++ 5 files changed, 127 insertions(+), 11 deletions(-) diff --git a/docs/TESTING.md b/docs/TESTING.md index dc3e3a587..336152a76 100644 --- a/docs/TESTING.md +++ b/docs/TESTING.md @@ -91,7 +91,7 @@ Do not add exemptions for scripts with branching logic. | `tests/test_bling.bats` | `ublue-bling` — shell config injection install/uninstall | | `tests/test_bling_preexec_rearm.bats` | `bling/bash-preexec-rearm.sh` — DEBUG trap re-arm with array/scalar `PROMPT_COMMAND`, idempotency, degradation when bash-preexec is absent | | `tests/test_luks_tpm2.bats` | `luks-tpm2-autounlock` — UUID parsing, device resolution, cryptenroll flag construction | -| `tests/test_rechunker_group_fix.bats` | `rechunker-group-fix` — group/gshadow append, duplicate detection, format | +| `tests/test_rechunker_group_fix.bats` | `rechunker-group-fix` — group/gshadow append, duplicate detection, format; service ordering contract and `systemd-analyze verify` cycle check | | `tests/test_bling_fastfetch.bats` | `ublue-bling-fastfetch` — all 9 accent colors, dconf/gsettings fallback chain, FASTFETCH_FORCE_THEME override | | `tests/test_changelog.bats` | `changelog.just` — LTS/non-LTS repo selection, URL construction, exit behaviour | | `tests/test_native_recipes.bats` | Native recipes with a leftover `bctl`: CLI setup, devmode, signed channel switching, VM setup, Flatpak bundles, and both reset confirmations | diff --git a/docs/skills/submodule-boundary.md b/docs/skills/submodule-boundary.md index 08c3503bd..ffea4727a 100644 --- a/docs/skills/submodule-boundary.md +++ b/docs/skills/submodule-boundary.md @@ -80,17 +80,28 @@ Key files (all live in `system_files/shared/`): ### Service ordering (critical — do not change without understanding this) -As of [common#530](https://github.com/projectbluefin/common/pull/530), the service must run with: +The service must run with: ```ini DefaultDependencies=no -Wants=local-fs.target -After=local-fs.target +After=systemd-remount-fs.service After=bootc-sysusers-shadow-sync.service Before=systemd-sysusers.service ``` -**Why:** `systemd-sysusers` is what fails if gshadow is corrupt. The service must run *before* sysusers, not after. `bootc-sysusers-shadow-sync.service` is the upstream fix shipped in bootc ≥1.16 ([bootc#2207](https://github.com/bootc-dev/bootc/pull/2207), merged May 2025); our service must run after it so they coexist correctly. `DefaultDependencies=no` is required for any early-boot unit. +**Why:** `systemd-sysusers` is what fails if gshadow is corrupt. The service must run *before* sysusers, not after, under the same preconditions sysusers itself requires (`After=systemd-remount-fs.service`, so `/etc` is writable). `bootc-sysusers-shadow-sync.service` is the upstream fix shipped in bootc ≥1.16 ([bootc#2207](https://github.com/bootc-dev/bootc/pull/2207), merged May 2025); our service must run after it so they coexist correctly. `DefaultDependencies=no` is required for any early-boot unit. + +**Never add `Wants=local-fs.target` / `After=local-fs.target`** (the ordering [common#530](https://github.com/projectbluefin/common/pull/530) shipped with). `systemd-sysusers.service` is ordered before `systemd-tmpfiles-setup-dev.service`, which is ordered before `local-fs-pre.target`, which is ordered before `local-fs.target`, so that edge closes an ordering cycle: + +```text +Found ordering cycle on systemd-sysusers.service/start; has dependency on rechunker-group-fix.service/start, local-fs.target/start, local-fs-pre.target/start, systemd-tmpfiles-setup-dev.service/start +``` + +systemd breaks the cycle by deleting whichever job it reaches first — `systemd-udevd`, `systemd-sysusers`, `systemd-tmpfiles-setup-dev`, `local-fs-pre.target`, `systemd-ask-password-console.path` (the LUKS password agent), … — so the failure is per-boot nondeterministic: 90 s device timeouts, `/var` never mounted, encrypted volumes never unlocked, or a black screen ([common#918](https://github.com/projectbluefin/common/issues/918), [bluefin-lts#628](https://github.com/projectbluefin/bluefin-lts/issues/628), [bluefin-lts#585](https://github.com/projectbluefin/bluefin-lts/issues/585), [bluefin-lts#466](https://github.com/projectbluefin/bluefin-lts/issues/466)). + +Because the unit runs before `local-fs-pre.target`, it must not run `systemd-tmpfiles`: `/var` and `/tmp` are not mounted yet and the pass exits 65. `systemd-tmpfiles-setup.service` performs the same pass after `local-fs.target` and after `systemd-sysusers.service`. + +`tests/test_rechunker_group_fix.bats` enforces this contract statically and by letting `systemd-analyze verify --root=` compute the boot transaction against a minimal fixture of the stock early-boot units. Downstream images must not try to fix ordering with a drop-in: systemd cannot reset `After=`/`Wants=` from a drop-in (`After=` with an empty value is a no-op for dependencies), so a drop-in can only *add* edges — `Before=local-fs-pre.target` on top of the old `After=local-fs.target` produced a tighter cycle. ### flock on gshadow writes (required) diff --git a/system_files/shared/usr/bin/rechunker-group-fix b/system_files/shared/usr/bin/rechunker-group-fix index 0d7894d6e..994bc2e0e 100755 --- a/system_files/shared/usr/bin/rechunker-group-fix +++ b/system_files/shared/usr/bin/rechunker-group-fix @@ -1,12 +1,14 @@ #!/usr/bin/env bash -# To use this script, you'll want to put this in your systemd service: +# To use this script, you'll want to put this in your systemd service +# (ordered before systemd-sysusers.service, see rechunker-group-fix.service): # rm /etc/gshadow # systemd-sysusers # (run this script) -# systemd-tmpfiles --create --remove --boot --exclude-prefix=/dev # This will populate /etc/group successfully, and then populate /etc/gshadow -# with any missing groups that we nuked when we removed /etc/gshadow +# with any missing groups that we nuked when we removed /etc/gshadow. +# The regular systemd-tmpfiles-setup.service pass runs afterwards, once the +# repaired groups exist and local filesystems are mounted. # # TODO: This entire script is a workaround for systemd-sysusers not managing # /etc/gshadow on bootc images. Remove once upstream resolves it: diff --git a/system_files/shared/usr/lib/systemd/system/rechunker-group-fix.service b/system_files/shared/usr/lib/systemd/system/rechunker-group-fix.service index 16cf6cedf..fbd5b256a 100644 --- a/system_files/shared/usr/lib/systemd/system/rechunker-group-fix.service +++ b/system_files/shared/usr/lib/systemd/system/rechunker-group-fix.service @@ -16,8 +16,19 @@ Description=Fix groups for Legacy rechunker ConditionPathExists=/run/ostree-booted DefaultDependencies=no -Wants=local-fs.target -After=local-fs.target +# Ordering contract: run under the same preconditions as systemd-sysusers.service +# (and bootc-sysusers-shadow-sync.service): after the root fs is remounted +# read-write, before systemd-sysusers.service. +# +# Do NOT order this unit after local-fs.target. systemd-sysusers.service is +# ordered before systemd-tmpfiles-setup-dev.service, which is ordered before +# local-fs-pre.target, which is ordered before local-fs.target. Adding +# After=local-fs.target here therefore closes an ordering cycle, and systemd +# breaks it by deleting an arbitrary early-boot job (systemd-udevd, +# systemd-sysusers, local-fs-pre.target, systemd-ask-password-console.path, ...), +# which results in device timeouts, an unmounted /var, a stalled LUKS unlock, or +# a black screen depending on which job is dropped. +After=systemd-remount-fs.service After=bootc-sysusers-shadow-sync.service Before=systemd-sysusers.service @@ -27,7 +38,10 @@ ExecStart=bash -c 'touch /etc/gshadow && chmod 600 /etc/gshadow' ExecStart=bash -c 'rm /etc/gshadow' ExecStart=systemd-sysusers ExecStart=rechunker-group-fix -ExecStart=systemd-tmpfiles --create --remove --boot --exclude-prefix=/dev +# No systemd-tmpfiles pass here: this unit now runs before local-fs-pre.target, +# so /var and /tmp are not mounted yet and the pass would fail (exit 65). +# systemd-tmpfiles-setup.service runs the same command after local-fs.target +# and after systemd-sysusers.service, i.e. after the groups repaired here exist. [Install] WantedBy=default.target multi-user.target diff --git a/tests/test_rechunker_group_fix.bats b/tests/test_rechunker_group_fix.bats index 170b5272f..d5f6cffca 100644 --- a/tests/test_rechunker_group_fix.bats +++ b/tests/test_rechunker_group_fix.bats @@ -89,3 +89,92 @@ teardown() { grep -q "^beta:" "${GSHADOW_FILE}" grep -q "^gamma:" "${GSHADOW_FILE}" } + +# --------------------------------------------------------------------------- +# Service ordering (regression for common#918 / bluefin-lts#628, #585, #466) +# --------------------------------------------------------------------------- + +SERVICE="$BATS_TEST_DIRNAME/../system_files/shared/usr/lib/systemd/system/rechunker-group-fix.service" + +@test "rechunker-group-fix.service: ordering contract matches systemd-sysusers.service" { + # Must run before sysusers, after the root fs is writable, and coexist + # with bootc's own shadow-sync unit. + grep -qE '^DefaultDependencies=no$' "${SERVICE}" + grep -qE '^Before=systemd-sysusers\.service$' "${SERVICE}" + grep -qE '^After=systemd-remount-fs\.service$' "${SERVICE}" + grep -qE '^After=bootc-sysusers-shadow-sync\.service$' "${SERVICE}" + + # Must not order after (or pull in) local-fs.target: sysusers is ordered + # before local-fs-pre.target, so that edge closes an ordering cycle. + run grep -E '^(Wants|Requires|After)=.*\blocal-fs(-pre)?\.target\b' "${SERVICE}" + [ "${status}" -ne 0 ] + + # Runs before /var is mounted, so it must not run a tmpfiles pass. + run grep -E '^ExecStart=.*systemd-tmpfiles' "${SERVICE}" + [ "${status}" -ne 0 ] +} + +# Build a minimal unit tree that reproduces the stock systemd early-boot +# ordering the service has to fit into: +# systemd-sysusers.service < systemd-tmpfiles-setup-dev.service +# < local-fs-pre.target < local-fs.target +# and let systemd itself compute the start transaction for default.target. +# With After=local-fs.target on the service this reports +# "Found ordering cycle ... rechunker-group-fix.service ..." and deletes a job. +_write_ordering_fixture() { + local root="$1" unitdir + unitdir="${root}/usr/lib/systemd/system" + mkdir -p "${unitdir}/sysinit.target.wants" "${unitdir}/default.target.wants" "${root}/usr/bin" + cp "${SCRIPT}" "${root}/usr/bin/rechunker-group-fix" + cp "${SERVICE}" "${unitdir}/rechunker-group-fix.service" + + printf '[Unit]\nDescription=Preparation for Local File Systems\n' \ + > "${unitdir}/local-fs-pre.target" + printf '[Unit]\nDescription=Local File Systems\nDefaultDependencies=no\nAfter=local-fs-pre.target\n' \ + > "${unitdir}/local-fs.target" + printf '[Unit]\nDescription=Remount Root and Kernel File Systems\nDefaultDependencies=no\nBefore=local-fs-pre.target local-fs.target\nWants=local-fs-pre.target\n[Service]\nType=oneshot\nExecStart=/bin/true\n' \ + > "${unitdir}/systemd-remount-fs.service" + printf '[Unit]\nDescription=Create System Users\nDefaultDependencies=no\nAfter=systemd-remount-fs.service\nBefore=systemd-tmpfiles-setup-dev.service\nBefore=sysinit.target\n[Service]\nType=oneshot\nExecStart=/bin/true\n' \ + > "${unitdir}/systemd-sysusers.service" + printf '[Unit]\nDescription=Create Static Device Nodes in /dev\nDefaultDependencies=no\nBefore=sysinit.target local-fs-pre.target systemd-udevd.service\nWants=local-fs-pre.target\n[Service]\nType=oneshot\nExecStart=/bin/true\n' \ + > "${unitdir}/systemd-tmpfiles-setup-dev.service" + printf '[Unit]\nDescription=Rule-based Manager for Device Events and Files\nDefaultDependencies=no\nAfter=systemd-sysusers.service\nBefore=sysinit.target\n[Service]\nExecStart=/bin/true\n' \ + > "${unitdir}/systemd-udevd.service" + printf '[Unit]\nDescription=System Initialization\nDefaultDependencies=no\nWants=local-fs.target\nAfter=local-fs.target\n' \ + > "${unitdir}/sysinit.target" + printf '[Unit]\nDescription=Default\nRequires=sysinit.target\nAfter=sysinit.target\n' \ + > "${unitdir}/default.target" + + local u + for u in systemd-sysusers.service systemd-tmpfiles-setup-dev.service systemd-udevd.service; do + ln -s "../${u}" "${unitdir}/sysinit.target.wants/${u}" + done + # WantedBy=default.target from the [Install] section, as `systemctl enable` does. + ln -s ../rechunker-group-fix.service "${unitdir}/default.target.wants/rechunker-group-fix.service" +} + +@test "rechunker-group-fix.service: systemd computes no ordering cycle for the boot transaction" { + command -v systemd-analyze >/dev/null 2>&1 || skip "systemd-analyze not available" + + _write_ordering_fixture "${WORKDIR}/root" + + run systemd-analyze verify --root="${WORKDIR}/root" default.target + echo "${output}" + [ "${status}" -eq 0 ] + [ "$(grep -c -e 'ordering cycle' -e 'deleted to break' <<< "${output}")" -eq 0 ] +} + +@test "rechunker-group-fix.service: ordering fixture detects the local-fs.target cycle" { + # Guard against the fixture silently passing: the pre-fix ordering + # (After=local-fs.target) must be reported as a cycle by systemd. + command -v systemd-analyze >/dev/null 2>&1 || skip "systemd-analyze not available" + + _write_ordering_fixture "${WORKDIR}/root" + sed -i 's/^After=systemd-remount-fs\.service$/Wants=local-fs.target\nAfter=local-fs.target/' \ + "${WORKDIR}/root/usr/lib/systemd/system/rechunker-group-fix.service" + + run systemd-analyze verify --root="${WORKDIR}/root" default.target + echo "${output}" + grep -q 'ordering cycle' <<< "${output}" + grep -q 'rechunker-group-fix.service' <<< "${output}" +}