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

Use patched golang1.7.1 for cross-builds targeting darwin #33803

Merged

Conversation

ixdy
Copy link
Member

@ixdy ixdy commented Sep 29, 2016

This PR extends #32517 to use the patched go1.7.1 introduced by that PR to build all darwin targets (e.g. kubectl).

This is necessary because binaries built with earlier versions of Go regularly segfault on macOS Sierra (see #32999 and #33070).

This solution is somewhat hacky, but we intend to cherry-pick this to 1.4, and switching all of 1.4 to build with go1.7.1 is very high risk.

I haven't yet pushed the cross build image yet, so this will fail to build. Will test locally and update with results.

First step of fixing #33801.

cc @luxas @pwittrock @david-mcmahon @liggitt @smarterclayton @jfrazelle @Starefossen @gerred


This change is Reviewable

@ixdy ixdy added this to the v1.4 milestone Sep 29, 2016
@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Sep 29, 2016
@david-mcmahon david-mcmahon added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 29, 2016
@ixdy ixdy added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Sep 29, 2016
@ixdy ixdy force-pushed the go1.7-darwin-client-binaries branch from b8d5356 to f769b02 Compare September 30, 2016 00:05
@k8s-ci-robot
Copy link
Contributor

Jenkins GCE e2e failed for commit f769b02. Full PR test history.

The magic incantation to run this job again is @k8s-bot gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins Kubemark GCE e2e failed for commit f769b02. Full PR test history.

The magic incantation to run this job again is @k8s-bot kubemark e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GCE e2e failed for commit f769b02. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gce e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 30, 2016
@gerred
Copy link

gerred commented Sep 30, 2016

@ixdy This is fantastic. Does the gcloud CLI kubectl distrubtion use anything else beyond what's OSS and available? I'd love to ensure this change is upstreamed into that tool as well.

We'll continue providing 1.3.7 binaries at Deis until GKE makes 1.4 GA. Then I don't see a need for my workaround anymore!

@k8s-ci-robot
Copy link
Contributor

Jenkins verification failed for commit f769b02. Full PR test history.

The magic incantation to run this job again is @k8s-bot verify test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@ixdy
Copy link
Member Author

ixdy commented Sep 30, 2016

@gerred I think the version of kubectl distributed by gcloud follows the official release supported by GKE, so it will likely update to 1.4.0 at some point in the near future. We'll probably need to cut a 1.4.1 with the go1.7.1 darwin kubectl, but I'm not sure how long that might take to make it through the distribution channels.

The current kubectl shipped by gcloud (1.3.7) is built with go1.6.2, so I imagine it's similarly broken on macOS Sierra right now.

@ixdy
Copy link
Member Author

ixdy commented Sep 30, 2016

I've built kubectl off the master branch locally using this PR and have uploaded the resulting (unofficial) binaries.

Can someone with macOS Sierra test these binaries to see if they work? Based on strings I'm pretty sure they used go1.7.1.
http://dl.k8s.io/ci/v1.5.0-alpha.0.1705+1d22e86776bc18-jgrafton/bin/darwin/386/kubectl
http://dl.k8s.io/ci/v1.5.0-alpha.0.1705+1d22e86776bc18-jgrafton/bin/darwin/amd64/kubectl

@luxas
Copy link
Member

luxas commented Sep 30, 2016

Can someone please manually verify it works?
@gerred maybe?

I do not have a machine to test on

Reapplying the LGTM label... @ixdy feel free to push

@luxas luxas assigned luxas and david-mcmahon and unassigned zmerlynn Sep 30, 2016
@gerred
Copy link

gerred commented Sep 30, 2016

@luxas Testing now.

@gerred
Copy link

gerred commented Sep 30, 2016

That binary works perfectly.

@ixdy
Copy link
Member Author

ixdy commented Sep 30, 2016

kube-cross:1.6.3-8 pushed.

@k8s-bot test this
@k8s-bot build this

@david-mcmahon david-mcmahon added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 30, 2016
@pwittrock pwittrock added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Sep 30, 2016
@pwittrock
Copy link
Member

This failure:

Tag v1.6.3-8 not found in repository gcr.io/google_containers/kube-cross

@luxas
Copy link
Member

luxas commented Sep 30, 2016

@k8s-bot gci gke e2e test this

@ixdy
Copy link
Member Author

ixdy commented Sep 30, 2016

@pwittrock which build was that from?

for some reason the GCI and Kubemark builds didn't re-trigger on my last comment.
@k8s-bot kubemark e2e test this
@k8s-bot gci gce e2e test this

@luxas
Copy link
Member

luxas commented Sep 30, 2016

@k8s-bot gci gce e2e test this

@luxas
Copy link
Member

luxas commented Sep 30, 2016

Haha, race condition between @ixdy and me :)

Let's hope everything's green now

@pwittrock When are you planning to release v1.4.1?

@pwittrock
Copy link
Member

@luxas Original plan was a week from Monday (10/10). Need to figure out if this warrants accelerating that timeline. WDYT?

@ixdy
Copy link
Member Author

ixdy commented Sep 30, 2016

We'll at least have Jenkins-built kubectl binaries on gs://kubernetes-release-dev/ci as soon as this gets cherrypicked.

@pwittrock
Copy link
Member

Nice. I can add that to the known issues in the release notes. Anywhere else we can publish it?

@luxas
Copy link
Member

luxas commented Sep 30, 2016

@pwittrock If possible, I'd prefer the 5th Oct or something like that, given this and the ARM issue. As soon as some other cherrypicks that are pending like #33280, #33520 and #33806 are merged, I think you should release.

Then, already more than 20 commits have been merged since v1.4.0, and that's quite a decent amount already

WDYT about Wednesday?

@k8s-ci-robot
Copy link
Contributor

Jenkins GKE smoke e2e failed for commit f769b02. Full PR test history.

The magic incantation to run this job again is @k8s-bot gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-github-robot
Copy link

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

@k8s-ci-robot
Copy link
Contributor

Jenkins GCI GKE smoke e2e failed for commit f769b02. Full PR test history.

The magic incantation to run this job again is @k8s-bot gci gke e2e test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@k8s-ci-robot
Copy link
Contributor

Jenkins unit/integration failed for commit f769b02. Full PR test history.

The magic incantation to run this job again is @k8s-bot unit test this. Please help us cut down flakes by linking to an open flake issue when you hit one in your PR.

@pwittrock
Copy link
Member

@k8s-bot gci gke e2e test this

@pwittrock
Copy link
Member

@k8s-bot unit test this

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 2628af0 into kubernetes:master Sep 30, 2016
k8s-github-robot pushed a commit that referenced this pull request Oct 1, 2016
…pstream-release-1.4

Automatic merge from submit-queue

Automated cherry pick of #33803

Cherry pick of #33803 on release-1.4.

Fixes #33801.
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.4" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
…f-#33803-upstream-release-1.4

Automatic merge from submit-queue

Automated cherry pick of kubernetes#33803

Cherry pick of kubernetes#33803 on release-1.4.

Fixes kubernetes#33801.
@ixdy ixdy deleted the go1.7-darwin-client-binaries branch May 15, 2018 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build-release cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", 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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants