Enable multi-arch build for cilium-cli image #2782

marcofranssen · 2024-09-02T08:36:22Z

This PR builds on top of #2842

It also includes the cilium-cli target to the build using multi-arch.

In the PR we levarage some buildx features like --cache and --link to improve the speed of builds for multi-arch.

Resolves #2780

maintainer-s-little-helper · 2024-09-02T08:36:25Z

Commits 42d8aa8, ed8ea68 do not match "Signed-off-by".

Please follow instructions provided in https://docs.cilium.io/en/stable/contributing/development/contributing_guide/#developer-s-certificate-of-origin

aanm

@michi-covalent @tklauser with the merge of cilium-cli into cilium/cilium it's becoming confusing to where contributors can submit their changes for cilium-cli.

aanm · 2024-09-03T07:59:37Z

Dockerfile

+# when bumping to a new version analyze the new version for security issues
+# then use crane to lookup the digest of that version so we are immutable
+# crane digest tonistiigi/xx:1.5.0
+FROM --platform=$BUILDPLATFORM tonistiigi/xx:1.5.0@sha256:0c6a569797744e45955f39d4f7538ac344bfb7ebf0a54006a0a4297b153ccf0f AS xx


Is this really necessary? It shouldn't be since we cross compile cilium without tonistiigi/xx as an example.

This is required within Docker, to compile with CGO_ENABLED=0. Don't recall the exact reason, I think it was related to alpine based builds (muslc, vs glibc).

I also use some buildx features that allow caching and parallel builds for all the supported platforms to have a speedy and optimized build.

I contributed similar to the Spire project couple of years ago.

In essence the tool does following and is basically a small little wrapper.

https://github.com/tonistiigi/xx?tab=readme-ov-file#go--cgo

It wraps the go command by setting some variables accordingly based on the buildx args.

Saves a lot of plumbing.

re-requested review from andre. let's see what he says 🙏

marcofranssen · 2024-09-04T09:18:09Z

@michi-covalent @tklauser with the merge of cilium-cli into cilium/cilium it's becoming confusing to where contributors can submit their changes for cilium-cli.

Yes, now I'm confused 😕

Should I create such PR on the cilium repo?

Maybe this repo has to be archived?

Let me know how to proceed.

michi-covalent · 2024-09-05T20:52:36Z

with the merge of cilium-cli into cilium/cilium it's becoming confusing to where contributors can submit their changes for cilium-cli.

sorry for the delay, we build release artifacts from this repo. assuming we want to build this docker image for each release tag, this is the right repo for this pull request.

@marcofranssen let's limit this pull request to Dockerfile changes only. once it gets approved & merged, i'll make necessary changes to github workflows.

Dockerfile.ci

michi-covalent · 2024-10-29T15:12:58Z

.github/workflows/images.yaml

+            platforms: linux/amd64,linux/arm64
+          - name: cilium-cli-ci
+            dockerfile: ./Dockerfile
+            platforms: linux/amd64
+


let's make this workflow change (pushing to quay.io/cilium/cilium-cli) in a separate pull request as a follow up. i (or @tklauser) needs to work with cilium maintainers to create new quay.io repo and such.

for this pull request, let's just push the multi arch image to quay.io/cilium/cilium-cli-ci.

edit: turned out https://quay.io/repository/cilium/cilium-cli?tab=settings already exists. we still need to set up proper credentials to push images to the repo 🚀🙏

Am i wrong to think this is quite duplicate of what i was trying to achieve with #2755 and PR #2770? we already have the option to build multiarch with the changes there

marcofranssen · 2024-11-01T10:53:46Z

@michi-covalent maybe merge this one first #2842

This PR builds on top of that and reduces the scope of this PR.

marcofranssen · 2024-11-11T09:51:44Z

Builds on top of #2842 to reduce the scope of this PR. Can we please move forward with both. These PRs are out there now for several weeks without progress on a review of these relative simple PRs. I'm waiting to be able to use the improvements.

tklauser · 2024-11-12T08:08:34Z

@marcofranssen this PR picked up a merge conflict and needs a rebase.

marcofranssen · 2024-11-12T11:35:03Z

@marcofranssen this PR picked up a merge conflict and needs a rebase.

Done

marcofranssen · 2024-11-12T11:36:21Z

Dockerfile

+    --mount=type=cache,target=/go/pkg/mod \
+    xx-go --wrap && \
+    make && \
+    xx-verify --static /go/src/github.com/cilium/cilium-cli/cilium


