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.21 #4529

Merged
merged 1 commit into from
Mar 28, 2024
Merged

Conversation

mboersma
Copy link
Contributor

@mboersma mboersma commented Feb 1, 2024

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This PR updates golang to v1.21 to match CAPI:

Which issue(s) this PR fixes:

N/A

Special notes for your reviewer:

See #3478 for prior art.

  • cherry-pick candidate

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests

Release note:

NONE

@k8s-ci-robot k8s-ci-robot added release-note-none Denotes a PR that doesn't merit a release note. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Feb 1, 2024
@mboersma
Copy link
Contributor Author

mboersma commented Feb 1, 2024

I'm assuming we also need to update testgrid e2e jobs for a newer container with Go 1.21. Kind of a chicken-and-egg situation...

Copy link

codecov bot commented Feb 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.72%. Comparing base (7f385fd) to head (ff7916f).
Report is 4 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4529      +/-   ##
==========================================
- Coverage   62.73%   62.72%   -0.02%     
==========================================
  Files         192      192              
  Lines       15644    15644              
==========================================
- Hits         9815     9813       -2     
- Misses       5146     5148       +2     
  Partials      683      683              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nawazkh
Copy link
Member

nawazkh commented Feb 1, 2024

I'm assuming we also need to update testgrid e2e jobs for a newer container with Go 1.21. Kind of a chicken-and-egg situation...

Does that mean we are merging this PR first and updating e2e jobs later on? Or is it the other way round?

@mboersma
Copy link
Contributor Author

mboersma commented Feb 1, 2024

Does that mean we are merging this PR first and updating e2e jobs later on?

I don't know actually. I'm not sure what we did in previous situations, have to do some research.

@mboersma mboersma self-assigned this Feb 1, 2024
@mboersma mboersma added this to the v1.14 milestone Feb 5, 2024
@mboersma
Copy link
Contributor Author

mboersma commented Feb 8, 2024

/retest

Just trying to get another read on #4553.

@Jont828
Copy link
Contributor

Jont828 commented Feb 9, 2024

Test seems to be failing b/c the go version is 1.20:

Detected go version: go version go1.20.13 linux/amd64.
Kubernetes requires go1.21.6 or greater.
Please install go1.21.6 or later.

@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 12, 2024
@nojnhuh
Copy link
Contributor

nojnhuh commented Feb 23, 2024

I'm assuming we also need to update testgrid e2e jobs for a newer container with Go 1.21. Kind of a chicken-and-egg situation...

It looks like our kubekins image pulls in Go 1.21 now:

% docker run --rm --entrypoint go -it gcr.io/k8s-staging-test-infra/kubekins-e2e:v20240219-207a9fb4d2-1.27 version
go version go1.21.7 linux/amd64

@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 23, 2024
@nojnhuh nojnhuh modified the milestones: v1.14, next Feb 29, 2024
@nojnhuh
Copy link
Contributor

nojnhuh commented Feb 29, 2024

While CAPI's main branch now uses Go 1.21, CAPI's 1.6 branch is still using Go 1.20, so it might be best to stay where we're at until we bump to CAPI 1.7 if we don't have any other need to upgrade.

/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 29, 2024
@mboersma
Copy link
Contributor Author

mboersma commented Mar 13, 2024

/hold cancel

/assign @nojnhuh

This needs to merge before we can incorporate CAPI v1.7.0-beta.0, because note: module requires Go 1.21.

I've been using Go 1.21 to work with CAPZ in general, so I think it's safe to bump up to that version while still having CAPI 1.6 in main.

@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 13, 2024
Copy link
Contributor

@nojnhuh nojnhuh left a comment

Choose a reason for hiding this comment

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

/lgtm

@@ -31,7 +31,7 @@ EOF
local go_version
IFS=" " read -ra go_version <<< "$(go version)"
local minimum_go_version
minimum_go_version=go1.20.0
minimum_go_version=go1.21.6
Copy link
Contributor

Choose a reason for hiding this comment

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

Not something we have to address now, but I'm wondering if we need this script at all since I think the go directive in go.mod already enforces this everywhere we'd need. I imagine this script was probably added to k/k like 8 years ago before Go modules and we inherited it from there.

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 13, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 4a4e83200d6b8e78feaaf557934a6bd521addb9b

@mboersma mboersma removed this from the next milestone Mar 13, 2024
@mboersma mboersma added this to the v1.15 milestone Mar 13, 2024
@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 26, 2024
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 26, 2024
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 26, 2024
Copy link
Contributor

@nojnhuh nojnhuh left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 28, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 2873ad17ed9e198fb83acac220639de28ccecb8b

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nojnhuh

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 28, 2024
@k8s-ci-robot k8s-ci-robot merged commit 309ac0c into kubernetes-sigs:main Mar 28, 2024
22 checks passed
@mboersma mboersma deleted the go-one-twenty-one branch March 28, 2024 18:07
@mboersma mboersma mentioned this pull request Mar 28, 2024
4 tasks
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. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants