kola: restore ACL-GRUB coverage for kargs/cgroupv1/fips tests - #30
Merged
bfjelds (bfjelds) merged 6 commits intoJul 24, 2026
Conversation
added 6 commits
July 23, 2026 20:23
The AB-update PR restricted cl.ignition.kargs and misc.cgroup1 (cl.cgroupv1) to Distros: ["cl"] only, reasoning that ACL uses UKI and these grub.cfg-based mechanisms do not apply there. True for ACL-UKI, but ACL-GRUB still boots via grub.cfg and still supports ignition kernel argument injection the same way CL does - that coverage was dropped instead of split. Restore Distros: ["acl", "cl"] for both tests and add a runtime skip (test -d /boot/EFI/Linux) so they self-skip on UKI-booted images instead of being unconditionally gated out for all of ACL, mirroring the existing acl.kdump / acl.kdump.grub split. Similarly split acl.misc.fips: keep it as the UKI-addon-swap variant (fails fast if run against a GRUB image), and add acl.misc.fips.grub which activates FIPS via ignition kernel_arguments (fips=1), matching how CL does it, self-skipping on UKI.
acl.kdump and acl.misc.fips are both registered with Distros: ["acl"] alongside their .grub counterparts, so both are scheduled on every ACL kola run regardless of boot mode. Hitting the wrong-boot-mode branch is therefore the routine, expected outcome on a GRUB run - not a scheduling error - so Fatalf there made every ACL-GRUB kola run permanently fail acl.kdump and (as introduced in this branch) would have made it fail acl.misc.fips too. Use Skip instead, matching the .grub variants intent.
Remove verbose comments above Distros field entries and inside test bodies (self-skip checks, pre-register.Register blocks in fips.go). Rely on the code and Skip messages to communicate intent.
Mirror the existing acl.kdump/acl.kdump.grub one-liner style for the other UKI/GRUB self-skip checks.
…orruptionCheck cl.verity/corruption intentionally corrupts /usr and expects systemd-veritysetup@usr.service to fail. The existing verity-suppression only covered the cascading 'Dependency failed for veritysetup.target' line (added alongside NoVerityCorruptionCheck); it missed the root 'Failed to start systemd-veritysetup@usr.service' line that precedes it, which is only gated by NoEmergencyShellCheck (not set on cl.verity). This made cl.verity fail its parent test deterministically whenever the corrupted machine's console reached that log line before becoming unreachable, even though the verify and corruption subtests both passed. Mirror the same skipIfMatch + dedicated-check pattern already used for Dependency failed for (.*verity.*) so Failed to start (.*verity.*) is consistently gated by NoVerityCorruptionCheck too.
cl.verity/corruption intentionally corrupts /usr and expects the machine to become unreachable/emergency-shell-adjacent - that is the whole point of the test, already verified directly via SSH in VerityVerify/ VerityCorruption. The console-check layer does not need to independently re-verify this via text pattern matching. The prior approach (skipIfMatch + dedicated NoVerityCorruptionCheck-gated checks mirroring 'Dependency failed for'/'Failed to start') is fragile here specifically: the corrupted machine's own console output can itself get garbled (observed literal U+FFFD replacement bytes splitting 'veritysetup' mid-word), which breaks the '.*verity.*' substring match those checks rely on, letting the check fire anyway. Adding NoEmergencyShellCheck directly to cl.verity's Flags sidesteps all of that - it covers the emergency-shell text check, the generic 'Failed to start' check, and the generic 'Dependency failed for' check unconditionally, regardless of exact wording or console corruption, for both the legacy inline-hash and new dedicated-hash-partition verity schemes.
bfjelds (bfjelds)
marked this pull request as ready for review
July 24, 2026 17:28
bfjelds (bfjelds)
merged commit Jul 24, 2026
9e4bfe4
into
user/bfjelds/adjust-acl-verity-tests-for-abupdate-20260708
2 checks passed
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 to #27. That branch restricted
cl.ignition.kargsandmisc.cgroup1(cl.cgroupv1) toDistros: ["cl"]only, reasoning ACL uses UKI and these grub.cfg-based mechanisms don't apply. True for ACL-UKI, but ACL-GRUB still boots via grub.cfg and still supports ignition kernel argument injection the same way CL does -- that coverage was dropped entirely instead of split.Changes
Distros: ["acl", "cl"]forcl.ignition.kargsandmisc.cgroup1, with a runtime skip (test -d /boot/EFI/Linux) so they self-skip on UKI-booted images instead of being gated out for all of ACL -- mirroring the existingacl.kdump/acl.kdump.grubsplit.acl.misc.fips: keep it as the UKI-addon-swap variant (fails fast if run against a GRUB image), and addacl.misc.fips.grubwhich activates FIPS via ignitionkernel_arguments(fips=1), matching how the CL test does it, self-skipping on UKI.cl.verity-verify/GetUsrHashDeviceNodeinkola/tests/misc/verity.goandkola/tests/util/update.goneeded no changes -- they already assume a dedicated hash-device scheme for anyacldistro image, which is now correct for both UKI and GRUB once azure-container-linux#36 lands.