-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add arm64 and ppc64le as new platforms #241
Conversation
I'm probably ignorant of the travis operation here, but is the point of the test -> build change for !amd64 because CNI doesn't currently pass tests on !amd64? Why wouldn't we want to run the tests for arm/ppc? |
It isn't possible to run arm, arm64 or ppc64le go binaries on an amd64 host, unfortunately, so we can't run the tests unless we're on a native machine But catching compilation errors is a good start |
Ah, OK. I guess there isn't any non-amd64 arch support in travis... |
Please split out the travis change from the vendoring bump. We'll deal with the latter first to see if that causes the tests to fail. Thanks! |
0acbe8f
to
0ec60ec
Compare
Tests passing, the only problem was that I replaced GOBIN because it can't be used when cross-compiling, but the test script used the variable. I fixed that now. Also, I added the arches to your releasing script. I haven't tested it, so please give feedback on it. We're gonna use cni now in docker-multinode, and it would help if binaries for all arches are easily available. |
Seems OK to me, but somebody who does actual releases should take a look too. |
@steveej Please take a look. If you want me to split the releasing code out from this patch, I'll do that and send a follow-up. |
Should I split it up now, so you may merge the code that makes it possible to compile for arm64 and ppc64le, and then we'll deal with the releasing later on? |
@luxas No need to split out the release script change IMO. I'll try the release script and let you know if it works out. On another note, have you tried running the cross-compiled binaries using qemu-user? I'm not sure if the netlink syscalls are supported but it'd be very useful for CI. |
Please adjust the commit message(s) to conform to https://github.com/containernetworking/cni/blob/master/CONTRIBUTING.md#format-of-the-commit-message, thanks a lot! |
@steveej Updated qemu-user is known to be bad with go and multithreading. Please take this PR into the v0.4 release, that would help Kubernetes multiarch |
The change from If you didn't mean to have that effect, |
ping @steveej |
Are there any blockers for this PR or haven't you just had time for it...? Side question: When is v0.4.0 going to be released? cc @dcbw @rosenhouse |
I'd be happy to merge this - just needs a rebase |
The current vendor of sys/unix is really old, and doesn't work on arm64 and ppc64le Updating to the latest version might also fix other issues ref containernetworking#209
… architectures This makes it possible to cross-compile cni like so: $ GOARCH=arm ./build $ GOARCH=arm64 ./build $ GOARCH=ppc64le ./build ref containernetworking#209
Cross-compile cni for arm, arm64 and ppc64le with go1.6 only Allow go tip to fail Set fast_finish to true, which means travis will instantly return build failure when any of the required builds fail ref containernetworking#209
Modify the releasing script to cross-compile for the new architectures, but also keep backwards-compability ref containernetworking#209
@tomdee Rebased. PTAL and merge. |
Made it possible to build for other arches using
GOARCH=something ./build
Made the CI spot compilation errors
Updated the
golang.org/x/sys/unix/
dependencytravis.yml now has the same layout as etcd: etcd-io/etcd#5450
@steveej