Bump to go1.8 and remove the edge GOROOT #41636
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
k8s-ci-robot
added
the
cncf-cla: yes
k8s-github-robot
added
size/M
|
This change is |
|
Can you try testing this locally before we push a new crossbuild image? |
|
also we probably want to update the |
|
@ixdy Ok, tests have been run, see gist: https://gist.github.com/luxas/6c0b9ced17e60072a63ff7c150574e48 All binaries were compiled, but etcd errored out a lot of times in the unit/integration tests, example: cc @kubernetes/sig-api-machinery-pr-reviews @wojtek-t @hongchaodeng @xiang90 @timothysc |
|
@ixdy However, there's nothing wrong with this image, so feel free to push it so we can run the pre-submit test suite on this PR, then it's easier to evalute and debug the failures |
|
Can we please not merge this before 1.6 is out? There's enough churn already... |
+1, I'm good with first thing in 1.7. |
timothysc
added
the
sig/scalability
Me too |
|
+1 |
|
But the faster we have an image to experiment with in the CI, the faster we'll catch bugs and things that should be changed on our side |
gmarek
added
the
do-not-merge
|
I'm a little hesitant to push a cross-build image before it's ready to be checked in, but I guess we don't have a better way, really. @luxas can you update the |
|
also as #41771 demonstrates there are a few other places we need to bump. |
|
@k8s-bot test this Things have been pretty unstable today, so I'm not sure how much signal we'll get. |
|
@k8s-bot cross build this |
|
@k8s-bot cross build this I don't understand what's going on. |
|
@k8s-bot test this |
|
The cross-build job doesn't appear to be significantly faster (59m, which is still gross), though we probably need more data points. |
k8s-github-robot
pushed a commit
that referenced
this pull request
Feb 23, 2017
Automatic merge from submit-queue (batch tested with PRs 41812, 41665, 40007, 41281, 41771) Bump golang versions to 1.7.5 **What this PR does / why we need it**: While #41636 might not make it in until 1.7, this would bump current golang versions from 1.7.4 to 1.7.5 to integrate the fixes from that patch version. This would include, among other things, a fix to ensure cross-built binaries for darwin don't have certificate validation errors (golang/go#18688) **Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: none **Special notes for your reviewer**: **Release note**: ```release-note Upgrade golang versions to 1.7.5 ```
k8s-github-robot
added
the
needs-rebase
|
The verification tests run in Does the generated code really depend on the go version? That's gross. |
I'm very confident they do. Nearly every time we've bumped go version earlier, generated code has changed and we've had a rebase hell party ;) @ixdy could you bump kubekins and try to get #44583 in today? That would unblock this in time for Friday (or maybe preferably Monday) |
It's unfortunate this means all kube developers must immediately switch to compiling with go1.8 :-/ |
|
Is it a big deal switching over to go1.8 though? Go's release policy deprecated support for 1.7 the moment 1.8 was out. |
|
I think a lot of our gogen scripts get around this issue by running in the kube-cross container (via |
|
@luxas it looks like you didn't run hack/update-staging-client-go.sh after you updated protobuf. I don't think the update/verify-staging-client-go.sh depends on the go version. |
This is the only golang usage I can think of inside of that script: https://github.com/kubernetes/kubernetes/blob/master/hack/verify-staging-client-go.sh#L26 . The copy step is just inspecting the dependency tree and making some copies. |
|
bazel PR is merged. @luxas can you rebase/regenerate stuff to see if that makes verification happy? |
|
@ixdy updated, let's see how it goes 👍 |
ixdy
removed
the
do-not-merge
|
Looks like it worked 🎉 ! |
|
/lgtm sweet! |
k8s-ci-robot
added
the
lgtm
|
/approve
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ixdy, luxas, smarterclayton
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
k8s-github-robot
added
the
approved
|
@smarterclayton thanks! @luxas might want to email kubernetes-dev@ as soon as this merges to note that we've updated to go1.8.1. |
|
Automatic merge from submit-queue (batch tested with PRs 41287, 41636, 44881, 44826) |
|
wohoo I didn't yet have time to look into results, but at least we didn't see anything broken by that. |
|
!!! thanks @luxas |
|
@ixdy now announced, thanks for reminding: https://groups.google.com/forum/#!topic/kubernetes-dev/0XRRz6UhhTM |
This was referenced May 1, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Bumps to go1.8; we get:
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged): fixes #38228Special notes for your reviewer:
@ixdy Please push the image ASAP so we can see if this passes all tests
Release note:
cc @ixdy @bradfitz @jessfraz @wojtek-t @timothysc @spxtr @thockin @smarterclayton @bprashanth @gmarek