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

Official ARM builds for every new release #19769

Merged
merged 1 commit into from Feb 9, 2016

Conversation

luxas
Copy link
Member

@luxas luxas commented Jan 17, 2016

This PR features support for building arm(, arm64 and ppc64le) binaries dynamically or statically on amd64 hosts. It builds docker images for kube-apiserver, kube-controller-manager, kube-scheduler, kube-proxy for the new architectures and pushes them.
Bug fixed: hyperkube (legacy name for hyperkube-amd64) is pushed too when releasing for compability with older setups

Earlier:

  • arm
    • Server support only when building on an ARM hosts
    • Client support also on amd64 hosts
  • No arm64 or ppc64le support at all.

Now:

Tracking issue: #17981
Depends on: #14873
A bit related to: #19703

When this is merged, it has the advantage that builds will fail when trying to merge only-working-on-amd64 code for the server targets, so it's easily noticed.

This is WIP for now because #14873 hasn't merged yet (and I have to rebase after that)

However, if it takes too long time for arm64 and ginkgo to work, I'm gonna comment out arm64 for the moment and merge that later.

Thoughts? This will make Kubernetes more cross-platform, and that will be awesome 👍

@brendandburns @thockin @zmerlynn @fgrzadkowski @david-mcmahon @mikedanese @gmarek @wojtek-t @qq511700505

@k8s-bot
Copy link

k8s-bot commented Jan 17, 2016

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

1 similar comment
@k8s-bot
Copy link

k8s-bot commented Jan 17, 2016

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

@k8s-github-robot
Copy link
Contributor

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Jan 17, 2016
@zmerlynn
Copy link
Member

@k8s-bot ok to test

@@ -1,4 +1,4 @@
# Copyright 2014 Google Inc. All rights reserved.
# Copyright 2016 Google Inc. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Please don't bump the copyright on the file unless it's been basically rewritten (it hasn't).

Copy link
Member Author

Choose a reason for hiding this comment

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

Aha, thought it could be updated... No problem

@k8s-bot
Copy link

k8s-bot commented Jan 19, 2016

GCE e2e build/test failed for commit e7f388d904953b155665fc19c5228ac385028a6d.

@luxas
Copy link
Member Author

luxas commented Jan 20, 2016

@zmerlynn Do you have any specific reason why you are building test targets for all client platforms? Isn't that a little bit unnecessary and slower?
I'm wondering if it could be removed from hack/build-cross.sh... (That code was modified last time 2014)

Of course test targets should be made for amd64, but for arm, windows and ppc64le for example it seems a little bit strange

