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

Upgrade Go version to v1.22.6 #2859

Merged
merged 3 commits into from
Aug 26, 2024
Merged

Upgrade Go version to v1.22.6 #2859

merged 3 commits into from
Aug 26, 2024

Conversation

tenzen-y
Copy link
Member

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

I upgraded the Go version to v1.22.6 since the minimum required Go version is upgraded to v1.22.6 in the hugo: https://github.com/gohugoio/hugo/blob/c9777473d1369f812d727a6c07dc57ad7be7bf62/go.mod#L172

If we do not upgrade the version, developers face some dependency issues when they use the IDEs depend on the "'go list -modfile" like JetBrains.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?

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. labels Aug 19, 2024
@k8s-ci-robot k8s-ci-robot requested review from mimowo and trasc August 19, 2024 17:10
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Aug 19, 2024
@netlify Netlify
Copy link

netlify bot commented Aug 19, 2024 •
edited
Loading

✅ Deploy Preview for kubernetes-sigs-kueue canceled.

Name Link
🔨 Latest commit 79fba12
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-kueue/deploys/66cce496f30f6f000891b515

mbobrovskyi
mbobrovskyi reviewed Aug 19, 2024
View reviewed changes
go.mod
@@ -1,6 +1,6 @@
module sigs.k8s.io/kueue

go 1.22.4
go 1.22.6
Copy link
Contributor

@mbobrovskyi mbobrovskyi Aug 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think it will not work well with gcb-docker-gcloud. The gcb-docker-gcloud now using go v1.22.5. If we will update it to go v1.22.6, we will have an error on post-kueue-push-images as we already had on #2611 and https://prow.k8s.io/view/gs/kubernetes-jenkins/logs/post-kueue-push-images/1812819196258553856.

There are two solution that I can see. We can try to use https://go.dev/doc/toolchain as recommended here. Or we need to update gcb-docker-gcloud image first (https://github.com/kubernetes/test-infra/blob/master/images/gcb-docker-gcloud/cloudbuild.yaml#L17).

@tenzen-y
Stop the Go patch version in the go module directive
35f9e60 
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
@mbobrovskyi mbobrovskyi mentioned this pull request Aug 26, 2024
Closed
@tenzen-y tenzen-y force-pushed the upgrade-go-version branch from a69e280 to 28edd58 Compare August 26, 2024 17:28
@tenzen-y
Copy link
Member Author

tenzen-y commented Aug 26, 2024 •
edited
Loading

@alculquicondor @mbobrovskyi After more investigating, I found another approach like this: cf781b2

How about this?

Indeed, building processes are available even if we can use the local gotoolchain like this:

$ docker run --rm -e GOTOOLCHAIN=local -it --name test-integration -v .:/go/src/kubernetes-sigs/kueue -w /go/src/kubernetes-sigs/kueue gcr.io/k8s-staging-test-infra/gcb-docker-gcloud:v20240718-5ef92b5c36 -c "make yq"

go: downloading github.com/mikefarah/yq/v4 v4.44.3
go: downloading gopkg.in/op/go-logging.v1 v1.0.0-20160211212156-b2cb9fa56473
go: downloading github.com/spf13/pflag v1.0.5
go: downloading github.com/spf13/cobra v1.8.1
go: downloading github.com/a8m/envsubst v1.4.2
go: downloading github.com/alecthomas/participle/v2 v2.1.1
go: downloading github.com/elliotchance/orderedmap v1.6.0
go: downloading github.com/fatih/color v1.17.0
go: downloading github.com/dimchansky/utfbom v1.1.1
go: downloading github.com/goccy/go-json v0.10.3
go: downloading github.com/goccy/go-yaml v1.12.0
go: downloading github.com/jinzhu/copier v0.4.0
go: downloading github.com/magiconair/properties v1.8.7
go: downloading github.com/pelletier/go-toml/v2 v2.2.2
go: downloading github.com/yuin/gopher-lua v1.1.1
go: downloading golang.org/x/net v0.27.0
go: downloading golang.org/x/text v0.16.0
go: downloading gopkg.in/yaml.v3 v3.0.1
go: downloading github.com/mattn/go-isatty v0.0.20
go: downloading github.com/mattn/go-colorable v0.1.13
go: downloading golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2
go: downloading golang.org/x/sys v0.22.0
$
$ echo $?
0

@tenzen-y tenzen-y force-pushed the upgrade-go-version branch from 28edd58 to 617f81f Compare August 26, 2024 17:33
tenzen-y
tenzen-y commented Aug 26, 2024
View reviewed changes
cmd/experimental/kjobctl/go.mod Outdated
Comment on lines 3 to 6
go 1.22.4

toolchain go1.22.6

Copy link
Member Author

Choose a reason for hiding this comment

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

kjobctl still requires the 1.22.4 explicitly.

https://prow.k8s.io/view/gs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_kueue/2859/pull-kueue-test-kjobctl/1828122376194232320

