handle GOTOOLCHAIN in kube::golang::verify_go_version

[BenTheElder]

for now:

• shim FORCE_HOST_GO to default GOTOOLCHAIN=local (ensures we really force local go with 1.21+)
• treat a set GOTOOLCHAIN like FORCE_HOST_GO (so users can start idiomatically using the standard env in the future)
• set GOTOOLCHAIN to the selected go version otherwise (potentially unnecessary in the future, but it enforces that .go-version is the source of truth otherwise for now), keep gimme as a fallback
• pass GOTOOLCHAIN through to build containers

TODO (future PR): set toolchain statements in go.mod files and keep them in sync?

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

https://go.dev/doc/toolchain#GOTOOLCHAIN

Does this PR introduce a user-facing change?

makefile and scripts now respect GOTOOLCHAIN and otherwise ensure ./.go-version is used

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

Holding for further discussion
/hold

[BenTheElder]

/sig release
/cc @liggitt @dims

Open question: Originally I implemented this to only treat GOTOOLCHAIN=local like FORCE_HOST_GO=y, since that's the equivalent setting and not a default value. However now I think it might make more sense to treat "GOTOOLCHAIN is set at all" to be like FORCE_HOST_GO=y and just defer to go's toolchain support. This would not make sense if we think users have errant GOTOOLCHAIN in their system env, which is unlikely unless they've done something like exported go env which will have GOTOOLCHAIN=auto by default.

Other than that debatable detail, I think this PR makes sense as a starting point to ensure no surprises when we go to switch to 1.21+

EDIT: In both cases FORCE_HOST_GO=y is still supported as well, I think we clearly need to keep that for the immediate future and it looks straightforward.

[BenTheElder]

/kind cleanup
/priority important-soon

[liggitt]

I think this makes sense, maybe wait for the go1.21 bump PR to merge first (#118996)?

[BenTheElder]

/retest
(CI pod pending timeout ...)

This was rewritten to:

1. If GOTOOLCHAIN has a non-auto value, respect it
2. If FORCE_HOST_GO is set, shim that to GOTOOLCHAIN=local
3. Otherwise, set GOTOOLCHAIN to go${GO_VERSION} (from ./.go-version file)
   3b. If go isn't installed or that doesn't result in the right version because go is <1.21, fallback on gimme (edited)

Someday we can drop 3b), and we already won't wind up running it if the host has 1.21+

[liggitt]

/lgtm
/approve
can unhold when you're ready

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: e6deacbfa505624746c2367fd40b2540460ade4b

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, liggitt

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:

• build/OWNERS [BenTheElder,liggitt]
• hack/OWNERS [BenTheElder,liggitt]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[BenTheElder]

Pretty confident this has reasonable behavior now.

I think somewhere in N+M releases we should revisit:

1. Dropping gimme when go 1.21+ is widely packaged etc. (though ... we may still want a fallback for users without go on the host at all ...)
2. Nudging towards GOTOOLCHAIN=local over FORCE_HOST_GO=y and dropping the latter
3. Drop .go-version and just use toolchain go.mod statements.

But this should be a reasonable starting point.

[BenTheElder]

/hold cancel

[BenTheElder]

/retest

[liggitt]

I think this should move inside the if block where we don't have the go version we want... I'm seeing behavior where setting GOTOOLCHAIN=go1.21.1 explicitly is making go1.21.1 try to redownload (maybe related to build modifiers like boringcrypto being enabled)

[BenTheElder]

I'd expect short of a go bug (maybe in goboring), that GOTOOLCHAIN == current version should not result in downloading go again, but if GOTOOLCHAIN != current version.

The intention is setting it here should ensure that go version wasn't selected e.g. because this script was invoked with pwd=sigs.k8s.io/kind and it read that go.mod or other ways that GOTOOLCHAIN gets auto-selected. Setting GOTOOLCHAIN even if go version gives the desired version at this early invocation should force consistency.

[liggitt]

go env GOVERSION reports go1.21.1 X:boringcrypto when boringcrypto is enabled. The go version check we're doing below ignores any experiments, so when we force GOTOOLCHAIN to exactly go1.21.1, we might be creating the mismatch.

Worth filing an issue for the go team to clarify how we should be using GOTOOLCHAIN in a situation like this.

[BenTheElder]

ACK, I suspect export GOTOOLCHAIN=local is the right answer when using a toolchain like this.
If not I think we could add a check for X: experiments early in the version check and assume that we should behave like GOTOOLCHAIN=local if an experimental toolchain is being used.

[liggitt]

ACK, I suspect export GOTOOLCHAIN=local is the right answer when using a toolchain like this

I don't think so ... GOTOOLCHAIN=local won't do the right thing when we propagate into a build container with a different go version (like our kube-cross images)

[BenTheElder]

[we've reached out for clarification with the go team and will follow-up...]

[BenTheElder]

forgot to circle back here -- The expectation is that users will use GOEXPERIMENT which works with GOTOOLCHAIN (and this PR), GOTOOLCHAIN will download the standard toolchain but you can use the standard toolchain with GOEXPERIMENT instead of a modified toolchain, and that issue generally exists for 1.21+ in all projects.