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

use new GOTOOLCHAIN env to manage go version #262

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

huww98
Copy link
Contributor

@huww98 huww98 commented Nov 8, 2024

Should prevent go from automatically downloading newer toolchains. And ensure the specified version is used.

BTW, this also enables support for platforms other than linux/amd64

Requires at least go1.21 to run prow.sh

See also:

It works like this:

$ go version
go version go1.23.1 darwin/arm64

$ run_with_go "" go version
2024年11月 9日 星期六 00时59分35秒 CST go1.23.1 /Users/huww98/Downloads$ go version
go version go1.23.1 darwin/arm64

$ run_with_go "1.23.0" go version
2024年11月 9日 星期六 00时59分51秒 CST go1.23.0 /Users/huww98/Downloads$ go version
go version go1.23.0 darwin/arm64

$ run_with_go "1.23.2" go version
go: downloading go1.23.2 (darwin/arm64)
2024年11月 9日 星期六 01时01分21秒 CST go1.23.2 /Users/huww98/Downloads$ go version
go version go1.23.2 darwin/arm64

$ export PATH=~/Downloads/go1.18.10/bin:$PATH
$ go version
go version go1.18.10 darwin/arm64
$ run_with_go "1.23.0" go version
ERROR: Please install Go 1.21+
exit
# remove go from PATH
$ run_with_go "1.21.0" go version
bash: go: command not found
ERROR: Please install Go 1.21+
exit

Should prevent go from automatically downloading newer toolchains. And ensure the specified version is used.
@k8s-ci-robot k8s-ci-robot added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Nov 8, 2024
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 8, 2024
@k8s-ci-robot
Copy link
Contributor

Hi @huww98. Thanks for your PR.

I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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 needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Nov 8, 2024
@huww98 huww98 mentioned this pull request Nov 8, 2024
@mowangdk
Copy link
Contributor

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Nov 13, 2024
@huww98
Copy link
Contributor Author

huww98 commented Nov 19, 2024

/retest

@k8s-ci-robot
Copy link
Contributor

k8s-ci-robot commented Nov 19, 2024

@huww98: The following test 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-csi-release-tools-csi-test 6b05f0f link false /test pull-kubernetes-csi-release-tools-csi-test

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.

Copy link
Contributor

@pohly pohly left a comment

Choose a reason for hiding this comment

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

The change itself is fine.

I'm just wondering whether a bigger revamp of how Go versions are picked might be possible. For example, CSI_PROW_GO_VERSION_E2E could be removed in favor of simply not overriding the Go version at all when building inside the k/k repo. Go should be able to figure out if it needs a newer Go version based on the go.mod there. If the current Go version is newer, then there should be no need to download an older version.

@huww98
Copy link
Contributor Author

huww98 commented Nov 20, 2024

If the current Go version is newer, then there should be no need to download an older version.

Downloading an older version should help with reproducible build/test result.

@jsafrane
Copy link
Contributor

jsafrane commented Dec 3, 2024

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: huww98, jsafrane

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 Dec 3, 2024
@k8s-ci-robot k8s-ci-robot merged commit f95c855 into kubernetes-csi:master Dec 3, 2024
6 of 7 checks passed
avorima added a commit to avorima/external-attacher that referenced this pull request Dec 5, 2024
kubernetes-csi/csi-release-tools@734c2b95 Merge kubernetes-csi/csi-release-tools#265 from Rakshith-R/consider-main-branch
kubernetes-csi/csi-release-tools@f95c855b Merge kubernetes-csi/csi-release-tools#262 from huww98/golang-toolchain
kubernetes-csi/csi-release-tools@3c8d966f Treat main branch as equivalent to master branch
kubernetes-csi/csi-release-tools@e31de525 Merge kubernetes-csi/csi-release-tools#261 from huww98/golang
kubernetes-csi/csi-release-tools@fd153a9e Bump golang to 1.23.1
kubernetes-csi/csi-release-tools@a8b3d050 pull-test.sh: fix "git subtree pull" errors
kubernetes-csi/csi-release-tools@6b05f0fc use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
avorima added a commit to avorima/external-attacher that referenced this pull request Dec 5, 2024
Commit summary:
Squashed 'release-tools/' changes from 227577e0..734c2b95

