etcd-io / etcd

Distributed reliable key-value store for the most critical data of a distributed system
https://etcd.io
Apache License 2.0
47.81k stars 9.76k forks source link

Setup a consistent way to manage go version #17857

Open ahrtr opened 6 months ago

ahrtr commented 6 months ago

What would you like to be added?

Currently there are multiple places setting the go versions, for examples,

We should set up a consistent way to manage go version. There should be only one source of truth for the go&toolchain version.

One problem is that if a dependency sets a higher go or toolchain version, then when running scripts/fix.sh will automatically update the go & toolchain version in our go.mod files. Refer to https://github.com/etcd-io/etcd/pull/17841/files#r1575344566

References:

cc @fuweid @henrybear327 @ivanvc @jmhbnz @MadhavJivrajani @serathius

Why is this needed?

Setup consistent way to manage go version

MadhavJivrajani commented 6 months ago

One problem is that if a dependency sets a higher go or toolchain version, then when running scripts/fix.sh will automatically update the go & toolchain version in our go.mod files.

We are trying to be more careful about this situation in Kubernetes (ref: https://github.com/kubernetes/kubernetes/issues/123744).

One thing we should keep in mind here also is that if dependabot bumps the go directive of the root go.mod and we don't reflect that change in the .go-version file then it could be problematic for reasons similar to https://github.com/kubernetes/kubernetes/issues/123744, if that happens, we are in a situation where:

In Kubernetes we added a pre-submit check to catch such accidental bumps if they occur: https://github.com/kubernetes/kube-openapi/pull/467

ahrtr commented 6 months ago

In Kubernetes we added a pre-submit check to catch such accidental bumps if they occur: kubernetes/kube-openapi#467

Thanks @MadhavJivrajani

I think we should do similar check for all etcd branches: including release-3.4, release-3.4 and main.

The other potential enhancement is to automatically populate the go versions for the go and toolchain lines in all go.mod (and go.sum) files based on the .go-version file.

MadhavJivrajani commented 6 months ago

I have some ideas on how we can go about doing this. Will create a PR and we can discuss there!

ahrtr commented 6 months ago

Followups:

ahrtr commented 6 months ago

I think we also need to backport the changes to both 3.5 and 3.4

ahrtr commented 6 months ago

We should follow the same rule to manage go version for all repositories under etcd-io. Note we only require users to depend on a minor version supported by golang team, i.e. 1.21, but we don't force users to depend on a patch version, i.e. 1.21.10.

ivanvc commented 6 months ago

Cross-posting and quoting my comment from https://github.com/etcd-io/etcd/pull/17876#issuecomment-2108783668:

This implementation is great (referring to #17876) and simplifies what would be tedious if done by hand. We recently had the Go version updated by a new contributor, and I believe it wasn't very clear to him how to update the toolchain in the different go.mod files (refer to: #17975 (comment)). I think we would benefit from documenting this somewhere in the repository. A potential improvement regarding documentation would be to document how to update the Go version. In the main branch with this new script, I think it's as easy as updating the value in .go-version and running make fix after 🎉