perf(ci): use native arm64 runners for docker builds#19759
Draft
perf(ci): use native arm64 runners for docker builds#19759
Conversation
Use ubuntu-24.04-arm for arm64 build-and-push-main and build-and-push-operator jobs instead of QEMU emulation on amd64. QEMU adds ~2x overhead to docker buildx RUN steps: - build-and-push-main arm64: 240s (QEMU) vs 111s (amd64 native) - build-and-push-operator arm64: similar This should cut ~2m from the arm64 critical path. Follows the pattern used in the collector repo. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Skipping CI for Draft Pull Request. |
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
runs-onexpression is duplicated betweenbuild-and-push-mainandbuild-and-push-operator; consider extracting the runner label selection into a reusable variable (e.g., viaenvorworkflow_callinput) to keep this logic in one place. - Using the
matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-24.04'idiom relies on the left-hand branch always evaluating to a truthy value; if you ever change the runner label to something that could be falsy (e.g. via an input), the expression will break, so it might be safer to use an explicitifwith separate jobs or a small wrapper step instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `runs-on` expression is duplicated between `build-and-push-main` and `build-and-push-operator`; consider extracting the runner label selection into a reusable variable (e.g., via `env` or `workflow_call` input) to keep this logic in one place.
- Using the `matrix.arch == 'arm64' && 'ubuntu-24.04-arm' || 'ubuntu-24.04'` idiom relies on the left-hand branch always evaluating to a truthy value; if you ever change the runner label to something that could be falsy (e.g. via an input), the expression will break, so it might be safer to use an explicit `if` with separate jobs or a small wrapper step instead.
## Individual Comments
### Comment 1
<location path=".github/workflows/build.yaml" line_range="413" />
<code_context>
build-and-push-main:
- runs-on: ubuntu-latest
+ # Use native arm64 runner to avoid QEMU emulation overhead (~2x slower).
+ runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-24.04-arm') || 'ubuntu-24.04' }}
needs:
- define-job-matrix
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating the `runs-on` expression by encoding the runner label in the matrix.
The same conditional `runs-on` expression is now duplicated across `build-and-push-main` and `build-and-push-operator`. To keep this mapping in one place and make future changes safer (e.g., updating Ubuntu version or runner label), consider adding `runner` to the matrix (e.g., `include: [{ arch: 'amd64', runner: 'ubuntu-24.04' }, { arch: 'arm64', runner: 'ubuntu-24.04-arm' }]`) and using `runs-on: ${{ matrix.runner }}` in each job.
Suggested implementation:
```
build-and-push-main:
# Runner selection is encoded in the matrix (arch -> runner mapping).
runs-on: ${{ matrix.runner }}
needs:
- define-job-matrix
- pre-build-cli
add_build_comment_to_pr
```
```
build-and-push-operator:
runs-on: ${{ matrix.runner }}
needs:
- define-job-matrix
- pre-build-go-binaries
```
To fully implement your suggestion, you also need to update the matrix definition used by these jobs (likely in the `define-job-matrix` job or a `strategy.matrix` block) to include the `runner` key alongside `arch`, for example:
- If using a direct matrix:
```yaml
strategy:
matrix:
include:
- arch: amd64
runner: ubuntu-24.04
- arch: arm64
runner: ubuntu-24.04-arm
```
- If `define-job-matrix` outputs the matrix, extend its outputs so that each matrix entry includes `runner` with the appropriate value.
Both `build-and-push-main` and `build-and-push-operator` must run with a matrix that provides `matrix.runner` for these changes to work.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated GitHub Actions workflow runner selection for two build jobs to dynamically choose architecture-specific runners based on a matrix variable. Jobs now use Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (4)
.github/workflows/build.yaml (4)
734-734: Consistent with build-and-push-main, but missing explanatory comment.The runner selection logic matches the
build-and-push-mainjob, which is good for maintainability. Consider adding the same explanatory comment for consistency:📝 Add explanatory comment
build-and-push-operator: + # Use native arm64 runner to avoid QEMU emulation overhead (~2x slower). runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-24.04-arm') || 'ubuntu-24.04' }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yaml at line 734, Add the same explanatory comment above the runs-on expression that selects ubuntu-24.04-arm for arm64 or ubuntu-24.04 otherwise (the line containing "runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-24.04-arm') || 'ubuntu-24.04' }}"); update the job to include a short comment explaining why we choose a different runner for arm64 (matching the comment used in the build-and-push-main job) so future readers understand the conditional runner selection.
817-819: Same QEMU optimization opportunity as build-and-push-main.This QEMU setup step has the same redundancy on native arm64 runners.
♻️ Optional: Skip QEMU on native arm64 runner
- name: Set up QEMU - if: matrix.arch != 'amd64' + if: matrix.arch == 'ppc64le' || matrix.arch == 's390x' uses: docker/setup-qemu-action@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yaml around lines 817 - 819, The "Set up QEMU" step (uses: docker/setup-qemu-action@v4) currently runs for any matrix.arch != 'amd64'; update its if condition to also skip when the job is running on a native arm64 runner by adding a check for runner.arch (e.g. change the if to something like: if: matrix.arch != 'amd64' && runner.arch != 'arm64'), so QEMU is not set up on native arm64 runners.
503-505: QEMU setup is now redundant for arm64 builds.With native arm64 runners, QEMU is no longer needed for
arm64builds. The conditionmatrix.arch != 'amd64'still triggers QEMU setup on the native arm64 runner, which is unnecessary overhead (though harmless).Consider tightening the condition to only run for architectures that actually require emulation:
♻️ Optional: Skip QEMU on native arm64 runner
- name: Set up QEMU - if: matrix.arch != 'amd64' + if: matrix.arch == 'ppc64le' || matrix.arch == 's390x' uses: docker/setup-qemu-action@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yaml around lines 503 - 505, The "Set up QEMU" step currently uses the broad condition if: matrix.arch != 'amd64' which still runs QEMU on native arm64 runners; change the condition on the "Set up QEMU" step (the line containing if: matrix.arch != 'amd64') to explicitly target only architectures that need emulation (e.g., if: matrix.arch == 'arm' || matrix.arch == 'armv7' || matrix.arch == 'ppc64le' || matrix.arch == 's390x' or use a contains(...) check against a short list), so native arm64 runners skip the QEMU setup.
412-413: Good optimization for arm64 builds.The conditional runner selection correctly routes arm64 builds to native runners while keeping amd64 (and ppc64le/s390x) on x86-64 runners, avoiding QEMU emulation overhead.
The container image
quay.io/stackrox-io/apollo-ci:stackrox-test-0.5.3must support arm64 for this to work—if it didn't, the job would fail immediately when pulled on the native arm64 runner. The fact that bothbuild-and-push-mainandbuild-and-push-operatorjobs now run on the arm64 runner with this image confirms multi-arch support is present.Minor optimization opportunity: The QEMU setup steps (lines 503–505, 817–819) run whenever
matrix.arch != 'amd64', which includes the native arm64 runner. On native arm64 runners, QEMU is unnecessary, though harmless. Consider adding&& matrix.arch != 'arm64'to the condition if reducing CI overhead is desired.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/build.yaml around lines 412 - 413, The QEMU setup steps currently run when matrix.arch != 'amd64', which also triggers on native arm64 runners; update the conditional for those QEMU setup steps (used by the build-and-push-main and build-and-push-operator jobs) to only run when matrix.arch is neither amd64 nor arm64 (e.g., change the check from "matrix.arch != 'amd64'" to "matrix.arch != 'amd64' && matrix.arch != 'arm64'") so QEMU is skipped on native arm64 while still running for other non-amd64 architectures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/build.yaml:
- Line 734: Add the same explanatory comment above the runs-on expression that
selects ubuntu-24.04-arm for arm64 or ubuntu-24.04 otherwise (the line
containing "runs-on: ${{ (matrix.arch == 'arm64' && 'ubuntu-24.04-arm') ||
'ubuntu-24.04' }}"); update the job to include a short comment explaining why we
choose a different runner for arm64 (matching the comment used in the
build-and-push-main job) so future readers understand the conditional runner
selection.
- Around line 817-819: The "Set up QEMU" step (uses:
docker/setup-qemu-action@v4) currently runs for any matrix.arch != 'amd64';
update its if condition to also skip when the job is running on a native arm64
runner by adding a check for runner.arch (e.g. change the if to something like:
if: matrix.arch != 'amd64' && runner.arch != 'arm64'), so QEMU is not set up on
native arm64 runners.
- Around line 503-505: The "Set up QEMU" step currently uses the broad condition
if: matrix.arch != 'amd64' which still runs QEMU on native arm64 runners; change
the condition on the "Set up QEMU" step (the line containing if: matrix.arch !=
'amd64') to explicitly target only architectures that need emulation (e.g., if:
matrix.arch == 'arm' || matrix.arch == 'armv7' || matrix.arch == 'ppc64le' ||
matrix.arch == 's390x' or use a contains(...) check against a short list), so
native arm64 runners skip the QEMU setup.
- Around line 412-413: The QEMU setup steps currently run when matrix.arch !=
'amd64', which also triggers on native arm64 runners; update the conditional for
those QEMU setup steps (used by the build-and-push-main and
build-and-push-operator jobs) to only run when matrix.arch is neither amd64 nor
arm64 (e.g., change the check from "matrix.arch != 'amd64'" to "matrix.arch !=
'amd64' && matrix.arch != 'arm64'") so QEMU is skipped on native arm64 while
still running for other non-amd64 architectures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: f382994c-35db-4321-89f9-3bd4dffbea28
📒 Files selected for processing (1)
.github/workflows/build.yaml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19759 +/- ##
==========================================
- Coverage 49.60% 49.58% -0.02%
==========================================
Files 2760 2761 +1
Lines 208144 208140 -4
==========================================
- Hits 103242 103214 -28
- Misses 97237 97260 +23
- Partials 7665 7666 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ility The CI container image (apollo-ci) is amd64-only and can't run on arm64 runners. Remove the container block from build-and-push-main and build-and-push-operator, moving secrets from container env to job-level env. These jobs only need docker buildx (available on ubuntu-latest) and don't depend on CI container tools. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
Images are ready for the commit at 44dab7e. To use with deploy scripts, first |
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
2 participants
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.
Description
Use
ubuntu-24.04-armfor arm64build-and-push-mainandbuild-and-push-operatorjobs instead of QEMU emulation on amd64 runners.QEMU adds ~2x overhead to docker buildx
RUNsteps. Current arm64 timings with QEMU:Build main images: 240s (vs 111s native amd64)Build roxctl image: 46s (vs 12s native amd64)Expected improvement: arm64 build-and-push-main from ~7.5m to ~5m (matching amd64). This removes arm64 as the PR build critical path bottleneck.
Follows the pattern used in the collector repo.
Does NOT change
pre-build-go-binaries— that uses Go cross-compilation (GOOS/GOARCH), not docker buildx, so QEMU isn't involved.User-facing documentation
Testing and quality
How I validated my change
CI validation — compare arm64 build-and-push-main timing vs previous runs.
🤖 Generated with Claude Code