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

[RFC] Setup a way to consistently manage go versions across scripts and go.mods #17876

Merged
merged 2 commits into from May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion Makefile
Expand Up @@ -55,7 +55,7 @@ fuzz:
verify: verify-gofmt verify-bom verify-lint verify-dep verify-shellcheck verify-goword \
verify-govet verify-license-header verify-receiver-name verify-mod-tidy \
verify-shellws verify-proto-annotations verify-genproto verify-yamllint \
verify-govet-shadow verify-markdown-marker
verify-govet-shadow verify-markdown-marker verify-go-versions

.PHONY: fix
fix: fix-bom fix-lint fix-yamllint
Expand Down Expand Up @@ -187,3 +187,11 @@ clean:
rm -rf ./tests/e2e/default.proxy
rm -rf ./bin/shellcheck*
find ./ -name "127.0.0.1:*" -o -name "localhost:*" -o -name "*.log" -o -name "agent-*" -o -name "*.coverprofile" -o -name "testname-proxy-*" -delete

.PHONY: verify-go-versions
verify-go-versions:
ahrtr marked this conversation as resolved.
Show resolved Hide resolved
./scripts/verify_go_versions.sh

.PHONY: sync-toolchain-directive
sync-toolchain-directive:
./scripts/sync_go_toolchain_directive.sh
MadhavJivrajani marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 2 additions & 0 deletions api/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/api/v3

go 1.22

toolchain go1.22.2

require (
github.com/coreos/go-semver v0.3.1
github.com/gogo/protobuf v1.3.2
Expand Down
2 changes: 2 additions & 0 deletions client/internal/v2/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/client/v2

go 1.22

toolchain go1.22.2

require (
go.etcd.io/etcd/api/v3 v3.6.0-alpha.0
go.etcd.io/etcd/client/pkg/v3 v3.6.0-alpha.0
Expand Down
2 changes: 2 additions & 0 deletions client/pkg/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/client/pkg/v3

go 1.22

toolchain go1.22.2

require (
github.com/coreos/go-systemd/v22 v22.5.0
github.com/stretchr/testify v1.9.0
Expand Down
2 changes: 2 additions & 0 deletions client/v3/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/client/v3

go 1.22

toolchain go1.22.2

require (
github.com/coreos/go-semver v0.3.1
github.com/dustin/go-humanize v1.0.1
Expand Down
2 changes: 2 additions & 0 deletions etcdctl/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/etcdctl/v3

go 1.22

toolchain go1.22.2

require (
github.com/bgentry/speakeasy v0.1.0
github.com/cheggaaa/pb/v3 v3.1.5
Expand Down
2 changes: 2 additions & 0 deletions etcdutl/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/etcdutl/v3

go 1.22

toolchain go1.22.2

replace (
go.etcd.io/etcd/api/v3 => ../api
go.etcd.io/etcd/client/pkg/v3 => ../client/pkg
Expand Down
2 changes: 2 additions & 0 deletions pkg/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/pkg/v3

go 1.22

toolchain go1.22.2

require (
github.com/creack/pty v1.1.18
github.com/dustin/go-humanize v1.0.1
Expand Down
2 changes: 2 additions & 0 deletions scripts/fix.sh
Expand Up @@ -36,5 +36,7 @@ run_for_modules run ${GO_CMD} fmt || exit 2
run_for_module tests bom_fix || exit 2
bash_ws_fix || exit 2

log_callout "Syncing go toolchain directives"
run ./scripts/sync_go_toolchain_directive.sh || exit 2
Copy link
Member

Choose a reason for hiding this comment

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

Would prefer to move subroutines from ./scripts/fix.sh to makefile targets. As you already have a makefile target sync-toolchain-directive why not add it as subtarget to fix?

Copy link
Member

Choose a reason for hiding this comment

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

why not add it as subtarget to fix?

Yes, it's another option. Either way works for me.

We can even breakdown the existing ./script/fix.sh into a couple of Makefile targets

  • fix-go-dependency (in which we will run mod_tidy_fix and the sync_go_toolchain_directive.sh)
  • fix-gofmt
  • fix-script-whitespace

It can be discussed & resolved in a separate PR.


log_success -e "\\nSUCCESS: etcd code is fixed :)"
14 changes: 14 additions & 0 deletions scripts/sync_go_toolchain_directive.sh
@@ -0,0 +1,14 @@
#!/usr/bin/env bash

# This script looks at the version present in the .go-version file and treats
# that to be the value of the toolchain directive that go should use. It then
# updates the toolchain directives of all go.mod files to reflect this version.
#
# We do this to ensure that .go-version acts as the source of truth for go versions.

set -euo pipefail

source ./scripts/test_lib.sh

TARGET_GO_VERSION="${TARGET_GO_VERSION:-"$(cat "${ETCD_ROOT_DIR}/.go-version")"}"
find . -name 'go.mod' -exec go mod edit -toolchain=go"${TARGET_GO_VERSION}" {} \;
24 changes: 24 additions & 0 deletions scripts/test_lib.sh
Expand Up @@ -456,3 +456,27 @@ function git_assert_branch_in_sync {
log_warning "Cannot verify consistency with the origin, as git is on detached branch."
fi
}

# The version present in the .go-verion is the default version that test and build scripts will use.
# However, it is possible to control the version that should be used with the help of env vars:
# - FORCE_HOST_GO: if set to a non-empty value, use the version of go installed in system's $PATH.
# - GO_VERSION: desired version of go to be used, might differ from what is present in .go-version.
# If empty, the value defaults to the version in .go-version.
function determine_go_version {
# Borrowing from how Kubernetes does this:
# https://github.com/kubernetes/kubernetes/blob/17854f0e0a153b06f9d0db096e2cd8ab2fa89c11/hack/lib/golang.sh#L510-L520
#
# default GO_VERSION to content of .go-version
GO_VERSION="${GO_VERSION:-"$(cat "${ETCD_ROOT_DIR}/.go-version")"}"
if [ "${GOTOOLCHAIN:-auto}" != 'auto' ]; then
# no-op, just respect GOTOOLCHAIN
:
elif [ -n "${FORCE_HOST_GO:-}" ]; then
export GOTOOLCHAIN='local'
else
GOTOOLCHAIN="go${GO_VERSION}"
export GOTOOLCHAIN
fi
}

determine_go_version
53 changes: 53 additions & 0 deletions scripts/verify_go_versions.sh
@@ -0,0 +1,53 @@
#!/usr/bin/env bash

# This script verifies that the value of the toolchain directive in the
# go.mod files always match that of the .go-version file to ensure that
# we accidentally don't test and release with differing versions of Go.

set -euo pipefail

source ./scripts/test_lib.sh

target_go_version="${target_go_version:-"$(cat "${ETCD_ROOT_DIR}/.go-version")"}"
log_info "expected go toolchain directive: go${target_go_version}"
log_info

toolchain_out_of_sync="false"
go_line_violation="false"

# verify_go_versions takes a go.mod filepath as an argument
# and checks if:
# (1) go directive <= version in .go-version
# (2) toolchain directive == version in .go-version
function verify_go_versions() {
# shellcheck disable=SC2086
toolchain_version="$(go mod edit -json $1 | jq -r .Toolchain)"
# shellcheck disable=SC2086
go_line_version="$(go mod edit -json $1 | jq -r .Go)"
if [[ "go${target_go_version}" != "${toolchain_version}" ]]; then
log_error "go toolchain directive out of sync for $1, got: ${toolchain_version}"
toolchain_out_of_sync="true"
fi
if ! printf '%s\n' "${go_line_version}" "${target_go_version}" | sort --check=silent --version-sort; then
log_error "go directive in $1 is greater than maximum allowed: go${target_go_version}"
go_line_violation="true"
fi
}

while read -r mod; do
verify_go_versions "${mod}";
done < <(find . -name 'go.mod')

if [[ "${toolchain_out_of_sync}" == "true" ]]; then
log_error
log_error "Please run scripts/sync_go_toolchain_directive.sh or update .go-version to rectify this error"
fi

if [[ "${go_line_violation}" == "true" ]]; then
log_error
log_error "Please update .go-version to rectify this error, any go directive should be <= .go-version"
fi

if [[ "${go_line_violation}" == "true" ]] || [[ "${toolchain_out_of_sync}" == "true" ]]; then
exit 1
fi
2 changes: 2 additions & 0 deletions server/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/server/v3

go 1.22

toolchain go1.22.2

require (
github.com/coreos/go-semver v0.3.1
github.com/coreos/go-systemd/v22 v22.5.0
Expand Down
2 changes: 2 additions & 0 deletions tests/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/tests/v3

go 1.22

toolchain go1.22.2

replace (
go.etcd.io/etcd/api/v3 => ../api
go.etcd.io/etcd/client/pkg/v3 => ../client/pkg
Expand Down
2 changes: 2 additions & 0 deletions tools/rw-heatmaps/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/tools/rw-heatmaps/v3

go 1.22

toolchain go1.22.2

require (
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5
Expand Down
2 changes: 2 additions & 0 deletions tools/testgrid-analysis/go.mod
Expand Up @@ -2,6 +2,8 @@ module go.etcd.io/etcd/tools/testgrid-analysis/v3

go 1.22

toolchain go1.22.2

require (
github.com/GoogleCloudPlatform/testgrid v0.0.173
github.com/google/go-github/v60 v60.0.0
Expand Down