[734c2b95](kubernetes-csi/csi-release-tools@734c2b95) Merge [pull request kubernetes-csi#265](kubernetes-csi/csi-release-tools#265) from Rakshith-R/consider-main-branch
[f95c855b](kubernetes-csi/csi-release-tools@f95c855b) Merge [pull request kubernetes-csi#262](kubernetes-csi/csi-release-tools#262) from huww98/golang-toolchain
[3c8d966f](kubernetes-csi/csi-release-tools@3c8d966f) Treat main branch as equivalent to master branch
[e31de525](kubernetes-csi/csi-release-tools@e31de525) Merge [pull request kubernetes-csi#261](kubernetes-csi/csi-release-tools#261) from huww98/golang
[fd153a9e](kubernetes-csi/csi-release-tools@fd153a9e) Bump golang to 1.23.1
[a8b3d050](kubernetes-csi/csi-release-tools@a8b3d050) pull-test.sh: fix "git subtree pull" errors
[6b05f0fc](kubernetes-csi/csi-release-tools@6b05f0fc) use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
Signed-off-by: Mario Valderrama <[email protected]>
avorima added a commit to avorima/external-provisioner that referenced this pull request Dec 5, 2024
kubernetes-csi/csi-release-tools@734c2b950 Merge kubernetes-csi/csi-release-tools#265 from Rakshith-R/consider-main-branch
kubernetes-csi/csi-release-tools@f95c855be Merge kubernetes-csi/csi-release-tools#262 from huww98/golang-toolchain
kubernetes-csi/csi-release-tools@3c8d966fe Treat main branch as equivalent to master branch
kubernetes-csi/csi-release-tools@e31de525b Merge kubernetes-csi/csi-release-tools#261 from huww98/golang
kubernetes-csi/csi-release-tools@fd153a9e2 Bump golang to 1.23.1
kubernetes-csi/csi-release-tools@a8b3d0504 pull-test.sh: fix "git subtree pull" errors
kubernetes-csi/csi-release-tools@6b05f0fcc use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
avorima added a commit to avorima/external-provisioner that referenced this pull request Dec 5, 2024
Commit summary:
Squashed 'release-tools/' changes from 227577e00..734c2b950

[734c2b950](kubernetes-csi/csi-release-tools@734c2b950) Merge [pull request kubernetes-csi#265](kubernetes-csi/csi-release-tools#265) from Rakshith-R/consider-main-branch
[f95c855be](kubernetes-csi/csi-release-tools@f95c855be) Merge [pull request kubernetes-csi#262](kubernetes-csi/csi-release-tools#262) from huww98/golang-toolchain
[3c8d966fe](kubernetes-csi/csi-release-tools@3c8d966fe) Treat main branch as equivalent to master branch
[e31de525b](kubernetes-csi/csi-release-tools@e31de525b) Merge [pull request kubernetes-csi#261](kubernetes-csi/csi-release-tools#261) from huww98/golang
[fd153a9e2](kubernetes-csi/csi-release-tools@fd153a9e2) Bump golang to 1.23.1
[a8b3d0504](kubernetes-csi/csi-release-tools@a8b3d0504) pull-test.sh: fix "git subtree pull" errors
[6b05f0fcc](kubernetes-csi/csi-release-tools@6b05f0fcc) use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
Signed-off-by: Mario Valderrama <[email protected]>
avorima added a commit to avorima/external-resizer that referenced this pull request Dec 5, 2024
kubernetes-csi/csi-release-tools@734c2b95 Merge kubernetes-csi/csi-release-tools#265 from Rakshith-R/consider-main-branch
kubernetes-csi/csi-release-tools@f95c855b Merge kubernetes-csi/csi-release-tools#262 from huww98/golang-toolchain
kubernetes-csi/csi-release-tools@3c8d966f Treat main branch as equivalent to master branch
kubernetes-csi/csi-release-tools@e31de525 Merge kubernetes-csi/csi-release-tools#261 from huww98/golang
kubernetes-csi/csi-release-tools@fd153a9e Bump golang to 1.23.1
kubernetes-csi/csi-release-tools@a8b3d050 pull-test.sh: fix "git subtree pull" errors
kubernetes-csi/csi-release-tools@6b05f0fc use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
avorima added a commit to avorima/external-resizer that referenced this pull request Dec 5, 2024
Commit summary:
Squashed 'release-tools/' changes from 227577e0..734c2b95

[734c2b95](kubernetes-csi/csi-release-tools@734c2b95) Merge [pull request kubernetes-csi#265](kubernetes-csi/csi-release-tools#265) from Rakshith-R/consider-main-branch
[f95c855b](kubernetes-csi/csi-release-tools@f95c855b) Merge [pull request kubernetes-csi#262](kubernetes-csi/csi-release-tools#262) from huww98/golang-toolchain
[3c8d966f](kubernetes-csi/csi-release-tools@3c8d966f) Treat main branch as equivalent to master branch
[e31de525](kubernetes-csi/csi-release-tools@e31de525) Merge [pull request kubernetes-csi#261](kubernetes-csi/csi-release-tools#261) from huww98/golang
[fd153a9e](kubernetes-csi/csi-release-tools@fd153a9e) Bump golang to 1.23.1
[a8b3d050](kubernetes-csi/csi-release-tools@a8b3d050) pull-test.sh: fix "git subtree pull" errors
[6b05f0fc](kubernetes-csi/csi-release-tools@6b05f0fc) use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
Signed-off-by: Mario Valderrama <[email protected]>
avorima added a commit to avorima/node-driver-registrar that referenced this pull request Dec 5, 2024
kubernetes-csi/csi-release-tools@734c2b95 Merge kubernetes-csi/csi-release-tools#265 from Rakshith-R/consider-main-branch
kubernetes-csi/csi-release-tools@f95c855b Merge kubernetes-csi/csi-release-tools#262 from huww98/golang-toolchain
kubernetes-csi/csi-release-tools@3c8d966f Treat main branch as equivalent to master branch
kubernetes-csi/csi-release-tools@e31de525 Merge kubernetes-csi/csi-release-tools#261 from huww98/golang
kubernetes-csi/csi-release-tools@fd153a9e Bump golang to 1.23.1
kubernetes-csi/csi-release-tools@a8b3d050 pull-test.sh: fix "git subtree pull" errors
kubernetes-csi/csi-release-tools@6b05f0fc use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
avorima added a commit to avorima/node-driver-registrar that referenced this pull request Dec 5, 2024
Commit summary:
Squashed 'release-tools/' changes from 227577e0..734c2b95

[734c2b95](kubernetes-csi/csi-release-tools@734c2b95) Merge [pull request kubernetes-csi#265](kubernetes-csi/csi-release-tools#265) from Rakshith-R/consider-main-branch
[f95c855b](kubernetes-csi/csi-release-tools@f95c855b) Merge [pull request kubernetes-csi#262](kubernetes-csi/csi-release-tools#262) from huww98/golang-toolchain
[3c8d966f](kubernetes-csi/csi-release-tools@3c8d966f) Treat main branch as equivalent to master branch
[e31de525](kubernetes-csi/csi-release-tools@e31de525) Merge [pull request kubernetes-csi#261](kubernetes-csi/csi-release-tools#261) from huww98/golang
[fd153a9e](kubernetes-csi/csi-release-tools@fd153a9e) Bump golang to 1.23.1
[a8b3d050](kubernetes-csi/csi-release-tools@a8b3d050) pull-test.sh: fix "git subtree pull" errors
[6b05f0fc](kubernetes-csi/csi-release-tools@6b05f0fc) use new GOTOOLCHAIN env to manage go version

git-subtree-dir: release-tools
git-subtree-split: 734c2b950c4b31f64b63052c64ffa5929d1c9b97
Signed-off-by: Mario Valderrama <[email protected]>
@Rakshith-R
Copy link
Contributor

hey @huww98 ,

I think this pr is responsible for all the k8s-1.29 pull job failures on all sidecar repos.

https://prow.k8s.io/view/gs/kubernetes-ci-logs/pr-logs/pull/kubernetes-csi_external-snapshot-metadata/83/pull-kubernetes-csi-external-snapshot-metadata-1-29-on-kubernetes-1-29/1866439937163792384


HEAD is now at 3f7a50f3 Release commit for Kubernetes v1.29.0
Tue Dec 10 11:29:33 UTC 2024 go1.23.3 /home/prow/go/src/k8s.io/kubernetes$ git clean -fdx
Tue Dec 10 11:29:33 UTC 2024 go1.21.5 $ make WHAT=test/e2e/e2e.test -C/home/prow/go/src/k8s.io/kubernetes
make: Entering directory '/home/prow/go/src/k8s.io/kubernetes'
+++ [1210 11:29:34] Building go targets for linux/amd64
    k8s.io/kubernetes/test/e2e/e2e.test (test)
make: Leaving directory '/home/prow/go/src/k8s.io/kubernetes'
Tue Dec 10 11:36:06 UTC 2024 go1.21.5 $ make WHAT=vendor/github.com/onsi/ginkgo/ginkgo -C/home/prow/go/src/k8s.io/kubernetes
make: Entering directory '/home/prow/go/src/k8s.io/kubernetes'
+++ [1210 11:36:06] Building go targets for linux/amd64
    github.com/onsi/ginkgo/v2/ginkgo (non-static)
go: downloading go1.21.5 (linux/amd64)
go: download go1.21.5 for linux/amd64: toolchain not available
!!! [1210 11:36:06] Call tree:
!!! [1210 11:36:06]  1: /home/prow/go/src/k8s.io/kubernetes/hack/lib/golang.sh:799 kube::golang::build_some_binaries(...)
!!! [1210 11:36:06]  2: /home/prow/go/src/k8s.io/kubernetes/hack/lib/golang.sh:958 kube::golang::build_binaries_for_platform(...)
!!! [1210 11:36:06]  3: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
!!! [1210 11:36:06] Call tree:
!!! [1210 11:36:06]  1: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)
!!! [1210 11:36:06] Call tree:
!!! [1210 11:36:06]  1: hack/make-rules/build.sh:27 kube::golang::build_binaries(...)

@avorima ^

cc @jsafrane @pohly

@Rakshith-R
Copy link
Contributor

Probably because the complete 1.21.z version is not defined in https://github.com/kubernetes/kubernetes/blob/release-1.29/go.mod#L9

@avorima
Copy link

avorima commented Dec 10, 2024

@Rakshith-R could we update the used k/k version to 1.30 or 1.31?

@huww98
Copy link
Contributor Author

huww98 commented Dec 10, 2024

@Rakshith-R It looks strange to me. The log you posted shows

go: downloading go1.21.5 (linux/amd64)
go: download go1.21.5 for linux/amd64: toolchain not available

But I got this locally.

GOTOOLCHAIN=go1.21.5 go version
go: downloading go1.21.5 (darwin/arm64)
go version go1.21.5 darwin/arm64

Can you check what the GOPROXY env is? The tool chain is downloaded there.

@Rakshith-R
Copy link
Contributor

@Rakshith-R could we update the used k/k version to 1.30 or 1.31?

I am not sure, I don't think so.

@Rakshith-R It looks strange to me. The log you posted shows

go: downloading go1.21.5 (linux/amd64)
go: download go1.21.5 for linux/amd64: toolchain not available

But I got this locally.

GOTOOLCHAIN=go1.21.5 go version
go: downloading go1.21.5 (darwin/arm64)
go version go1.21.5 darwin/arm64

Can you check what the GOPROXY env is? The tool chain is downloaded there.

I added prow job logs here
#262 (comment)
I didn't find any GOPROXY env .

@huww98
Copy link
Contributor Author

huww98 commented Dec 10, 2024

I think I've figured it out. v1.29 failed because It has GOPROXY=off in kube::golang::build_some_binaries. v1.30 is OK because it has the new kube::golang::internal::verify_go_version to trigger the download before the build.

@avorima
Copy link

avorima commented Dec 10, 2024

@Rakshith-R yes, my bad i was thinking about the default version in prow.sh, but the test runs on 1.29 which can't be changed.

@huww98 i see the GOPROXY=off in the k/k repo. is that something that needs to be fixed there or do you think it can be fixed here?

@huww98
Copy link
Contributor Author

huww98 commented Dec 10, 2024

@avorima I think #267 could fix this. It should ensure that Kubernetes also use the toolchain that we download, and does not need to download again.

@Rakshith-R
Copy link
Contributor

Thanks for the quick fix !

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. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants