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

[1.30] .*: set godebug defaults to go1.22 #129534

Open
wants to merge 2 commits into
base: release-1.30
Choose a base branch
from
Open

[1.30] .*: set godebug defaults to go1.22 #129534

wants to merge 2 commits into from

Conversation

MadhavJivrajani
Copy link
Contributor

Kubernetes 1.30 was first released with go1.22.x.
Set compatibility defaults to that version.

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Partial pick of #127271
Needed to bump release branches to go1.23

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

Sets defaults of unset GODEBUG variables to be the defaults that go1.21 had.
See GODEBUG history: https://go.dev/doc/godebug

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

@MadhavJivrajani
.*: set godebug defaults to go1.22
2d979f4 
Kubernetes 1.30 was first released with go1.22.x.
Set compatibility defaults to that version.

Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
@MadhavJivrajani
.go-version: set go version for build and test to go1.23
1d918c0 
Signed-off-by: Madhav Jivrajani <madhav.jiv@gmail.com>
@k8s-ci-robot k8s-ci-robot added this to the v1.30 milestone Jan 9, 2025
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. labels Jan 9, 2025
@k8s-ci-robot
Copy link
Contributor

This cherry pick PR is for a release branch and has not yet been approved by Release Managers.
Adding the do-not-merge/cherry-pick-not-approved label.

To merge this cherry pick, it must first be approved (/lgtm + /approve) by the relevant OWNERS.

If you didn't cherry-pick this change to all supported release branches, please leave a comment describing why other cherry-picks are not needed to speed up the review process.

If you're not sure is it required to cherry-pick this change to all supported release branches, please consult the cherry-pick guidelines document.

AFTER it has been approved by code owners, please leave the following comment on a line by itself, with no leading whitespace: /cc kubernetes/release-managers

(This command will request a cherry pick review from Release Managers and should work for all GitHub users, whether they are members of the Kubernetes GitHub organization or not.)

For details on the patch release process and schedule, see the Patch Releases page.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. 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. do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Jan 9, 2025
@k8s-ci-robot
Copy link
Contributor

This issue is currently awaiting triage.

If a SIG or subproject determines this is a relevant issue, they will accept it by applying the triage/accepted label and provide further guidance.

The triage/accepted label can be added by org members by writing /triage accepted in a comment.

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-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added the needs-priority Indicates a PR lacks a `priority/foo` label and requires one. label Jan 9, 2025
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: MadhavJivrajani
Once this PR has been reviewed and has the lgtm label, please assign andrewsykim, deads2k, derekwaynecarr, smarterclayton for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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 requested review from apelisse, brianpursley and a team January 9, 2025 00:37
@k8s-ci-robot k8s-ci-robot added area/apiserver area/cloudprovider area/code-generation area/dependency Issues or PRs related to dependency changes area/kube-proxy area/kubectl area/kubelet sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 9, 2025
@k8s-ci-robot k8s-ci-robot added sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. labels Jan 9, 2025
@k8s-ci-robot
Copy link
Contributor

@MadhavJivrajani: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-kubernetes-integration 1d918c0 link true /test pull-kubernetes-integration
pull-kubernetes-node-e2e-containerd 1d918c0 link true /test pull-kubernetes-node-e2e-containerd
pull-kubernetes-integration-go-compatibility 1d918c0 link true /test pull-kubernetes-integration-go-compatibility
pull-kubernetes-unit-go-compatibility 1d918c0 link true /test pull-kubernetes-unit-go-compatibility
pull-kubernetes-dependencies 1d918c0 link true /test pull-kubernetes-dependencies
pull-kubernetes-linter-hints 1d918c0 link false /test pull-kubernetes-linter-hints
pull-kubernetes-e2e-kind-kms 1d918c0 link false /test pull-kubernetes-e2e-kind-kms
pull-kubernetes-verify 1d918c0 link true /test pull-kubernetes-verify

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-sigs/prow repository. I understand the commands that are listed here.

@BenTheElder
Copy link
Member

Needed to bump release branches to go1.23

Isn't the opposite true? you have to bump to 1.23 to use this?

golang/go#65573 (comment)

cc @liggitt

@BenTheElder BenTheElder mentioned this pull request Jan 9, 2025
Open
@BenTheElder
Copy link
Member

Ah, another commit was added with 1.23 .go-version, but that's not in the title/description/release note.

Yeah, I think this will have to be an overall 1.23 update patch unfortunately.

