Update the discovery service protocol for v3 discovery #583
Conversation
ahrtr
force-pushed
the
v3_discovery
branch
2 times, most recently
from
5b101ff to
9cbfd3b
Compare
|
@spzala Thanks for you comments, which make sense to me! All resolved. |
| @@ -33,85 +36,72 @@ UUID=$(uuidgen) | |||
| The discovery token expects a cluster size that must be specified. The size is used by the discovery service to know when it has found all members that will initially form the cluster. | |||
|
|
|||
| ``` | |||
| curl -X PUT http://example.com/v2/keys/_etcd/registry/${UUID}/_config/size -d value=${cluster_size} | |||
| etcdctl --endpoints=http://example.com:2379 put /_etcd/registry/${UUID}/_config/size ${cluster_size} | |||
There was a problem hiding this comment.
Not sure we can switch the curl to etcdctl as it is not obvious how developer would reimplement it. V2 discovery protocol was based on V2 api which was simple REST api that could be called via curl, showcasing how to implement it. Problem is that V3 api is using grpc which is binary protocol, so it's no longer obvious how we should explain it.
Could we maybe provide link to grpc proto definition and list which methods need to be implemented. It would be also great if we could provide examples by using client similar to curl however one native to grpc.
There was a problem hiding this comment.
The developers just need to follow the guide if they want to bootstrap a new cluster using v3 discovery. Probably we should add a simple example (as a new section) to help developers to understand it.
With regard to migrating from v2 discovery to v3 discovery, previously @ptabor has a comment . Both v2 & v3 discovery are only useful at the very first bootstrapping. Once etcd members have local data, they do not depend on the discovery service any more. So it doesn't make much sense to migrate the previous v2 discovery records?
We don't know the real data on how many users deploy the v2 discovery in their production environment. Most likely the use cases are very few. So users can manually perform the migration if they want.
What do you think?
There was a problem hiding this comment.
The developers just need to follow the guide if they want to bootstrap a new cluster using v3 discovery. Probably we should add a simple example (as a new section) to help developers to understand it.
My understanding of this documentation is not for developer to learn it, but to be able to implement their service serving discover without deploying full etcd. For example https://github.com/etcd-io/discoveryserver. I would expect that this should be cough when we were migrating tests from V2 to V3, but it didn't.
With regard to migrating from v2 discovery to v3 discovery, previously @ptabor has a comment . Both v2 & v3 discovery are only useful at the very first bootstrapping. Once etcd members have local data, they do not depend on the discovery service any more. So it doesn't make much sense to migrate the previous v2 discovery records?
I think so.
We don't know the real data on how many users deploy the v2 discovery in their production environment. Most likely the use cases are very few. So users can manually perform the migration if they want.
I think we could look into metrics of https://github.com/etcd-io/discoveryserver which is the public etcd service discovery maintained by etcd project.
What do you think?
Feels like we should also consider implementing V3 discovery in https://github.com/etcd-io/discoveryserver. However I would be worried to change and deploy a project that haven't seen much updates for last couple of years.
TODO:
- Look at usage metrics of discoveryserver
- Decide if we want to implement V3 to discoveryserver
- Add tests to etcd that use discoveryserver, to make sure we don't break things in future
There was a problem hiding this comment.
My understanding of this documentation is not for developer to learn it, but to be able to implement their service serving discover without deploying full etcd. For example https://github.com/etcd-io/discoveryserver. I would expect that this should be cough when we were migrating tests from V2 to V3, but it didn't.
Not really per my understanding. The discovery.etcd.io or discoveryserver is just a router, which routes the client requests to backend etcd cluster. Please note that users do not necessarily to use the public discovery API, they can just deploy an etcd cluster as the discovery service themselves.
I think we could look into metrics of https://github.com/etcd-io/discoveryserver which is the public etcd service discovery maintained by etcd project.
This is a good point. But just as I mentioned above, users do not necessarily use the public discovery API. But anyway, we can take a look at the metrics firstly. Do you or @ptabor know the info on the backend etcd server supporting the public discovery service?
- Decide if we want to implement V3 to discoveryserver
I don't think we should implement a public discovery service for V3 discovery. But it's open to discussion.
There was a problem hiding this comment.
I read some discoveryserver documentation, you are right it's just a router to etcd server underneath. Question should we also implement it to support v3 discovery to avoid removing ability for users to use public discovery service in v3.6?
There was a problem hiding this comment.
A couple of comments:
- There are already several e2e cases to cover the v2 discovery. We can remove them in 3.7 or 3.8 when clientv2 is completely removed.
- With regard to the question whether should we support public v3 discovery service, we can discuss it in separate session after we investigate the existing data in pubic v2 discovery service. Personally I don't think it's a priority for now.
- I will try to add a new example section into the doc in a separate PR.
This PR should be good.
|
Can we get this PR merged so that I can go ahead to submit a separate PR to add an example on how to use v3 discovery? |
|
@ahrtr sounds good to me to address the comments in a separate PR. Thanks! cc @serathius |
|
@spzala @serathius @ptabor Can we get this PR merged? thx |
@ahrtr yes, your approach sounds good to me. Thanks! |
Thanks @spzala . I have no permission on this repo ( I will enhance the doc in separate PRs. Note that we also need to update https://etcd.io/docs/v3.5/op-guide/clustering/#discovery. |
@ahrtr sounds good. Will be working on your permission as well. Thanks! |
The related issue is issues/13624, and PR is pull/13635
cc @philips @lucperkins @serathius @ptabor @spzala