Skip to content

go.mod, renovate: specify and update Go toolchain version #27820

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
Merged
merged 2 commits into from
Aug 30, 2023
Merged

go.mod, renovate: specify and update Go toolchain version #27820

merged 2 commits into from
Aug 30, 2023

Conversation

tklauser
Copy link
Member

@tklauser tklauser commented Aug 30, 2023 •
edited
Loading

According to [1] as of Go 1.21 we either need to specify the full toolchain version in the go directive or add a toolchain directive with the concrete toolchain version. Opt for the former and make sure it's kept up to date by renovate bot.

[1] golang/go#62278 (comment)

@tklauser tklauser added the release-note/misc This PR makes changes that have no direct user impact. label Aug 30, 2023
@tklauser tklauser requested review from a team as code owners August 30, 2023 09:02
@tklauser tklauser requested review from rolinh and qmonnet August 30, 2023 09:02
@tklauser tklauser force-pushed the pr/tklauser/go.mod-toolchain branch from ae59455 to af6a864 Compare August 30, 2023 09:02
@tklauser
Copy link
Member Author

/test

@tklauser tklauser requested a review from a team as a code owner August 30, 2023 09:34
@tklauser
Copy link
Member Author

/ci-runtime

@tklauser
Copy link
Member Author

/test

@tklauser tklauser force-pushed the pr/tklauser/go.mod-toolchain branch from 87ef862 to e261c36 Compare August 30, 2023 10:09
@tklauser tklauser requested review from a team as code owners August 30, 2023 10:09
@tklauser tklauser requested a review from brlbil August 30, 2023 10:09
@tklauser
Copy link
Member Author

/test

@tklauser
Copy link
Member Author

Pushed an additional commit to run the privileged runtime tests with the proper Go toolchain version. See commit message of the 1st commit for details.

@tklauser tklauser force-pushed the pr/tklauser/go.mod-toolchain branch from e261c36 to cc4198c Compare August 30, 2023 10:35
@tklauser
Copy link
Member Author

/test

@tklauser tklauser removed the request for review from a team August 30, 2023 11:07
qmonnet
qmonnet approved these changes Aug 30, 2023
View reviewed changes
Copy link
Member

@qmonnet qmonnet left a comment

Choose a reason for hiding this comment

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

Looks good as far as I can tell

brlbil
brlbil approved these changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@brlbil brlbil left a comment

Choose a reason for hiding this comment

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

LGTM

@tklauser
.github/workflows: run privileged tests with proper Go toolchain version
e438cd5 
Currently, the privileged runtime tests are built and run with whatever
Go toolchain version the LVH images ship. To make sure these tests are
run with the same Go toolchain version as the rest of the tests install
a copy of that Go toolchain before running the test.

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser
go.mod, renovate: specify and update Go toolchain version
79387b0 
According to [1] as of Go 1.21 we either need to specify the full
toolchain version in the `go` directive or add a `toolchain` directive
with the concrete toolchain version. Opt for the former and make sure
it's kept up to date by renovate bot.

[1] golang/go#62278 (comment)

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@tklauser tklauser force-pushed the pr/tklauser/go.mod-toolchain branch from cc4198c to 79387b0 Compare August 30, 2023 12:30
@tklauser
Copy link
Member Author

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Aug 30, 2023
@tklauser tklauser merged commit 2f7e2f3 into main Aug 30, 2023
@tklauser tklauser deleted the pr/tklauser/go.mod-toolchain branch August 30, 2023 13:30
@tklauser tklauser mentioned this pull request Aug 31, 2023
Merged
tklauser added a commit that referenced this pull request Aug 31, 2023
@tklauser
ci: run verifier tests with proper Go toolchain version
c166510 
Currently, the verifier tests are built and run with whatever Go
toolchain version the LVH images ship. To make sure these tests are run
with the same Go toolchain version as the rest of the tests install a
copy of that Go toolchain before running the test.

This wasn't caught in CI on #27820
because the verifier workflow wasn't run passed on the ariane exclusion
list.

Reported-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
aanm pushed a commit that referenced this pull request Sep 1, 2023
@tklauser @aanm
ci: run verifier tests with proper Go toolchain version
bb3eec4 
Currently, the verifier tests are built and run with whatever Go
toolchain version the LVH images ship. To make sure these tests are run
with the same Go toolchain version as the rest of the tests install a
copy of that Go toolchain before running the test.

This wasn't caught in CI on #27820
because the verifier workflow wasn't run passed on the ariane exclusion
list.

Reported-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
@joestringer
Copy link
Member

joestringer commented Sep 5, 2023 •
edited
Loading

I'm getting errors in my editor like this after merging this PR:

Error loading workspace /home/joe/git/cilium/go.mod:4: invalid go version '1.21.0': must match format 1.23

I've updated my local Go binary to v1.21.0 but that doesn't seem to help.

EDIT: Looks like in might just be coc / coc.nvim / gopls not supporting micro versions..? No change updating gopls to v0.13.x.

@tklauser
Copy link
Member Author

tklauser commented Sep 6, 2023

I'm getting errors in my editor like this after merging this PR:

Error loading workspace /home/joe/git/cilium/go.mod:4: invalid go version '1.21.0': must match format 1.23

I've updated my local Go binary to v1.21.0 but that doesn't seem to help.

Without knowing your exact editor setup it's hard to tell what's wrong. I assume you're using some sort of Go integration like go-vim or similar? These extensions usually rely on various supporting tool binaries (e.g. gopls, linters etc). Could you try rebuilding these using Go 1.21 as well? Also cleaning all Go caches using go clean -cache -modcache -testcache -fuzzcache might help.

@joestringer
Copy link
Member

joestringer commented Sep 7, 2023 •
edited
Loading

👍 thanks for the tips. go clean didn't help, neither did PlugUpdate in vim to update my plugins. I tracked it down to coc.nvim. Looks like for some reason it was using some custom version of gopls. Configuring go.goplsPath to my ~/go/bin/gopls (+ updating that binary) fixed the issue.

@rolinh rolinh mentioned this pull request Sep 28, 2023
Merged
@joestringer
Copy link
Member

@tklauser why do we need to keep the patch version here in sync with upstream? Is this how we derive the Go version to install and build with in CI? Put another way, is it possible to keep the go.mod at 1.21.0 until we migrate to 1.22?

@joestringer joestringer mentioned this pull request Oct 18, 2023
Merged
@tklauser
Copy link
Member Author

@tklauser why do we need to keep the patch version here in sync with upstream? Is this how we derive the Go version to install and build with in CI? Put another way, is it possible to keep the go.mod at 1.21.0 until we migrate to 1.22?

@joestringer the idea here was to keep up with the latest patch version in case anything (e.g. CI) or anyone is relying on go.mod as a source of truth for the Go version. I agree that this might cause friction and from quickly glancing through our CI workflows it doesn't look like any of them is using the Go version in go.mod as a source of truth, so SGTM to keep go.mod at 1.x.0 until we switch to 1.(x+1).0.

giorio94 pushed a commit that referenced this pull request Jan 23, 2024
@tklauser @giorio94
ci: run verifier tests with proper Go toolchain version
da2640b 
[ upstream commit bb3eec4 ]

Currently, the verifier tests are built and run with whatever Go
toolchain version the LVH images ship. To make sure these tests are run
with the same Go toolchain version as the rest of the tests install a
copy of that Go toolchain before running the test.

This wasn't caught in CI on #27820
because the verifier workflow wasn't run passed on the ariane exclusion
list.

Reported-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
julianwiedmann pushed a commit that referenced this pull request Jan 24, 2024
@tklauser @julianwiedmann
ci: run verifier tests with proper Go toolchain version
1d2db9f 
[ upstream commit bb3eec4 ]

Currently, the verifier tests are built and run with whatever Go
toolchain version the LVH images ship. To make sure these tests are run
with the same Go toolchain version as the rest of the tests install a
copy of that Go toolchain before running the test.

This wasn't caught in CI on #27820
because the verifier workflow wasn't run passed on the ariane exclusion
list.

Reported-by: Julian Wiedmann <jwi@isovalent.com>
Signed-off-by: Tobias Klauser <tobias@cilium.io>
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rolinh rolinh rolinh approved these changes

@brlbil brlbil brlbil approved these changes

@qmonnet qmonnet qmonnet approved these changes

Assignees
No one assigned
Labels
ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tklauser @joestringer @rolinh @brlbil @qmonnet