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

The endpoint commands should either work with --discovery-srv or should indicate that they ignore the flag #17886

Open
chrislalos opened this issue Apr 26, 2024 · 1 comment
Labels
area/etcdctl priority/backlog Higher priority than priority/awaiting-more-evidence. type/feature

Comments

@chrislalos
Copy link

What would you like to be added?

Many etcdctl commands, eg get, put, etc, allow for service discovery via the --discovery-srv flag, which indicates the command should lookup the endpoints via DNS SRV records.

endpoint health etc do not work this way. They ignore the flag entirely.

$ etcdctl --discovery-srv redacted-domain.dev endpoint health
{"level":"warn","ts":"2024-04-26T09:07:44.811296-0500","logger":"client","caller":"v3@v3.5.12/retry_interceptor.go:62","msg":"retrying of unary invoker failed","target":"etcd-endpoints://0x140003d0a80/127.0.0.1:2379","attempt":0,"error":"rpc error: code = DeadlineExceeded desc = latest balancer error: last connection error: connection error: desc = \"transport: Error while dialing: dial tcp 127.0.0.1:2379: connect: connection refused\""}
127.0.0.1:2379 is unhealthy: failed to commit proposal: context deadline exceeded
Error: unhealthy cluster

--discovery-srv is a global flag in etcdctl, and perfectly valid for the endpoint commands, but it is silently ignored, even though the intent of the user to use --discovery-srv is clear.

At the least, I'd like a warning that I need to explicitly specify endpoints with the --endpoints flag.

What I'd really like is for the endpoint commands to actually use--discovery-srv consistently with other commands. From looking at the code, this is non-trivial. Most commands use mustClientFromCmd() which abstract the SRV lookup so it happens transparently to the command implementation. The endpoint commands don't do this, which makes sense since they are checking the health of API endpoints, not calling the API itself. But an implementation should still be possible, and perhaps it wouldn't be too challenging.

Why is this needed?

Because ... Consistency is Nice

@chrislalos
Copy link
Author

Btw I closed this issue mistakenly -- I noticed I'd posted to the etcd repo, which I thought was a mistake and that it belonged in an etcdctl specific repo. But it appears everything's under one repo. Sorry for any confusion!

@jmhbnz jmhbnz added area/etcdctl priority/backlog Higher priority than priority/awaiting-more-evidence. labels May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/etcdctl priority/backlog Higher priority than priority/awaiting-more-evidence. type/feature
Development

No branches or pull requests

2 participants