firmware: never leave the board without a kernel on switch/repository change - #977
firmware: never leave the board without a kernel on switch/repository change#977igorpecovnik wants to merge 2 commits into
Conversation
github-actions
Bot
added
08
|
Warning Review limit reached
Next review available in: 5 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughKernel switching now installs all requested packages before removing stale kernel packages. Cleanup uses exact package names and excludes the new kernel. Installation failures preserve the current kernel and return an error. The command verifies an installed kernel and matching boot image. Repository switching now checks kernel reinstallation results and returns a warning and nonzero status when reinstallation fails. Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/modules/system/module_armbian_firmware.sh`:
- Around line 267-280: Before the stale-package cleanup loop, validate that
linux-image-${branch}-${linuxfamily} is installed and has a boot image, using
the existing kernel verification mechanisms. If validation fails, return an
error immediately and do not run apt-get autopurge; only proceed with cleanup
after confirming the replacement image is available.
- Line 279: Replace apt-get autopurge with apt-get purge in the scoped cleanup
command for the stale package list, preserving the existing noninteractive
flags, output redirection, and error handling so only explicitly listed packages
are removed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 26f99005-2cb2-4e5b-ab13-3daeb93d8c31
📒 Files selected for processing (1)
tools/modules/system/module_armbian_firmware.sh
| # New kernel is on disk. Now prune only the OTHER kernel packages a | ||
| # branch/family switch leaves behind (e.g. current -> edge), by EXACT | ||
| # name and explicitly excluding what we just installed. NEVER a | ||
| # 'linux-image*' wildcard — that also matches the kernel we just put on | ||
| # and is exactly what used to delete the running kernel. | ||
| local keep=" " | ||
| for pkg in ${packages[@]}; do keep+="${pkg%%=*} "; done | ||
| local stale=() | ||
| while IFS= read -r ipkg; do | ||
| [[ -n "$ipkg" && "$keep" != *" $ipkg "* ]] && stale+=("$ipkg") | ||
| done < <(dpkg-query -W -f='${Package}\n' 'linux-image-*' 'linux-dtb-*' 'linux-headers-*' 2>/dev/null) | ||
| if [[ ${#stale[@]} -gt 0 ]]; then | ||
| DEBIAN_FRONTEND=noninteractive apt-get autopurge -y "${stale[@]}" > /dev/null 2>&1 || true | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Require a replacement image before stale cleanup.
The show command can omit an unavailable linux-image version while retaining linux-dtb or headers. In that case, keep contains no replacement image package. Cleanup can then remove every existing linux-image package. The final check runs after this removal.
Before Line 267, verify that linux-image-${branch}-${linuxfamily} is installed and has a boot image. Return an error before cleanup if the verification fails.
Proposed fix
if ! DEBIAN_FRONTEND=noninteractive apt-get install --allow-downgrades -y ${packages[@]} > /dev/null 2>&1; then
rm -f /etc/apt/preferences.d/armbian-upgrade-policy
echo "Error: kernel install failed — current kernel left in place. Try again later and report to the Armbian forums."
return 1
fi
+ local target_image="linux-image-${branch}-${linuxfamily}"
+ if ! dpkg-query -W -f='${db:Status-Status}\n' "$target_image" 2>/dev/null | grep -qx 'installed'; then
+ echo "Error: replacement kernel image is not installed — current kernel left in place."
+ return 1
+ fi
+
# New kernel is on disk. Now prune only the OTHER kernel packages a🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tools/modules/system/module_armbian_firmware.sh` around lines 267 - 280,
Before the stale-package cleanup loop, validate that
linux-image-${branch}-${linuxfamily} is installed and has a boot image, using
the existing kernel verification mechanisms. If validation fails, return an
error immediately and do not run apt-get autopurge; only proceed with cleanup
after confirming the replacement image is available.
… change
Switching kernel branch or repository (e.g. stable -> rolling) could strand
the board with NO kernel — seen after a rolling switch left the SD card with
the kernel packages missing, so the board no longer booted.
The install path removed the old kernel BEFORE the new one was on disk:
for pkg in $packages; do pkg_remove 'linux-image*' ...; done # purge first
pkg_install $packages # then install
Three ways that bricks a board:
* the purge runs first, so there is a window with zero kernels installed —
any interruption or failure in between leaves nothing bootable;
* the 'linux-image*' wildcard also matches the kernel about to be installed;
* pkg_install returns an unreliable exit code in the dialog-gauge path (the
gauge pipeline masks apt's real status), so a failed install right after
the purge looked like success.
Reorder to be brick-safe (downloads already happen first, unchanged):
1. install the NEW kernel first, as one apt-get transaction from the local
cache, with a trustworthy exit code — apt replaces same-named packages in
place so a bootable kernel is present at every step; on failure the current
kernel is left untouched and we bail out;
2. prune only the OTHER kernels a branch/family switch leaves behind, by
EXACT name excluding what we just installed — never a 'linux-image*'
wildcard;
3. assert a bootable kernel remains (installed linux-image package + an image
in /boot); fail loudly instead of letting the caller reboot into a dead
system.
The repository command now propagates an install failure (source list switched
but kernel could not be reinstalled from the new repo) instead of reporting a
clean switch.
Signed-off-by: Igor Pecovnik <igor@armbian.com>
58dffda to
3de6c93
Compare
…ernel Switching to a repository that publishes no kernel for this board (e.g. a rolling/beta that has none built yet) used to report success: the source list was rewritten and `install` found a package because `apt-cache show` also reports the currently-INSTALLED package, masking that the repo is empty. The board was then left pointing at a repo it can neither be reinstalled from nor upgraded against. Before committing to a switch, verify the TARGET repo actually offers the kernel using `apt-cache madison` (which lists only repository-provided versions, not the installed one). If it offers none, revert the source list to the previous mirror host and return non-zero — the board stays on its working repo and kernel. Also fold the two mirror-host branches into a prev_host/target_host mapping. Signed-off-by: Igor Pecovnik <igor@armbian.com>
1 participant
Problem
Switching kernel branch or repository (e.g. stable → rolling) could strand the board with no kernel at all. Seen in the lab: after a rolling switch the SD card had its kernel packages missing and the board no longer booted.
The install path in
module_armbian_firmwareremoved the old kernel before the new one was on disk:Three ways that bricks a board:
linux-image*wildcard also matches the kernel about to be installed.pkg_installreturns an unreliable exit code in the non-readdialog-gauge path (the gauge pipeline masks apt's real status), so a failed install right after the purge looked like success.Fix
Reorder to be brick-safe (the download-first step is unchanged):
apt-gettransaction from the local cache, with a trustworthy exit code — apt replaces same-named packages in place so a bootable kernel is present at every step; on failure the current kernel is left untouched and we bail out.linux-image*wildcard.linux-imagepackage + an image in/boot); fail loudly instead of letting the caller reboot into a dead system.The
repositorycommand now propagates an install failure (source list switched but kernel could not be reinstalled from the new repo) instead of reporting a clean switch the caller would reboot into.Verified:
bash -nclean; stale-selection logic unit-tested (branch switch prunes only the old branch; same-branch version bump prunes nothing).Related
Companion safety net on the test side: armbian/autotests#92 stops the pipeline when a board never returns from a reboot/power-cycle.