fix: install bootupd and unblock the common E2E suite (#492) - #509
fix: install bootupd and unblock the common E2E suite (#492)#509hanthor wants to merge 2 commits into
Conversation
The common repo's post-merge E2E suite fails on every run against bluefin:lts-testing: - "bootupd is required for ostree-based installs" during bootc install to-disk - "bootupctl ... opening EFI dir: No such file or directory" from bootloader-update.service at boot Install bootupd explicitly in base.toml instead of relying on the centos-bootc base image shipping it, and regenerate the bootupd update payload during the build: the Containerfile mounts a tmpfs over /boot, which hides the base image's /boot/efi content, so the EFI bootloader packages are reinstalled to materialize their payload before running `bootupctl backend generate-update-metadata` (the result lands in /usr/lib/bootupd/updates, which is preserved in the image). The third E2E failure, "Cannot operate; LockLayering=true", comes from system_files/etc/rpm-ostreed.conf and is deliberate Bluefin policy (local layering is unsupported); it is intentionally left unchanged. Adds a bats regression test asserting bootupd stays in the install list. Fixes #492 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KSjBPVDcnK71WveusaK7Nc
castrojo
left a comment
There was a problem hiding this comment.
The approach matches the #492 diagnosis and the base.toml addition plus bats regression test look right — but the new build step breaks the image build itself. From the x86_64 build log (run 31239447088, job 93057856610), after the reinstall succeeds:
+ rm -rf /usr/lib/bootupd/updates
+ bootupctl backend generate-update-metadata
Generated update layout for BIOS: grub2-tools-1:2.12-54.el10.x86_64
Moving usr/lib/ostree-boot/efi/EFI to usr/lib/bootupd/updates/EFI
error: generating metadata failed: Failed to find "/usr/lib/ostree-boot/efi/EFI"
Error: building at STEP "RUN --mount=type=tmpfs,dst=/opt ...": while running runtime: exit status 1
Reinstalling grub2-efi-x64/shim-x64 materializes the EFI payload, but not at /usr/lib/ostree-boot/efi/EFI where bootupctl's generate-update-metadata looks for it. Both arch image builds (x86_64 and aarch64) and the nvidia build fail the same way, so this can't merge as-is — the fix needs to place the EFI tree where bootupctl expects it (or point bootupctl at the right source) before generate-update-metadata runs.
Two smaller notes: (1) Lint & syntax also fails here, but that's the pre-existing hadolint DL3063 on Containerfile:13 (fixed by #501), not this change. (2) The deliberate decision to leave LockLayering=true is well-reasoned in the PR body — agreed that belongs on the harness side.
|
@castrojo you were right that this can't merge, and digging into why turned up something worse: the build step this PR adds is not just broken, it's destructive — and the problem it's solving doesn't exist. 1. The base image already ships a complete bootupd payload. From 2. And so does the image we actually ship. From So both the 3. The rm -rf /usr/lib/bootupd/updates # deletes the CORRECT payload the base image ships
bootupctl backend generate-update-metadata # then fails, aborting the buildToday the build fails, which is loud and safe. If someone "fixed" this by making the last line non-fatal, the image would ship with no bootupd payload at all — strictly worse than doing nothing, and a silent boot-update failure rather than a build failure. 4.
Which is exactly the mismatch in your review — and it's unfixable by relocating files, because bootupd wants the ostree-boot staging tree that only exists mid-compose. Where this leaves #492. The stated cause — "bootupd missing, breaking Moving to draft. The right end state for this branch is almost certainly delete the whole build step and the I'll take #492 back to the E2E logs and re-diagnose from there. (The |
Pull request was converted to draft
… test The centos-bootc base image already ships bootupd with a complete payload under /usr/lib/bootupd/updates. The reinstall step rebuilt files bootupctl cannot use (compose-time one-shot), and the rm -rf of the shipped payload was destructive. Assert instead that build scripts never remove the payload or re-run generate-update-metadata. Assisted-by: Kimi K3 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Deep-dive assessment: I believe this PR's premise is invalid against the current image, and the right fix for #492 is elsewhere. bootupd and its EFI payload are already present in current builds. Verified by inspecting the images directly (x86_64):
The E2E failures in #492 were measured against a stale tag. Proposed direction instead of this PR:
Because the image is already correct, the |
Problem
The
commonpost-merge E2E suite fails againstbluefin:lts-testingon every run (#492) with three distinct errors:error: Installing to disk: bootupd is required for ostree-based installsbootupctl[1019]: error: Failed adopt and update: opening EFI dir: No such file or directory(→bootloader-update.servicefails at boot)error: Cannot operate; \LockLayering=true` in configuration`Changes
build_scripts/packages/base.toml— addbootupdto the[install]list. The LTS image never installed bootupd itself and relied on thequay.io/centos-bootc/centos-bootc:c10sbase shipping it; installing it explicitly guaranteesbootc install to-diskhas it regardless of base-image churn. This addresses failure 1. (Upstreamprojectbluefin/bluefindoes the same —bootupdis in itsbuild_files/packages/base.tomlinstall list.)build_scripts/26-packages-post.sh— populate bootupd's update payload at build time. The Containerfile mounts a tmpfs over/bootduring the build, which hides the base image's/boot/eficontent, so bootupd has no EFI files to work from. The script now reinstalls the arch-appropriate EFI bootloader packages (grub2-efi-x64/shim-x64, or-aa64on aarch64) to materialize their payload, then runsbootupctl backend generate-update-metadata, which copies it into/usr/lib/bootupd/updates— under/usr, so it is preserved in the final image. This addresses failure 2.tests/unit/packages_test.bats— regression test assertingbootupdstays in the[install]list.What was NOT changed, and why
LockLayering=true(failure 3) is left as-is. It is set by this repo insystem_files/etc/rpm-ostreed.conf, but the comment there marks it as deliberate Bluefin policy: local layering withrpm-ostree installis an unsupported configuration that can lead to upgrade issues. The E2E harness hits it because it usesrpm-ostree installto inject its test tooling into the VM; the right fix is on the harness side (e.g. flipLockLayering=falsein the VM before installing test tooling, or deliver tooling another way), not weakening the shipped image's policy.Fixes #492
🤖 Generated with Claude Code
https://claude.ai/code/session_01KSjBPVDcnK71WveusaK7Nc
Generated by Claude Code