It also allows verifying the binary, see here.

marcofranssen · 2024-11-12T11:36:52Z

Dockerfile

+COPY --link --from=xx / /
+RUN --mount=type=cache,target=/root/.cache/go-build \
+    --mount=type=cache,target=/go/pkg/mod \
+    xx-go --wrap && \


Small little wrapper that sets the correct go build flags and such based on TARGETPLATFORM and TARGETARCH and such.

marcofranssen · 2024-11-12T11:37:11Z

Dockerfile

-COPY --from=builder --chown=root:root --chmod=755 /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
-COPY --from=builder /go/src/github.com/cilium/cilium-cli/cilium /usr/local/bin/cilium
+COPY --link --from=builder --chown=root:root --chmod=755 /etc/ssl/certs/ca-certificates.crt /etc/ssl/certs/
+COPY --link --from=builder --chown=1000:1000 --chmod=755 /go/src/github.com/cilium/cilium-cli/cilium /usr/local/bin/cilium


Run as non root

marcofranssen · 2024-11-12T11:40:01Z

.github/workflows/images.yaml

@@ -57,10 +60,10 @@ jobs:
          ref: ${{ steps.tag.outputs.tag }}

      # main branch or tag pushes
-      - name: CI Build ${{ matrix.name }}


renamed because the wf now builds both the ci and plain cli image.

marcofranssen · 2024-11-20T19:07:06Z

Rebased the branch again to resolve conflicts

Signed-off-by: Marco Franssen <[email protected]>

marcofranssen · 2024-11-25T12:17:25Z

Did another rebase to get branch fully in sync with upstream

michi-covalent

LGTM overall, only one feedback regarding when to push to the non-CI repo.

pinging @aanm for re-review 🏓

michi-covalent · 2024-11-25T15:07:29Z

.github/workflows/images.yaml

@@ -57,10 +60,10 @@ jobs:
          ref: ${{ steps.tag.outputs.tag }}

      # main branch or tag pushes
-      - name: CI Build ${{ matrix.name }}
+      - name: Build ${{ matrix.name }}
        if: ${{ github.event_name != 'pull_request_target' }}


let's run step only on tag push, not branch push. https://quay.io/repository/cilium/cilium-cli?tab=tags should only contain released images 🙏

Suggested change

if: ${{ github.event_name != 'pull_request_target' }}

if: ${{ github.ref_type == 'tag' }}

marcofranssen requested review from a team as code owners September 2, 2024 08:36

marcofranssen requested review from aanm and christarazi September 2, 2024 08:36

marcofranssen requested a review from derailed September 2, 2024 08:36

maintainer-s-little-helper bot added the dont-merge/needs-sign-off label Sep 2, 2024

marcofranssen had a problem deploying to ci September 2, 2024 08:36 — with GitHub Actions Error

marcofranssen force-pushed the cilium-cli-image branch from ed8ea68 to 0b948bf Compare September 2, 2024 08:38

maintainer-s-little-helper bot removed the dont-merge/needs-sign-off label Sep 2, 2024

marcofranssen temporarily deployed to ci September 2, 2024 08:38 — with GitHub Actions Inactive

marcofranssen force-pushed the cilium-cli-image branch from 0b948bf to e06aa0d Compare September 2, 2024 08:46

marcofranssen temporarily deployed to ci September 2, 2024 08:46 — with GitHub Actions Inactive

marcofranssen force-pushed the cilium-cli-image branch from e06aa0d to 2bf1e74 Compare September 2, 2024 08:49

marcofranssen had a problem deploying to ci September 2, 2024 08:50 — with GitHub Actions Error

marcofranssen force-pushed the cilium-cli-image branch from 2bf1e74 to ed22c94 Compare September 2, 2024 08:50

marcofranssen temporarily deployed to ci September 2, 2024 08:50 — with GitHub Actions Inactive

marcofranssen force-pushed the cilium-cli-image branch from ed22c94 to 8b4242d Compare September 2, 2024 09:04

marcofranssen temporarily deployed to ci September 2, 2024 09:04 — with GitHub Actions Inactive

aanm requested changes Sep 3, 2024

View reviewed changes

aanm requested review from tklauser and michi-covalent September 3, 2024 08:01

michi-covalent reviewed Sep 5, 2024

View reviewed changes

Dockerfile.ci Outdated Show resolved Hide resolved

tklauser removed their request for review September 11, 2024 09:02

christarazi marked this pull request as draft September 30, 2024 17:35

