Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cross-compile for arm64 and mount go cache #30

Merged
merged 1 commit into from
May 27, 2024
Merged

Cross-compile for arm64 and mount go cache #30

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 15 additions & 5 deletions Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,21 @@
ARG GOARCH="amd64"
FROM golang:1.22 AS builder
FROM --platform=$BUILDPLATFORM golang:1.22 AS builder

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have defaults for all these variables for amd64 and linux?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None of these arguments need to be set manually. The build platform argument as used in the from line tells docker to always use the native architecture rather than to emulate with qemu as we use cross compilation. The target arguments get set based on the platform flag which is provided in the makefile.

https://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guide/

Copy link
Contributor

@aojea aojea May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I knew I remember something about this but I didn't know what.

Here is the problem, with multiplatform the --loadflag fails docker/buildx#65

make image-build
docker buildx build . \
        --platform="linux/amd64,linux/arm64" \
        --tag="gcr.io/k8s-staging-networking/kube-network-policies:v20240527-abf99c2-dirty" \
  --load
[+] Building 0.0s (0/0)                                                                                                                                                 
ERROR: docker exporter does not currently support exporting manifest lists
make: *** [Makefile:41: image-build] Error 1

This seems to solve the problem

diff --git a/Makefile b/Makefile
index 5067609..ade162c 100644
--- a/Makefile
+++ b/Makefile
@@ -40,8 +40,7 @@ PLATFORMS?=linux/amd64,linux/arm64
 image-build:
        docker buildx build . \
                --platform="${PLATFORMS}" \
-               --tag="${IMAGE}" \
-               --load
+               --tag="${IMAGE}"

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vaskozl you resolved this comment but using the --load flag with multiple archs fails for me, is not failing for you?

Copy link
Contributor Author

@vaskozl vaskozl May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I haven't resolved it since the first comment. --load does indeed work for me. Apologies if it's still showing that way, I didn't mean to resolve it in the first place, github ate my comment and made me type it out twice earlier on my phone :/.

% make image-build
docker buildx build . \
		--platform="linux/amd64,linux/arm64" \
		--tag="gcr.io/k8s-staging-networking/kube-network-policies:v20240527-abf99c2" \
		--load
...
% docker images                            
REPOSITORY                                            TAG                 IMAGE ID       CREATED         SIZE
gcr.io/k8s-staging-networking/kube-network-policies   v20240527-abf99c2   04cd1b9d68b4   8 seconds ago   163MB

It's not necessary though if one doesn't plan to use the image locally. Looks like it might not work on older version of docker, so I suggest:

  • remove --load if we just want to test builds on all architectures without pushing on image-build
  • if we want to load it to be able to test locally, we can instead leave --load and remove the --platform flag. It will only build for the host arch in that case.

What do you think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented here https://github.com/kubernetes-sigs/kube-network-policies/pull/30/files#r1616335414 , defaulting to one platform is compatible

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ii docker-ce 5:20.10.213-0debian-bullseye amd64 Docker: the open-source application container engine

:(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you not prefer to keep PLATFORMS set to the platforms we push for, and then work around the "local" limitation by not using --platform in bulild-image (only in build-push)?

That way one wouldn't need to set PLATFORMS even if working locally and make image-push would should push upstream without error?

Copy link
Contributor Author

@vaskozl vaskozl May 27, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I say that because I personally think the fewer variables one has to juggle the better :D. I've already removed --platform from the local build. It's pretty redundant as you would never need a local alternate arch unless you ran that image with qemu.

Edit: Also looking at other projects, they seem to keep PLATFORMS populated with everything they push for.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert here, I trust you

WORKDIR /src

COPY go.mod go.sum .
RUN --mount=type=cache,target=/go/pkg \
go mod download

COPY . .
# build
RUN go mod download
RUN CGO_ENABLED=0 go build -o /go/bin/netpol ./cmd

ARG TARGETOS TARGETARCH
RUN --mount=type=cache,target=/root/.cache/go-build \
--mount=type=cache,target=/go/pkg \
CGO_ENABLED=0 GOOS=$TARGETOS GOARCH=$TARGETARCH \
go build -o /go/bin/netpol ./cmd

# STEP 2: Build small image
FROM registry.k8s.io/build-image/distroless-iptables:v0.5.2
COPY --from=builder --chown=root:root /go/bin/netpol /bin/netpol

CMD ["/bin/netpol"]
14 changes: 10 additions & 4 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,15 @@ REGISTRY?=gcr.io/k8s-staging-networking
TAG?=$(shell echo "$$(date +v%Y%m%d)-$$(git describe --always --dirty)")
# the full image tag
IMAGE?=$(REGISTRY)/$(IMAGE_NAME):$(TAG)
PLATFORMS?=linux/amd64,linux/arm64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
PLATFORMS?=linux/amd64,linux/arm64
PLATFORMS?=linux/amd64

default only to one platfor, this make it works locally and for multiplatform we can always set the variable


# required to enable buildx
export DOCKER_CLI_EXPERIMENTAL=enabled
image-build:
# docker buildx build --platform=${PLATFORMS} $(OUTPUT) --progress=$(PROGRESS) -t ${IMAGE} --pull $(EXTRA_BUILD_OPT) .
docker build . -t ${IMAGE}
docker buildx build . \
--tag="${IMAGE}" \
--load

image-push:
docker buildx build . \
--platform="${PLATFORMS}" \
--tag="${IMAGE}" \
--push
Loading