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

e2e: add cluster version test with rolling start servers #11287

Merged
merged 1 commit into from
Nov 26, 2019

Conversation

YoyinZyc
Copy link
Contributor

In #11282, we added a test for cluster version check. However, the cluster in test environment will start servers at the same time so that this test will not cover cluster version compatibility check.
In this pr, the added test will start servers one by one and then verify the cluster version.
@wenjiaswe @jingyih Please have a look.

@YoyinZyc YoyinZyc changed the title e2e: add cluster version test for rolling start servers e2e: add cluster version test with rolling start servers Oct 22, 2019
@codecov-io
Copy link

codecov-io commented Oct 22, 2019

Codecov Report

Merging #11287 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11287      +/-   ##
==========================================
- Coverage   64.65%   64.65%   -0.01%     
==========================================
  Files         403      403              
  Lines       37956    37956              
==========================================
- Hits        24542    24541       -1     
- Misses      11777    11783       +6     
+ Partials     1637     1632       -5
Impacted Files Coverage Δ
client/client.go 65.03% <0%> (-18.96%) ⬇️
etcdserver/api/v3rpc/lease.go 69.31% <0%> (-5.69%) ⬇️
proxy/grpcproxy/watcher.go 89.79% <0%> (-4.09%) ⬇️
pkg/testutil/recorder.go 77.77% <0%> (-3.71%) ⬇️
clientv3/op.go 72.5% <0%> (-1.25%) ⬇️
clientv3/balancer/resolver/endpoint/endpoint.go 81.73% <0%> (-0.87%) ⬇️
etcdserver/server.go 68.59% <0%> (+0.33%) ⬆️
clientv3/watch.go 92% <0%> (+0.42%) ⬆️
clientv3/leasing/cache.go 87.77% <0%> (+0.55%) ⬆️
clientv3/leasing/kv.go 90.36% <0%> (+0.66%) ⬆️
... and 17 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 b5afbdd...4243ebd. Read the comment docs.

@jingyih
Copy link
Contributor

jingyih commented Oct 23, 2019

For context, this is needed in order to exercise the following code path where in a 3-node cluster, 2 nodes are up and therefore has a leader but the cluster version is undecided because the info of the 3rd node is unknown.

etcd/etcdserver/server.go

Lines 2463 to 2467 in b5afbdd

if s.Leader() != s.ID() {
continue
}
v := decideClusterVersion(s.getLogger(), getVersions(s.getLogger(), s.cluster, s.id, s.peerRt))

Copy link
Contributor

@jingyih jingyih left a comment

Choose a reason for hiding this comment

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

LGTM

for i := range epc.procs {
go func(n int) { readyC <- f(epc.procs[n]) }(i)
// make sure the servers do not start at the same time
time.Sleep(time.Second)
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we should avoid using Sleep() for timing, but I do not have a better idea.

@jingyih jingyih merged commit 419ffb7 into etcd-io:master Nov 26, 2019
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.

3 participants