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

ctlv2: report unhealthy in cluster-health if any node is unavailable #8070

Merged
merged 2 commits into from
Jun 9, 2017

Conversation

heyitsanthony
Copy link
Contributor

Fixes #8061 and #7032

@xiang90
Copy link
Contributor

xiang90 commented Jun 9, 2017

lgtm

fmt.Println("cluster is healthy")
} else {
case 0:
fmt.Println("cluster is unavailable")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

degraded is more descriptive?

@xiang90
Copy link
Contributor

xiang90 commented Jun 9, 2017

lgtm

@heyitsanthony heyitsanthony merged commit 933aa09 into etcd-io:master Jun 9, 2017
@heyitsanthony heyitsanthony deleted the etcdctl-cluster-health branch June 9, 2017 21:57
@codecov-io
Copy link

Codecov Report

Merging #8070 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #8070      +/-   ##
==========================================
- Coverage   76.51%   76.49%   -0.03%     
==========================================
  Files         342      342              
  Lines       26548    26549       +1     
==========================================
- Hits        20314    20309       -5     
- Misses       4774     4785      +11     
+ Partials     1460     1455       -5
Impacted Files Coverage Δ
etcdctl/ctlv2/command/cluster_health.go 11.84% <0%> (-0.16%) ⬇️
auth/simple_token.go 86.79% <0%> (-6.61%) ⬇️
clientv3/namespace/watch.go 93.93% <0%> (-6.07%) ⬇️
proxy/grpcproxy/watch.go 89.58% <0%> (-3.48%) ⬇️
etcdmain/etcd.go 45.49% <0%> (-0.86%) ⬇️
clientv3/watch.go 95.5% <0%> (-0.5%) ⬇️
etcdserver/server.go 80.39% <0%> (-0.35%) ⬇️
etcdserver/v3_server.go 71.73% <0%> (-0.24%) ⬇️
pkg/adt/interval_tree.go 88.71% <0%> (+0.3%) ⬆️
etcdserver/api/v3rpc/watch.go 93.42% <0%> (+0.93%) ⬆️
... and 5 more

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 b194276...3fcb833. Read the comment docs.

@discordianfish
Copy link
Contributor

Was this somehow reverted in a recent change? I depend on this behavior here: https://github.com/itskoko/kubecfn/blob/master/kubernetes.yaml#L1049

And cluster-health currently returns 0 even if there are unhealthy/unreachable members:

May 22 10:21:08 ip-172-20-181-238 etcd-signal-health[1894]: member f96dcaf66e42e5 is healthy: got healthy result from https://ip-172-20-182-16.ec2.internal:2379
May 22 10:21:08 ip-172-20-181-238 etcd-signal-health[1894]: member 3117c78e122ac7cb is healthy: got healthy result from https://ip-172-20-184-45.ec2.internal:2379
May 22 10:21:09 ip-172-20-181-238 etcd-signal-health[1894]: failed to check the health of member 96a7254e2e1542e1 on https://ip-172-20-180-128.ec2.internal:2379: Get https://ip-172-20-180-128.ec2.internal:2379/health: dial tcp 172.20.180.128:2379: getsockopt: no r
May 22 10:21:09 ip-172-20-181-238 etcd-signal-health[1894]: member 96a7254e2e1542e1 is unreachable: [https://ip-172-20-180-128.ec2.internal:2379] are all unreachable
May 22 10:21:09 ip-172-20-181-238 etcd-signal-health[1894]: member a2d7a63532026c28 is healthy: got healthy result from https://ip-172-20-181-238.ec2.internal:2379
May 22 10:21:09 ip-172-20-181-238 etcd-signal-health[1894]: cluster is healthy

"Container Linux by CoreOS 1688.5.3 (Rhyolite)"
etcdctl version: 3.2.15
API version: 2

discordianfish added a commit to itskoko/kubecfn that referenced this pull request May 22, 2018
According to etcd-io/etcd#8070, cluster-health
should check this but apparently this broke in some recent releases, so
we're checking this explicitly in the script now.
@gyuho
Copy link
Contributor

gyuho commented May 22, 2018

@discordianfish I see this change is only available from v3.3.0 release https://github.com/coreos/etcd/blob/v3.3.0/etcdctl/ctlv2/command/cluster_health.go? Can you try v3.3?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants