etcd not compatible with grpc v1.30.0

[xiegeo]

To reproduce:

$ go get google.golang.org/grpc@latest
go: google.golang.org/grpc latest => v1.30.0
$ go mod tidy
...
go.etcd.io/etcd/v3/clientv3/naming imports
        google.golang.org/grpc/naming: module google.golang.org/grpc@latest found (v1.30.0), but does not contain package google.golang.org/grpc/naming

This is a problem for dependents of etcd who want to use the latest grpc.

The naming package was removed at https://github.com/grpc/grpc-go/pull/3314 and was deprecated in favour of resolver.

I can work on a pull request if that's helpful.

[cfc4n]

I think rootcause is like https://github.com/etcd-io/etcd/issues/12068 , there is no go.mod in release-3.3 branch. go mod will download latest version of dependent package.

[ptabor]

As a workaround, you can pin it to 1.29.1:

require (
google.golang.org/grpc v1.29.1
)

But we should migrate out of the deprecated naming namespace.
(and in theory grpc should not deprecate within the same major version).

[junhwong]

go.mod
replace google.golang.org/grpc v1.30.0 => google.golang.org/grpc v1.26.0

[xiegeo]

I don't need a work around, I want to use grpc v1.30.0 to keep up to date.

[xiegeo]

After a bit of investigation. It looks like upgrading (switch from grpc/naming to grpc/resolver) isn't possible without breaking compatibility.

clientv3/naming has public function signatures that depend on grpc/naming. A migration, which needs to be complete eventually requires 2 steps:

1. Create a new package clientv3/resolver and depreciate clientv3/naming, this allows usage of grpc up to version 1.29.
2. Eventually, remove clientv3/naming, allowing usage of grpc v1.30 and up.

Alternatively, we can copy grpc/naming over to this project or its own namespace and rewrite imports. This still produces a breaking change but is much easier for users to adopt. Since grpc/naming should not be used anymore, switching to resolver is still the goal, but this allow keeping clientv3/naming and update grpc at the same time.

[xiegeo]

I have good news and bad news:

I did a little experimentation. Adding replace google.golang.org/grpc/naming => github.com/xiegeo/grpc-naming v1.29.1-alpha to go.mod file allows uploading grpc to v1.30.0 without any coding changes.

But another part of etcd is exposed as incompatible:

# go.etcd.io/etcd/v3/clientv3/balancer/picker
..\..\..\..\pkg\mod\go.etcd.io\etcd\[email protected]\clientv3\balancer\picker\err.go:25:9: cannot use &errPicker literal (type *errPicker) as type Picker in return argument:
        *errPicker does not implement Picker (wrong type for Pick method)
                have Pick(context.Context, balancer.PickInfo) (balancer.SubConn, func(balancer.DoneInfo), error)
                want Pick(balancer.PickInfo) (balancer.PickResult, error)
..\..\..\..\pkg\mod\go.etcd.io\etcd\[email protected]\clientv3\balancer\picker\roundrobin_balanced.go:33:9: cannot use &rrBalanced literal (type *rrBalanced) as type Picker in return argument:
        *rrBalanced does not implement Picker (wrong type for Pick method)
                have Pick(context.Context, balancer.PickInfo) (balancer.SubConn, func(balancer.DoneInfo), error)
                want Pick(balancer.PickInfo) (balancer.PickResult, error)

This is because of this change: https://github.com/grpc/grpc-go/commit/4eb418e5b2e10614c9638bc7748fb524636e5c4a

The grpc maintiners went through the trouble of creating a V2Picker, then broke semantic versioning intentionally.

I think the best option is to bite the bullet and upgrade with the incompatible changes, we can do this while adding go.mod which already changes the module path.

[zerospiel]

@xiegeo Hi there, did you migrate proxy.grpcproxy package to your own grpc.resolver implementation? I've had some issues in getting SCs for proxy.grpcproxy.clusterProxy struct, so I just dropped the whole implementation.

In case you stacked with clientv3.balancer, let me know, it's fairy simple to migrate to new interfaces as of [email protected].

[xiegeo]

@zerospiel Nope, still waiting for the etcd core maintainers to decide what to keep and what to drop at each stage.

[nikhita]

We (Kubernetes) would also like to see etcd updated to use the latest grpc. Ideally, use [email protected] when it's released to include https://github.com/grpc/grpc-go/issues/2628 (needed for https://github.com/kubernetes/kubernetes/issues/93320).

@gyuho :wave: do you have suggestions on which of the above approaches to take here?

[zerospiel]

If anyone in a hurry to update to [email protected] and don't use service discovery using grpcproxy package — here is a fork https://github.com/ozonru/etcd/releases/tag/v3.3.0-rc.0-grpc1.30.0 where I just removed implementation for grpcproxy (due to undefined decision of how to implement it using grpc/resolver)

In go.mod: require github.com/ozonru/etcd/v3 v3.3.0-rc.0-grpc1.30.0

[pracucci]

I've attempted an upgrade in this draft PR https://github.com/etcd-io/etcd/pull/12155 but I'm having an issue due to the protobuf upgrade too. I didn't have much time to investigate it, but a second pair of 👀 would be very appreciated.

[nikhita]

@gyuho :wave: can you take a look?

[gyuho]

Hi,

Upgrading gRPC has always been painful where we had to basically rewrite client library. I will take a look to estimate how much work we need to upgrade gRPC and post updates here.

I am currently busy with other tasks.
In the meantime, did this workaround work? Does Kubernetes need new gRPC updates?

As a workaround, you can pin it to 1.29.1:

require (
google.golang.org/grpc v1.29.1
)

But we should migrate out of the deprecated naming namespace.
(and in theory grpc should not deprecate within the same major version).

/cc @jpbetz

[gyuho]

I am afraid if this requires us to rewrite our client third time just to make it compatible with gRPC :/

I will be working on some design doc about our long term plan. I would like to decouple clientv3 from gRPC as much as possible.

[dfawley]

I am afraid if this requires us to rewrite our client third time just to make it compatible with gRPC :/

I would recommend not using gRPC APIs explicitly marked as experimental in the documentation. This will avoid any potential future breakages. If you are missing functionality you need from our stable APIs, please reach out to us by filing an issue, and we will work with you to find a solution.

[gyuho]

@dfawley Thanks. Yes, going forward, we will rely less on the experimental APIs. Will definitely reach out to the gRPC team.

[sebito91]

Is it possible to update the etcd deps to support grpc using v1.31.0? There is a fix to the logger subsystem there that prevents extraneous logging by tools like loopyWriter:

https://github.com/grpc/grpc-go/commit/506b7730668b5a13465224b0d8133f974a3f843d

[derekperkins]

Is there a timeline for the 3.5 milestone and whatever fix ends up getting committed?

[xiang90]

@dfawley

I would recommend not using gRPC APIs explicitly marked as experimental in the documentation. This will avoid any potential future breakages. If you are missing functionality you need from our stable APIs, please reach out to us by filing an issue, and we will work with you to find a solution.