@tenzen-y
Point the Go 1.22 in the root module.
cf781b2 
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
@tenzen-y tenzen-y force-pushed the upgrade-go-version branch from 617f81f to cf781b2 Compare August 26, 2024 17:35
@mbobrovskyi
Copy link
Contributor

@alculquicondor @mbobrovskyi After more investigating, I found another approach like this: 617f81f

Yes, but it's still automatically updating go to 1.22.6 after go mod tidy.

@tenzen-y
Copy link
Member Author

@alculquicondor @mbobrovskyi After more investigating, I found another approach like this: 617f81f

Yes, but it's still automatically updating go to 1.22.6 after go mod tidy.

Could you check the latest commit? I can not observe the automatic updates.

@mbobrovskyi
Copy link
Contributor

mbobrovskyi commented Aug 26, 2024 •
edited
Loading

@alculquicondor @mbobrovskyi After more investigating, I found another approach like this: 617f81f

Yes, but it's still automatically updating go to 1.22.6 after go mod tidy.

Could you check the latest commit? I can not observe the automatic updates.

Yes, the same. I'm trying to execute go mod tidy on hack/internal/tools directory.

diff --git a/hack/internal/tools/go.mod b/hack/internal/tools/go.mod
index 49c409b31..5379feff7 100644
--- a/hack/internal/tools/go.mod
+++ b/hack/internal/tools/go.mod
@@ -1,6 +1,8 @@
 module sigs.k8s.io/kueue/internal/tools
 
-go 1.22
+go 1.22.6
+
+toolchain go1.23.0
 
 require (
        github.com/gohugoio/hugo v0.133.0

@tenzen-y
Copy link
Member Author

@alculquicondor @mbobrovskyi After more investigating, I found another approach like this: 617f81f

Yes, but it's still automatically updating go to 1.22.6 after go mod tidy.

Could you check the latest commit? I can not observe the automatic updates.

Yes, the same. I'm trying to execute go mod tidy on hack/internal/tools directory.

diff --git a/hack/internal/tools/go.mod b/hack/internal/tools/go.mod
index 49c409b31..5379feff7 100644
--- a/hack/internal/tools/go.mod
+++ b/hack/internal/tools/go.mod
@@ -1,6 +1,8 @@
 module sigs.k8s.io/kueue/internal/tools
 
-go 1.22
+go 1.22.6
+
+toolchain go1.23.0
 
 require (
        github.com/gohugoio/hugo v0.133.0

Oh, I see. I could observe it...

@alculquicondor
Copy link
Contributor

Can you leave it as go mod tidy wants and add GOTOOLCHAIN=auto in the cloudbuild.yaml env variables?

@tenzen-y
Explictly use Go 1.22.6 and GOTOOLCHAIN=auto
79fba12 
Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
@tenzen-y
Copy link
Member Author

Can you leave it as go mod tidy wants and add GOTOOLCHAIN=auto in the cloudbuild.yaml env variables?

Sure. I applied changes to the latest commit.

alculquicondor
alculquicondor reviewed Aug 26, 2024
View reviewed changes
Copy link
Contributor

@alculquicondor alculquicondor left a comment

Choose a reason for hiding this comment

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

Let's hope for the best and revert if it doesn't work :)

/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 Aug 26, 2024
@k8s-ci-robot
Copy link
Contributor

LGTM label has been added.

Git tree hash: ae40493f528fdea7c3a095526daeabbc1e43a9c9

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alculquicondor, tenzen-y

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:
  • OWNERS [alculquicondor,tenzen-y]

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 merged commit a1d9d9f into kubernetes-sigs:main Aug 26, 2024
17 checks passed
@k8s-ci-robot k8s-ci-robot added this to the v0.9 milestone Aug 26, 2024
@tenzen-y tenzen-y deleted the upgrade-go-version branch August 26, 2024 21:20
@alculquicondor alculquicondor mentioned this pull request Aug 27, 2024
Merged
kannon92 pushed a commit to openshift-kannon92/kubernetes-sigs-kueue that referenced this pull request Nov 19, 2024
@tenzen-y @kannon92
Upgrade Go version to v1.22.6 (kubernetes-sigs#2859)
7a4f42a 
* Stop the Go patch version in the go module directive

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

* Point the Go 1.22 in the root module.

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

* Explictly use Go 1.22.6 and GOTOOLCHAIN=auto

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>

---------

Signed-off-by: Yuki Iwai <yuki.iwai.tz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbobrovskyi mbobrovskyi mbobrovskyi left review comments

@alculquicondor alculquicondor alculquicondor left review comments

@mimowo mimowo Awaiting requested review from mimowo

@trasc trasc Awaiting requested review from trasc

Assignees

@alculquicondor alculquicondor

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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v0.9
Development

Successfully merging this pull request may close these issues.
4 participants
@tenzen-y @mbobrovskyi @alculquicondor @k8s-ci-robot