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.

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

Makefile add optional CONTAINER_PLATFORM flag to container builds #7347

Merged
merged 1 commit into from
Jun 19, 2023

Conversation

guymguym
Copy link
Member

@guymguym guymguym commented Jun 16, 2023

Explain the changes

  1. See https://lima-vm.io/?file=docs/multi-arch.md
  2. See https://github.com/containerd/nerdctl/blob/main/docs/multi-platform.md
  3. used CONTAINER_PLATFORM=amd64 for building x86_64 images on Mac arm64.
  4. In a lima shell:
make noobaa CONTAINER_ENGINE=nerdctl CONTAINER_PLATFORM=amd64

CC @Utkarsh-pro

Issues: Fixed #xxx / Gap #xxx

  1. NA

Testing Instructions:

  1. NA
  • Doc added/updated
  • Tests added

Signed-off-by: Guy Margalit <guymguym@gmail.com>
@guymguym guymguym changed the title Makefile add optional CONTAINER_PLATFORM to docker build calls Makefile add optional CONTAINER_PLATFORM flag to container builds Jun 18, 2023
Copy link
Contributor

@liranmauda liranmauda left a comment

Choose a reason for hiding this comment

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

LGTM

@guymguym guymguym merged commit bebd857 into noobaa:master Jun 19, 2023
5 of 6 checks passed
@@ -3,6 +3,14 @@ ifeq ($(CONTAINER_ENGINE),)
CONTAINER_ENGINE=$(shell podman version >/dev/null 2>&1 && echo podman)
endif

# see https://github.com/containerd/nerdctl/blob/main/docs/multi-platform.md
# e.g use CONTAINER_PLATFORM=amd64 for building x86_64 on arm.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# e.g use CONTAINER_PLATFORM=amd64 for building x86_64 on arm.
# e.g use CONTAINER_PLATFORM_FLAG=amd64 for building x86_64 on arm.

Copy link
Member Author

Choose a reason for hiding this comment

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

no, notice that I had to change it because it failed otherwise. The user needs to set the CONTAINER_PLATFORM=amd64 and then the Makefile will sets the CONTAINER_PLATFORM_FLAG=--platform=amd64 (approppriately) for all the container build calls...

@tangledbytes
Copy link
Member

@guymguym, docker supports multi platform builds as well.

$ docker run --platform linux/amd64 busybox uname -a
Linux a7476d751565 5.15.82-0-virt #1-Alpine SMP Mon, 12 Dec 2022 09:15:17 +0000 x86_64 GNU/Linux

$ docker build --platform linux/arm64,linux/amd64,linux/ppc64le ...` # This requires a custom docker builder  though - docker buildx create ...

@guymguym
Copy link
Member Author

docker supports multi platform builds as well.

@Utkarsh-pro even better then! I just tried with lima and nerdctl, but I assumed this flag could be applicable for other container engines as well.

@liranmauda
Copy link
Contributor

@guymguym @Utkarsh-pro
we need to see that podman support that too.
Currently, we all should move to podman.

@guymguym
Copy link
Member Author

@liranmauda it was very simple for me to check that podman has the exact same --platform flag too. However I am not trying to make any suggestions about running this downstream. But if you want to try, now you can with a simple arg.

@liranmauda
Copy link
Contributor

@liranmauda it was very simple for me to check that podman has the exact same --platform flag too. However I am not trying to make any suggestions about running this downstream. But if you want to try, now you can with a simple arg.

I remember that at some point we could not do it as podman was behind.
In any case, those flows are not triggered downstream, so we are safe there. (we are not using our makefile there).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants