[github-actions] Add multi-arch support#844
Conversation
e701026 to
20917af
Compare
There was a problem hiding this comment.
Pull request overview
This PR extends the kernel build and test workflow to support multiple architectures (x86_64 and aarch64) by introducing dynamic matrix generation and parallel execution across architectures.
Changes:
- Renamed workflow file from kernel-build-and-test-x86_64.yml to kernel-build-and-test-multiarch.yml with architecture input parameter
- Converted build, boot, test-kselftest, and compare-results jobs to use dynamic matrix strategy based on enabled architectures
- Updated artifact naming to be architecture-specific and modified PR creation logic to aggregate results from both architectures
- Modified create-pr-body.sh script to handle both single-arch and multi-arch reporting formats
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/kernel-build-and-test-multiarch.yml | Adds matrix-based multi-arch support with dynamic architecture selection, separate runners per architecture, and consolidated PR creation with per-architecture results |
| .github/scripts/create-pr-body.sh | Updates PR body generation script to handle both single and multi-architecture parameter formats with conditional formatting |
Comments suppressed due to low confidence (9)
.github/workflows/kernel-build-and-test-multiarch.yml:283
- The base_branch output should be architecture-specific or handled differently. Since this is now a matrix job, each architecture instance will try to set this output, but only one value will be retained. If the base branch detection logic differs or if one architecture fails while another succeeds, this could lead to inconsistent or missing output values in the create-pr job.
.github/workflows/kernel-build-and-test-multiarch.yml:550 - The regression detection logic checks if comparison_status equals "failed" for each architecture, but the comparison_status outputs from the matrix job will be unreliable (see comment on lines 284-285). Even if that were fixed, this logic doesn't account for the case where a comparison might not exist for a particular architecture (empty string vs "skipped" vs "failed"). The conditional should explicitly check for empty strings to avoid false negatives.
.github/workflows/kernel-build-and-test-multiarch.yml:285 - The approach for handling outputs from matrix jobs is problematic. Matrix jobs cannot reliably set job-level outputs for individual matrix instances - only the last matrix job to complete will have its outputs available. This means:
- If x86_64 completes last, comparison_status_aarch64 will be empty regardless of its value
- If aarch64 completes last, comparison_status_x86_64 will be empty regardless of its value
This will cause the regression detection logic in the create-pr job (lines 536-550) to fail silently. Consider using artifacts to store comparison results instead, or restructure the workflow to have separate comparison jobs per architecture that don't use matrix.
.github/workflows/kernel-build-and-test-multiarch.yml:728
- This appears to be referencing a development branch 'shreeya_kernelci_main' instead of the main branch. This is likely a mistake and should be reverted to fetch from 'main' to ensure the workflow uses the stable version of the script. Using a personal/development branch in production workflows creates maintenance and stability issues.
.github/workflows/kernel-build-and-test-multiarch.yml:814 - The logic for passing parameters to the script when both architectures are enabled is unclear and potentially incorrect. The script expects 13 positional parameters for multi-arch (architecture, build_time, total_time, passed, failed, run_id, comparison_section, repo, commit_message_file, build_time_aarch64, total_time_aarch64, passed_aarch64, failed_aarch64), but the script invocation at lines 832-843 doesn't align properly with this expectation. The EXTRA_PARAMS variable is appended at line 842, but it's a space-separated string which could be interpreted as multiple arguments. This needs to be restructured to properly pass all 13 arguments in the correct order.
.github/workflows/kernel-build-and-test-multiarch.yml:278 - The condition checks only for exit status success() or failure(), but doesn't handle the skipped or cancelled states. If test-kselftest is skipped for any reason, the compare-results job will still run, which might not be desirable. Consider being more explicit about when this should run, e.g., 'if: always() && needs.test-kselftest.result != 'cancelled''.
.github/workflows/kernel-build-and-test-multiarch.yml:391 - The workflow references 'kernel-build-and-test-multiarch.yml' at line 391, but this is checking against past runs. If this workflow was recently renamed from 'kernel-build-and-test-x86_64.yml', there may be historical runs under the old workflow name that should still be considered as valid baselines. Consider checking both workflow names during the transition period to avoid losing baseline comparisons.
.github/workflows/kernel-build-and-test-multiarch.yml:803 - The comparison status case statement doesn't handle empty or undefined values properly. If the outputs are empty (which they will be due to the matrix job output issue), the case will match the '*' wildcard and display "Failed - Regression detected" even when no comparison was performed. Add an explicit check for empty strings before entering the case statement.
.github/workflows/kernel-build-and-test-multiarch.yml:842 - The script invocation constructs EXTRA_PARAMS as a string with space-separated values and then appends it with '$EXTRA_PARAMS' at line 842. Without proper quoting (should be "$EXTRA_PARAMS"), this will cause word splitting and the individual parameters won't be passed as separate positional arguments to the script. This breaks the 13-argument expectation for multi-arch. Each parameter should be passed separately on its own line or the string should be properly quoted.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
bf45ad7 to
93aba2a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (5)
.github/workflows/kernel-build-and-test-multiarch.yml:285
compare-resultsis a matrix job, but joboutputsare not reliably available per-matrix-run. The conditional outputs here will be set independently in each matrix run, and downstreamneeds.compare-results.outputs.*will not give you both architectures' statuses (it typically resolves to a single run’s outputs). This can cause regressions on one architecture to be missed. Consider writing per-arch comparison results to artifacts and adding a non-matrix aggregation job that downloads them and exposes stable outputs forcreate-pr(or split compare into separate non-matrix jobs gated byif).
.github/workflows/kernel-build-and-test-multiarch.yml:547- These
needs.compare-results.outputs.comparison_status_*checks depend oncompare-resultsmatrix job outputs. With the current matrix setup,needs.compare-results.outputswill not reliably contain both architectures’ results, so this regression gate may incorrectly allow PR creation. Use an explicit aggregation step/job to collect per-arch statuses before evaluatingREGRESSION_DETECTED.
.github/workflows/kernel-build-and-test-multiarch.yml:690 build_time_*/total_time_*are always suffixed withs, even when the grep fallback producesN/A. That yields values likeN/As, andcreate-pr-body.sh’sconvert_timewill then fail arithmetic expansion underset -e. Either emit plainN/A(withouts) or change both the workflow and script to use a numeric fallback (e.g.,0s) and/or teachconvert_timeto pass through non-numeric inputs.
.github/workflows/kernel-build-and-test-multiarch.yml:728- Hard-coding
shreeya_kernelci_mainas the source ofcreate-pr-body.shmakes this workflow fragile (it will fail if that branch doesn’t exist or is renamed, and it’s unexpected in a reusable workflow). Prefer checking out the script from the current ref, the repository default branch, or a pinned commit SHA/tag so the workflow is portable and reproducible.
.github/workflows/kernel-build-and-test-multiarch.yml:48 - The dynamic matrix generation doesn’t validate that at least one supported architecture was selected. If
inputs.architecturesis empty/invalid,MATRIX_ITEMSstays empty and the downstream matrix jobs won’t run, leading to confusing workflow behavior. Consider failing thesetupjob with a clear error when no supported architectures are detected (or normalize/validate the input list).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
7d9014d to
cd17ba4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (3)
.github/workflows/kernel-build-and-test-multiarch.yml:285
compare-resultsis a matrix job, but job-leveloutputsaren’t reliably aggregated across matrix runs. As written,needs.compare-results.outputs.comparison_status_x86_64/comparison_status_aarch64(and evenbase_branch) will typically reflect only one matrix child (often the last), causing the regression gate and PR body to miss one architecture or show empty/incorrect statuses. Consider replacing these outputs with per-arch artifacts (e.g., upload a small JSON/text status file keyed by arch) and havecreate-prdownload/parse them, or splitcompare-resultsinto two non-matrix jobs (one per arch) plus an aggregation job that sets a single canonical output forcreate-pr.
.github/workflows/kernel-build-and-test-multiarch.yml:49- The dynamic matrix generation allows
architecturesvalues that produce an emptyincludelist (e.g., typo/unsupported arch). An empty matrix will make downstream jobs fail/skip in a confusing way. Add an explicit validation that at least one supported arch was selected and exit with a clear error message if not.
.github/workflows/kernel-build-and-test-multiarch.yml:723 - This step hard-codes fetching
.github/scripts/create-pr-body.shfrom theshreeya_kernelci_multiarchbranch. That makes the workflow depend on a non-default branch that may not exist in forks or after the branch is deleted, and it also overwrites the script version from the current workflow ref. Prefer using the script from the currently checked out commit, or fetch it from the repository default branch (e.g.,main) if that’s the intent.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
95c358d to
6738971
Compare
6738971 to
2107133
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
.github/workflows/kernel-build-and-test-multiarch.yml:48
- If
inputs.architecturesdoesn’t contain any supported values, this generates an emptyincludematrix ({"include":[]}) and downstream jobs will be skipped in a non-obvious way. Consider validating the input here and failing fast with a clear error when no architectures are selected / recognized.
.github/workflows/kernel-build-and-test-multiarch.yml:285 compare-resultsis a matrix job, but job-leveloutputsdon’t safely aggregate per-matrix-run values. As written,needs.compare-results.outputs.comparison_status_x86_64/_aarch64will be empty or nondeterministic, which can cause regressions to be missed (or misreported) increate-pr. Consider persisting each arch’s comparison result (e.g., as an artifact or as a JSON line in$GITHUB_OUTPUTuploaded per-arch) and add a non-matrix “collect” job to download/merge them, or splitcompare-resultsinto separate jobs per arch so each can expose a stable output.
.github/workflows/kernel-build-and-test-multiarch.yml:562- This regression gate relies on
needs.compare-results.outputs.comparison_status_x86_64/aarch64, but those values won’t be reliable whilecompare-resultsruns as a matrix job (matrix job outputs aren’t aggregated per-arch). This can let a regression through without blocking PR creation. After fixing the output propagation, also consider treating an empty/unknown status as a hard failure so regressions can’t be silently ignored.
.github/workflows/kernel-build-and-test-multiarch.yml:847 - The workflow still checks out
.github/scripts/create-pr-body.shfromorigin/mainearlier in this job, but this PR changes the script interface to named args. During runs on this branch (before merge),origin/mainwill likely contain the old positional-args script, so this invocation can fail. Prefer using the script from the current ref (don’t overwrite it frommain), or pin the checkout to a commit that is guaranteed to include the named-arg version.
.github/workflows/kernel-build-and-test-multiarch.yml:689 - These
grep ... | head -1command substitutions have no fallback when the timer markers aren’t present. With GitHub Actions’ defaultbash -e -o pipefail, a non-match will exit the step and block PR creation. Add a safe fallback (e.g.,|| echo "0"/|| echo "N/A") or explicitly handle the no-match case before writing outputs.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
If we change this will this break all the other branches ? Should actually be keeping the x86_64 version then all the other inherited branches will have to migrate to using multiarch. Once all the branches are using multi arch we can just delete this one? |
Yes, I will not merge the changes before we transform all other branches to use this new workflow file. I just wanted to get review from the kernel team to see if it all looks good before I make the transition. |
|
ok, looks good to me, we can probably just merge in all the current stuff, then just have the two rather than renaming that way we don't have a "flag day" of coordination if that makes sense? |
Extended the kernel build and test workflow to support multiple architectures. The workflow now builds and tests kernels for both x86_64 and aarch64 platforms in parallel. Important changes :- - Renamed workflow from kernel-build-and-test-x86_64.yml to kernel-build-and-test-multiarch.yml - Added dynamic matrix generation based on architecture input parameter - Separate runners for each architecture (kernel-build for x86_64, kernel-build-arm64 for aarch64) - Per-architecture artifact uploads and comparison results - Architecture-specific kselftest baseline tracking and regression detection - PR creation includes results from both architectures with separate status sections Signed-off-by: Shreeya Patel <[email protected]>
Signed-off-by: Shreeya Patel <[email protected]>
be4d621
2107133 to
be4d621
Compare
Extended the kernel build and test workflow to support multiple architectures.
The workflow now builds and tests kernels for both x86_64 and aarch64 platforms in parallel.
Important changes :-