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
![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
Adopt the same trusted-publishing approach used by bootc-dev/bootc so that cargo publish happens automatically when a v* tag is pushed, with no stored API tokens needed. The new crates-release.yml workflow obtains an OIDC token via rust-lang/crates-io-auth-action and publishes idempotently (skips if the version is already on crates.io). The release checklist is updated to note the automation, remove the now-unnecessary crates.io account requirements, and fix the vendor tarball extension (.tar.gz → .tar.zstd). One-time setup: configure a trusted publisher on crates.io for the bootupd crate (owner: coreos/bootupd, workflow: crates-release.yml). Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
|
Added one more check |
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 the update path would fail because list_dev_current_root() bailed when it could not find a block device from /boot or /sysroot. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
cgwalters
force-pushed
the
ephemeral-noupdate
branch
from
32258d5 to
218236d
Compare
|
This won't work on Live systems. Here is my branch with fix, mind checking it? Logs from live-iso: Logs from qemu: |
3 participants
Fix the problem that
bcvk ephemeral run quay.io/fedora/fedora-bootc:43shows a systemd error by default.