liggitt
liggitt reviewed Jan 9, 2025
View reviewed changes
go.mod
@@ -268,3 +268,5 @@ replace (
k8s.io/sample-cli-plugin => ./staging/src/k8s.io/sample-cli-plugin
k8s.io/sample-controller => ./staging/src/k8s.io/sample-controller
)

godebug default=go1.22
Copy link
Member

Choose a reason for hiding this comment

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

nit: move these to the top of the file, by the go version

@liggitt
Copy link
Member

liggitt commented Jan 9, 2025

interestingly, I think all the jobs that failed are not honoring the .go-version file ... that's expected for the *-go-compatibility jobs, but none of the others... are they not using kube::golang::setup_env to set up their go env?

@liggitt
Copy link
Member

liggitt commented Jan 9, 2025

interestingly, I think all the jobs that failed are not honoring the .go-version file ... that's expected for the *-go-compatibility jobs, but none of the others... are they not using kube::golang::setup_env to set up their go env?

either that, or we have to bump the go.mod file to go1.23.0 to use godebug default :-/

@liggitt
Copy link
Member

liggitt commented Jan 9, 2025

Yeah, I think this will have to be an overall 1.23 update patch unfortunately.

Yeah, I think our go 1.23 bump PRs to release branches will have to include all of these commits:

  1. Actually change the go builder to go 1.23
  2. Bump go.mod go directives to go 1.23.0 and add godebug default=go1.$original directives
  3. Include https://github.com/kubernetes/kubernetes/pulls?q=is%3Apr+SA1006+-base%3Amaster+ picks (for <1.31 where it already merged)

@liggitt liggitt mentioned this pull request Jan 9, 2025
Open
@MadhavJivrajani
Copy link
Contributor Author

Ah, another commit was added with 1.23 .go-version, but that's not in the title/description/release note.

Yeah, I think this will have to be an overall 1.23 update patch unfortunately.

Yes this will need to be done together :/
The .go-version commit is only for testing here, we'll need to bump that with the actual builder

@MadhavJivrajani
Copy link
Contributor Author

interestingly, I think all the jobs that failed are not honoring the .go-version file ... that's expected for the *-go-compatibility jobs, but none of the others... are they not using kube::golang::setup_env to set up their go env?

  • pull-kubernetes-*-go-compatibility and pull-kubernetes-linter-hints are genuine failures which is expected.
  • pull-kubernetes-node-e2e-containerd seems like an infra related flake.

The remaining failures don't seem to be honouring the .go-version file. Coincidentally (?) they are also the containerized jobs, I know we've had some issues with those specifcially wrt the Go version setup. Need to look more closely here though.

@MadhavJivrajani
Copy link
Contributor Author

Let me also open up a PR for 1.31 since that has some of the static analysis fixes. Should get a better idea of what's going on there

@MadhavJivrajani
Copy link
Contributor Author

#129551

@cici37
Copy link
Contributor

cici37 commented Jan 9, 2025

/remove-sig api-machinery

@k8s-ci-robot k8s-ci-robot removed the sig/api-machinery Categorizes an issue or PR as relevant to SIG API Machinery. label Jan 9, 2025
@MadhavJivrajani MadhavJivrajani mentioned this pull request Jan 10, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@liggitt liggitt liggitt left review comments

@apelisse apelisse Awaiting requested review from apelisse

@brianpursley brianpursley Awaiting requested review from brianpursley

Assignees
No one assigned
Labels
area/apiserver area/cloudprovider area/code-generation area/dependency Issues or PRs related to dependency changes area/kube-proxy area/kubectl area/kubelet cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/cherry-pick-not-approved Indicates that a PR is not yet approved to merge into a release branch. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/architecture Categorizes an issue or PR as relevant to SIG Architecture. sig/auth Categorizes an issue or PR as relevant to SIG Auth. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/cloud-provider Categorizes an issue or PR as relevant to SIG Cloud Provider. sig/cluster-lifecycle Categorizes an issue or PR as relevant to SIG Cluster Lifecycle. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. sig/network Categorizes an issue or PR as relevant to SIG Network. sig/node Categorizes an issue or PR as relevant to SIG Node. sig/storage Categorizes an issue or PR as relevant to SIG Storage. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
SIG Auth
Status: Needs Triage
SIG CLI
Status: Needs Triage
SIG Node: code and documentation PRs
Status: not-only-sig-node
Milestone
v1.30
Development

Successfully merging this pull request may close these issues.
5 participants
@MadhavJivrajani @k8s-ci-robot @BenTheElder @liggitt @cici37