Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Bump Go version to 1.17 #1446

Merged
merged 6 commits into from
Jan 11, 2022
Merged

Conversation

rajathagasthya
Copy link
Member

@rajathagasthya rajathagasthya commented Jan 4, 2022

What this PR does / why we need it

Bumps Go version to 1.17.

Which issue(s) this PR fixes

Related #857

Describe testing done for PR

$ make lint
$ go test ./...
$ make build-install-cli-all

Release note

Tanzu framwork now uses Go 1.17 for all builds and language feature support.

PR Checklist

  • Squash the commits into one or a small number of logical commits
  • Use good commit messages
  • Ensure PR contains terms all contributors can understand and links all contributors can access

Additional information

Special notes for your reviewer

@stmcginnis
Copy link
Contributor

Hmm, I hope this isn't something we're going to start hitting in our GitHub Actions regularly:

Error: No space left on device

@github-actions
Copy link

github-actions bot commented Jan 4, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1446/20220104155314/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

github-actions bot commented Jan 4, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1446/20220104155353/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

github-actions bot commented Jan 4, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1446/20220104183328/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

github-actions bot commented Jan 4, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1446/20220104192215/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@rajathagasthya rajathagasthya marked this pull request as ready for review January 5, 2022 16:31
@rajathagasthya rajathagasthya requested review from a team as code owners January 5, 2022 16:31
Copy link
Contributor

@stmcginnis stmcginnis left a comment

Choose a reason for hiding this comment

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

Looks great to me. We should probably get some more feedback on the timing of moving to 1.17 though. My opinion: 1.17 has been out long enough now and has improvements that I think it would be good to make the bump.

Copy link
Contributor

@vuil vuil left a comment

Choose a reason for hiding this comment

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

lgtm. I second the bump as well. I had to build some targets in cluster-api main and it plain didn't work until I switched to 1.17.

addons/go.mod Show resolved Hide resolved
@vijaykatam
Copy link
Contributor

Do we need to bump base golang images in Dockerfiles as well?

@stmcginnis
Copy link
Contributor

I do see a few other places where 1.16 is referenced. I haven't looked closely to see if these all require updating.

  • docs/cli/getting-started.md
  • docs/release/release-notes-gathering-process.md
  • addons/Dockerfile
  • pkg/v1/tkr/Dockerfile
  • addons/pinniped/post-deploy/Dockerfile
  • pkg/v1/sdk/capabilities/Dockerfile
  • pkg/v1/sdk/features/Dockerfile
  • pkg/v1/builder/template/plugintemplates/gomod.tmpl
  • pkg/v1/builder/template/plugintemplates/github_workflow_release.yaml.tmpl
  • pkg/v1/builder/template/plugintemplates/gitlab-ci.yaml.tmpl

* Updates all go.mod files to include go 1.17
* Prunes module graph by running `go mod tidy -go=1.17`
Adds `sigs.k8s.io/cluster-api/test` in the `replace` section to avoid
Go 1.17 modules consuming tanzu-framework erroring out.
@rajathagasthya
Copy link
Member Author

rajathagasthya commented Jan 7, 2022

@stmcginnis I changed most of the files listed in your previous comment. The ones I didn't change were plugintemplates files. It refers to an old version of tanzu-framework, which I tried to update along with changing Go version to 1.17. But it doesn't seem to like it:

go list -m: github.com/vmware-tanzu/tanzu-framework@v0.14.0 requires
	sigs.k8s.io/cluster-api/test@v1.0.1 requires
	sigs.k8s.io/cluster-api@v0.0.0-00010101000000-000000000000: invalid version: unknown revision 000000000000

I think this commit fixes it, but we will need to merge this PR first and then reference it in plugintemplates/gomod.tmpl. I will do it as a follow-up.

@github-actions
Copy link

github-actions bot commented Jan 7, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1446/20220107174614/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

github-actions bot commented Jan 7, 2022

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/1446/20220107193829/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@stmcginnis
Copy link
Contributor

Do we want to get any other folks giving explicit feedback? I think this is ready to go, but don't want to merge it if there are any other key stakeholders that should weigh in first.

@rajathagasthya
Copy link
Member Author

@vuil I will leave it to you to add the ok-to-merge label. I think this is ready to go.

@stmcginnis
Copy link
Contributor

Maybe a good argument for updating go versions, even if we're not super specific on the bugfix version: https://nvd.nist.gov/vuln/detail/CVE-2021-44716

@vuil vuil added the ok-to-merge PRs should be labelled with this before merging label Jan 11, 2022
@vuil
Copy link
Contributor

vuil commented Jan 11, 2022

Done.
May be a good idea to add a release note that stuff is built with 1.17 after the change

@stmcginnis stmcginnis merged commit b2816e1 into vmware-tanzu:main Jan 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area/repo maintenance cla-not-required kind/enhancement Categorizes issue or PR as related to an enhancement ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants