firmware: don't reboot into a half-applied major upgrade - #10647
Closed
cclements wants to merge 1 commit into
Closed
firmware: don't reboot into a half-applied major upgrade#10647cclements wants to merge 1 commit into
cclements wants to merge 1 commit into
Conversation
Two spots treated destructive upgrade stages as if they cannot fail: upgrade.sh checked for a pending kernel with 'opnsense-update -K -c', installed it, discarded the result and rebooted no matter what. The kernel install moves /boot/kernel out of the way, consumes the pending marker and extracts the new kernel in place, so a failed extraction (disk full, I/O error) leaves no bootable default kernel and nothing to retry. Rebooting at that point strands the machine at the loader prompt, which on a remote firewall means a console visit. Now the reboot only happens when no kernel was pending or the apply succeeded; a failed apply falls through to the existing abort path, which also clears the deferred sets so the boot-time hook will not pile base and packages on top. The early syshook looped K, B and P, rebooting after the first stage that reported success. Since a stage exits 1 both when nothing is pending and when the apply blows up, a broken kernel stage was indistinguishable from a no-op and the loop happily applied base or packages on top of it and rebooted. Keep exit 1 as "nothing pending" and treat any other failure as an aborted apply and stop. Older opnsense-update versions only exit 0 or 1, so behaviour there is unchanged; the companion opnsense/update change makes failed applies exit 2 (and restores the previous kernel). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
|
Not without a structured report and discussion. I’ll allow changes to opnsense-update if the scope fits but it’s a bit silly to jump to conclusions here and change reliable code. |
Author
|
Fair enough! I'm just looking to contribute back where I have the opportunity and I try to be careful and mindful of not creating busywork. I'm a happy user and appreciate this project. |
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
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.
Two places in the major upgrade path assume the destructive stages cannot fail, and
both of them reboot afterwards.
The first is upgrade.sh. It checks for a pending kernel with
opnsense-update -K -c,applies it with
opnsense-update -K, throws the result away and reboots regardless.The apply is the dangerous part: opnsense-update consumes the pending marker first,
moves /boot/kernel to kernel.old and untars the new kernel straight into place. If
the extraction dies halfway (out of space, I/O error) you are left with a partial
/boot/kernel, no marker to retry from, and a reboot already on its way. The loader
boots /boot/kernel by default, so a remote box comes back... to the loader prompt,
waiting for someone with console access to type
boot kernel.old. After this changethe reboot only happens when there was no pending kernel or the apply came back
clean; a failure drops through to the existing abort path, whose
opnsense-update -esconveniently also flushes the deferred sets, so the boot hookbelow won't try to finish the job on top of the damage.
The second is the early syshook, 05-upgrade. It loops K, B, P and reboots after the
first stage that reports success. The problem is that
opnsense-update -Kexits 1both when there is nothing pending (the normal every-boot case) and when the apply
actually blew up, so the loop cannot tell "no kernel to do, move along" from "the
kernel is now in pieces" — and in the latter case it will happily install base and
packages on top and reboot.
I first tried gating the loop with
-c, but that has its own trap: RELEASE is bakedinto the opnsense-update binary at build time, so on the first boot of a major
upgrade the still-old updater sees a version mismatch and
-K -creports true withnothing pending, which would have broken the normal flow. So the loop now keeps
exit 1 as "nothing pending" and treats anything else as a failed apply and stops.
Current opnsense-update only ever exits 0 or 1, so behaviour with today's updater is
completely unchanged; the companion PR (opnsense/update#106) makes failed applies
exit 2 and, more importantly, puts the old kernel back when the install fails.
Also added a
breakafter the reboot call so we stop feeding stages to a systemthat is already going down.
🤖 Generated with Claude Code