Skip to content

add exception to acl.kdump for qemu + aarch64 - #29

Open
mayankfz wants to merge 2 commits into
aclmainfrom
mayansingh/kdump
Open

add exception to acl.kdump for qemu + aarch64#29
mayankfz wants to merge 2 commits into
aclmainfrom
mayansingh/kdump

Conversation

@mayankfz

@mayankfz mayankfz commented Jul 10, 2026

Copy link
Copy Markdown

Summary

add exception to acl.kdump for qemu + aarch64

Change Log

  • add exception to acl.kdump for qemu + aarch64

Type of Change

  • Image build change (base image, sysexts, OEM images)
  • Package/SPEC update
  • CI/automation change
  • SDK/toolchain update
  • Configuration change
  • Documentation update
  • Bug fix

Does this affect the image build?

  • Yes
  • No

Associated Issues

Merge Checklist

All applicable boxes should be checked before merging

  • Image builds successfully with this change (or image build is not affected)
  • Any updated packages/SPECs build successfully
  • Relevant kola tests pass
  • All package sources are available
  • Source files have up-to-date hashes/manifests
  • Documentation has been updated to match any changes
  • Ready to merge

@mayankfz
mayankfz marked this pull request as ready for review July 10, 2026 08:45
@mayankfz
mayankfz requested a review from a team as a code owner July 10, 2026 08:45
Copilot AI lite review requested due to automatic review settings July 10, 2026 08:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.kdump on platforms: [qemu] + architectures: [aarch64].
  • Add an exception for acl.kdump.grub on platforms: [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>
Copilot AI review requested due to automatic review settings July 28, 2026 05:55

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 both acl.kdump and acl.kdump.grub in the enforced list and adds a matching QEMU+aarch64 exception for acl.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:

Copilot AI review requested due to automatic review settings July 30, 2026 06:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.kdump for qemu + aarch64, but the new exception is attached to acl.kdump.grub (and this hunk also re-enables both acl.kdump and acl.kdump.grub in the enforcing list). Please either (a) move/add the qemu/aarch64 exception under acl.kdump if that’s what is intended, or (b) update the PR title/description/changelog to reflect that the exception is for acl.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.

Copilot AI review requested due to automatic review settings July 30, 2026 08:52

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 for acl.kdump.grub. If both are required, the PR description should mention both test entries; otherwise consider dropping the acl.kdump.grub change 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.kdump for all qemu architectures. If the intent is to only exempt the TCG-emulated arm64 case (as described), scope this exception with architectures: [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]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we skipping for qemu both archs here, but only aarch64 for the grub flavor of the test on purpose?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, if we want we can enable acl.kdump for qemu+amd64. thanks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if that works reliably, then lets have it enabled consistently

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jiria

Copy link
Copy Markdown
Member

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.

@jiria

Copy link
Copy Markdown
Member

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:

--- FAIL: acl.kdump (4226.62s)

@mayankfz

mayankfz commented Aug 8, 2026

Copy link
Copy Markdown
Author

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:

--- FAIL: acl.kdump (4226.62s)

Agreed

@mayankfz

mayankfz commented Aug 8, 2026

Copy link
Copy Markdown
Author

Latest run with the changes (acl-scripts + mantle) -> https://dev.azure.com/mariner-org/ACL/_build/results?buildId=1178396&view=results

Copilot AI review requested due to automatic review settings August 10, 2026 06:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.kdump on qemu+aarch64, but this hunk also re-enables acl.kdump/acl.kdump.grub enforcement (previously commented out) and adds the same qemu+aarch64 exception to acl.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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants