AB update: fix ACL verity tests for partition layout changes by bfjelds · Pull Request #27 · microsoft/azure-container-linux-mantle

bfjelds (bfjelds) · 2026-07-10T16:00:01Z

Updates the kola test harness and verity tests to work with the ab-update partition layout, where the usr data device and hash device are separate (rather than hash appended at an offset within the same device).

Required PRs

Validation

acldevel (uki): https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1192019&view=results
acldevel (grub): https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1192018&view=results
acl-pre-release-tests; https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1186587&view=results
aks-e2e-tests: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1186588&view=results
acl-conformance-tests: https://dev.azure.com/mariner-org/mariner/_build/results?buildId=1186589&view=results

Changes

Update kola tests for the new partitioning and UKIs in ACL.

for ACL, discover the data and hash devices from veritysetup
for non-ACL, retain existing methods.

kola/harness.go
For verity-corruption tests, split the dependency failure serial scanning to differentiate between verity and non-verity failures:

The generic "Dependency failed for (.*)" console check now evaluates output line-by-line (new perLine mode) and skips only verity-related dependency-failure lines (skipIfMatch), so a non-verity dependency failure is still reported even when a verity one is present in the same output.
Add a dedicated check for verity dependency failures that routes them to NoVerityCorruptionCheck (so verity corruption tests can opt out) instead of the generic NoEmergencyShellCheck.

For kola tests, assume ACL images are UKI images, so tests that validate ignition kernel argument injection should be skipped:

cl.ignition.kargs: add Distros: ["cl"] (was ungated, so it previously ran on ACL).
cl.cgroupv1: Distros: ["acl","cl"] -> ["cl"] (cgroup-v1 karg is injected via GRUB, not applied on UKI).
acl.kdump.grub: skips at runtime on UKI-booted images.
acl.misc.fips: fix addon activation to discover the real UKI basename (vmlinuz-<kver>.efi) at runtime instead of the hardcoded acl.efi.

Copilot

Pull request overview

Updates Mantle’s kola verity verification and harness log handling to support the ACL ab-update partition layout where dm-verity uses separate data and hash block devices (instead of an appended hash region within one device).

Changes:

Update ACL /usr device discovery to parse veritysetup status output and add a helper to discover the hash device.
Split verity verification logic: ACL verifies using separate data/hash devices; non-ACL keeps --hash-offset verification on a single device.
Refine harness console-check rules so verity-related systemd dependency failures can be handled via NoVerityCorruptionCheck instead of the generic emergency-shell suppression.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
kola/tests/util/update.go	Switch `/usr` backing-device discovery for ACL to `veritysetup status` and add hash-device discovery helper.
kola/tests/misc/verity.go	Update verity verification flow for ACL’s separate data/hash devices while preserving non-ACL offset-based verification.
kola/harness.go	Add a verity-specific dependency-failure console rule and adjust the generic dependency-failure rule to defer when verity is involved.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.

Copilot

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (2)

kola/harness.go:243

Same issue for the dedicated verity-related "Dependency failed for" console check: it only matches lines containing "verity", but the truncated console output variant you already handle elsewhere can present only the "Integrity Protection Setup" tail. That case will be skipped by the generic dependency-failure check and missed here, suppressing the failure entirely and bypassing NoVerityCorruptionCheck behavior.

			desc:     "systemd dependency unit related to verity failed to start",
			match:    regexp.MustCompile("Dependency failed for (.*verity.*)"),
			skipFlag: &[]register.Flag{register.NoVerityCorruptionCheck}[0],

kola/tests/ignition/kernel.go:16

PR description says this test should be limited to Container Linux ("Distros: ["cl"]") because ACL images are assumed UKI and kargs injection is GRUB-based. The code currently whitelists both "acl" and "cl", which still schedules the test for ACL on any future GRUB-booted ACL images and also makes the "cl." prefix misleading. If the intent is to skip ACL entirely as described, restrict Distros to "cl".

		Name:        "cl.ignition.kargs",
		Distros:     []string{"acl", "cl"},
		Run:         check,

…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.

…/boot/EFI/Linux - harness.go: widen skipIfMatch regex for the generic "Failed to start" and "Dependency failed for" console checks to also match "Integrity Protection Setup" (the systemd unit Description= for veritysetup), since long unit names get elided with U+2026 on narrow consoles and can truncate "verity" itself mid-word. - verity.go: drop register.NoEmergencyShellCheck from cl.verity now that the generic checks correctly skip verity-related failures on their own; keeps the harness able to flag genuinely unrelated dependency failures instead of blanket-suppressing all emergency-shell checks. - kdump.go, fips.go: acl-activate-kdump.service and acl-activate-fips.service are UKI-only test scaffolding but previously only gated on ConditionKernelCommandLine=. On GRUB-booted ACL images that condition passes (no crashkernel/fips karg set), so the unit ran anyway, didn't find its UKI addon dir, and exited 1 -- failing the boot before the test's own runtime skip logic ever executed. Added ConditionPathExists=/boot/EFI/Linux so the unit itself skips cleanly on GRUB. Validated via full kola suites (grub_amd64, uki_amd64, parallel=60): acl.kdump/acl.misc.fips now skip cleanly on GRUB and pass on UKI; previously-flaky cl.ignition.*/coreos.ignition.* tests pass consistently once the arm64-style device-timeout override is also applied to amd64. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The generic "Failed to start"/"Dependency failed for" checks were widened to also match the "Integrity Protection Setup" tail that survives console truncation, but the dedicated verity-corruption checks (gated on NoVerityCorruptionCheck, active for every test except cl.verity) still only matched the literal "verity" substring. If a genuine verity-corruption failure line got truncated the same way, neither check would catch it: the generic check correctly defers to the dedicated one, but the dedicated one would never fire. Widen both dedicated match regexes the same way. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- kola/tests/util/uki.go: IsUki() now probes systemd-stub's volatile StubInfo EFI variable (Boot Loader Interface) instead of checking for installed UKI files under /boot/EFI/Linux. The directory can be populated on a GRUB boot too (e.g. while UKIs are staged/installed), which would misclassify the boot mode for every caller (kernel.go, kdump.go, fips.go, cgroup1.go). Mirrors the same StubInfo probe already used by kdumpGRUBTest. - kola/tests/misc/update.go: add register.NoVerityCorruptionCheck to cl.update.badverity's flags. This test intentionally corrupts the USR verity hash to exercise recovery; without this flag, harness.go's new dedicated verity-failure console check would flag the expected failure and could fail the test wherever it runs (it's Distros: cl / ExcludeDistros: acl, so it's not exercised by ACL's own pipeline, but the harness change is shared code). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

acl.disk.raid0.root/raid1.root wiped and relabeled a fixed by-id/virtio-primary-disk-partN device to clear the stale ROOT label left behind once /dev/md/rootarray takes over the ROOT label. The partition numbers (part5 for ACL/UKI, part9 for CL/GRUB) were already stale as of PR #28 (verity hash partitions renumbered ACL ROOT to part7 and CL ROOT to part11), so the test was wiping the wrong partition (ACL: HASH-B) instead of the real stale ROOT partition. Reference the target by partlabel (/dev/disk/by-partlabel/ROOT) instead of a hardcoded partition number, since both layouts label their ROOT partition ROOT. This is robust to any future renumbering. Reported-by: frhuelsz See: #27 (comment) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

bfjelds (bfjelds) · 2026-09-09T16:47:04Z

🔵 Needs a closer look

Boot-time UKI activation can still run on GRUB, and FIPS can target an inactive staged UKI.

Have fix, am testing now.

bfjelds (bfjelds) · 2026-09-09T18:26:26Z

rebased, included fix for copilot comment

Copilot

🟡 Changes recommended

Boot-mode detection can misroute tests, and the FIPS service can incorrectly run on GRUB boots.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 10/10 changed files
Comments generated: 3
Review effort level: Balanced

…d multi-UKI ESP - acl-activate-fips.service and acl-activate-kdump.service now gate on ConditionPathExistsGlob=/sys/firmware/efi/efivars/StubInfo-* instead of ConditionPathExists=/boot/EFI/Linux. The directory can exist on a GRUB boot too (e.g. while UKIs are installed/staged), so the boot-time unit could still run and mutate the ESP before the runtime IsUki() skip in fipsUKITest/kdumpUKITest ever gets a chance to fire. StubInfo matches the same signal already used by IsUki() and kdumpGRUBTest. - UKI_NAME discovery in both activation scripts now asserts exactly one vmlinuz-*.efi candidate on the ESP and fails loudly otherwise, instead of silently taking the first glob match (which defaulted to the fictitious acl.efi when no match existed at all). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

UKI discovery mishandles missing files, and SSH failures can be incorrectly reported as GRUB-based test skips.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

kola/tests/util/uki.go:33

IsUki maps an SSH transport/session failure to false, making it indistinguishable from a confirmed GRUB boot. In fipsUKITest, that turns a machine/SSH failure into c.Skip, masking the failure. Return the probe error (for example, (bool, error)) or accept a test cluster that can fail the probe, and have callers handle it before branching.

	out, _, err := m.SSH("ls /sys/firmware/efi/efivars/StubInfo-* >/dev/null 2>&1 && echo uki || echo grub")
	if err != nil {
		return false

Files reviewed: 10/10 changed files
Comments generated: 2
Review effort level: Balanced

- IsUki(m) now returns (bool, error) instead of silently treating an SSH/probe failure as a GRUB boot. All four call sites (fipsUKITest, fipsGRUBTest, kdumpUKITest, ignition/kernel.go check, cgroup1.go CgroupV1Test) now check the error and c.Fatalf on probe failure instead of masking it as a skip/GRUB-variant dispatch. - fips.go and kdump.go UKI activation scripts now shopt -s nullglob before building UKI_CANDIDATES=(...). Without it, an unmatched glob leaves the literal pattern string as a single array element, so the exactly-one-candidate check added previously could not detect the zero-UKI case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🟡 Changes recommended

Generic systemd failure checks no longer honor existing NoEmergencyShellCheck opt-outs for tests that intentionally trigger non-verity boot failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Files reviewed: 10/10 changed files
Comments generated: 1
Review effort level: Balanced

Address Copilot review: exported helper name used the established initialism spelling inconsistently. UKI is capitalized consistently elsewhere (kdumpUKITest, fipsUKITest); IsUki was the outlier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

🔵 Needs a closer look

Ignition test scheduling remains overly broad, and the PR description misstates two distro-gating changes.

Review details

Suppressed comments (3)

kola/tests/misc/cgroup1.go:57

The PR description says this registration is changed to Distros: ["cl"], but the implementation deliberately keeps ACL enabled and skips only UKI boots, so ACL-GRUB still runs. Update the description to document the implemented ACL-GRUB/UKI split; otherwise reviewers and test selectors are given the wrong scheduling behavior.

	// UKI images have no grub.cfg for ignition to inject the cgroup-v1 karg
	isUki, err := util.IsUKI(m)
	if err != nil {
		c.Fatalf("failed to probe boot mode: %v", err)
	}
	if isUki {
		c.Skip("cgroup-v1 karg injection is grub.cfg-based; not applicable on a UKI-booted image")

kola/tests/ignition/kernel.go:32

This runtime check only excludes UKI boots, while the registration still omits Distros (which defaults to all distributions in kola/register/register.go:55). As a result, this Flatcar/ACL-specific kernel-argument test remains scheduled on every non-UKI distro. Explicitly whitelist acl and cl so the intended ACL-GRUB coverage does not broaden the test to unrelated distributions.

	// UKI images have no grub.cfg for ignition to inject kargs into
	isUki, err := util.IsUKI(m)

kola/tests/ignition/kernel.go:37

The PR description says cl.ignition.kargs is now restricted to Distros: ["cl"], but this implementation preserves ACL-GRUB coverage and skips only UKI boots. Update the description to document that runtime split so it matches the implemented scheduling behavior.

	if isUki {
		c.Skip("ignition kernel_arguments injection is grub.cfg-based; not applicable on a UKI-booted image")

Files reviewed: 10/10 changed files
Comments generated: 0 new
Review effort level: Balanced

bfjelds (bfjelds) mentioned this pull request Jul 10, 2026

AB update: fix ACL verity tests for partition layout changes #26

Closed

bfjelds (bfjelds) requested a lite review from Copilot July 14, 2026 18:09

Copilot started reviewing on behalf of bfjelds (bfjelds) July 14, 2026 18:11 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread kola/tests/util/update.go

Comment thread kola/tests/util/update.go

bfjelds (bfjelds) requested a review from Copilot July 14, 2026 18:45

Copilot started reviewing on behalf of bfjelds (bfjelds) July 14, 2026 18:46 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

Comment thread kola/tests/util/update.go

Comment thread kola/tests/util/update.go

Comment thread kola/harness.go Outdated

bfjelds (bfjelds) requested a review from Copilot July 14, 2026 19:38

Copilot started reviewing on behalf of bfjelds (bfjelds) July 14, 2026 19:39 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

bfjelds (bfjelds) requested a review from Copilot July 22, 2026 18:29

Copilot started reviewing on behalf of bfjelds (bfjelds) July 22, 2026 18:29 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

Comment thread kola/tests/misc/kdump.go

Comment thread kola/tests/misc/fips.go

bfjelds (bfjelds) requested a review from Copilot July 22, 2026 19:11

Copilot started reviewing on behalf of bfjelds (bfjelds) July 22, 2026 19:12 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

bfjelds (bfjelds) marked this pull request as ready for review July 22, 2026 20:54

bfjelds (bfjelds) requested a review from a team as a code owner July 22, 2026 20:54

bfjelds (bfjelds) mentioned this pull request Jul 22, 2026

feat: enable ABUpdate for UKI-based ACL images microsoft/azure-container-linux#28

Open

Copilot AI review requested due to automatic review settings July 22, 2026 22:55

Copilot started reviewing on behalf of bfjelds (bfjelds) July 22, 2026 22:56 View session

Copilot AI reviewed Jul 22, 2026

View reviewed changes

bfjelds (bfjelds) mentioned this pull request Jul 23, 2026

kola: restore ACL-GRUB coverage for kargs/cgroupv1/fips tests #30

Merged

Copilot AI review requested due to automatic review settings July 24, 2026 17:30

Copilot started reviewing on behalf of bfjelds (bfjelds) July 24, 2026 17:32 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread kola/tests/ignition/kernel.go Outdated

Comment thread kola/tests/misc/fips.go

Comment thread kola/tests/misc/verity.go Outdated

Copilot AI review requested due to automatic review settings July 24, 2026 21:59

Copilot started reviewing on behalf of bfjelds (bfjelds) July 24, 2026 22:00 View session

Copilot AI reviewed Jul 24, 2026

View reviewed changes

Comment thread kola/harness.go Outdated

Copilot AI review requested due to automatic review settings July 24, 2026 22:54

Brian Fjeldstad and others added 10 commits September 9, 2026 15:04

remove new Distros directive

55b50bf

create helper for UKI detection

98ab20a

missed some usage for IsUki

1848104

kola/tests/util: update copyright header to Microsoft

1e33e61

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

fintelia previously approved these changes Sep 9, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings September 9, 2026 18:24

bfjelds (bfjelds) force-pushed the user/bfjelds/adjust-acl-verity-tests-for-abupdate-20260708 branch from 78be15c to 1e33e61 Compare September 9, 2026 18:24

Copilot started reviewing on behalf of bfjelds (bfjelds) September 9, 2026 18:24 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread kola/tests/misc/fips.go Outdated

Comment thread kola/tests/util/uki.go Outdated

Comment thread kola/tests/misc/kdump.go Outdated

Copilot AI review requested due to automatic review settings September 9, 2026 18:32

bfjelds (bfjelds) dismissed fintelia’s stale review via 42dee6c September 9, 2026 18:32

Copilot started reviewing on behalf of bfjelds (bfjelds) September 9, 2026 18:32 View session

Copilot AI reviewed Sep 9, 2026

View reviewed changes

Comment thread kola/tests/misc/fips.go

Comment thread kola/tests/misc/kdump.go

Copilot AI review requested due to automatic review settings September 10, 2026 00:17

Copilot started reviewing on behalf of bfjelds (bfjelds) September 10, 2026 00:18 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Comment thread kola/tests/util/uki.go Outdated

Copilot AI review requested due to automatic review settings September 10, 2026 01:13

Copilot started reviewing on behalf of bfjelds (bfjelds) September 10, 2026 01:14 View session

Copilot AI reviewed Sep 10, 2026

View reviewed changes

Uh oh!

Conversation

bfjelds (bfjelds) commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Required PRs

Validation

Changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

bfjelds (bfjelds) commented Sep 9, 2026

🔵 Needs a closer look

Uh oh!

bfjelds (bfjelds) commented Sep 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (1)

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (3)

bfjelds (bfjelds) commented Jul 10, 2026 •

edited

Loading