-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
etcdctl: use cluster endpoints when passed --cluster #8143
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm. thanks
etcdctl/README.md
Outdated
@@ -565,6 +565,10 @@ Prints a humanized table of the member IDs, statuses, names, peer addresses, and | |||
|
|||
ENDPOINT provides commands for querying individual endpoints. | |||
|
|||
#### Options | |||
|
|||
- cluster -- fetch and use all endpoints from the etcd cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from the etcd cluster membership?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also suffers from the same issue as the previous auto-sync
option. if users have a proxy (not necessarily etcd proxy or etcd gPRC proxy) in front of etcd, and they can only access the cluster though the proxy, then cluster option will try to bypass the proxy. this is not the desired behavior. we probably need to warn about that here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It says it's fetching it from the cluster though; if there's a pass-through proxy then the cluster wouldn't know about it. Would "fetch and use all endpoints from the etcd cluster member list" be OK?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok. let us make it this way. if users have issue, we can improve. just something we should keep an eye on.
etcdctl/ctlv3/command/ep_command.go
Outdated
}() | ||
membs, err := c.MemberList(ctx) | ||
if err != nil { | ||
ExitWithError(ExitError, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
have you checked what the error looks like?
can we provide more context here: "failed to get all endpoints from etcd cluster membership: %v"
lgtm except some document improvement suggestions. |
Queries the cluster for endpoints to use for the endpoint commands. Fixes etcd-io#8117
c6d7673
to
1a2be43
Compare
Query the cluster for endpoints when given --cluster for the endpoint commands.
Fixes #8117