ci: harden the kvm_x86_64 build/boot workflow and boot harness - #1137
Open
bhouse-nexthop wants to merge 1 commit into
Open
ci: harden the kvm_x86_64 build/boot workflow and boot harness#1137bhouse-nexthop wants to merge 1 commit into
bhouse-nexthop wants to merge 1 commit into
Conversation
Follow-up fixes to the CI added in 1a24708, found in review by @mshych on PR opencomputeproject#1126. Grouped here because they all fix already-merged code rather than anything that PR adds. Workflow: - The cross-toolchain cache key omitted inputs that change the toolchain. xtools.make derives XTOOLS_VERSION from ONIE_ARCH and LINUX_RELEASE, which live in machine/kvm_x86_64/machine.make and kernel-download.make -- neither of which was hashed. A kernel bump therefore kept HITTING the key while the build/x-tools/<XTOOLS_VERSION> directory name changed underneath it: the restored toolchain was the wrong one, "make xtools" rebuilt from scratch, and the save step (gated on a cache miss) skipped it, so every later run paid the same rebuild with no signal saying why. Hash those inputs, and add a guard that warns when a cache hit still had to build a toolchain -- the symptom of a key that is missing an input. - push: and pull_request: were both unfiltered, running the whole pipeline twice for every push to a branch with an open PR. The concurrency group cannot collapse them because the two events carry different refs. Add a small "gate" job that skips the push run when an open PR in the same repository already covers the commit. This is a query rather than the simpler branch filter on push: because a branch filter also removes CI from every topic branch in a fork, which is where this workflow does most of its pre-submit work. A fork's pull requests are opened against the upstream repository, so they raise no pull_request event in the fork, and workflow_dispatch is unavailable there unless the workflow is also on the fork's default branch -- a filtered branch would be left with no CI at all. The query has no such blind spot: in a fork it finds no pull request and the push run proceeds. - Drop --privileged from both docker run invocations. The Dockerfile ends with USER build, so the build is unprivileged and cannot use any capability --privileged grants; the image build uses fakeroot and mtools, not loop mounts. - Add a workflow-level "permissions: contents: read" and per-job timeout-minutes. Nothing here writes to the repository, and a wedged build would otherwise burn the 6-hour default. Dockerfile: - groupadd -g $GID failed when the host GID already exists in debian:11, which the low GID range is full of (30 dip, 50 staff, 100 users). A developer whose primary group lands there -- common with central/NFS accounts, and the norm on macOS -- could not build the image at all, while CI never saw it because the runner's GID happens to be free. Reuse the group when the GID is taken, and chown by numeric id since the "build" group then does not exist. - The /sbin:/usr/sbin PATH addition never took effect. It was appended to ~/.bashrc, but the workflow runs "bash -lc" and Debian's default .bashrc returns early for non-interactive shells. The root cause is a level deeper: /etc/profile *replaces* PATH for non-root users with one that omits both sbin dirs, so ENV alone would be undone under bash -l. Set both ENV and an /etc/profile.d snippet, which is sourced after /etc/profile. emulation/ci-boot-test.sh: - The "ONIE userspace" milestone matched strings GRUB itself echoes from grub-iso.cfg ("ONIE: Rescue Mode ...", "Version :"), so it duplicated the "GRUB reached" check and proved nothing about userspace. Assert "Info: BIOS mode:" from init-arch instead, which GRUB cannot produce. - secureboot mode never checked that Secure Boot was actually enforced: if the varstore failed to enrol the guest booted with SB off and the "must boot" assertion passed for the wrong reason. Assert the guest's own "Info: Secure Boot: Active." (read from the SecureBoot EFI variable by init-arch), and assert its absence in the relaxed run. - A failed virt-fw-vars was silently ignored: it was piped through "| sed | grep ... || true", which dropped its exit status twice over. Move enrolment into a helper that fails the run. - A QEMU that never started was indistinguishable from a firmware that refused the image -- both left an empty serial log -- so the negative control could score a broken harness as a successful rejection. Keep QEMU's stderr and hard-fail when it exits immediately. Also capture the OVMF debug console when the device is available, as a second evidence channel on debug OVMF builds. - The documented accel=kvm:tcg fallback did not work for secureboot at -smp 2: without KVM the boot dies in rcu_sched stalls and burns the whole timeout, where -smp 1 boots in ~31s. Use one vCPU when /dev/kvm is not usable. CI has /dev/kvm and is unaffected. Signed-off-by: Brad House <bhouse@nexthop.ai>
bhouse-nexthop
force-pushed
the
bhouse.ci-review-fixes
branch
from
August 18, 2026 14:56
a286b37 to
24eeb15
Compare
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
1 participant
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.
Follow-up fixes to the CI added in
1a24708f, all found in review by @mshych on #1126. They are here rather than in that PR because they fix already-merged code, which is how he asked for them to be split.Every item was re-verified against the tree before being applied; the notes are his numbering.
Workflow
Cross-toolchain cache key omitted inputs that change the toolchain.
xtools.make:25derivesXTOOLS_VERSION = $(ONIE_ARCH)-g$(GCC_VERSION)-lnx$(LINUX_RELEASE)-..., butLINUX_RELEASEis defined inbuild-config/make/kernel-download.make:22andONIE_ARCHinmachine/kvm_x86_64/machine.make:10— neither was hashed. A kernel bump therefore keeps hitting the key while thebuild/x-tools/<XTOOLS_VERSION>directory name changes underneath it: the restored toolchain is the wrong one,make xtoolsrebuilds from scratch, and the save step (gated on a cache miss) skips it, so every later run pays the same rebuild. Silent and self-perpetuating.GCC_VERSION/XTOOLS_LIBCdefault incrosstool-ng.make, which is already hashed, so those two were the only gaps. Also adds his guard: a::warningwhen a cache hit still had to build a toolchain.This is the one worth landing before the kernel PR (kernel: move to the 6.18 LTS kernel #1130) does exactly this bump.
push:andpull_request:both unfiltered ran the pipeline twice for every push to a branch with an open PR; the concurrency group can't collapse them because the refs differ. Fixed, but not with the branch filter that was suggested — see the note at the bottom. A smallgatejob now skips the push run when an open PR in the same repository already covers the commit.--privilegeddropped from bothdocker runinvocations. The Dockerfile ends withUSER build, so the build is unprivileged and can't use anything--privilegedgrants. Confirmed the build needs no loop mounts — it goes throughfakerootandmtools, andonie-mk-iso.shhardcodes/sbin/mkdosfs.permissions: contents: readat workflow level plustimeout-minuteson each job (180/45/60).Dockerfile
groupadd -g $GIDfailed when the host GID already exists. Confirmed indebian:11:groupadd -g 30 build→rc=4, GID '30' already exists. The low range is full of system groups (30 dip, 50 staff, 100 users), which is common with central/NFS accounts and the norm on macOS (GID 20 = dialout there). CI never sees it because the runner's GID is free. Reuses the group when the GID is taken, and chowns by numeric id sincebuildthen isn't a group name. Test-built at GID 30 and GID 1001 — both work now.The
/sbin:/usr/sbinPATH line never took effect. It was appended to~/.bashrc, and Debian's default.bashrcreturns early for non-interactive shells, sobash -lcnever reaches it. As he noted,ENValone isn't enough either:/etc/profilelines 5–9 replace PATH for non-root users with one that omits both sbin dirs. Verified in the image —bash -lcas non-root gets/usr/local/bin:/usr/bin:/bin:/usr/local/games:/usr/gamesandblkidis off PATH. Fixed withENVplus an/etc/profile.dsnippet (profile.d is sourced at line 27, after the reset). Verified both login and non-login shells now resolve/sbin/blkid.emulation/ci-boot-test.sh
The "ONIE userspace" milestone was matched by GRUB output.
build-config/recovery/grub-iso.cfg:32echoes"ONIE: Rescue Mode ..."and:24echoes"Version : ...", both before the kernel loads — so the check duplicated "GRUB reached". Now assertsInfo: BIOS mode:, printed unconditionally fromrootconf/grub-arch/sysroot-lib-onie/init-arch:130.secureboot mode never checked that Secure Boot was enforced. Now asserts the guest's own
Info: Secure Boot: Active.(init-arch:121, read from the SecureBoot EFI variable via efivar), and asserts its absence on the relaxed run.A failed
virt-fw-varswas silently ignored — piped through| sed | grep ... || true, which dropped its status twice over. Enrolment moved into a helper that fails the run.A QEMU that never started could be scored as a successful SB rejection. QEMU's stderr is kept and an immediate exit is now a hard error. Verified with a stub QEMU that exits 1: the harness prints the stderr and exits 2, where before it reported
FAIL no serial output captured/PASS ONIE did not boot without db. Also captures the OVMF debug console whenisa-debugconis present — belt-and-braces, since as he corrected himself the release OVMF does put itsBdsDxe: ... Access Deniedline on ttyS0.One vCPU when
/dev/kvmis unusable, so the documentedaccel=kvm:tcgfallback works for local runs. CI has/dev/kvmand still gets two.Smoke-tested locally against real OVMF: logs are written as expected, the firmware console is empty on Ubuntu's release OVMF (as he predicted), and the milestone assertions behave correctly on a non-ONIE ISO.
Note on the duplicate-run fix
@mshych suggested restricting
push:to the long-lived branches, which is the usual answer. I took a different route because the filter has a blind spot that bites this exact patch series.A fork's pull requests are opened against the upstream repository, so they raise no
pull_requestevent in the fork; the fork's only trigger ispush. Filteringpushby branch name therefore leaves every topic branch in a fork with no CI at all, andworkflow_dispatchcannot fill the gap unless the workflow also exists on the fork's default branch (it does not — this workflow lives ononie-modernization-2026, and forks default tomaster).That is not hypothetical: with the branch filter applied, pushing this very branch to my fork produced zero runs, which is how I noticed.
The
gatejob targets the actual condition instead — "an open PR here already covers this commit" — so it drops the duplicate upstream while a fork, where the query finds nothing, keeps full push CI. Verified on the fork:No open pull request in bhouse-nexthop/onie for bhouse.ci-review-fixes; running.Onlybuilddepends on the gate; the other jobs chain from it and skip automatically.If you would rather have the simple branch filter, say so and I will switch it — the tradeoff is that fork-based development loses pre-submit CI.