feat: add ARM64 support for AgentENV - #86
Conversation
|
🔍 OpenCodeReview found 2 issue(s) in this PR.
[security · high] 📄
|
| run: echo "OWNER=${GITHUB_REPOSITORY_OWNER,,}" >> "$GITHUB_ENV" | ||
|
|
||
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@v4 |
There was a problem hiding this comment.
[security · high]
This newly added publishing job runs Docker-maintained third-party actions by mutable tags while holding packages: write; the following login action also receives GITHUB_TOKEN. If either tag is retargeted or compromised, its code can steal the token or publish a tampered manifest. Pin both docker/setup-buildx-action and docker/login-action in this job to audited full commit SHAs.
| docker-publish: | ||
| name: Publish multi-architecture Docker image | ||
| needs: docker-build | ||
| runs-on: ubuntu-24.04 |
There was a problem hiding this comment.
[other · medium]
This new registry-publishing job has no timeout-minutes. A stalled Buildx setup, registry manifest operation, or inspection can occupy the runner and leave the release workflow blocked until the platform limit. Add a bounded timeout appropriate for registry operations (for example, 30 minutes).
Suggestion:
| docker-publish: | |
| name: Publish multi-architecture Docker image | |
| needs: docker-build | |
| runs-on: ubuntu-24.04 | |
| docker-publish: | |
| name: Publish multi-architecture Docker image | |
| needs: docker-build | |
| runs-on: ubuntu-24.04 | |
| timeout-minutes: 30 |
|
Where could I build vmlinux myself |
This is just a Linux kernel built from Firecracker's microvm-kernel-ci-aarch64-6.1.config. You can build your own kernel using this config and enable additional options as needed. |
cfa3d2c to
ba9871d
Compare
| name: e2e-tests (${{ matrix.e2e_name }}, ${{ matrix.os }}) | ||
| # Ubuntu 24.04's Linux 6.17 kernel can crash in ublk ADD_DEV. | ||
| runs-on: ubuntu-22.04 | ||
| runs-on: ${{ matrix.os }} |
There was a problem hiding this comment.
[other · medium]
This expanded matrix has no timeout-minutes, so any hung KVM/ublk or compose test can occupy both x64 and ARM runners indefinitely. Add a job-level timeout sized for the slowest ARM leg to bound resource consumption.
Suggestion:
| runs-on: ${{ matrix.os }} | |
| runs-on: ${{ matrix.os }} | |
| timeout-minutes: 60 |
| os: | ||
| - ubuntu-22.04 | ||
| - ubuntu-22.04-arm |
There was a problem hiding this comment.
[performance · medium]
The new architecture legs share the same BuildKit GHA cache scopes (agentenv-runtime, agentenv-gateway, and agentenv-scheduler). Because both OS legs can export those scopes concurrently on main, the last writer can replace the other architecture's cache, causing persistent misses or cache thrashing. Include the architecture/OS in each cache-from and cache-to scope (for example, ${{ runner.arch }}), as the release workflow does.
| docker-publish: | ||
| name: Publish multi-architecture Docker image | ||
| needs: docker-build | ||
| runs-on: ubuntu-24.04 |
There was a problem hiding this comment.
[other · medium]
Add timeout-minutes to this new publication job. Registry login, manifest creation, or inspection can otherwise stall until GitHub's much longer default job limit, leaving a release run and runner occupied. A timeout consistent with the adjacent Docker build job (for example, 30 minutes) would bound this failure mode.
Suggestion:
| docker-publish: | |
| name: Publish multi-architecture Docker image | |
| needs: docker-build | |
| runs-on: ubuntu-24.04 | |
| docker-publish: | |
| name: Publish multi-architecture Docker image | |
| needs: docker-build | |
| runs-on: ubuntu-24.04 | |
| timeout-minutes: 30 |
| permissions: | ||
| contents: read | ||
| packages: write | ||
| steps: | ||
| - name: Set lowercase owner |
There was a problem hiding this comment.
[security · low]
This manifest-only job does not check out or otherwise read repository contents, so contents: read is unnecessary. Set contents: none (or omit it, since declaring job permissions makes unspecified scopes none) to reduce the token privileges exposed to the publishing steps.
Suggestion:
| permissions: | |
| contents: read | |
| packages: write | |
| steps: | |
| - name: Set lowercase owner | |
| permissions: | |
| packages: write | |
| steps: | |
| - name: Set lowercase owner |
f350b27 to
b042bc8
Compare
b042bc8 to
9903e36
Compare
| --tag "${image}:${version}" \ | ||
| --tag "${image}:latest" \ |
There was a problem hiding this comment.
[bug · low]
Every tag-triggered workflow unconditionally rewrites the shared latest tag, and this workflow has no concurrency or version-order guard. Overlapping or retried releases can therefore let an older release finish last and point latest backward. Serialize publication and/or update latest only after verifying this tag is the newest release; alternatively publish only the immutable version tag.
Suggestion:
| --tag "${image}:${version}" \ | |
| --tag "${image}:latest" \ | |
| --tag "${image}:${version}" \ |
What
Add ARM64/aarch64 support across AgentENV runtime dependency provisioning, release artifacts, and Docker images.
Why
AgentENV installation and Server releases previously assumed x86_64. On ARM64 hosts, the installer rejected the architecture and dependency setup could not resolve an architecture-specific kernel artifact.
Related issue
Closes #60, #63
Scope and non-goals
Included:
Not included:
Design and behavior changes
aenv-server-linux-aarch64.tar.gzto the release artifacts and publish the Docker image as a multi-architecture image.Compatibility and operations
kvcache-ai/firecrackerGitHub Release and include architecture-specific asset names.Validation
make fmtmake clippymake test-unitmake -C services test(required whenservices/changes)maketargetCommands and results:
Skipped checks and reasons:
This PR does not change
services/and doesn't have performance-sensitive change either.Risks and reviewer notes
Checklist