I know that it might be another discussion though and that it might break backwards-compability (I don't know if you run integration tests on windows)
So I'm just asking...

@ixdy ixdy assigned zmerlynn and unassigned ixdy Jan 20, 2016
@ixdy
Copy link
Member

ixdy commented Jan 20, 2016

FYI also @ihmccreery @david-mcmahon

@zmerlynn
Copy link
Member

@luxas: Yes, we could probably take the test code out of the client build.

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 20, 2016
darwin/amd64
darwin/386
windows/amd64
windows/386
Copy link
Member

Choose a reason for hiding this comment

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

Did you add 32-bit windows for a reason?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because I saw that a compiler in the earlier kube-build:cross was built for windows/386, but it wasn't listed as a client platform. Supposed it just had been forgotten.

But it doesn't harm to build kubectl for 32-bit win also.

@zmerlynn
Copy link
Member

This looks okay. I'm a little concerned about bumping golang given what lead up to 19487 #17524, so I'm not sure how to adjudicate that.

@ixdy
Copy link
Member

ixdy commented Jan 20, 2016

#17524 I think you mean?

In any case, it'd probably be good to bump the golang version separate from other changes. #14873 is for that, I think.

@zmerlynn
Copy link
Member

Yes, I have no idea where I pulled the other number from.

@zmerlynn zmerlynn mentioned this pull request Jan 21, 2016
@luxas
Copy link
Member Author

luxas commented Jan 21, 2016

golang should be updated in another PR, probably #14873.
I just have to include it now in this WIP stage so it builds.
Will rebase when #14873 gets in

Will rebase and rework the PR...

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 21, 2016
@@ -97,6 +97,7 @@ readonly GCS_STAGE="${LOCAL_OUTPUT_ROOT}/gcs-stage"
# The set of master binaries that run in Docker (on Linux)
# Entry format is "<name-of-binary>,<base-image>".
# Binaries are placed in /usr/local/bin inside the image.
# TODO: Make base images for multiple server platforms
Copy link
Member Author

Choose a reason for hiding this comment

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

@zmerlynn Do you have some suggestions how to nicely handle this?
We need to be able to specify different base images for different arches

@k8s-bot
Copy link

k8s-bot commented Jan 21, 2016

GCE e2e build/test failed for commit 2360e14e8225c02c1be9363c21a43ec5edbfc334.

@luxas
Copy link
Member Author

luxas commented Jan 21, 2016

Looks like a e2e flake.
It built binaries and packaged them as it should.

But some test didn't start in time or something...

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 21, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 7, 2016

GCE e2e build/test failed for commit d58844e952f8c6adc35cc7472c1b26b90fca398d.

@luxas
Copy link
Member Author

luxas commented Feb 7, 2016

To rebase to go1.4 was an attempt to do this anyway, and see if it works, but it didn't.
linux/arm64 and linux/ppc64le was introduced in go1.5

I'll try to get working this only for linux/arm, since that's the primary goal for this patch
We'll continue with the others post-v1.2 when we have go1.5.3 (or go1.6?) from the beginning

@luxas luxas changed the title Add support for building arm, arm64 and ppc64le server and client targets Add support for building arm server and client targets in the release process. Prepare for arm64 and ppc64le Feb 7, 2016
@k8s-bot
Copy link

k8s-bot commented Feb 7, 2016

GCE e2e test build/test passed for commit c969c04.

@luxas luxas mentioned this pull request Feb 9, 2016
@zmerlynn
Copy link
Member

zmerlynn commented Feb 9, 2016

LGTM

@zmerlynn zmerlynn added the lgtm Indicates that a PR is ready to be merged. label Feb 9, 2016
@k8s-github-robot
Copy link
Contributor

@k8s-bot test this

Tests are more than 48 hours old. Re-running tests.

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit c969c04.

@k8s-github-robot
Copy link
Contributor

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit c969c04.

@k8s-github-robot
Copy link
Contributor

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Feb 9, 2016

GCE e2e test build/test passed for commit c969c04.

@k8s-github-robot
Copy link
Contributor

Automatic merge from submit-queue

k8s-github-robot added a commit that referenced this pull request Feb 9, 2016
@k8s-github-robot k8s-github-robot merged commit f36b604 into kubernetes:master Feb 9, 2016
@luxas
Copy link
Member Author

luxas commented Feb 9, 2016

Thanks @zmerlynn!
BTW, could we add a release note for this? e.g.

Official ARM builds for every release (#19769, @luxas)

I think it would be great to announce that we now support ARM :)
(If I remember correctly there's a label for release notes)

@david-mcmahon
Copy link
Contributor

@luxas To add this as a release note, add the release-note label and update the PR description to whatever you want the release note to say.

@mikedanese
Copy link
Member

@david-mcmahon adding a label requires repo write access which not everyone has

@david-mcmahon
Copy link
Contributor

@mikedanese thanks for the clarification. Adding release-note label. @luxas please adjust release note to taste.

@david-mcmahon david-mcmahon added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Feb 11, 2016
@luxas luxas changed the title Add support for building arm server and client targets in the release process. Prepare for arm64 and ppc64le Official ARM builds for every new release Feb 12, 2016
@luxas luxas mentioned this pull request Feb 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/release-eng Issues or PRs related to the Release Engineering subproject lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet