Added Arm Multi-arch Make Logic #36

firethestars · 2020-03-28T05:41:16Z

I am hoping to resolve part of Issue #18 with this pull request (would have included ppc64le, but I only have amd64 and arm hardware to test against). I tried to follow the logic in the velero Makefile as closely as possible, so this should look familiar. This will require/create extra dockerhub repositories of velero-plugin-for-aws-amd64, velero-plugin-for-aws-arm, and velero-plugin-for-aws-arm64; which are needed to manifest the multi-arch velero-plugin-for-aws image.

Added file Dockerfile-arm for building arm 32bit builds
Added file Dockerfile-arm64 for building arm 64bit builds
modified Makefile to try and have similar logic to the multi-arch Makefile in the base velero project adding targets all-containers, all-push, manifest, etc. Original targets should continue to work as is.
updated hack/docker-push to use the new targets to build multi-arch images

Signed-off-by: Chris Lynch [email protected]

- Added file Dockerfile-arm for building arm 32bit builds - Added file Dockerfile-arm64 for building arm 64bit builds - modified Makefile to try and have similar logic to the multi-arch Makefile in the base velero project adding targets all-containers, all-push, manifest, etc. Original targets should continue to work as is. - updated hack/docker-push to use the new targets to build multi-arch images Signed-off-by: Chris Lynch <[email protected]>

firethestars · 2020-03-30T18:49:39Z

TravisCI seems to be stuck as it has been at this for several days without returning. If I had to guess, I would say that it is the changes to hack/docker-push.sh that it might object to. Let me know if there is something on my side that requires changing or that I can help with.

nrb · 2020-04-17T16:40:01Z

Hi @firethestars, sorry for the long delay in replying. https://travis-ci.org/github/vmware-tanzu/velero-plugin-for-aws/builds/667981259 says that it finished, but I don't see that it's updated here. I'm going to close and re-open the PR to see if that kicks it off again.

firethestars · 2020-04-18T05:19:42Z

@nrb, thanks for kicking it off again. Looks like it passed the automatic checks. Just let me know if there is anything I can do to help on my side.

xunholy · 2020-04-19T10:04:42Z

Rather than creating multiple Dockerfiles essentially duplicating the same content, you could approach it using docker buildx to build for a much wider variety of architectures currently supported. Here is an example of how that might look in reality - https://github.com/raspbernetes/multi-arch-images/blob/master/build/velero-aws-plugin/Dockerfile

firethestars · 2020-04-25T04:59:49Z

@xunholy That is a pretty interesting tool that I did not even know existed. If that is the preferred way to do multi-arch builds, then I can certainly alter the pull request to use that instead (might just submit a new parallel one).

My main concern about moving to buildx would be that then it will be using a different build/make workflow than the base velero project (which cannot as easily take up buildx because they are doing some extra work-around stuff in the dockerfile for powerpc).

If you don't mind my asking, in the example Dockerfile you linked to, I note your initial from does not have a "--platform=$BUILDPLATFORM" switch. My limited reading and trying it seems to indicate it is necessary for cross-platform builds, so I wonder if that is a quirk of your compilation environment or if I am doing something wrong.

xunholy · 2020-04-25T05:50:02Z

Yep, buildx is a fantastic tool that will help multi-architecture images become more mainstream in the year(s) to come.

If you don't mind my asking, in the example Dockerfile you linked to, I note your initial from does not have a "--platform=$BUILDPLATFORM" switch. My limited reading and trying it seems to indicate it is necessary for cross-platform builds, so I wonder if that is a quirk of your compilation environment or if I am doing something wrong.

This is because in a lot of my examples I'm compiling the Binary for multi-architecture - It's not enough to only have a multi-arch image if the binary is still compiled for a specific OS and ARCH. Thankfully Golang is good at handling the OS, ARCH, and even the ARM version to use.

Also the reason there is no switch is because that is a native Docker env variable - not a regular build arg which you might be more familiar with although that ARG actually does change for each architecture which is how the compilation works and exports the correct versions.

nrb · 2020-04-27T19:09:48Z

@xunholy Thanks for pointing that command out! I think we should definitely look into cleaning up our build pipeline, but I think I also agree with @firethestars - I'd prefer to do it in core Velero and work out to the plugins we support (that's AWS, GCP, Azure, and CSI). Right now, that's not going to make it in for our v1.4 release.

I've made vmware-tanzu/velero#2464 to capture that so we don't lose it. Thanks for the suggestion!

nrb

👍 Thanks for this! Apologies again for the long delay.

firethestars · 2020-06-04T15:24:33Z

@nrb - I just wanted to follow up and see if there are any action items I could take on to help move this forward.

…n-for-aws

carlisia · 2020-06-22T23:47:45Z

@firethestars thank you for this. You are missing the DCO signature, please add it so it passes the check.

@nrb and @ashish-amarnath PTAL when you have a chance, you two are way more well versed in this than I am.

ashish-amarnath · 2020-06-24T16:03:39Z

Dockerfile-arm

+
+
+FROM arm32v7/ubuntu:bionic
+#RUN mkdir /plugins


Please remove commented steps in the Dockerfiles.

Comment removed from both dockerfiles.

ashish-amarnath · 2020-06-24T16:04:26Z

Dockerfile-arm

@@ -0,0 +1,25 @@
+# Copyright 2017, 2019 the Velero contributors.


Copyright boilerplate should have 2020 as the year.

Year has been updated.

…r file after 1.1 update Signed-off-by: Chris Lynch <[email protected]>

Signed-off-by: Chris Lynch <[email protected]>

firethestars · 2020-06-27T06:34:00Z

Sorry I was busy all week, but the requested changes should now be in place. Please let me know if anything else needs attention.

ashish-amarnath

Thank you for making the changes.
LGTM.
I have one comment about ppv64le.

LGTM otherwise.

ashish-amarnath · 2020-07-08T16:32:10Z

Dockerfile-arm64

+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+FROM golang:1.13-buster AS build


Can we please switch this to golang 1.14?

This needs to be done on the other dockerfile too.
Going to clean this in a follow-up PR

ashish-amarnath · 2020-07-08T16:32:56Z

Dockerfile-arm64

+RUN GOARCH=arm64 CGO_ENABLED=0 GOOS=linux go build -v -o /go/bin/velero-plugin-for-aws ./velero-plugin-for-aws
+
+
+FROM arm64v8/ubuntu:bionic


Can we use the more recent ubuntu:focal instead?

This needs to be done on the other dockerfile too.
Going to clean this in a follow-up PR

ashish-amarnath · 2020-07-08T16:37:57Z

Makefile

 VERSION 	?= master

+CONTAINER_PLATFORMS ?= amd64 arm arm64 # ppc64le


should ppv64le be uncommented?

I think ppc64le needs to be removed entirely; it's not supported by Velero itself.

nrb

Lets get this merged for now and fix the issues in a follow up PR in order to reduce the back and forth.

Thanks for your patience @firethestars!

nrb closed this Apr 17, 2020

nrb reopened this Apr 17, 2020

nrb mentioned this pull request Apr 27, 2020

Investigate moving multiarch builds to docker buildx vmware-tanzu/velero#2464

Closed

nrb previously approved these changes Apr 27, 2020

View reviewed changes

nrb requested review from carlisia, skriss and ashish-amarnath April 27, 2020 19:34

Merge branch 'master' of https://github.com/vmware-tanzu/velero-plugi…

51ee020

…n-for-aws

firethestars dismissed nrb’s stale review via 8bc156d June 20, 2020 02:42

carlisia requested a review from nrb June 22, 2020 21:59

ashish-amarnath suggested changes Jun 24, 2020

View reviewed changes

firethestars added 2 commits June 27, 2020 02:25

Updates to keep arm64 and arm dockerfiles in line with original docke…

11913e8

…r file after 1.1 update Signed-off-by: Chris Lynch <[email protected]>

Making changes to dockerfiles requested by review

e79a53a

Signed-off-by: Chris Lynch <[email protected]>

skriss removed their request for review July 6, 2020 16:18

carlisia requested a review from ashish-amarnath July 8, 2020 13:53

ashish-amarnath approved these changes Jul 8, 2020

View reviewed changes

nrb approved these changes Jul 8, 2020

View reviewed changes

nrb merged commit 46c18b3 into vmware-tanzu:master Jul 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added Arm Multi-arch Make Logic #36

Added Arm Multi-arch Make Logic #36

firethestars commented Mar 28, 2020

firethestars commented Mar 30, 2020

nrb commented Apr 17, 2020

firethestars commented Apr 18, 2020

xunholy commented Apr 19, 2020

firethestars commented Apr 25, 2020

xunholy commented Apr 25, 2020 •

edited

Loading

nrb commented Apr 27, 2020

nrb left a comment

firethestars commented Jun 4, 2020

carlisia commented Jun 22, 2020

ashish-amarnath Jun 24, 2020

firethestars Jun 27, 2020

ashish-amarnath Jun 24, 2020

firethestars Jun 27, 2020

firethestars commented Jun 27, 2020

ashish-amarnath left a comment

ashish-amarnath Jul 8, 2020

ashish-amarnath Jul 8, 2020

ashish-amarnath Jul 8, 2020

ashish-amarnath Jul 8, 2020

ashish-amarnath Jul 8, 2020

nrb Jul 8, 2020

nrb left a comment

		@@ -0,0 +1,25 @@
		# Copyright 2017, 2019 the Velero contributors.

		RUN GOARCH=arm64 CGO_ENABLED=0 GOOS=linux go build -v -o /go/bin/velero-plugin-for-aws ./velero-plugin-for-aws


		FROM arm64v8/ubuntu:bionic

		VERSION ?= master

		CONTAINER_PLATFORMS ?= amd64 arm arm64 # ppc64le

Added Arm Multi-arch Make Logic #36

Added Arm Multi-arch Make Logic #36

Conversation

firethestars commented Mar 28, 2020

firethestars commented Mar 30, 2020

nrb commented Apr 17, 2020

firethestars commented Apr 18, 2020

xunholy commented Apr 19, 2020

firethestars commented Apr 25, 2020

xunholy commented Apr 25, 2020 • edited Loading

nrb commented Apr 27, 2020

nrb left a comment

Choose a reason for hiding this comment

firethestars commented Jun 4, 2020

carlisia commented Jun 22, 2020

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

firethestars commented Jun 27, 2020

ashish-amarnath 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nrb left a comment

Choose a reason for hiding this comment

xunholy commented Apr 25, 2020 •

edited

Loading