[build] Cross-arch builds: components - #932
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis change updates multiple Makefiles and Dockerfiles across the repository to support configurable, multi-architecture Docker builds. Hardcoded architecture and platform flags are replaced with variables ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Makefile
participant Docker Buildx
participant Dockerfile
User->>Makefile: make image (with BUILDER, PLATFORM)
Makefile->>Docker Buildx: buildx build --builder=$(BUILDER) --platform=$(PLATFORM)
Docker Buildx->>Dockerfile: Passes TARGETOS, TARGETARCH as build args
Dockerfile->>Dockerfile: Use TARGETOS, TARGETARCH in build/install steps
Dockerfile->>Docker Buildx: Outputs multi-arch image
Docker Buildx->>Makefile: Build complete
Makefile->>User: Build result (multi-arch image)
Possibly related PRs
Suggested labels
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
382a0f8 to
4a2c67e
Compare
|
All modified Dockerfiles were successfully built on amd64 and arm64. |
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
Signed-off-by: nbykov0 <166552198+nbykov0@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (3)
packages/apps/kubernetes/Makefile (3)
35-38: Build context ordering issue repeated. See the comment on lines 17-20 for the correct placement of the build context.
52-55: Build context ordering issue repeated. See the comment on lines 17-20 for the correct placement of the build context.
70-73: Build context ordering issue repeated. See the comment on lines 17-20 for the correct placement of the build context.
🧹 Nitpick comments (25)
packages/system/dashboard/Makefile (1)
20-21: Switched off automatic Dockerfile updates—consider enforcing manual update
Printing a reminder (@echo Update dockerfiles manually) is helpful, but the target still succeeds even if Dockerfiles are stale. Consider failing theupdate-dockerfilestarget (e.g.,exit 1) or adding precise instructions to prevent drift.packages/system/dashboard/images/kubeapps-apis/Dockerfile (2)
6-6: Introduce a dedicated 'source' stage—pin Alpine for reproducibility
Pulling from the genericalpineimage can introduce breakages over time.
Consider specifying a fixed version (e.g.,alpine:3.18) to ensure consistent downloads and caching.
74-74: Cross‐compile plugin builds—consider DRY refactor
The GOOS/GOARCH flags repeat across each plugin build block. You could DRY this by defining aGOENV="GOOS=$TARGETOS GOARCH=$TARGETARCH GOPROXY=..."variable and reusing it:ARG GOENV="GOOS=$TARGETOS GOARCH=$TARGETARCH GOPROXY=... " RUN --mount=... $GOENV go build ...Also applies to: 83-83, 93-93
packages/core/testing/images/e2e-sandbox/Dockerfile (3)
10-11: Combineapt updateandapt installinto one layer
Merging these into a singleRUNreduces image layers and ensures package lists stay in sync:RUN apt update -q && apt install -yq --no-install-recommends ... && rm -rf /var/lib/apt/lists/*
18-18: Dynamicyqdownload—good approach
Fetchingyq_${TARGETOS}_${TARGETARCH}ensures the correct binary.
Consider adding checksum validation as well.
19-19: Pin FluxCD installer version for reproducibility
Curling the latest install script always fetches the bleeding edge.
Pin to a specific FluxCD release or include a version argument to avoid unpredictable changes.packages/system/kubeovn-webhook/Makefile (1)
10-11: EnsureBUILDERandPLATFORMvariables are defined and documentedIntroducing
--builder=$(BUILDER)and--platform=$(PLATFORM)flags assumes these variables are set (e.g., via CI environment orcommon-envs.mk). Please verify that:
$(BUILDER)and$(PLATFORM)have sensible defaults or error clearly if undefined.- The required values and examples are documented in
scripts/common-envs.mkor the repository’s contributing guidelines.packages/system/kamaji/Makefile (1)
17-18: Verify cross-architecture variables in build targetThe added
--builder=$(BUILDER)and--platform=$(PLATFORM)flags enable multi-arch builds. Ensure that:
BUILDERandPLATFORMare exported or defaulted incommon-envs.mk.- Downstream CI or
makeinvocations supply these variables correctly.Optionally, add a fallback in the Makefile for local development, e.g.:
BUILDER?=default PLATFORM?=linux/amd64packages/apps/kubernetes/images/kubevirt-csi-driver/Dockerfile (1)
8-11: Approve cross-compilation build args; consider static linkingGood use of BuildKit’s automatic
TARGETOSandTARGETARCHargs to parameterizeGOOS/GOARCHfor multi-arch builds. As an enhancement, consider disabling cgo to produce statically linked binaries by adding:ENV CGO_ENABLED=0before
RUN make build.packages/system/kubeovn-webhook/images/kubeovn-webhook/Dockerfile (1)
3-4: Consider specifying default values forTARGETOSandTARGETARCH.Without defaults, a plain
docker build(without--build-arg) may produce emptyGOOS/GOARCH, leading to unpredictable builds. You can set defaults for backward compatibility, e.g.:ARG TARGETOS=linux ARG TARGETARCH=amd64packages/system/kamaji/images/kamaji/Dockerfile (1)
5-6: Add default values for architecture build arguments.To ensure builds still work without explicit
--build-arg, consider:ARG TARGETOS=linux ARG TARGETARCH=amd64packages/system/bucket/images/s3manager/Dockerfile (1)
5-6: IntroduceTARGETOSandTARGETARCHbuild args.Running
go buildwithGOOS/GOARCHnow enables cross-compilation. Consider setting defaults (linux/amd64) to preserve behavior when args are omitted.packages/system/cozystack-controller/images/cozystack-controller/Dockerfile (1)
3-4: Default values for build arguments.For environments that omit build-args, add:
ARG TARGETOS=linux ARG TARGETARCH=amd64packages/system/bucket/Makefile (1)
16-17: Ensure default values forBUILDERandPLATFORM.If these variables are not set externally, the build will fail. Consider adding at the top of the Makefile:
BUILDER ?= default PLATFORM ?= linux/amd64packages/apps/kubernetes/images/kubevirt-cloud-provider/Dockerfile (2)
4-7: Add default values for build arguments. To improve local development and maintain backward compatibility when arguments are omitted, consider specifying defaults:ARG TARGETOS=linux ARG TARGETARCH=amd64This ensures the build still succeeds in environments where these args aren’t explicitly passed.
22-22: Cross-compilation build flags look solid. UsingCGO_ENABLED=0with dynamicGOOS/GOARCHenables static, multi-arch binaries. For enhanced reproducibility, you might also include-trimpathin the-ldflags.packages/system/cozystack-api/images/cozystack-api/Dockerfile (2)
3-4: Consider defaulting build args. Adding defaults like:ARG TARGETOS=linux ARG TARGETARCH=amd64can help local builds and avoid failures if args are omitted.
16-16: Static build flags are appropriate. Using-extldflags=-staticalong withCGO_ENABLED=0produces a static binary. For reproducibility, consider adding-trimpathto the-ldflags.packages/apps/kubernetes/Makefile (1)
14-15: Consolidate repeated flags. The build commands for each image repeat identical flags (--provenance,--builder,--platform, cache and metadata options). Extract these into a variable (e.g.,DOCKER_BUILD_OPTS) to DRY up the Makefile:DOCKER_BUILD_OPTS := --provenance false --builder=$(BUILDER) --platform=$(PLATFORM) \ --cache-from type=registry,ref=$(REGISTRY)/$(IMAGE):latest \ --cache-to type=inline --metadata-file $(IMAGE).json \ --push=$(PUSH) --label "org.opencontainers.image.source=…" \ --load=$(LOAD)Then reference
$(DOCKER_BUILD_OPTS)in each target.packages/apps/http-cache/images/nginx-cache/Dockerfile (2)
12-13: Introduce build args for cross-arch support.TARGETOSandTARGETARCHare now available. Consider defaulting them (linux/amd64) for improved local builds:ARG TARGETOS=linux ARG TARGETARCH=amd64
16-18: Optimize package installation. Switching toaptwith--no-install-recommendsreduces image size. For consistency, add--no-install-suggestshere and ensure removal of/var/lib/apt/lists/*in this stage to slim the builder image.packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile (1)
7-10: Provide sensible defaults for build arguments.
ARG TARGETOSandARG TARGETARCHcurrently have no default values. Omitting these at build time will result in emptyGOOS/GOARCH, causing cross-compilation to fail. Consider adding defaults, for example:- ARG TARGETOS - ARG TARGETARCH + ARG TARGETOS=linux + ARG TARGETARCH=amd64packages/apps/kubernetes/images/ubuntu-container-disk/Dockerfile (3)
11-11: Review necessity of bash-completion package.
Addingbash-completionincreases image size and may not be required in a non-interactive build. If it's only for debugging, consider moving it to a dev-only stage or removing it altogether.
17-18: Consider providing defaults for TARGETOS and TARGETARCH.
Without defaults (for example,ARG TARGETOS=linuxandARG TARGETARCH=amd64), omitting these flags will break the build. Adding defaults guards against missing build-args.
40-41: Combine update and install into one transaction.
Mergingapt-get update -qandapt-get install -yq containerd.iointo a single command can reduce image layers and improve cache efficiency:- && guestfish --remote command "apt-get update -q" \ - && guestfish --remote command "apt-get install -yq containerd.io" \ + && guestfish --remote sh "apt-get update -q && apt-get install -yq containerd.io" \
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
packages/apps/clickhouse/Makefile(1 hunks)packages/apps/http-cache/Makefile(1 hunks)packages/apps/http-cache/images/nginx-cache/Dockerfile(6 hunks)packages/apps/kubernetes/Makefile(4 hunks)packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile(2 hunks)packages/apps/kubernetes/images/kubevirt-cloud-provider/Dockerfile(2 hunks)packages/apps/kubernetes/images/kubevirt-csi-driver/Dockerfile(1 hunks)packages/apps/kubernetes/images/ubuntu-container-disk/Dockerfile(3 hunks)packages/apps/mysql/Makefile(1 hunks)packages/apps/postgres/Makefile(1 hunks)packages/core/testing/Makefile(1 hunks)packages/core/testing/images/e2e-sandbox/Dockerfile(1 hunks)packages/extra/monitoring/Makefile(1 hunks)packages/system/bucket/Makefile(1 hunks)packages/system/bucket/images/s3manager/Dockerfile(1 hunks)packages/system/bucket/values.yaml(1 hunks)packages/system/cilium/Makefile(1 hunks)packages/system/cozystack-api/Makefile(1 hunks)packages/system/cozystack-api/images/cozystack-api/Dockerfile(1 hunks)packages/system/cozystack-controller/images/cozystack-controller/Dockerfile(1 hunks)packages/system/dashboard/Makefile(3 hunks)packages/system/dashboard/images/kubeapps-apis/Dockerfile(7 hunks)packages/system/kamaji/Makefile(1 hunks)packages/system/kamaji/images/kamaji/Dockerfile(2 hunks)packages/system/kubeovn-webhook/Makefile(1 hunks)packages/system/kubeovn-webhook/images/kubeovn-webhook/Dockerfile(1 hunks)packages/system/kubeovn/Makefile(1 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
packages/system/bucket/images/s3manager/Dockerfile
[error] 10-10: Use COPY instead of ADD for files and folders
(DL3020)
🔇 Additional comments (39)
packages/system/kubeovn/Makefile (1)
22-23: Approve dynamic builder and platform usage
The flags--builder=$(BUILDER)and--platform=$(PLATFORM)correctly replace hardcoded values and align with the repository's multi-arch build approach.packages/core/testing/Makefile (1)
20-21: Approve multi-arch build enhancements
The new--builder=$(BUILDER)and--platform=$(PLATFORM)flags enable flexible, cross-architecture builds for the end-to-end sandbox image.packages/extra/monitoring/Makefile (1)
18-19: Approve parameterized build flags
Replacing the fixed platform with--builder=$(BUILDER)and--platform=$(PLATFORM)is consistent and improves build flexibility across architectures.packages/apps/postgres/Makefile (1)
12-13: Approve dynamic multi-arch flags
Using--builder=$(BUILDER)and--platform=$(PLATFORM)instead of hardcoded values aligns with the cross-architecture build strategy and ensures consistency.packages/apps/mysql/Makefile (1)
10-13: Parameterize Docker build for multi-architecture—good improvement
Replacing hardcoded--platform linux/amd64and--build-arg ARCH=amd64with--builder=$(BUILDER)and--platform=$(PLATFORM)makes the build flexible across architectures.
Ensure that$(BUILDER)and$(PLATFORM)are documented and have sensible defaults (e.g., incommon-envs.mk) to avoid build failures when not set.packages/apps/clickhouse/Makefile (1)
10-13: Parameterize Docker build for multi-architecture—good improvement
As with other packages, removing hardcoded flags in favor of$(BUILDER)and$(PLATFORM)enables cross-arch builds.
Please verify that these variables are defined by default or documented incommon-envs.mkto prevent undefined-variable errors.packages/system/dashboard/Makefile (2)
32-33: Parameterize dashboard build for multi-arch—solid enhancement
Adding--builder=$(BUILDER)and--platform=$(PLATFORM)aligns this target with others.
Confirm thatcommon-envs.mkor CI injects these variables consistently.
54-55: Parameterize kubeapps-apis build for multi-arch—solid enhancement
Using$(BUILDER)and$(PLATFORM)here too standardizes the process.
Ensure that downstream CI/Makefiles pass valid values or default fallbacks.packages/system/dashboard/images/kubeapps-apis/Dockerfile (3)
33-33: Cross‐compile lint installation—good use of GOOS/GOARCH
ApplyingGOOS=$TARGETOS GOARCH=$TARGETARCHto thegolangci-lintinstall is correct.
EnsureTARGETOS/TARGETARCHare validated upstream.
47-47: Cross‐compilego mod download—looks correct
UsingGOOS/GOARCHhere ensures module caching is arch‐aware.
All good, assuming defaults are set.
65-65: Cross‐compile main server build—approved
EmbeddingGOOSandGOARCHinto the build command aligns with the multi-arch goal.packages/core/testing/images/e2e-sandbox/Dockerfile (1)
17-17: Helm installer script—verify multi-arch support
The official Helm install script infers OS/ARCH; please confirm it selects the correct$TARGETOS/$TARGETARCHor pin a versioned binary for deterministic builds.packages/system/bucket/values.yaml (1)
1-1: Validate default bucketName change for backward compatibilitySetting the default
bucketNameto"cozystack"will apply a non-empty name in environments that rely on the Helm chart’s defaults. Confirm that this won’t break existing deployments that assume an empty default (or document the new required override). Consider allowing an environment variable override or documenting the change in the chart’sREADME.md.packages/system/cozystack-api/Makefile (1)
12-13: Align builder/platform flags inimage-cozystack-apitargetThe replacement of hardcoded platform with
--builder=$(BUILDER)and--platform=$(PLATFORM)is consistent with other packages. Please confirm that:
- Environment variables
BUILDERandPLATFORMare centrally defined.- There is documentation or an example invocation to illustrate their expected values.
packages/system/kubeovn-webhook/images/kubeovn-webhook/Dockerfile (2)
8-8: Good cache layer for module dependencies.The separate
go mod downloadstep, scoped byGOOS/GOARCH, efficiently caches dependencies across architectures. Looks solid.
11-11: Cross-compilation build command is correct.Explicitly setting
CGO_ENABLED=0alongsideGOOS/GOARCHensures a static binary compatible with the target platform. Nice.packages/system/kamaji/images/kamaji/Dockerfile (2)
2-2: StandardizeASkeyword casing.The change from lowercase to uppercase
ASfollows Dockerfile best practices and improves readability. Good catch.
15-15: Build step correctly uses cross-compile flags.The
RUN GOOS=$TARGETOS GOARCH=$TARGETARCH CGO_ENABLED=0 go buildinvocation is aligned with other Dockerfiles and supports multi-arch output. Nice.packages/system/bucket/images/s3manager/Dockerfile (1)
12-12: Cross-compilation flags applied correctly.Using
CGO_ENABLED=0and static linking flags is consistent with other components and ensures portable binaries.packages/system/cozystack-controller/images/cozystack-controller/Dockerfile (3)
1-1: Align builder stage with Go Alpine variant.Switching to
golang:1.23-alpinereduces image size while retaining the Go toolchain. Approved.
9-9: Dependency download step is well-placed.Using
GOOS/GOARCHduringgo mod downloadpre-populates the module cache for the correct target. Approved.
16-16: Static build flags are correct.The
-extldflags=-staticoption plusCGO_ENABLED=0ensures a fully static binary. Looks good.packages/system/bucket/Makefile (2)
9-9: No-op update target is intentional.Replacing the old chart update commands with a no-op avoids unnecessary operations. Approved.
14-14: Parameterized build command.Switching to
buildx buildwith--builderand--platformoptions aligns with multi-arch requirements. Good.packages/apps/kubernetes/images/kubevirt-cloud-provider/Dockerfile (1)
2-2: Ensure consistent builder stage aliasing. TheAS builderkeyword is correctly capitalized, aligning with other Dockerfiles.packages/system/cozystack-api/images/cozystack-api/Dockerfile (2)
1-1: Builder stage declaration is correct. TheAS builderalias is well‐placed on the Alpine base for a lean build.
9-9: Cross-platform module download. Prefixinggo mod downloadwith dynamicGOOS/GOARCHis valid; just confirm it doesn’t inadvertently affect other tool invocations in subsequent steps.packages/apps/http-cache/images/nginx-cache/Dockerfile (5)
1-1: Multi-stage build alias is correct. UsingAS stageon Ubuntu base aligns with the repository’s pattern for builder stages.
75-75: Dynamic package architecture for IP2Location. Replacing hardcodedamd64with${TARGETARCH}incheckinstallensures correct package naming per architecture.
104-104: Dynamic package architecture for IP2Proxy.--pkgarch=${TARGETARCH}is correctly applied for the second library.
151-151: Dynamic package architecture for Nginx.${TARGETARCH}ensures the Nginx package matches the target architecture.
172-173: Minimal final stage dependencies. The final stage usesaptwith both--no-install-recommendsand--no-install-suggestsand cleans up apt lists—best practice for minimal images.packages/apps/kubernetes/images/cluster-autoscaler/Dockerfile (1)
24-27: Confirm final-stage ARG and copy semantics.
You’ve reintroducedARG TARGETARCHin the final stage soCOPY --from=builder .../cluster-autoscaler-${TARGETARCH}works correctly. Verify this matches your build output naming, and note thatTARGETOSisn’t used here—if future multi-OS builds are desired, you may need a correspondingARG TARGETOSand adjust the binary naming accordingly.packages/apps/kubernetes/images/ubuntu-container-disk/Dockerfile (6)
2-2: Specify explicit image version for reproducibility.
Pinning toubuntu:22.04is valid given guestfish compatibility, but please document this choice (e.g., in a README or central build guide) so future maintainers understand the rationale.
15-15: Approve multi-stage build introduction.
Splitting into aguestfishstage and abuilderstage improves isolation and keeps the final image minimal—this follows Docker best practices.
21-21: Verify dynamic image URL works across architectures.
Switching tonoble-server-cloudimg-${TARGETARCH}.imgrelies on the Ubuntu cloud-images endpoint supporting each${TARGETARCH}. Please confirm and handle download failures gracefully.
34-34: Removearch=filter in Docker apt source.
Droppingarch=amd64correctly enables multi-arch package retrieval from the Docker repository.
38-38: Validate apt repository integrity before installs.
Runningapt-get check -qafter adding new sources is an excellent sanity check for package-index consistency.
46-46: Ensure containerd configuration is valid.
containerd config dump >/dev/nullis an effective smoke test to verify the generated/etc/containerd/config.tomlis syntactically correct.
|
Nikita (@nbykov0) I see no changes in CI. Will it still work with these changes? |
Yes, as #907 is merged. By default everything will be built only for the platform supported by a builder (amd64). |
Components with existing dockerfiles will be updated in this PR. Part of #519 <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added support for multi-architecture and cross-platform Docker image builds across various components, enabling builds for different operating systems and CPU architectures. - **Chores** - Updated Docker build commands in multiple Makefiles to use configurable builder and platform variables, improving build flexibility. - Standardized Dockerfile build arguments and environment variables for cross-compilation. - Improved package installation commands for quieter and more minimal installs in Dockerfiles. - Changed the default bucket name configuration to "cozystack" in system bucket settings. - Updated some maintenance targets and manual update reminders in Makefiles. <!-- end of auto-generated comment: release notes by coderabbit.ai --> (cherry picked from commit 4f78b13) Signed-off-by: Timofei Larkin <lllamnyp@gmail.com>
Components with existing dockerfiles will be updated in this PR.
Part of #519
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Chores