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.

Sign up for GitHub

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

Jump to bottom

Bump Go version to 1.8.3 #46429

Merged
merged 2 commits into from
May 26, 2017
Merged

Bump Go version to 1.8.3 #46429

merged 2 commits into from
May 26, 2017

Conversation

wojtek-t
Copy link
Member

@wojtek-t wojtek-t commented May 25, 2017 •
edited
Loading

This PR also removed this patched version of Go 1.8.1 which we used to use to workaround performance problem of Go 1.8.1.

Fix #45216
Ref #46391

@timothysc @bradfitz

@wojtek-t wojtek-t added the release-note-none Denotes a PR that doesn't merit a release note. label May 25, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 25, 2017
@k8s-github-robot k8s-github-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 25, 2017
@wojtek-t wojtek-t mentioned this pull request May 25, 2017
Closed
@wojtek-t
Copy link
Member Author

@ixdy - I'm getting this from bazel:

W0525 11:20:22.607] ERROR: /workspace/k8s.io/kubernetes/WORKSPACE:35:1: Traceback (most recent call last):
W0525 11:20:22.633] 	File "/workspace/k8s.io/kubernetes/WORKSPACE", line 35
W0525 11:20:22.634] 		go_repositories(go_version = "1.8.3")
W0525 11:20:22.635] 	File "/root/.cache/bazel/_bazel_root/e9f728bbd90b3fba632eb31b20e1dacd/external/io_bazel_rules_go/go/private/go_repositories.bzl", line 216, in go_repositories
W0525 11:20:22.636] 		fail("go_repositories: unsupported ve...())))
W0525 11:20:22.674] go_repositories: unsupported version 1.8.3; supported versions are 1.7.5 1.8 1.8.1.

Do you know how to fix it?

@timothysc
Copy link
Member

/lgtm

@timothysc timothysc added this to the 1.7 milestone May 25, 2017
@timothysc timothysc added the sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. label May 25, 2017
@cblecker
Copy link
Member

build/cni/Makefile and cluster/images/etcd/Makefile actually look like they were missed in the bump to 1.8. Should we bump those too (they're on 1.7.5 right now on master) ?

ixdy
ixdy reviewed May 25, 2017
View reviewed changes
build/root/WORKSPACE
@@ -33,7 +33,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_repositories")
load("@io_bazel_rules_docker//docker:docker.bzl", "docker_repositories")

go_repositories(
go_version = "1.8.1",
go_version = "1.8.3",
Copy link
Member

Choose a reason for hiding this comment

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

need to update the rules_go dependency to https://github.com/bazelbuild/rules_go/archive/a9df110cf04e167b33f10473c7e904d780d921e6.tar.gz for this, along with updating the sha256.

Copy link
Member

@ixdy ixdy May 25, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Specifically,

$ sha256sum a9df110cf04e167b33f10473c7e904d780d921e6.tar.gz
64294fd0e74d2aafa03ec3a1f2f9c167e27d17c9a5cf393e8bf79e43258de73d  a9df110cf04e167b33f10473c7e904d780d921e6.tar.gz

So change L1-6 to

http_archive(
    name = "io_bazel_rules_go",
    sha256 = "64294fd0e74d2aafa03ec3a1f2f9c167e27d17c9a5cf393e8bf79e43258de73d",
    strip_prefix = "rules_go-a9df110cf04e167b33f10473c7e904d780d921e6",
    urls = ["https://github.com/bazelbuild/rules_go/archive/a9df110cf04e167b33f10473c7e904d780d921e6.tar.gz"],
)

(explanation: the rules_go repo added 1.8.3 in that commit - bazel-contrib/rules_go@a9df110)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

@wojtek-t
Copy link
Member Author

@ixdy - PTAL

@dchen1107 dchen1107 modified the milestones: 1.7, v1.7 May 25, 2017
@wojtek-t
Copy link
Member Author

@k8s-bot pull-kubernetes-node-e2e test this
@k8s-bot pull-kubernetes-unit test this

luxas
luxas approved these changes May 25, 2017
View reviewed changes
Copy link
Member

@luxas luxas left a comment

Choose a reason for hiding this comment

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

LGTM

@luxas
Copy link
Member

luxas commented May 25, 2017

build/cni/Makefile and cluster/images/etcd/Makefile actually look like they were missed in the bump to 1.8. Should we bump those too (they're on 1.7.5 right now on master) ?

@cblecker etcd shouldn't use v1.8 yet. build/cni now actually is outdated as we're gonna start using the official upstream bins. However, I'll work on that next week.

@wojtek-t I think we could use a release note for this both given the possible crypto issue and the removal of the patch. Agree?

@luxas
Copy link
Member

luxas commented May 25, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 25, 2017
@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: luxas, timothysc, wojtek-t

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@cblecker
Copy link
Member

I don't think we've cut a release with go1.8 yet, so would the release note from #41636 suffice?

Thanks for the clarification on etcd/cni, @luxas!

@wojtek-t
Copy link
Member Author

@k8s-bot unit test this
@k8s-bot node e2e test this

@luxas
Copy link
Member

luxas commented May 25, 2017

@cblecker Both https://github.com/kubernetes/kubernetes/releases/tag/v1.7.0-alpha.3 and https://github.com/kubernetes/kubernetes/releases/tag/v1.7.0-alpha.4 have been cut since.
I think this deserves a release note. We should also notify the mailing list thread about this.

@ixdy
Copy link
Member

ixdy commented May 25, 2017

lgtm2

@ixdy
Copy link
Member

ixdy commented May 25, 2017

@k8s-bot pull-kubernetes-node-e2e test this

@cblecker
Copy link
Member

@k8s-bot pull-kubernetes-node-e2e test this

@ixdy
Copy link
Member

ixdy commented May 26, 2017

/release-note

@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels May 26, 2017
@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented May 26, 2017 •
edited
Loading

@wojtek-t: The following test(s) failed:

Test name Commit Details Rerun command
pull-kubernetes-federation-e2e-gce 97b6701 link @k8s-bot pull-kubernetes-federation-e2e-gce test this

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@k8s-github-robot
Copy link

Automatic merge from submit-queue (batch tested with PRs 46429, 46308, 46395, 45867, 45492)

@k8s-github-robot k8s-github-robot merged commit 59ee250 into kubernetes:master May 26, 2017
@wojtek-t wojtek-t deleted the bump_go_to_183 branch May 29, 2017 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ixdy ixdy ixdy left review comments

@luxas luxas luxas approved these changes

Assignees

@luxas luxas

@ixdy ixdy

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. 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. sig/scalability Categorizes an issue or PR as relevant to SIG Scalability. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
v1.7
Development

Successfully merging this pull request may close these issues.
8 participants
@wojtek-t @timothysc @cblecker @luxas @k8s-github-robot @ixdy @k8s-ci-robot @dchen1107