add exception to acl.kdump for qemu + aarch64 - #29
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the enforced kola test policy to allow acl.kdump-related tests to be exempted on QEMU + aarch64, acknowledging that kdump validation under TCG emulation is not reliably actionable and that arm64 coverage is handled via Azure.
Changes:
- Add an exception for
acl.kdumponplatforms: [qemu]+architectures: [aarch64]. - Add an exception for
acl.kdump.grubonplatforms: [qemu]+architectures: [aarch64].
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Mayank Singh <mayansingh@microsoft.com>
7be14bb to
1d8fcaf
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
acl/tests/kola_enforcing.yaml:66
- The PR title/description says this change only adds a QEMU+aarch64 exception for
acl.kdump, but this diff also re-enables bothacl.kdumpandacl.kdump.grubin the enforced list and adds a matching QEMU+aarch64 exception foracl.kdump.grub. Please update the PR description (and optionally the title) to reflect the full scope so reviewers understand the behavioral change in enforcement.
- name: acl.kdump
exceptions:
- bootloader: [grub]
reason: acl.kdump requires UKI boot (addon-based crashkernel)
- platforms: [qemu]
architectures: [aarch64]
reason: Crash dump under TCG emulation is too slow and I/O-variable to be reliable, aarch64 coverage via Azure.
- name: acl.kdump.grub
exceptions:
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
acl/tests/kola_enforcing.yaml:70
- The PR title/description says this change adds an exception to
acl.kdumpforqemu+aarch64, but the new exception is attached toacl.kdump.grub(and this hunk also re-enables bothacl.kdumpandacl.kdump.grubin the enforcing list). Please either (a) move/add theqemu/aarch64exception underacl.kdumpif that’s what is intended, or (b) update the PR title/description/changelog to reflect that the exception is foracl.kdump.grub(and that both tests are being re-enabled).
- name: acl.kdump.grub
exceptions:
- platforms: [azure]
reason: writing /oem/grub.cfg does not persist across reboot on cloud platforms.
- bootloader: [uki]
reason: acl.kdump.grub is for GRUB images only
- platforms: [qemu]
architectures: [aarch64]
reason: Crash dump under TCG emulation is too slow and I/O-variable to be reliable.
c249e93 to
d978ea5
Compare
d978ea5 to
57555f8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
acl/tests/kola_enforcing.yaml:72
- The PR title/description mention adding an exception to
acl.kdump, but this change also adds a new qemu+aarch64 exception foracl.kdump.grub. If both are required, the PR description should mention both test entries; otherwise consider dropping theacl.kdump.grubchange to match the stated scope.
- name: acl.kdump.grub
exceptions:
- platforms: [azure]
reason: writing /oem/grub.cfg does not persist across reboot on cloud platforms.
- bootloader: [uki]
reason: acl.kdump.grub is for GRUB images only
- platforms: [qemu]
architectures: [aarch64]
reason: Crash dump under TCG emulation is too slow and I/O-variable to be reliable.
acl/tests/kola_enforcing.yaml:62
- The PR title/description call out an exception for qemu+aarch64, but this rule disables
acl.kdumpfor all qemu architectures. If the intent is to only exempt the TCG-emulated arm64 case (as described), scope this exception witharchitectures: [aarch64](or update the PR description if the broader exemption is intentional).
- platforms: [qemu]
reason: Crash dump cycle is too slow and I/O-variable for qemu CI.
| exceptions: | ||
| - bootloader: [grub] | ||
| reason: acl.kdump requires UKI boot (addon-based crashkernel) | ||
| - platforms: [qemu] |
There was a problem hiding this comment.
Are we skipping for qemu both archs here, but only aarch64 for the grub flavor of the test on purpose?
There was a problem hiding this comment.
yes, if we want we can enable acl.kdump for qemu+amd64. thanks
There was a problem hiding this comment.
if that works reliably, then lets have it enabled consistently
There was a problem hiding this comment.
No worries, have enabled the test for qemu + amd64.
pipeline runs ->
https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1179467&view=results
https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1179477&view=results
|
Kicked off https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1177926&view=results to make sure we have a good run post reintroduction or if we should merge microsoft/azure-container-linux-mantle#25 first. |
QEMU aarch64 kdump is still slow sometimes, so I think we will need the mantle changes first: |
Agreed |
|
Latest run with the changes (acl-scripts + mantle) -> https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1178396&view=results |
57555f8 to
c551a31
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (1)
acl/tests/kola_enforcing.yaml:73
- PR title/description say this change is only an exception for
acl.kdumpon qemu+aarch64, but this hunk also re-enablesacl.kdump/acl.kdump.grubenforcement (previously commented out) and adds the same qemu+aarch64 exception toacl.kdump.grub. If this broader scope is intended, the PR metadata should be updated to reflect it; otherwise consider limiting the change to just the intended test(s).
- name: acl.kdump
exceptions:
- bootloader: [grub]
reason: acl.kdump requires UKI boot (addon-based crashkernel)
- platforms: [qemu]
architectures: [aarch64]
reason: Crash dump under TCG emulation is too slow and I/O-variable to be reliable.
- name: acl.kdump.grub
exceptions:
- platforms: [azure]
reason: writing /oem/grub.cfg does not persist across reboot on cloud platforms.
- bootloader: [uki]
reason: acl.kdump.grub is for GRUB images only
- platforms: [qemu]
architectures: [aarch64]
reason: Crash dump under TCG emulation is too slow and I/O-variable to be reliable.
| # reason: writing /oem/grub.cfg does not persist across reboot on cloud platforms. | ||
| # - bootloader: [uki] | ||
| # reason: acl.kdump.grub is for GRUB images only | ||
| - name: acl.kdump |
There was a problem hiding this comment.
Blocking merge order: these entries re-enable kdump enforcement, but the mantle-side enabling/managed-reboot change is still only in microsoft/azure-container-linux-mantle#25 (open). Build 1179466 consumed acldevel.azurecr.io/mantle:latest; its log shows an intermediate PR #25 implementation (acl.kdump.grub still fails at kdump.go:131 on UKI), rather than mantle aclmain or PR #25 current head. If this lands first and mantle:latest is rebuilt from mantle aclmain, both tests are again excluded for ACL and the evaluator can fail with "enforced test NOT SELECTED". Please merge #25 first, refresh the mantle image, rebase this branch, and rerun PR #29.
Summary
add exception to acl.kdump for qemu + aarch64
Change Log
Type of Change
Does this affect the image build?
Associated Issues
Merge Checklist
All applicable boxes should be checked before merging