Hmm... What gRPC-go did was to replace an experimental API with another experimental API (see https://pkg.go.dev/google.golang.org/[email protected]/resolver) with no migration path.

@gyuho Can we simply remove the gRPC naming support in etcd latest? I see no points on supporting it if gRPC-go cannot figure out its naming store in and keep it experimental for 4 years.

The users of etcd/naming package also depends on gRPC/naming. So it is OK just to remove it as we upgrade gRPC.

[dfawley]

Hmm... What gRPC-go did was to replace an experimental API with another experimental API (see pkg.go.dev/google.golang.org/[email protected]/resolver) with no migration path.

gRPC did provide a migration path. From 1.26 (Dec 18) until 1.30 (Mar 21), both the old and new APIs were available. See https://github.com/grpc/grpc-go/issues/3180 for details, which was pinned in our repo for several months.

[xiang90]

that is a notice. not a document on how to migrate or a tool to do the auto migration. Either way we shall just remove the etcd backend naming support for grpc since it is removed in grpc itself.

[skyao]

I'm trying to update grpc version in etcd to v1.32.0.

For naming resolver, it is hard to change all the code in etcd packages (naming and grpcproxy) from grpc naming to grpc resolver. A simple way is to copy the grpc naming package (in fact just one file naming.go containing 200 lines code) into etcd naming package.

For loadbalancer, there are some small breaking api changes (easy to fix) and one BIG change in Balancer interface...

Is there any plan to update to new grpc version? Or is somebody also trying to do similar work?

[ptabor]

I have PR (https://github.com/etcd-io/etcd/pull/12397) ready that refreshes protos as much as possible in 3.5:

    github.com/gogo/protobuf                        v1.2.1,v1.0.0 -> v1.3.1
    github.com/golang/protobuf                             v1.3.2 -> v1.3.5
    github.com/grpc-ecosystem/grpc-gateway  v1.9.5,v1.4.1,v1.15.2 -> v1.14.6
    google.golang.org/grpc                                v1.26.0 -> v1.29.1

It does not solve this very problem, but reduces risk due to a version skew.

[gyuho]

Either way we shall just remove the etcd backend naming support for grpc since it is removed in grpc itself.

@ptabor As we migrate clientv3 as a separate module, can we see if we can remove naming package that had breaking changes?

[skyao]

@gyuho I have a PR submit to update grpc-go version to v1.32.0. Please help to review.

we can't simply remove etcd naming package because it's used by etcd grpcproxy package.

[ptabor]

@gyuho I have a PR submit to update grpc-go version to v1.32.0. Please help to review.

we can't simply remove etcd naming package because it's used by etcd grpcproxy package.

I will take a look.

I wonder how more complicated is proper change of grpcproxy code to make it depend on grpc/resolver packages.
Do you have any unfinished prototype to compare ? High level cloning of deprecated 'naming.go' sounds like caring over the debt, while migration is paying it.

git grep '(grpc|client)/naming' shows that the usage is very 'local':

clientv3/naming/doc.go://               "google.golang.org/grpc/naming"
clientv3/naming/grpc.go:        "google.golang.org/grpc/naming"
clientv3/naming/grpc.go://      "google.golang.org/grpc/naming"

proxy/grpcproxy/cluster.go:     gnaming "google.golang.org/grpc/naming"
proxy/grpcproxy/cluster.go:  naming "google.golang.org/grpc/naming"
proxy/grpcproxy/cluster.go:     "go.etcd.io/etcd/v3/clientv3/naming"
proxy/grpcproxy/register.go:    gnaming "google.golang.org/grpc/naming"
proxy/grpcproxy/register.go:    "go.etcd.io/etcd/v3/clientv3/naming"

tests/integration/clientv3/grpc_test.go:        "google.golang.org/grpc/naming"
tests/integration/proxy/grpcproxy/register_test.go:     gnaming "google.golang.org/grpc/naming"

[skyao]

etcd naming package is public, I don't know if some project ( out of ectd project) will use it or not.

Removing a public API is not a good idea.

[ptabor]

1. In 3.5 we will release this on a new package names:
   go.etcd.io/etcd/client/v3 instead of go.etcd.io/etcd/clientv3
   so either way it will be opt-in change for customers migrating to 3.5.

2. customer that depended on google.golang.org/grpc/naming.Update
   will need to change their code do use go.etcd.io/etcd/client/v3/naming.Update
   (as this is struct, not interface), so either way it will be a change requiring a code adaptation.

[skyao]

One more detail, in etcd we use the struct of grpc naming.Udpate to do json marshal/unmarshal.

func (gr *GRPCResolver) Update(ctx context.Context, target string, nm gnaming.Update, opts ...etcd.OpOption) (err error) {
    switch nm.Op {
    case gnaming.Add:
        var v []byte
        if v, err = json.Marshal(nm); err != nil {
            return status.Error(codes.InvalidArgument, err.Error())
        }
        _, err = gr.Client.KV.Put(ctx, target+"/"+nm.Addr, string(v), opts...)
......
}

and:

    updates := make([]*gnaming.Update, 0, len(resp.Kvs))
    for _, kv := range resp.Kvs {
        var jupdate gnaming.Update
        if err := json.Unmarshal(kv.Value, &jupdate); err != nil {
            continue
        }
    }

So there are some data saved in etcd using the json format as naming.Update. Can we simply changed it (means all the data invalid after version updated)?

[ptabor]

Ouch. I assume we need to preserve the json's...
In fact I see you copy in your PR only naming.go (interfaces + structs) and not the implementation, so its not as bad as I expected from the description.

[trusch]

Is there a timeline for the 3.5 release and is it safe to assume that this will be fixed then? I'm currently building a simple gRPC server which happens to also talk to and etcd server. I think I would have to downgrade protoc-gen-grpc-go + the pin on grpc 1.29.1 :-(

[maja42]

I have multiple projects using gRPC, including one that uses grpc-gateway. Since I also use etcd-client, I needed to downgrade all of them, including some protoc compilers to stay compatible.
I really hope this will be fixed soon, because it's a lot of overheard to find and use compatible versions and debug issues resulting from it.
This is a ticking timebomb...

[bufdev]

A solution would be for the grpc-go team to:

• Revert the breaking of relevant experimental elements, and provide a backwards-compatible solution, so this very important dependency can move forward, if nothing else then through aliases.
• Put all experimental APIs in the future under an experimental package, to make it clear that the APIs are experimental by Golang API standards.

The grpc-go project is v1.0, and people (understandably) use features from it without checking the Godoc if something is experimental or not, as the general expectation in the Golang community is that a SemVer tag of v1.0+ means the Golang API is stable. Additionally, there have been things in grpc-go that have been experimental for years that the community comes to rely on (for example, interceptors were marked as experimental for a number of years, even though github.com/grpc-ecosystem had code that relied on them). Notices in a GitHub issue such as https://github.com/grpc/grpc-go/issues/3180 aren't sufficient by Golang standards to denote a Golang API breakage, especially in such a core, practically standard-library project as grpc-go.

Perhaps the etcd team can work with the gRPC team to make sure this can happen? This seems to be breaking a significant number of people.

[dfawley]

A solution would be for the grpc-go team to:

We have no plans to change our development practices. We require the ability to experiment with new APIs and change them over time. There is no feasible way to restrict these to a separate package or module. We very clearly label our experimental APIs and our versioning policy.

The interceptors you reference are actually a success story: they were recently marked stable upon a request from a user. If you have something you want to use, and it is marked experimental, and you cannot tolerate breakages from upgrades, then please reach out to us in our repo and we will work with you to find an alternative or stabilize the necessary API. Your use case could help us determine whether the current version of the API is acceptable or needs work.

The solution to this issue is for etcd to stop using grpc APIs that are explicitly marked experimental.

[bufdev]

Almost every Golang project in the industry does not consider Godocs to be clear labeling, and considers the only versioning policy to be SemVer.

A GitHub issue giving three months notice to delete a Godoc-labeled-experimental package, along with a markdown document explaining a different versioning policy, would not be considered acceptable for any other org in the industry - moreover, no other org does this.

If you need to experiment, you're more than able to with clear labeling (such as an experimental package) that doesn't cause non-experimental packages to break SemVer, and cause so much downstream pain for users of such a key library.

The grpc-go team not abiding by this has caused lots of pain in the Golang community - I think this needs to be re-considered.

[maja42]

I think there are two issues here and you are arguing about the wrong one.
While the development process of grpc-go might or might not be good for the community, that issue should be discussed on it's own on their github page.
Their current approach certainly caused confusion down the line and the current mess for people trying to use both grpc and etcd.

What should be discussed here, though, is a plan on how to get out of the current situation and if there is a timeline people can expect.
I already considered using a different store than etcd due to this issue and will probably do so once I get in a situation where I need libraries that are inherently unusable due to grpc versioning. I wouldn't be surprised if this issue already drove off others.

[jan-xyz]

On @maja42's note: In our org everyone is moving to the newer version of protobuf and gRPC except the components which rely on etcd. It is really painful from a maintenance perspective to have snowflake services which must be treated differently from others and cannot use newer features or bug fixes.

[fpcr]

golang current package management is worse than c++ in the 1990s. What a mess.

[vangent]

+1 to what @maja42 said. Is there a plan and timeline for how to get out of the current situation? Lots of people naturally want to move to modern versions of grpc and anything that depends on later versions of grpc, and are currently blocked.

[UnderTreeTech]

Any updates?

[breathbath]

sorry guys does anyone know an alternative etcd client? we use some basic CRUD methods of etcd client and we don't need to import huge library which transitively fetches whole etcd code (+server). Maybe there is a way to call GRPC methods with a regular client?

[zerospiel]

@breathbath Hey Andrey, as I mentioned before, you can use this as a temporary decision, we use it in a pretty big production since July, and (so far) everything is okay

[breathbath]

@zerospiel thanks for the hint we will try it

[vooon]

@zerospiel i tried to use your fork trough replace, but not succeeded. go get -t -u ./... gives an error.

replace go.etcd.io/etcd/v3 => github.com/ozonru/etcd/v3 v3.3.0-rc.0-grpc1.30.0

So i reverted back to old PR:

// etcd issue: https://github.com/etcd-io/etcd/issues/12124
// PR: https://github.com/etcd-io/etcd/pull/12155
replace go.etcd.io/etcd/v3 => github.com/pracucci/etcd/v3 v3.3.0-rc.0.0.20200720164748-650087d78168