marcofranssen force-pushed the cilium-cli-image branch from 8b4242d to 1a22c85 Compare October 29, 2024 07:47

marcofranssen temporarily deployed to ci October 29, 2024 07:47 — with GitHub Actions Inactive

marcofranssen temporarily deployed to ci October 29, 2024 09:27 — with GitHub Actions Inactive

This was referenced Oct 29, 2024

Improve cacheability of Docker layers and add ca-certs to scratch image #2842

Merged

Slim version of the cilium-cli image #2780

Open

michi-covalent requested a review from aanm October 29, 2024 15:09

michi-covalent reviewed Oct 29, 2024

View reviewed changes

christarazi marked this pull request as draft November 6, 2024 19:53

marcofranssen force-pushed the cilium-cli-image branch from cafcdf6 to f1f64f7 Compare November 11, 2024 09:40

marcofranssen had a problem deploying to ci November 11, 2024 09:40 — with GitHub Actions Error

marcofranssen force-pushed the cilium-cli-image branch from f1f64f7 to 6e217b5 Compare November 11, 2024 09:46

marcofranssen temporarily deployed to ci November 11, 2024 09:46 — with GitHub Actions Inactive

marcofranssen marked this pull request as ready for review November 11, 2024 09:50

tklauser added the needs-rebase This PR needs to be rebased because it has merge conflicts. label Nov 12, 2024

marcofranssen force-pushed the cilium-cli-image branch from 6e217b5 to 4c1a5a2 Compare November 12, 2024 11:34

marcofranssen temporarily deployed to ci November 12, 2024 11:34 — with GitHub Actions Inactive

marcofranssen commented Nov 12, 2024

View reviewed changes

marcofranssen force-pushed the cilium-cli-image branch from 4c1a5a2 to f73e9f6 Compare November 20, 2024 19:06

marcofranssen temporarily deployed to ci November 20, 2024 19:06 — with GitHub Actions Inactive

Enable multi-arch Docker build

0a6057e

Signed-off-by: Marco Franssen <[email protected]>

marcofranssen force-pushed the cilium-cli-image branch from f73e9f6 to 0a6057e Compare November 25, 2024 12:17

marcofranssen temporarily deployed to ci November 25, 2024 12:17 — with GitHub Actions Inactive

michi-covalent reviewed Nov 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable multi-arch build for cilium-cli image #2782

Enable multi-arch build for cilium-cli image #2782

marcofranssen commented Sep 2, 2024 •

edited

Loading

maintainer-s-little-helper bot commented Sep 2, 2024

aanm left a comment

aanm Sep 3, 2024

marcofranssen Sep 4, 2024 •

edited

Loading

marcofranssen Oct 29, 2024

michi-covalent Oct 29, 2024

marcofranssen commented Sep 4, 2024

michi-covalent commented Sep 5, 2024

michi-covalent Oct 29, 2024 •

edited

Loading

leppeK Nov 1, 2024 •

edited

Loading

marcofranssen commented Nov 1, 2024

marcofranssen commented Nov 11, 2024

tklauser commented Nov 12, 2024

marcofranssen commented Nov 12, 2024

marcofranssen Nov 12, 2024

marcofranssen Nov 12, 2024

marcofranssen Nov 12, 2024

marcofranssen Nov 12, 2024

marcofranssen commented Nov 20, 2024

marcofranssen commented Nov 25, 2024

michi-covalent left a comment

michi-covalent Nov 25, 2024

	if: ${{ github.event_name != 'pull_request_target' }}
	if: ${{ github.ref_type == 'tag' }}

Enable multi-arch build for cilium-cli image #2782

Are you sure you want to change the base?

Enable multi-arch build for cilium-cli image #2782

Conversation

marcofranssen commented Sep 2, 2024 • edited Loading

maintainer-s-little-helper bot commented Sep 2, 2024

aanm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcofranssen Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcofranssen commented Sep 4, 2024

michi-covalent commented Sep 5, 2024

michi-covalent Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

leppeK Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

marcofranssen commented Nov 1, 2024

marcofranssen commented Nov 11, 2024

tklauser commented Nov 12, 2024

marcofranssen commented Nov 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcofranssen commented Nov 20, 2024

marcofranssen commented Nov 25, 2024

michi-covalent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcofranssen commented Sep 2, 2024 •

edited

Loading

marcofranssen Sep 4, 2024 •

edited

Loading

michi-covalent Oct 29, 2024 •

edited

Loading

leppeK Nov 1, 2024 •

edited

Loading