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

etcdctl: don't ask password twice for etcdctl endpoint health --cluster #9136

Merged
merged 1 commit into from
Jan 12, 2018

Conversation

mitake
Copy link
Contributor

@mitake mitake commented Jan 12, 2018

Current etcdctl endpoint health --cluster asks password twice if auth
is enabled. This is because the command creates two client instances:
one for the purpose of checking endpoint health and another for
getting cluster members with MemberList(). The latter client doesn't
need to be authenticated because MemberList() is a public RPC. This
commit makes the client with no authed one.

Fix #9094

/cc @lyddragon @gyuho @hexfusion

Current etcdctl endpoint health --cluster asks password twice if auth
is enabled. This is because the command creates two client instances:
one for the purpose of checking endpoint health and another for
getting cluster members with MemberList(). The latter client doesn't
need to be authenticated because MemberList() is a public RPC. This
commit makes the client with no authed one.

Fix etcd-io#9094
@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@c5532eb). Click here to learn what that means.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##             master   #9136   +/-   ##
========================================
  Coverage          ?   75.9%           
========================================
  Files             ?     359           
  Lines             ?   30004           
  Branches          ?       0           
========================================
  Hits              ?   22775           
  Misses            ?    5641           
  Partials          ?    1588
Impacted Files Coverage Δ
etcdctl/ctlv3/command/ep_command.go 60.74% <0%> (ø)

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 c5532eb...b337674. Read the comment docs.

@gyuho
Copy link
Contributor

gyuho commented Jan 12, 2018

Confirmed that it fixes:

$ ETCDCTL_API=3 ./bin/etcdctl endpoint health --cluster
http://127.0.0.1:22379 is unhealthy: failed to commit proposal: etcdserver: user name is empty
http://127.0.0.1:2379 is unhealthy: failed to commit proposal: etcdserver: user name is empty
http://127.0.0.1:32379 is unhealthy: failed to commit proposal: etcdserver: user name is empty
Error: unhealthy cluster

$ ETCDCTL_API=3 ./bin/etcdctl --user root:123 endpoint health --cluster
http://127.0.0.1:22379 is healthy: successfully committed proposal: took = 1.135362ms
http://127.0.0.1:32379 is healthy: successfully committed proposal: took = 1.573943ms
http://127.0.0.1:2379 is healthy: successfully committed proposal: took = 1.199876ms

@gyuho
Copy link
Contributor

gyuho commented Jan 12, 2018

Thanks @mitake !

@gyuho gyuho merged commit 6f7eda3 into etcd-io:master Jan 12, 2018
@mitake mitake deleted the fix-9094 branch January 13, 2018 01:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants