k8snetworkplumbingwg / ovs-cni

Open vSwitch CNI plugin
Apache License 2.0
224 stars 71 forks source link

Fix: multiarch build by using crosscompilation #336

Open ykulazhenkov opened 3 days ago

ykulazhenkov commented 3 days ago

What this PR does / why we need it:

Use quay.io/centos/centos:stream9 with the –platform=$BUILDPLATFORM flag, in this case the image is pulled for the builder host's current architecture. The BUILDOS and BUILDARCH args are used to download the right go binary for the build platform. Cross-compilation occurs in the builder image using TARGETOS and TARGETARCH build arguments to determine the target OS/arch. These args are set automatically by the multiarch-build process (with docker buildx). The final container image (registry.access.redhat.com/ubi9/ubi-minimal) is pulled for the correct target architecture, such as amd64 or arm64.

This update fixes support for multiarch builds and speeds up image building, as cross-compilation is faster than compiling on a non-native platform.

Special notes for your reviewer:

Fixes support for multiarch builds added in https://github.com/k8snetworkplumbingwg/ovs-cni/pull/307

Multiarch build was broken by https://github.com/k8snetworkplumbingwg/ovs-cni/pull/319

Release note:

NONE
kubevirt-bot commented 3 days ago

Hi @ykulazhenkov. Thanks for your PR.

I'm waiting for a k8snetworkplumbingwg member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes-sigs/prow](https://github.com/kubernetes-sigs/prow/issues/new?title=Prow%20issue:) repository.
ykulazhenkov commented 3 days ago

@phoracek @SchSeba please take a look on this one

ykulazhenkov commented 3 days ago

Additional context is in this message https://github.com/k8snetworkplumbingwg/ovs-cni/issues/260#issuecomment-2495530500

Example of the job with the code from this PR: https://github.com/ykulazhenkov/ovs-cni/actions/runs/12008233711/job/33470460458 And artifacts https://github.com/ykulazhenkov/ovs-cni/pkgs/container/ovs-cni-plugin/311494909?tag=v0.101.0

kubevirt-bot commented 3 days ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: oshoval, ykulazhenkov Once this PR has been reviewed and has the lgtm label, please assign alonakaplan for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/k8snetworkplumbingwg/ovs-cni/blob/main/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
ashokpariya0 commented 17 hours ago

@ykulazhenkov We should update the GOARCH ?= amd64 line (as seen in this Makefile) to: GOARCH ?= $(shell uname -m | sed 's/x86_64/amd64/') This change will dynamically set GOARCH based on the host architecture, converting x86_64 to amd64 as needed.

@oshoval changes look good to me, assuming the idea is to provide multiplatform support only through GitHub Actions, exclusively for the Docker container runtime, and not through make commands or for the Podman container runtime.

oshoval commented 17 hours ago

@oshoval changes look good to me, assuming the idea is to provide multiplatform support only through GitHub Actions, exclusively for the Docker container runtime, and not through make commands or for the Podman container runtime.

thanks

at the end we will do need also images that are multi platform though isnt it? (as the issue / PRs you work on) it is fine to do so in steps, as long as those steps can co exists

ashokpariya0 commented 16 hours ago

@oshoval changes look good to me, assuming the idea is to provide multiplatform support only through GitHub Actions, exclusively for the Docker container runtime, and not through make commands or for the Podman container runtime.

thanks

at the end we will do need also images that are multi platform though isnt it? (as the issue / PRs you work on) it is fine to do so in steps, as long as those steps can co exists

Yes, we can consider adding multiplatform support using a Makefile(using make commands) for both Docker and Podman in the future. This would allow us to build images on our local machine, in a specific environment, or within any CI system, giving us more control over the build process and environment. For Podman, there are no additional dependencies required to build multiplatform images, while for Docker, we would need the docker buildx tool.

ykulazhenkov commented 15 hours ago

We should update the GOARCH ?= amd64 line (as seen in this Makefile) to: GOARCH ?= $(shell uname -m | sed 's/x86_64/amd64/') This change will dynamically set GOARCH based on the host architecture, converting x86_64 to amd64 as needed.

done