Add multi platform support #444

ashokpariya0 · 2024-12-11T11:58:05Z

What this PR does / why we need it:
The upstream kubemacpool image (link) currently supports only the amd64 architecture. The proposed changes in this PR enable multi-platform support for the kubemacpool image, broadening its compatibility to include additional architectures.

Special notes for your reviewer:
Multiplatform Build Instruction: https://gist.github.com/ashokpariya0/33cf22401d17153b7f91b40295397ccd

Multi-platform build support is enabled for both Docker and Podman container runtimes, and the necessary changes have been made accordingly.
The PLATFORMS variable defines the target platforms for building the manager image, allowing support for multiple architectures.

To use this feature, you need to:

For Docker:

Have access to docker buildx. More information: [Docker Buildx Documentation](https://docs.docker.com/build/building/multi-platform/)
Ensure that BuildKit is enabled. More information: [Docker BuildKit Enhancements](https://docs.docker.com/develop/develop-images/build_enhancements/)
For Podman:

No additional dependencies are required to build multi-platform images. Podman supports this feature natively.

Release note:

None

kubevirt-bot · 2024-12-11T11:58:15Z

Hi @ashokpariya0. Thanks for your PR.

PRs from untrusted users cannot be marked as trusted with /ok-to-test in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

ashokpariya0 · 2024-12-12T13:04:56Z

@RamLavi Please take a look at this PR whenever you have time.

RamLavi

@ashokpariya0 thanks for the refactor, it is now much more easier to review. Added some questions

hack/build-kubemacpool-docker.sh

hack/build-kubemacpool-podman.sh

RamLavi · 2024-12-12T20:05:21Z

/test pull-kubemacpool-e2e-k8s
/test pull-kubemacpool-unit-test

RamLavi · 2024-12-15T07:16:12Z

/test all

RamLavi · 2024-12-15T07:27:00Z

Hey @ashokpariya0, I'm seeing WARN lines when running the podman option:

WARN[0000] missing "BUILDOS" build argument. Try adding "--build-arg BUILDOS=<VALUE>" to the command line 
WARN[0000] missing "BUILDARCH" build argument. Try adding "--build-arg BUILDARCH=<VALUE>" to the command line

Can we do something about these?

RamLavi · 2024-12-15T07:28:40Z

build/Dockerfile

+COPY go.mod .
+COPY go.sum .
+RUN GO_VERSION=$(sed -En 's/^go +(.*)$/\1/p' go.mod) && \
+    wget https://dl.google.com/go/go${GO_VERSION}.${BUILDOS}-${BUILDARCH}.tar.gz && \


nit: the wget is giving off a long output that's not very helpful. Please consider adding the --quiet flag.

Sure, Done.

RamLavi · 2024-12-15T07:38:17Z

@ashokpariya0 docker case is failing on my local:

selecting docker as container runtime
+ registry_port=32822
+ push_registry=localhost:32822
+ manifest_registry=registry:5000
+ config_dir=./config/test
+ REGISTRY=localhost:32822
+ make container
make[1]: Entering directory '/root/github.com/k8snetworkplumbingwg/kubemacpool'
./hack/build-multiarch-kubemacpool.sh amd64 linux/amd64 localhost:32822/kubevirt/kubemacpool:latest localhost:32822/kubevirt/kubemacpool:v0.44.0-9-g48f76fe4 kubemacpool-docker-builder docker
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
            Install the buildx component to build images with BuildKit:
            https://docs.docker.com/go/buildx/

unknown flag: --push
See 'docker build --help'.
make[1]: *** [Makefile:101: container] Error 125
make[1]: Leaving directory '/root/github.com/k8snetworkplumbingwg/kubemacpool'
make: *** [Makefile:118: cluster-sync] Error 2

[UPDATE]: installing the new docker-ce worked - thanks @oshoval !

oshoval · 2024-12-15T07:45:19Z

Please install new one using https://docs.docker.com/engine/install/fedora/
or switch to podman (both fixed it for me)

RamLavi · 2024-12-15T08:11:34Z

Please install new one using https://docs.docker.com/engine/install/fedora/ or switch to podman (both fixed it for me)

yeah podman worked locally, I was just making sure both work.
installing the new docker-ce worked - thanks!

ashokpariya0 · 2024-12-15T08:39:52Z

Hey @ashokpariya0, I'm seeing WARN lines when running the podman option:
WARN[0000] missing "BUILDOS" build argument. Try adding "--build-arg BUILDOS=<VALUE>" to the command line 
WARN[0000] missing "BUILDARCH" build argument. Try adding "--build-arg BUILDARCH=<VALUE>" to the command line
Can we do something about these?

I think this is a non-critical warning intended to inform you that these build arguments are available and can be set explicitly. However, since default values are already provided in your Dockerfile, there is no need to worry if the defaults are acceptable. The warning can be safely ignored without affecting the build process. I can see BUILDOS and BUILDARCH correctly set by podman and docker on amd64,s390x build machine, so we are good here,

These changes enable building kubemacpool container images for multiple platforms (amd64, s390x, arm64) from a single Dockerfile. Signed-off-by: Ashok Pariya [email protected]

RamLavi · 2024-12-18T08:31:09Z

@ashokpariya0 please rebase, I think I'm good otherwise

ashokpariya0 · 2024-12-18T09:03:10Z

@ashokpariya0 please rebase, I think I'm good otherwise

Thanks @RamLavi I’m working on it. I’ve realigned my changes based on your updates in PR: #445. Currently testing a few scenarios for both local and non-local registries. I’ll raise a PR shortly.

ashokpariya0 · 2024-12-18T11:03:04Z

@ashokpariya0 please rebase, I think I'm good otherwise

Done, Please check.

RamLavi · 2024-12-18T11:31:49Z

/approve

@oshoval feel free to lgtm when you're good with this

kubevirt-bot · 2024-12-18T11:31:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RamLavi

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [RamLavi]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

oshoval

Thanks
Please consider to always mention podman first, i.e PR desc (twice), commits etc

oshoval · 2024-12-22T14:47:45Z

Makefile

+# PLATFORMS ?= linux/amd64,linux/arm64,linux/s390x
+# Alternatively, you can set the PLATFORMS variable using:
+# export PLATFORMS=linux/arm64,linux/s390x,linux/amd64
+# or export PLATFORMS=all to automatically include all supported platforms.


There should be a PR that set platforms=all on the release right ? (unless there is already)

@oshoval I don't see any post-submit Prow jobs that handle the release for the macpool CNI image.

@RamLavi Could you please let me know how the image is being built and pushed to quay.io?
Also, please note that we need to set PLATFORMS=all before running make build to ensure the image is built with multi-platform support.

github/ci/prow-deploy/files/jobs/k8snetworkplumbingwg/kubemacpool/kubemacpool-postsubmits.yaml ?

Yes, thanks! I found it here: kubemacpool-postsubmits.yaml.

I’ll go ahead and submit the PR for that.

Done, PR link: kubevirt/project-infra#3855

oshoval · 2024-12-22T14:54:34Z

hack/build-multiarch-kubemacpool.sh

+OCI_BIN=$6
+REGISTRY=$7
+
+if [ "$OCI_BIN" == "docker" ]; then


Please switch order, as we encourage to use podman all over

I don’t think the order will make a difference here since we’re passing the OCI_BIN value from the Makefile, and the Makefile already takes care of it. You can see the relevant part of the Makefile here: link to Makefile.

it doesnt make a difference of course, it is just semantic that podman is the preferable suggestion please
same as in the line you mentioned, and on readmes, podman is first
anyhow this is a nit

oshoval · 2024-12-22T15:13:47Z

/test all

These changes provide multi-platform build support for the Kubemacpool CNI, enabling builds for Docker and Podman container runtimes. Signed-off-by: Ashok Pariya [email protected]

oshoval · 2024-12-23T09:18:50Z

Thanks

/lgtm

just please consider this nit
#444 (review)

oshoval · 2024-12-23T09:19:01Z

/test all

kubevirt-bot added release-note-none dco-signoff: yes labels Dec 11, 2024

kubevirt-bot requested review from RamLavi and rhrazdil December 11, 2024 11:58

kubevirt-bot added the size/L label Dec 11, 2024

ashokpariya0 mentioned this pull request Dec 11, 2024

Enable Multi-Platform Support for kubemacpool Image Builds #441

Closed

RamLavi reviewed Dec 12, 2024

View reviewed changes

hack/build-kubemacpool-docker.sh Outdated Show resolved Hide resolved

hack/build-kubemacpool-podman.sh Show resolved Hide resolved

ashokpariya0 force-pushed the add-multi-platform-support branch from 48f76fe to ac56357 Compare December 13, 2024 05:05

ashokpariya0 mentioned this pull request Dec 13, 2024

Enable support for multiple platforms in the Linux Bridge CNI. kubevirt/cluster-network-addons-operator#1927

Merged

RamLavi reviewed Dec 15, 2024

View reviewed changes

ashokpariya0 force-pushed the add-multi-platform-support branch from ac56357 to 1d654be Compare December 15, 2024 08:35

kubevirt-bot added the needs-rebase label Dec 16, 2024

Modify Dockerfile to support multi-platform builds.

cacc9f7

These changes enable building kubemacpool container images for multiple platforms (amd64, s390x, arm64) from a single Dockerfile. Signed-off-by: Ashok Pariya [email protected]

ashokpariya0 force-pushed the add-multi-platform-support branch from 1d654be to 5c7d35f Compare December 18, 2024 11:00

kubevirt-bot removed the needs-rebase label Dec 18, 2024

kubevirt-bot added the approved label Dec 18, 2024

oshoval reviewed Dec 22, 2024

View reviewed changes

Add Multi-Platform support for Kubemacpool Image using make command.

2c03979

These changes provide multi-platform build support for the Kubemacpool CNI, enabling builds for Docker and Podman container runtimes. Signed-off-by: Ashok Pariya [email protected]

ashokpariya0 force-pushed the add-multi-platform-support branch from 5c7d35f to 2c03979 Compare December 23, 2024 09:16

kubevirt-bot assigned oshoval Dec 23, 2024

kubevirt-bot added the lgtm label Dec 23, 2024

kubevirt-bot merged commit 2cf12d0 into k8snetworkplumbingwg:main Dec 23, 2024
4 of 5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add multi platform support #444

Add multi platform support #444

ashokpariya0 commented Dec 11, 2024

kubevirt-bot commented Dec 11, 2024

ashokpariya0 commented Dec 12, 2024

RamLavi left a comment

RamLavi commented Dec 12, 2024

RamLavi commented Dec 15, 2024

RamLavi commented Dec 15, 2024

RamLavi Dec 15, 2024

ashokpariya0 Dec 15, 2024

RamLavi commented Dec 15, 2024 •

edited

Loading

oshoval commented Dec 15, 2024

RamLavi commented Dec 15, 2024

ashokpariya0 commented Dec 15, 2024

RamLavi commented Dec 18, 2024

ashokpariya0 commented Dec 18, 2024

ashokpariya0 commented Dec 18, 2024

RamLavi commented Dec 18, 2024

kubevirt-bot commented Dec 18, 2024

oshoval left a comment

oshoval Dec 22, 2024

ashokpariya0 Dec 22, 2024

oshoval Dec 22, 2024 •

edited

Loading

ashokpariya0 Dec 22, 2024

ashokpariya0 Dec 23, 2024

oshoval Dec 22, 2024

ashokpariya0 Dec 22, 2024

oshoval Dec 22, 2024 •

edited

Loading

ashokpariya0 Dec 23, 2024

oshoval commented Dec 22, 2024

oshoval commented Dec 23, 2024

oshoval commented Dec 23, 2024

Add multi platform support #444

Add multi platform support #444

Conversation

ashokpariya0 commented Dec 11, 2024

kubevirt-bot commented Dec 11, 2024

ashokpariya0 commented Dec 12, 2024

RamLavi left a comment

Choose a reason for hiding this comment

RamLavi commented Dec 12, 2024

RamLavi commented Dec 15, 2024

RamLavi commented Dec 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RamLavi commented Dec 15, 2024 • edited Loading

oshoval commented Dec 15, 2024

RamLavi commented Dec 15, 2024

ashokpariya0 commented Dec 15, 2024

RamLavi commented Dec 18, 2024

ashokpariya0 commented Dec 18, 2024

ashokpariya0 commented Dec 18, 2024

RamLavi commented Dec 18, 2024

kubevirt-bot commented Dec 18, 2024

oshoval left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval Dec 22, 2024 • edited Loading

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

Choose a reason for hiding this comment

oshoval Dec 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oshoval commented Dec 22, 2024

oshoval commented Dec 23, 2024

oshoval commented Dec 23, 2024

RamLavi commented Dec 15, 2024 •

edited

Loading

oshoval Dec 22, 2024 •

edited

Loading

oshoval Dec 22, 2024 •

edited

Loading