Makefile: add multiarch building support #203

lubinsz · 2018-12-17T10:16:34Z

Signed-off-by: Bin Lu bin.lu@arm.com

lubinsz · 2018-12-18T06:37:23Z

@marquiz
Any comments?

marquiz · 2019-01-17T15:20:25Z

I understand the motivation behind this. But, there are some details I don't fully understand, and, I think the current implementation is a bit too hackish.

Some quick thoughts and questions:

First, what are the Docker requirements for this? At least my docker CE v17.09 does not support this?
What about buildah?
I think there should be more detailed checks to make sure that the build tool (only Docker?) supports multiple platforms.
If the build steps start to be complex, maybe those should be moved into a separate build.sh script or smth(?)
What is the idea behind using qemu-static? I dislike the blind "wgeting" of these and creating per-arch Dockerfiles, leaving the repo dirty.
Makefile could have one target per arch, possibly utilizing make's pattern rules?
Better error handling would be needed. I think it should error out if one build fails.

lubinsz · 2019-01-25T08:16:18Z

@marquiz
I have reorganized the code according to the compilation method in Kubernetes.
Any comments?
Thanks.

marquiz

I think the patch could be streamlined still

Makefile

Dockerfile

marquiz · 2019-02-13T14:09:51Z

Dockerfile

-FROM debian:stretch-slim
+FROM BASEIMAGE2
+
+CROSS_BUILD_COPY qemu-QEMUARCH-static /usr/bin/


Again, could this be commented out, and, QEMUARCH turned into a build arg

Dockerfile

Makefile

marquiz · 2019-02-13T14:32:11Z

Makefile

+	docker run --rm --privileged multiarch/qemu-user-static:register --reset
+	curl -sSL https://github.com/multiarch/qemu-user-static/releases/download/$(QEMUVERSION)/x86_64_qemu-$(QEMUARCH)-static.tar.gz | tar -xz -C ./ 
+        # Ensure we don't get surprised by umask settings
+	chmod 0755 qemu-$(QEMUARCH)-static


Would it be possible to run this inside the Dockerfile, or, put the temporary files in a separate build directory in order to minimize the dirtying of the git work dir?

Done.
I added a new layer in Dockerfile to get the qemu file.

Makefile

fejta-bot · 2019-05-14T14:36:07Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

lubinsz · 2019-05-23T10:36:21Z

@marquiz
Any comments?

marquiz

Looks quite good, now, in my eyes. I think we could merge this after resolving the few comments I still have.

I think pulling qemu in a separate image is ok, definitely better than (re-)downloading it with every build.

Dockerfile

Makefile

marquiz · 2019-06-04T14:33:15Z

/remove-lifecycle stale

marquiz · 2019-08-27T14:13:17Z

README.md

+Optional, for example.
+```
+cd <project-root>
+docker run --rm --privileged multiarch/qemu-user-static:register --reset


Would make pre-cross be better?

marquiz · 2019-08-27T14:16:35Z

README.md

+docker run --rm --privileged multiarch/qemu-user-static:register --reset
+make image ARCH=arm64
+```
+Or, you can build all supported amd64/non-amd64 images together.


I'd suggest Or, you can build all supported architectures with one command: or similar, drop the amd64 here...

marquiz · 2019-08-27T14:21:14Z

Makefile

+	docker run --rm --privileged multiarch/qemu-user-static:register --reset
+
+# Building multi-arch docker images
+images-all: pre-cross


I think pre-cross should be listed as a phony target, or, removed from here. With the former, we always would run pre-cross when building images-all which prolly is ok

marquiz · 2019-08-27T14:30:22Z

README.md

