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

Pin etcd client version #148

Merged

Conversation

JamesLaverack
Copy link
Contributor

Thanks to the work of etcd-io/etcd#11477 we can pin our etcd client version to v3.4.3 instead of the v3.3.17 we were using. This has the limitation of needing to be careful upgraded when new versions come out (using the script in the previously linked pull request against etcd).

Note that the go.sum still references other versions of etcd:

github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/etcd v3.3.15+incompatible h1:+9RjdC18gMxNQVvSiXvObLu29mOFmkgdsB4cRTlV+EE=
github.com/coreos/etcd v3.3.15+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=

Some investigation with go mod tree seem to suggest that github.com/coreos/etcd v3.3.15 comes from k8s.io/apiserver and k8s.io/apiextensions-apiserver.

github.com/coreos/etcd v3.3.10 ultimately comes from sigs.k8s.io/kind, via github.com/spf13/cobra and github.com/spf13/viper.

There's not a lot I can do about these, but I don't believe our code uses them.

Always use `go.etcd.io/etcd` over `github.com/coreos/etcd` or
`github.com/etcd-io/etcd`.
@JamesLaverack JamesLaverack requested a review from wallrj February 4, 2020 13:14
@JamesLaverack JamesLaverack added the housekeeping Cleanups or optimisations label Feb 4, 2020
@JamesLaverack JamesLaverack requested review from munnerz and removed request for wallrj February 5, 2020 09:46
Copy link
Contributor

@munnerz munnerz left a comment

Choose a reason for hiding this comment

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

No big comments/anything important!

@@ -4,6 +4,8 @@ go 1.13

// Pin k8s.io/* dependencies to kubernetes-1.16.0 to match controller-runtime v0.4.0
replace (
// This must match the version below
go.etcd.io/etcd => go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, this doesn't have to match the version below. replace directives are always used regardless of the require directives below IF you are building the current module.

If something imports this module, then the replace directives are ignored, at which point those people depending on us may need a similar replace directive in their own module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you say it's 'good practice' to match as below?

github.com/dustinkirkland/golang-petname v0.0.0-20190613200456-11339a705ed2
github.com/go-logr/logr v0.1.0
github.com/google/go-cmp v0.3.0
github.com/google/gofuzz v1.0.0
github.com/robfig/cron/v3 v3.0.0
github.com/stretchr/testify v1.4.0
go.etcd.io/etcd v3.3.17+incompatible
// Pin spesific etcd version via tag. See https://github.com/etcd-io/etcd/pull/11477
go.etcd.io/etcd v0.0.0-20191023171146-3cf2f69b5738
Copy link
Contributor

Choose a reason for hiding this comment

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

iirc these get re-written by go mod tidy anyway - if you just had the commit ref here it should work (and go mod will automatically compute to v0.0.0-* version of the ref)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These get re-written by go mod tidy

It doesn't seem to. I've run tidy a few times since I added this with no change.

I'm pretty much following the documentation for the version workaround.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I mean is if you added/manually specified the commit ref here, go mod tidy would rewrite it into the form you've written here 😄

I don't think using that linked script is necessary.. simply getting the ref for that tag and adding it here, then running go mod tidy should do it.

@JamesLaverack JamesLaverack merged commit eac0f09 into improbable-eng:master Feb 5, 2020
@JamesLaverack JamesLaverack deleted the pin-client-version branch February 5, 2020 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
housekeeping Cleanups or optimisations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants