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

clientv3: Fix grpc-go(v1.27.0) incompatible changes to balancer/resolver. #11564

Merged
merged 1 commit into from
Mar 6, 2020

Conversation

eddycjy
Copy link
Contributor

@eddycjy eddycjy commented Jan 29, 2020

clientv3: Fix grpc-go (v1.27.0) incompatible modification of balancer/resolver API.

Modify the API changed by balancer / resolver to ensure consistency with grpc-go (v1.27.0), otherwise clientv3 will not be able to be pulled by go mod, which will affect the direct use of users.

References:

  1. balancer/resolver: remove temporary backward-compatibility type aliases
  2. Notice: Upcoming Experimental Balancer/Resolver API Changes

Fixes #11563

@eddycjy eddycjy requested review from gyuho and jpbetz January 29, 2020 07:13
@eddycjy eddycjy changed the title Fix grpc-go(v1.27.0) incompatible changes to balancer/resolver. clientv3: Fix grpc-go(v1.27.0) incompatible changes to balancer/resolver. Jan 29, 2020
@codecov-io
Copy link

codecov-io commented Jan 29, 2020

Codecov Report

Merging #11564 into master will increase coverage by 1.51%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11564      +/-   ##
==========================================
+ Coverage   64.39%   65.91%   +1.51%     
==========================================
  Files         403      401       -2     
  Lines       38097    36568    -1529     
==========================================
- Hits        24534    24105     -429     
+ Misses      11913    10958     -955     
+ Partials     1650     1505     -145
Impacted Files Coverage Δ
clientv3/balancer/resolver/endpoint/endpoint.go 84.48% <100%> (+1.87%) ⬆️
clientv3/balancer/picker/err.go 100% <100%> (ø) ⬆️
clientv3/balancer/picker/roundrobin_balanced.go 100% <100%> (ø) ⬆️
auth/options.go 35% <0%> (-57.5%) ⬇️
clientv3/balancer/utils.go 53.84% <0%> (-46.16%) ⬇️
client/client.go 47.71% <0%> (-36.28%) ⬇️
pkg/transport/timeout_conn.go 60% <0%> (-20%) ⬇️
auth/jwt.go 51.68% <0%> (-16.89%) ⬇️
etcdserver/api/v3rpc/util.go 51.61% <0%> (-16.13%) ⬇️
etcdserver/api/membership/store.go 60.68% <0%> (-15.67%) ⬇️
... and 121 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3898452...4258cdd. Read the comment docs.

@jingyih
Copy link
Contributor

jingyih commented Jan 30, 2020

cc @jpbetz @gyuho

@eddycjy
Copy link
Contributor Author

eddycjy commented Feb 18, 2020

@jpbetz @gyuho @jingyih ping review.

@jpbetz
Copy link
Contributor

jpbetz commented Feb 18, 2020

Thanks @eddycjy. Would you also update the changelog (3.5 since this is on master) with a clientv3 line item about this change? Please include which grpc versions are compatible and which are incompatible (they're overlapping ranges since there was a period of backward compatibility via the typerefs).

@eddycjy
Copy link
Contributor Author

eddycjy commented Feb 19, 2020

@jpbetz Thanks for your reply and reminder, I have submitted CHANGELOG-3.5.

@eddycjy eddycjy mentioned this pull request Feb 19, 2020
@jpbetz
Copy link
Contributor

jpbetz commented Feb 19, 2020

Next time lets include the changelog update in the same PR as the main change. But I'm okay with separate for this issue since they are both already open.

LGTM

@jpbetz
Copy link
Contributor

jpbetz commented Feb 19, 2020

@gyuho WDYT?

@satyajituk
Copy link

I get the following error when I simply try to go build main.go in a program which uses "go.etcd.io/etcd/clientv3".

# github.com/coreos/etcd/clientv3/balancer/resolver/endpoint
../../../../pkg/mod/github.com/coreos/etcd@v3.3.18+incompatible/clientv3/balancer/resolver/endpoint/endpoint.go:114:78: undefined: resolver.BuildOption
../../../../pkg/mod/github.com/coreos/etcd@v3.3.18+incompatible/clientv3/balancer/resolver/endpoint/endpoint.go:182:31: undefined: resolver.ResolveNowOption
# github.com/coreos/etcd/clientv3/balancer/picker
../../../../pkg/mod/github.com/coreos/etcd@v3.3.18+incompatible/clientv3/balancer/picker/err.go:37:44: undefined: balancer.PickOptions
../../../../pkg/mod/github.com/coreos/etcd@v3.3.18+incompatible/clientv3/balancer/picker/roundrobin_balanced.go:55:54: undefined: balancer.PickOptions

Will this PR solve the issue?

Copy link

@skyjia skyjia left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ecifreo
Copy link

ecifreo commented Mar 3, 2020

Any ETA on merging this? We are hitting the go mod dependency issue.

@gdbelvin
Copy link

Any ETA on releasing 3.3.19? We are also hitting the go mod dependency issue.

@swathinsankaran
Copy link

When this merge will be released? We are also facing the same issue.

Regards,
Swathin

humblec added a commit to humblec/ceph-csi that referenced this pull request Apr 14, 2020
etcd-io/etcd#11564

Signed-off-by: Humble Chirammal <hchiramm@redhat.com>
@rcgoodfellow
Copy link

Not releasing this really screws projects that have dependencies with protobufs that have been generated by github.com/golang/protobuf/protoc-gen-go v1.3.5 or later. As the only way to get etcd co-exist with these dependencies is to directly import this commit via

go get go.etcd.io/etcd@221f0cc107cb3497eeb20fb241e1bcafca2e9115

Which then translates into

go.etcd.io/etcd v0.5.0-alpha.5.0.20200306183522-221f0cc107cb

in go.mod parlance.

Given that etcd's go modules are totally broken anyway, we'll likely still have to import commit hashes directly, but at least they'll be pointing at a tagged release that can be matched to etcd server deployment. In the absence of an actual release, every downstream project with recent protoc-gen-go dependencies is relegated to a mishmash of client/server versions that maybe works.

The common workaround noted on this issue and others of replacing grpc's module version with v1.2.6 is not viable for projects that actually depend on generated protobuf code that has elements like grpc.ClientConnInterface that are specific to later versions of protoc-gen-go.

@pipe0482
Copy link

pipe0482 commented May 4, 2020

Please please please release this fix, our whole team is completely stuck because no builds are working

@gdbelvin
Copy link

gdbelvin commented May 4, 2020

@mitake @gyuho This issue is holding up a significant portion of the Golang ecosystem updating their dependencies. Please release a version of etcd that contains this commit.

@pipe01
Copy link

pipe01 commented May 6, 2020

Workaround: add

replace google.golang.org/grpc => google.golang.org/grpc v1.26.0

to you go.mod file. It's not perfect, but it'll allow you to at least partially update

xiang90 pushed a commit that referenced this pull request May 13, 2020
* clientv3: fix grpc-go(v1.27.0) incompatible changes to balancer/resolver.

* vendor: upgrade gRPC Go to v1.24.0

Picking up some performance improvements and bug fixes.

https://github.com/grpc/grpc-go/releases/tag/v1.24.0

Signed-off-by: Gyuho Lee <leegyuho@amazon.com>

* vendor: update gRPC Go to v1.26.0 (#11522)

* GO111MODULE=on go mod vendor

* GO111MODULE=on go mod vendor go 1.14

Bump travis 2

Co-authored-by: EDDYCJY <313687982@qq.com>
Co-authored-by: Gyuho Lee <leegyuho@amazon.com>
Co-authored-by: Yuchen Zhou <yczhou@google.com>
@AFMiziara
Copy link

@rcgoodfellow, did you find any solutions for now? All go clients from googleapis relies on grpc v1.28.0. This simple fix is making impossible to use other packages because of dependencies issues.

@gdbelvin
Copy link

@AFMiziara 3.4.8 contains the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

clientv3: grpc-go (v1.27.0) made API changes to balancer / resolver.