update: Skip bootloader update when no block devices back the root#1072
update: Skip bootloader update when no block devices back the root#1072cgwalters wants to merge 2 commits intocoreos:mainfrom
Conversation
In environments without block-backed boot filesystems (virtiofs in bcvk ephemeral, NFS root, ISO boot, etc.) there is no on-disk bootloader to manage. Previously bootloader-update.service would fail because get_devices() bailed when it could not find a block device from /boot or /sysroot. Change get_devices() to return Ok(None) instead of an error when no block-backed filesystem is found, and propagate this through prep_before_update() so the update and adopt-and-update paths exit cleanly with an informative message. The EFI validate path also skips gracefully in this case. This is more general than checking for specific filesystem types: any environment without backing block devices is handled, which also prepares for the transition to list_dev_current_root() in PR coreos#1068. Assisted-by: OpenCode (Claude Opus 4)
Add a CI job that builds the container image (using Fedora 43, where bootloader-update.service is enabled by preset) and boots it via bcvk ephemeral (direct kernel boot with virtiofs root). The test verifies that the service ran successfully at boot and that manual bootupctl update also skips cleanly. Assisted-by: OpenCode (Claude Opus 4)
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of bootupctl update failing in environments without block-backed root filesystems, such as ephemeral VMs using virtiofs. The core change to make get_devices() return an Option is well-implemented and propagated correctly to the callers, allowing them to gracefully skip the update. The addition of ci/ephemeral-test.sh is a great way to ensure this new behavior is tested. I have one main concern about the completeness of the fix regarding other components, which I've detailed in a specific comment.
| let Some(devices) = crate::blockdev::get_devices("/")? else { | ||
| return Ok(ValidationResult::Skip); | ||
| }; |
There was a problem hiding this comment.
This change correctly handles the case where no block devices are found for the EFI component's validation. It's highly likely that a similar change is needed for the validate implementation in bios.rs and any other Component implementations that call get_devices(). Without updating all call sites, bootupctl validate could still fail for other components (like BIOS) in the exact ephemeral environments this PR aims to support.
| run: sudo podman build --build-arg=base=quay.io/fedora/fedora-bootc:43 -t localhost/bootupd:latest -f Dockerfile . | ||
| - name: Smoke test (bcvk ephemeral) | ||
| timeout-minutes: 10 | ||
| run: bcvk ephemeral run-ssh localhost/bootupd:latest -- /usr/libexec/bootupd-tests/ephemeral-test.sh |
There was a problem hiding this comment.
Overall LGTM, maybe you should add sudo to run bcvk, as image localhost/bootupd:latest is built with sudo
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
2 participants
Fix the problem that
bcvk ephemeral run quay.io/fedora/fedora-bootc:43shows a systemd error by default.