Add multi-arch build workflows and update CI configurations#16
Add multi-arch build workflows and update CI configurations#16rodneyosodo merged 24 commits intomainfrom
Conversation
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new multi-platform build workflow, refactors CD into matrix Docker build/push jobs, fixes Dependabot config, updates Dockerfiles for cross-compilation across architectures, and enables rustls for Changes
Sequence DiagramsequenceDiagram
autonumber
participant GH as GitHub (event)
participant GHA as GitHub Actions
participant GoMatrix as Go build matrix
participant PropletBuild as Proplet build jobs
participant ArtifactStorage as Actions Artifacts
participant Registry as Container Registry
participant Release as Release job
GH->>GHA: push / pull_request / tag / workflow_dispatch
GHA->>GoMatrix: run matrix builds (manager/cli/proxy × arch)
GoMatrix->>ArtifactStorage: upload `dist/**` artifacts
GHA->>PropletBuild: run proplet matrix builds (cargo/docker per arch)
PropletBuild->>Registry: build & push multi-arch images
Note over GHA: if ref matches tag `v*`
GHA->>Release: download artifacts, create GitHub Release with `dist/**`
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Poem
🚥 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 |
Reviewer's GuideAdds a dedicated multi-architecture Go build/release workflow and restructures CD to build and push multi-arch Docker images (including proplet variants) to GHCR, while updating the main Dockerfile for buildx compatibility and cleaning up CI configs. Sequence diagram for docker-go multi-arch image build and pushsequenceDiagram
participant GitHub as GitHub
participant Runner as Actions_runner
participant QEMU as setup_qemu_action
participant Buildx as setup_buildx_action
participant DockerLogin as docker_login_action
participant BuildPush as build_push_action
participant GHCR as GitHub_Container_Registry
GitHub->>Runner: Trigger docker-go job (matrix svc)
Runner->>Runner: Checkout code
Runner->>Runner: Compute build metadata
Runner->>QEMU: Set up QEMU emulation
Runner->>Buildx: Set up Docker Buildx builder
Runner->>DockerLogin: Login to REGISTRY with GITHUB_TOKEN
Runner->>BuildPush: Build and push multi-arch image for svc
BuildPush->>GHCR: Push tags latest and commit
Flow diagram for multi-arch Dockerfile build processflowchart TD
Start[Start docker buildx build]
ReadArgs[Read build args SVC, TARGETARCH, TARGETVARIANT, GOARCH, GOARM]
SetGOARCH[Set GOARCH to TARGETARCH if GOARCH not provided]
DeriveGOARM[Derive _goarm from GOARM or TARGETVARIANT without prefix v]
RunMake[Run make SVC with GOARCH and optional GOARM]
Compress[Run upx --best build/SVC and ignore failures]
MoveExe[Move build/SVC to /exe]
End[Create final image FROM scratch with /exe]
Start --> ReadArgs
ReadArgs --> SetGOARCH
SetGOARCH --> DeriveGOARM
DeriveGOARM --> RunMake
RunMake --> Compress
Compress --> MoveExe
MoveExe --> End
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The IMAGE_PREFIX/REGISTRY configuration in cd.yml is hardcoded to ghcr.io/rodneyosodo/propeller; consider deriving the image namespace from
github.repository(or a single top-level variable) so the workflow remains reusable if the repo or owner changes. - The Go toolchain versions are specified in multiple places (golang:1.26-alpine3.23 in Dockerfile and 1.25.8 in build.yml); aligning these to a single version source (and one that actually exists) will avoid subtle build differences between CI and container builds.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The IMAGE_PREFIX/REGISTRY configuration in cd.yml is hardcoded to ghcr.io/rodneyosodo/propeller; consider deriving the image namespace from `github.repository` (or a single top-level variable) so the workflow remains reusable if the repo or owner changes.
- The Go toolchain versions are specified in multiple places (golang:1.26-alpine3.23 in Dockerfile and 1.25.8 in build.yml); aligning these to a single version source (and one that actually exists) will avoid subtle build differences between CI and container builds.
## Individual Comments
### Comment 1
<location path=".github/workflows/build.yml" line_range="51" />
<code_context>
+ - name: Setup Go
+ uses: actions/setup-go@v6
+ with:
+ go-version: 1.25.8
+ cache-dependency-path: go.sum
+
</code_context>
<issue_to_address>
**issue (bug_risk):** The configured Go version `1.25.8` is likely invalid for `actions/setup-go` and may cause workflow failures.
`actions/setup-go` requires a valid released Go version or range, and `1.25.8` doesn’t match any current Go release, so the workflow will likely fail when resolving the toolchain. Please switch to a valid version (e.g. `1.22.x` or a specific released version) and align it with whatever Go version is used in your Docker image so CI and containers stay consistent.
</issue_to_address>
### Comment 2
<location path=".github/workflows/cd.yml" line_range="159" />
<code_context>
+ # OpenVINO backend is x86-only
+ platforms: linux/amd64
+ push: true
+ tags: ${{ env.IMAGE_PREFIX }}/proplet:wasi-nn
+ cache-from: type=gha
+ cache-to: type=gha,mode=max
</code_context>
<issue_to_address>
**suggestion:** The wasi-nn image is only tagged with a static `wasi-nn` tag, which reduces traceability to specific builds.
Other Docker jobs tag images with both `latest` and a commit-derived tag so running images can be mapped back to the exact revision. Here you only publish a static `wasi-nn` tag. Please also add a commit SHA or version tag (as done in `proplet` and `docker-go` jobs), which may require adding the same metadata step to this job.
Suggested implementation:
```
- name: Build and push
uses: docker/build-push-action@v6
with:
context: .
file: docker/Dockerfile.proplet-wasinn
# OpenVINO backend is x86-only
platforms: linux/amd64
push: true
tags: |
${{ env.IMAGE_PREFIX }}/proplet:wasi-nn
${{ env.IMAGE_PREFIX }}/proplet:wasi-nn-${{ github.sha }}
```
If other jobs in this workflow use a different tagging convention (e.g. `latest` + short SHA or tags generated via `docker/metadata-action`), you may want to align this block with that existing pattern. For example, if those jobs use `steps.meta.outputs.tags` from a metadata step, add a similar metadata step to this job and replace the hard-coded `tags:` block with `tags: ${{ steps.meta.outputs.tags }}` to keep the tagging strategy consistent across images.
</issue_to_address>
### Comment 3
<location path=".github/workflows/cd.yml" line_range="7" />
<code_context>
+ pull_request:
+ branches:
+ - main
+ - feat/ci
+ workflow_dispatch:
+
</code_context>
<issue_to_address>
**question (bug_risk):** Including the feature branch `feat/ci` in the CD workflow trigger may unintentionally publish production images from a non-main branch.
Since this workflow runs for both `main` and `feat/ci`, it will also publish images from the feature branch. That risks overwriting tags (e.g. `latest`) that are expected to reflect `main` only. If `feat/ci` is for experimentation, consider gating publish steps on `github.ref == 'refs/heads/main'` or removing `feat/ci` from the trigger once you’re done.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Approve with suggestions
This PR introduces multi-arch builds with several minor architectural and maintainability suggestions, such as potential UPX compatibility issues and CI configuration sync risks.
🌟 Strengths
- Introduces robust multi-architecture build support for broader platform compatibility.
- Cleans up and modernizes CI configurations with GitHub Actions best practices.
| Priority | File | Category | Impact Summary (≤12 words) | Anchors |
|---|---|---|---|---|
| P2 | .github/workflows/cd.yml | Architecture | UPX may fail silently on RISC-V, causing inconsistent binary sizes. | path:docker/Dockerfile |
| P2 | .github/workflows/cd.yml | Architecture | Registry change breaks existing deployment scripts without coordination. | |
| P2 | docker/Dockerfile | Maintainability | Complex GOARM derivation logic is brittle for edge cases. | |
| P2 | .github/workflows/build.yml | Testing | Release job not validated on feat/ci branch, risking breakage. | |
| P2 | docker/Dockerfile.proplet | Architecture | Dead riscv64 code in Dockerfile unsupported by CI platform. | path:.github/workflows/cd.yml |
🔍 Notable Themes
- Several findings highlight the need for better synchronization between Dockerfile logic and CI job configurations to avoid dead code and maintenance hazards.
⚠️ **Unanchored Suggestions (Manual Review Recommended)**
The following suggestions could not be precisely anchored to a specific line in the diff. This can happen if the code is outside the changed lines, has been significantly refactored, or if the suggestion is a general observation. Please review them carefully in the context of the full file.
📁 File: docker/Dockerfile
The logic for deriving GOARM from TARGETVARIANT is clever but introduces complexity and a potential edge case. TARGETVARIANT for ARMv7 is typically v7. The ${TARGETVARIANT#v} operation correctly yields 7. However, if TARGETVARIANT is empty or set to a non-v-prefixed value (e.g., for another architecture or a future Docker buildx change), _goarm could be set to an empty string or an unexpected value. While the ${_goarm:+GOARM=${_goarm}} syntax prevents setting GOARM to an empty value, the derivation logic is brittle and its behavior for unsupported variants is not explicitly documented in the Dockerfile.
Suggestion:
# Consider adding a comment and explicit default.
# Example: For ARMv7, TARGETVARIANT is 'v7'. We strip the 'v' to get GOARM.
ARG _goarm=""
RUN if [ -n "${GOARM}" ]; then _goarm="${GOARM}"; \
elif [ "${TARGETARCH}" = "arm" ] && [ -n "${TARGETVARIANT}" ]; then \
_goarm="${TARGETVARIANT#v}"; \
fi && \
make $SVC GOARCH=${GOARCH} ${_goarm:+GOARM=${_goarm}}Related Code:
ARG TARGETARCH
ARG TARGETVARIANT
ARG GOARCH=${TARGETARCH}
...
&& _goarm="${GOARM:-${TARGETVARIANT#v}}" \
&& make $SVC GOARCH=${GOARCH} ${_goarm:+GOARM=${_goarm}} \💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/cd.yml (1)
3-12:⚠️ Potential issue | 🟠 MajorDon't publish stable tags from
feat/ci.With Line 7 enabled, every push to that branch writes the same
:latesttags asmain. Iffeat/ciis only for workflow testing, gate stable pushes tomainor give non-main runs branch-qualified tags instead.Also applies to: 67-69, 121-123
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/cd.yml around lines 3 - 12, The workflow currently triggers pushes for both "main" and "feat/ci" under the on: push: branches list which causes stable tags (e.g., :latest) to be published from the feature branch; update the push triggers or tagging logic so only main produces stable tags: either remove "feat/ci" from the branches list used for stable publishing or add a conditional in the jobs that builds/pushes images to detect github.ref_name (or github.ref) and append the branch name to tags when ref != "refs/heads/main" (so IMAGE_PREFIX-based tags become branch-qualified for feat/ci). Apply this same change to the other publish/tagging sections referenced (the blocks corresponding to lines 67-69 and 121-123).
🧹 Nitpick comments (1)
docker/Dockerfile (1)
3-20: Pin the Go builder to$BUILDPLATFORM.This multi-arch path currently executes
apk,make, andupxunder the target architecture, so multi-platform builds incur the QEMU emulation penalty even though Go can cross-compile natively. UsingFROM --platform=$BUILDPLATFORMkeeps these RUN steps on the native builder while still allowing cross-compilation viaTARGETARCH.Suggested change
-FROM golang:1.26-alpine3.23 AS builder +FROM --platform=$BUILDPLATFORM golang:1.26-alpine3.23 AS builder🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile` around lines 3 - 20, The Dockerfile's RUN steps (apk, make, upx) are executed for the target architecture causing QEMU emulation; update the builder stage to use the host build platform by adding or changing the base image declaration to include --platform=$BUILDPLATFORM (e.g. FROM --platform=$BUILDPLATFORM <builder-image>) so that ARGs like TARGETARCH/TARGETVARIANT/GOARCH still control Go cross-compilation but package manager and build tooling run natively; ensure this change is applied to the builder stage that contains the RUN block which invokes apk, make and upx and that existing ARGs (TARGETARCH, TARGETVARIANT, GOARCH, GOARM, VERSION, COMMIT, TIME) and the WORKDIR/COPY/RUN sequence remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/cd.yml:
- Around line 151-159: The workflow step "Build and push" uses
docker/build-push-action@v6 and currently only pushes the mutable tag listed
under the tags key; add an immutable, commit- or version-qualified tag so the
wasi-nn image has a stable rollback/provenance tag (for example add a second tag
like ${{ env.IMAGE_PREFIX }}/proplet:wasi-nn:${{ github.sha }} or ${{
env.IMAGE_PREFIX }}/proplet:wasi-nn:${{ env.VERSION }}), keeping the existing
mutable tag as well and ensuring push: true remains set; update the tags entry
to a YAML list with both tags so both the human-friendly and the immutable tags
are pushed.
In `@docker/Dockerfile.proplet`:
- Around line 50-64: The Dockerfile currently downloads and extracts the
Wasmtime tarball using the WASMTIME_VERSION and WASMTIME_ARCH variables without
verifying integrity; update the RUN block to also download the corresponding
checksum or signature for the tarball (for the same
WASMTIME_VERSION/WASMTIME_ARCH), verify the tarball using sha256sum -c or a GPG
signature check, and fail the build if verification fails before running tar -xf
and mv; ensure the verification step removes the tarball on failure and cleans
up checksum/signature files after successful verification.
---
Outside diff comments:
In @.github/workflows/cd.yml:
- Around line 3-12: The workflow currently triggers pushes for both "main" and
"feat/ci" under the on: push: branches list which causes stable tags (e.g.,
:latest) to be published from the feature branch; update the push triggers or
tagging logic so only main produces stable tags: either remove "feat/ci" from
the branches list used for stable publishing or add a conditional in the jobs
that builds/pushes images to detect github.ref_name (or github.ref) and append
the branch name to tags when ref != "refs/heads/main" (so IMAGE_PREFIX-based
tags become branch-qualified for feat/ci). Apply this same change to the other
publish/tagging sections referenced (the blocks corresponding to lines 67-69 and
121-123).
---
Nitpick comments:
In `@docker/Dockerfile`:
- Around line 3-20: The Dockerfile's RUN steps (apk, make, upx) are executed for
the target architecture causing QEMU emulation; update the builder stage to use
the host build platform by adding or changing the base image declaration to
include --platform=$BUILDPLATFORM (e.g. FROM --platform=$BUILDPLATFORM
<builder-image>) so that ARGs like TARGETARCH/TARGETVARIANT/GOARCH still control
Go cross-compilation but package manager and build tooling run natively; ensure
this change is applied to the builder stage that contains the RUN block which
invokes apk, make and upx and that existing ARGs (TARGETARCH, TARGETVARIANT,
GOARCH, GOARM, VERSION, COMMIT, TIME) and the WORKDIR/COPY/RUN sequence remain
unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e44d72b4-65d1-4d89-ae3c-1cfd74629c73
📒 Files selected for processing (5)
.github/dependabot.yml.github/workflows/build.yml.github/workflows/cd.ymldocker/Dockerfiledocker/Dockerfile.proplet
💤 Files with no reviewable changes (1)
- .github/dependabot.yml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docker/Dockerfile.proplet (1)
8-10: Unused build arguments:VERSION,COMMIT,TIME.These arguments are declared and passed from the CI workflow but are never used in the
cargo buildcommand. If the intent is to embed version metadata into the binary, consider passing them via environment variables or Rust build flags (e.g.,PROPLET_VERSION=$VERSION cargo build ...).If version info is embedded through other means (e.g.,
Cargo.tomlor build scripts), this can be ignored.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docker/Dockerfile.proplet` around lines 8 - 10, The Dockerfile declares ARG VERSION, ARG COMMIT, and ARG TIME but never uses them; update the Dockerfile to propagate those ARGs into environment variables and pass them to the build step (e.g., after the ARG lines add ENV PROPLET_VERSION=$VERSION PROPLET_COMMIT=$COMMIT PROPLET_TIME=$TIME and then ensure the RUN that invokes cargo build picks up those env vars, or explicitly prefix the build: PROPLET_VERSION=$VERSION PROPLET_COMMIT=$COMMIT PROPLET_TIME=$TIME cargo build ...); reference the existing ARG VERSION / ARG COMMIT / ARG TIME declarations and the cargo build invocation when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docker/Dockerfile.proplet`:
- Around line 17-18: The comment claiming "upx is applied where supported (amd64
/ arm64); it silently skips others." is stale and misleading—remove that clause
(or replace it with a note that UPX is not installed/used) so the Dockerfile
comment accurately reflects current behavior; update the comment block
containing the cross-compilation description to only mention installed
toolchains and that UPX is not used if you wish to document it.
---
Nitpick comments:
In `@docker/Dockerfile.proplet`:
- Around line 8-10: The Dockerfile declares ARG VERSION, ARG COMMIT, and ARG
TIME but never uses them; update the Dockerfile to propagate those ARGs into
environment variables and pass them to the build step (e.g., after the ARG lines
add ENV PROPLET_VERSION=$VERSION PROPLET_COMMIT=$COMMIT PROPLET_TIME=$TIME and
then ensure the RUN that invokes cargo build picks up those env vars, or
explicitly prefix the build: PROPLET_VERSION=$VERSION PROPLET_COMMIT=$COMMIT
PROPLET_TIME=$TIME cargo build ...); reference the existing ARG VERSION / ARG
COMMIT / ARG TIME declarations and the cargo build invocation when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 82aedb77-f382-463f-8c19-bfa58580bf4a
📒 Files selected for processing (3)
.github/workflows/build.ymldocker/Dockerfiledocker/Dockerfile.proplet
✅ Files skipped from review due to trivial changes (1)
- .github/workflows/build.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- docker/Dockerfile
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
- Add runner to proplet matrix to use macos-latest for darwin targets - Conditionally install cross-compilation toolchains only on ubuntu - Conditionally set linker env vars only for Linux targets
These targets fail due to aws-lc-sys compilation issues on Linux cross-compiler. Native FreeBSD toolchain would be needed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 144-148: The mv commands run after "cd proplet && cargo build ..."
are using paths relative to the repository root and omit the Windows extension;
update both branches so the move uses paths relative to the proplet working
directory and includes ${ext}: after the cd, move
target/.../release/proplet${ext} to ../dist/proplet-${{ matrix.goos }}-${{
matrix.goarch }}${ext} (i.e., change "mv proplet/target/..." to "mv
target/.../release/proplet${ext} ../dist/proplet-...${ext}"), or alternatively
avoid cd entirely and run cargo in the proplet subdir while keeping mv source as
proplet/target/.../release/proplet${ext}; apply the same fix for both the
matrix-targeted and non-targeted branches.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 384e1f78-1770-4967-87c4-3bde2dafdaaa
📒 Files selected for processing (1)
.github/workflows/build.yml
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
This reverts commit ba9de19. Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
Signed-off-by: Rodney Osodo <[email protected]>
What type of PR is this?
What does this do?
Which issue(s) does this PR fix/relate to?
Have you included tests for your changes?
Did you document any new/modified features?
Notes
Signed-off-by: Rodney Osodo [email protected]
Summary by Sourcery
Introduce multi-architecture build and release workflows and extend Docker image publishing for multiple services and platforms.
Build:
CI:
Summary by CodeRabbit
Chores
New Features