@@ -593,6 +593,19 @@ cd <project-root>
 make
 ```

+**Build non-amd64 image on amd64:**<br>


NIT: this just looks a bit clumsy heading 😄

How about Multiarch builds or Build for target architectures or smth? If AMD64 is mentioned I think it should be capital case.

marquiz · 2019-08-27T14:32:15Z

Hmm, now my test build fails with:

Step 18/24 : RUN go install   -ldflags "-s -w -X sigs.k8s.io/node-feature-discovery/pkg/version.version=$NFD_VERSION"   ./cmd/*
 ---> Running in f0534f7d6494
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault (core dumped)

Need to investigate this. I'd like to be able to run the build myself before merging 😄

fejta-bot · 2019-11-25T15:01:44Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

marquiz · 2019-12-12T14:42:00Z

I still have issues running make image on any other ARCH than amd64. I've tried with multiple different hardware and mutiple different OSes and on most systems the build gets stuck in the go install phase. However, I was able to successfully build it on Ubuntu 16.04 with Docker.io v18.09. I haven't had the time to look at the problem closer.

What system and docker version are you using to test this on?

Could you rebase this PR?

/remove-lifecycle stale

k8s-ci-robot · 2019-12-12T14:42:08Z

@lubinsz: PR needs rebase.

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/test-infra repository.

marquiz · 2019-12-13T12:44:03Z

Dockerfile

+ARG IMAGEARCH
+FROM alpine:3.9.2 as qemu
+RUN apk add --no-cache curl
+ARG QEMUVERSION=2.9.1


QEMU version needs to be update

Suggested change

ARG QEMUVERSION=2.9.1

ARG QEMUVERSION=4.1.1-1

marquiz · 2019-12-13T12:57:09Z

OK, I think I cracked this one. QEMU version was too old and buggy. Used QEMUVERSION=4.1.1-1 and it started to work.

@lubinsz please rebase and apply that change and I think we can merge this.

fejta-bot · 2020-03-12T13:11:09Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

fejta-bot · 2020-04-11T13:54:39Z

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

marquiz · 2020-04-11T19:11:17Z

Please rebase

/remove-lifecycle rotten

marquiz · 2020-05-19T06:27:08Z

ping @lubinsz

pmundt · 2020-05-20T15:20:47Z

Since this PR was opened, Docker has also changed the way in which it does multiarch builds, now preferring the buildx extension, which amongst other things, takes care of wrapping the backing QEMU instances automatically. I suspect much of this could now be simplified by wrapping TARGETPLATFORM in the Dockerfile.

pmundt · 2020-05-20T16:28:22Z

Following up on buildx builds, I can confirm that there are no source-level modifications required in order to do multi-arch builds. I've just successfully built and run amd64 and arm64 containers on the current git HEAD as-is:

https://hub.docker.com/repository/docker/adaptant/node-feature-discovery/tags?page=1

The change to the pstate file should be split out and applied separately.

pmundt · 2020-05-21T17:54:45Z

Actually, I see that the pstate workaround has already been applied, so it's possible to close this PR.

marquiz · 2020-05-26T12:46:15Z

Following up on buildx builds, I can confirm that there are no source-level modifications required in order to do multi-arch builds. I've just successfully built and run amd64 and arm64 containers on the current git HEAD as-is:

Thanks for the investigation and testing. This could be an option if amd64 builds are not affected, not on older docker versions or other builders like podman (or docker ce).

If you have time and wish to do so you could open another PR utilising buildx(?)

pmundt · 2020-05-26T14:29:25Z

As I said, there are no source-level modifications required for buildx - it just works out of the box. The only thing I had to change was the addition of 32-bit ARM support, which was already merged in #322 . I could make a PR that documents it in the README, but to be honest, there is nothing special about its invocation within the NFD context, so the standard buildx instructions are more than sufficient.

fejta-bot · 2020-08-24T14:32:53Z

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

ArangoGutierrez · 2020-09-06T21:57:31Z

/close

k8s-ci-robot · 2020-09-06T21:57:38Z

@ArangoGutierrez: Closed this PR.

In response to this:

/close

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/test-infra repository.

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 17, 2018

k8s-ci-robot requested review from balajismaniam and marquiz December 17, 2018 10:16

k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 17, 2018

lubinsz changed the title ~~Add multiarch building support~~ Makefile: add multiarch building support Dec 17, 2018

lubinsz force-pushed the pr_multi branch 5 times, most recently from 167ac83 to bf6c489 Compare January 25, 2019 08:15

lubinsz force-pushed the pr_multi branch 6 times, most recently from 97975fa to 102a089 Compare January 25, 2019 09:08

marquiz requested changes Feb 13, 2019

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 14, 2019

lubinsz force-pushed the pr_multi branch 4 times, most recently from 5b3a4e3 to 61a6c6f Compare May 23, 2019 10:32

marquiz requested changes Jun 4, 2019

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jun 4, 2019

marquiz reviewed Aug 27, 2019

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 25, 2019

k8s-ci-robot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 12, 2019

marquiz reviewed Dec 13, 2019

View reviewed changes

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Mar 12, 2020

k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Apr 11, 2020

k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Apr 11, 2020

billimek mentioned this pull request May 27, 2020

Add support for node-feature-discovery raspbernetes/multi-arch-images#96

Closed

k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 24, 2020

k8s-ci-robot closed this Sep 6, 2020

marquiz mentioned this pull request Jan 21, 2021

Add support for arm64, armV7, armV8 #426

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Makefile: add multiarch building support #203

Makefile: add multiarch building support #203

lubinsz commented Dec 17, 2018

lubinsz commented Dec 18, 2018

marquiz commented Jan 17, 2019

lubinsz commented Jan 25, 2019

marquiz left a comment

marquiz Feb 13, 2019

marquiz Feb 13, 2019

lubinsz May 23, 2019

fejta-bot commented May 14, 2019

lubinsz commented May 23, 2019

marquiz left a comment

marquiz commented Jun 4, 2019

marquiz Aug 27, 2019

marquiz Aug 27, 2019

marquiz Aug 27, 2019

marquiz Aug 27, 2019

marquiz commented Aug 27, 2019

fejta-bot commented Nov 25, 2019

marquiz commented Dec 12, 2019

k8s-ci-robot commented Dec 12, 2019

marquiz Dec 13, 2019

marquiz commented Dec 13, 2019

fejta-bot commented Mar 12, 2020

fejta-bot commented Apr 11, 2020

marquiz commented Apr 11, 2020

marquiz commented May 19, 2020

pmundt commented May 20, 2020

pmundt commented May 20, 2020

pmundt commented May 21, 2020

marquiz commented May 26, 2020

pmundt commented May 26, 2020

fejta-bot commented Aug 24, 2020

ArangoGutierrez commented Sep 6, 2020

k8s-ci-robot commented Sep 6, 2020

Makefile: add multiarch building support #203

Makefile: add multiarch building support #203

Conversation

lubinsz commented Dec 17, 2018

lubinsz commented Dec 18, 2018

marquiz commented Jan 17, 2019

lubinsz commented Jan 25, 2019

marquiz left a 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

fejta-bot commented May 14, 2019

lubinsz commented May 23, 2019

marquiz left a comment

Choose a reason for hiding this comment

marquiz commented Jun 4, 2019

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

marquiz commented Aug 27, 2019

fejta-bot commented Nov 25, 2019

marquiz commented Dec 12, 2019

k8s-ci-robot commented Dec 12, 2019

Choose a reason for hiding this comment

marquiz commented Dec 13, 2019

fejta-bot commented Mar 12, 2020

fejta-bot commented Apr 11, 2020

marquiz commented Apr 11, 2020

marquiz commented May 19, 2020

pmundt commented May 20, 2020

pmundt commented May 20, 2020

pmundt commented May 21, 2020

marquiz commented May 26, 2020

pmundt commented May 26, 2020

fejta-bot commented Aug 24, 2020

ArangoGutierrez commented Sep 6, 2020

k8s-ci-robot commented Sep 6, 2020