Skip to content

fix(boot): break rechunker ordering cycle - #495

Closed
castrojo wants to merge 10 commits into
projectbluefin:mainfrom
castrojo:fix/466-rechunker-ordering
Closed

fix(boot): break rechunker ordering cycle#495
castrojo wants to merge 10 commits into
projectbluefin:mainfrom
castrojo:fix/466-rechunker-ordering

Conversation

@castrojo

@castrojo castrojo commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #466. Adds an LTS systemd drop-in to remove the local-fs.target ordering cycle from rechunker-group-fix.service while retaining its ordering before systemd-sysusers. Focused assertions and git diff --check pass; bats, just, and systemd-analyze are unavailable locally. Assisted-by: Goose

Reset the common service ordering that waits for local-fs.target and anchor the migration repair at local-fs-pre.target before systemd-sysusers. Add regression coverage for the drop-in.

Assisted-by: Goose

@hanthor hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean, well-scoped fix. This supersedes #494.

What I checked:

  • Drop-in resets both After= and Wants= (unlike #494 which only resets After=)
  • Adds Before=local-fs-pre.target to put the service outside the cycle
  • Retains After=bootc-sysusers-shadow-sync.service and Before=systemd-sysusers.service
  • Test verifies the resets, ordering, and absence of local-fs.target
  • Matching fix already approved in common#921

Recommend closing #494 in favor of this PR since this is the more complete version of the same approach.

@hanthor

hanthor commented Aug 7, 2026

Copy link
Copy Markdown
Member

Diagnosed the bats failure — it's self-inflicted and one line to fix:

not ok 35 rechunker ordering: does not add local-fs.target fails because the test greps the whole drop-in for (^|[[:space:]])local-fs\.target([[:space:]]|$), and line 2 of the comment in ordering.conf contains local-fs.target ("…but its local-fs.target ordering creates a cycle…"). The actual directives are fine (local-fs-pre.target doesn't match). That's also why #494 passed bats with the same directive approach.

Recommended fix in tests/unit/rechunker_ordering_test.bats — make the test ignore comment lines:

@test "rechunker ordering: does not add local-fs.target" {
    run bash -c "grep -v '^#' '${DROP_IN}' | grep -E '(^|[[:space:]])local-fs\.target([[:space:]]|\$)'"
    [ "$status" -ne 0 ]
}

The "Lint & syntax" failure on this PR is the repo-wide hadolint DL3063 breakage, fixed by #501 — re-running after that merges should clear it. With those two, this is green and it's the designated fix for #466 (#494 and #471 are closed as superseded).


Generated by Claude Code

github-actions Bot and others added 7 commits August 27, 2026 17:54
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
@kubestellar-hive

Copy link
Copy Markdown
Contributor

CI diagnosis: shared infrastructure failure, not PR code. Compose succeeded through Buildah, then push failed at https://github.com/projectbluefin/bluefin-lts/actions/runs/33409012764/job/99543629858 with denied: installation not allowed to Write organization package; wrapper reports podman exited with code 125. PR comes from fork castrojo/bluefin-lts-1, so pull_request GITHUB_TOKEN cannot write projectbluefin GHCR package. No safe branch/code fix or blind retry justified.

🐝 Hive Agent: scanner | Instance: hosted-projectbluefin-knuckle-gjvq | SHA: unknown

— hive: agent=scanner backend=copilot model=gpt-5.6-luna

@fabsah

fabsah commented Sep 1, 2026

Copy link
Copy Markdown

Thanks for this fix — I'm the reporter of #466. Heads-up from testing the same approach locally on bluefin-lts (systemd 257):

  1. Empty-assignment resets in a drop-in did not take effect here. I first tried After= (empty) + re-adds in a drop-in; after daemon-reload, systemctl show rechunker-group-fix.service -p After still listed local-fs.target and systemd-analyze verify still reported the cycle — I ended up shipping a full unit shadow instead. Since this PR relies on the same After=/Wants= reset mechanism, and the bats tests grep the file rather than exercise systemd, it's worth verifying on a real box that systemctl show -p After -p Wants no longer lists local-fs.target and systemd-analyze verify default.target is cycle-free.

  2. Anchor formulation: After=local-fs-pre.target re-anchoring still left 2 cycles in my testing (local-fs-pre → rechunker → sysusers → tmpfiles-setup-dev → local-fs-pre). This PR's Before=local-fs-pre.target formulation is acyclic in my measurements — good — but it all hinges on the resets from (1) landing.

  3. ExecStart side effect: with the unit no longer ordered after local-fs, its ExecStart=systemd-tmpfiles --create --remove --boot runs before /var is mounted and fails on read-only paths (unit then shows failed each boot). Prefixing that line with - works locally; the stock systemd-tmpfiles-setup.service covers the work after local-fs — may be worth folding in here.

For reference: the full-unit-shadow equivalent of this fix has run on my machine since Aug 17 — verify 16 → 0 cycle lines, 4+ clean boots, 41–43 s, no regressions. Happy to test this PR branch on hardware (drop my shadow, apply the drop-in, run verify + several reboots) — just say the word.

sec-check[bot] added 2 commits September 1, 2026 08:02
Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
@castrojo

castrojo commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by #527, which merged the rechunker ordering drop-in and its regression coverage. This branch now conflicts because the equivalent fix is already on main.

@castrojo castrojo closed this Sep 5, 2026
auto-merge was automatically disabled September 5, 2026 02:06

Pull request was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

intermittent BLOCKING boot hang (rechunker-group-fix ordering cycle)

4 participants