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

Update Go to v1.16 #10892

Merged
merged 1 commit into from
Mar 4, 2021
Merged

Conversation

bmelbourne
Copy link
Contributor

@bmelbourne bmelbourne commented Feb 20, 2021

What this PR does / why we need it:
Upgrade Go to v1.16.

Which issue(s) this PR fixes:
N/A

Special notes for your reviewer:
Support for GOPATH mode will be deprecated in Go 1.17 and GO111MODULE environment variable will be ignored,

https://blog.golang.org/go116-module-changes

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Feb 20, 2021
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Feb 20, 2021
@bmelbourne bmelbourne changed the title Upgrade Go v1.16 [WIP] Upgrade Go v1.16 Feb 20, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 20, 2021
@bmelbourne bmelbourne changed the title [WIP] Upgrade Go v1.16 [WIP] Upgrade Go 1.16 Feb 20, 2021
@bmelbourne bmelbourne changed the title [WIP] Upgrade Go 1.16 Upgrade Go 1.16 Feb 20, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 20, 2021
@bmelbourne
Copy link
Contributor Author

/assign @hakman

@bharath-123
Copy link
Contributor

Have submitted a similar PR #10864 which got closed....

@olemarkus
Copy link
Member

Thanks for this. I think we want to hold this one waiting for some of our larger deps to upgrade. at least the other kubernetes libs.

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 20, 2021
@bmelbourne
Copy link
Contributor Author

bmelbourne commented Feb 20, 2021

ok no probs. I did a search for any open PRs before raising this hence why I missed the closed PR.

Happy to hold until the Kubernetes and other dependencies are updated.

As already mentioned in Slack, I'm going to raise new PRs to cover upgrades to other Go module dependencies.

@bmelbourne bmelbourne changed the title Upgrade Go 1.16 [WIP] Upgrade Go 1.16 Feb 20, 2021
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 20, 2021
@hakman
Copy link
Member

hakman commented Feb 20, 2021

We should wait for kubernetes/kubernetes#98572 to merge and for 1.12.1 beta.1 to be released at least.

@hakman hakman changed the title [WIP] Upgrade Go 1.16 [WIP] Update Go to v1.16 Feb 20, 2021
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 21, 2021
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Feb 26, 2021
@olemarkus
Copy link
Member

I think the test above should pass if make gomod is run with golang1.16 now.

@rifelpet
Copy link
Member

rifelpet commented Mar 4, 2021

/retest

@bmelbourne
Copy link
Contributor Author

I think the test above should pass if make gomod is run with golang1.16 now.

It certainly did last week, but I'm just waiting for Kubernetes to create the next v1.21.0-beta.1 release, and then I'll do some further testing locally and do a final rebase.

@rifelpet
Copy link
Member

rifelpet commented Mar 4, 2021

I think the go upgrade is independent of any kubernetes release and our use of the kubernetes dependencies.

I bumped the go versions in the go.mod files to 1.16 and running make gomod gave me the same go.mod and go.sum changes that the failing verify-gomod is expecting. Perhaps rebasing and committing the those changes can get this to succeed? All PRs are now blocked on this issue, so I would lean towards getting this merged sooner rather than later.

@hakman
Copy link
Member

hakman commented Mar 4, 2021

Tried it also and worked as @rifelpet described. Would be nice to get this in soon.

@bmelbourne
Copy link
Contributor Author

We should wait for kubernetes/kubernetes#98572 to merge and for 1.12.1 beta.1 to be released at least.

@hakman @olemarkus @rifelpet
If the general consensus is not to wait for the next Kubernetes release built on Go v1.16, then I'll start working on the final rebase and local testing tonight and hopefully, all going well, it'll be ready for approval soon.

@hakman
Copy link
Member

hakman commented Mar 4, 2021

No worries, we all agreed that it's ok to merge now and upgrade the deps to 1.21-beta.1 next week.

@k8s-ci-robot k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Mar 4, 2021
@hakman
Copy link
Member

hakman commented Mar 4, 2021

Thanks @bmelbourne! 👍
/retest

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman, olemarkus

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@olemarkus
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 4, 2021
@bmelbourne bmelbourne changed the title [WIP] Update Go to v1.16 Update Go to v1.16 Mar 4, 2021
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 4, 2021
@k8s-ci-robot k8s-ci-robot merged commit fd9b9d8 into kubernetes:master Mar 4, 2021
@k8s-ci-robot k8s-ci-robot added this to the v1.21 milestone Mar 4, 2021
@bmelbourne bmelbourne deleted the upgrade-go-1.16 branch March 4, 2021 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants