From f499c6b9ec960592f6e89f5d0d759dfac623d066 Mon Sep 17 00:00:00 2001 From: David Wang <00107082@163.com> Date: Thu, 21 Jul 2022 22:14:07 +0800 Subject: [PATCH] Test: wait for network changes in TestNetworkDBNodeJoinLeaveIteration In network node change test, the expected behavior is focused on how many nodes left in networkDB, besides timing issues, things would also go tricky for a leave-then-join sequence, if the check (counting the nodes) happened before the first "leave" event, then the testcase actually miss its target and report PASS without verifying its final result; if the check happened after the 'leave' event, but before the 'join' event, the test would report FAIL unnecessary; This code change would check both the db changes and the node count, it would report PASS only when networkdb has indeed changed and the node count is expected. Signed-off-by: David Wang <00107082@163.com> --- libnetwork/networkdb/networkdb_test.go | 59 ++++++++++++++++++-------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/libnetwork/networkdb/networkdb_test.go b/libnetwork/networkdb/networkdb_test.go index 5033d2bb8a..c490a47f29 100644 --- a/libnetwork/networkdb/networkdb_test.go +++ b/libnetwork/networkdb/networkdb_test.go @@ -13,6 +13,7 @@ import ( "github.com/docker/docker/pkg/stringid" "github.com/docker/go-events" "github.com/hashicorp/memberlist" + "github.com/hashicorp/serf/serf" "github.com/sirupsen/logrus" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" @@ -479,22 +480,47 @@ func TestNetworkDBCRUDMediumCluster(t *testing.T) { func TestNetworkDBNodeJoinLeaveIteration(t *testing.T) { dbs := createNetworkDBInstances(t, 2, "node", DefaultConfig()) + var ( + dbIndex int32 + staleNetworkTime [2]serf.LamportTime + expectNodeCount int + network = "network1" + ) + dbChangeWitness := func(t poll.LogT) poll.Result { + db := dbs[dbIndex] + networkTime := db.networkClock.Time() + if networkTime <= staleNetworkTime[dbIndex] { + return poll.Continue("network time is stale, no change registered yet.") + } + count := -1 + db.Lock() + if nodes, ok := db.networkNodes[network]; ok { + count = len(nodes) + } + db.Unlock() + if count != expectNodeCount { + return poll.Continue("current number of nodes is %d, expect %d.", count, expectNodeCount) + } + return poll.Success() + } + // Single node Join/Leave + staleNetworkTime[0], staleNetworkTime[1] = dbs[0].networkClock.Time(), dbs[1].networkClock.Time() err := dbs[0].JoinNetwork("network1") assert.NilError(t, err) - if len(dbs[0].networkNodes["network1"]) != 1 { - t.Fatalf("The networkNodes list has to have be 1 instead of %d", len(dbs[0].networkNodes["network1"])) - } + dbIndex, expectNodeCount = 0, 1 + poll.WaitOn(t, dbChangeWitness, poll.WithTimeout(3*time.Second), poll.WithDelay(5*time.Millisecond)) + staleNetworkTime[0], staleNetworkTime[1] = dbs[0].networkClock.Time(), dbs[1].networkClock.Time() err = dbs[0].LeaveNetwork("network1") assert.NilError(t, err) - if len(dbs[0].networkNodes["network1"]) != 0 { - t.Fatalf("The networkNodes list has to have be 0 instead of %d", len(dbs[0].networkNodes["network1"])) - } + dbIndex, expectNodeCount = 0, 0 + poll.WaitOn(t, dbChangeWitness, poll.WithTimeout(3*time.Second), poll.WithDelay(5*time.Millisecond)) // Multiple nodes Join/Leave + staleNetworkTime[0], staleNetworkTime[1] = dbs[0].networkClock.Time(), dbs[1].networkClock.Time() err = dbs[0].JoinNetwork("network1") assert.NilError(t, err) @@ -503,37 +529,34 @@ func TestNetworkDBNodeJoinLeaveIteration(t *testing.T) { // Wait for the propagation on db[0] dbs[0].verifyNetworkExistence(t, dbs[1].config.NodeID, "network1", true) - if len(dbs[0].networkNodes["network1"]) != 2 { - t.Fatalf("The networkNodes list has to have be 2 instead of %d - %v", len(dbs[0].networkNodes["network1"]), dbs[0].networkNodes["network1"]) - } + dbIndex, expectNodeCount = 0, 2 + poll.WaitOn(t, dbChangeWitness, poll.WithTimeout(3*time.Second), poll.WithDelay(5*time.Millisecond)) if n, ok := dbs[0].networks[dbs[0].config.NodeID]["network1"]; !ok || n.leaving { t.Fatalf("The network should not be marked as leaving:%t", n.leaving) } // Wait for the propagation on db[1] dbs[1].verifyNetworkExistence(t, dbs[0].config.NodeID, "network1", true) - if len(dbs[1].networkNodes["network1"]) != 2 { - t.Fatalf("The networkNodes list has to have be 2 instead of %d - %v", len(dbs[1].networkNodes["network1"]), dbs[1].networkNodes["network1"]) - } + dbIndex, expectNodeCount = 1, 2 + poll.WaitOn(t, dbChangeWitness, poll.WithTimeout(3*time.Second), poll.WithDelay(5*time.Millisecond)) if n, ok := dbs[1].networks[dbs[1].config.NodeID]["network1"]; !ok || n.leaving { t.Fatalf("The network should not be marked as leaving:%t", n.leaving) } // Try a quick leave/join + staleNetworkTime[0], staleNetworkTime[1] = dbs[0].networkClock.Time(), dbs[1].networkClock.Time() err = dbs[0].LeaveNetwork("network1") assert.NilError(t, err) err = dbs[0].JoinNetwork("network1") assert.NilError(t, err) dbs[0].verifyNetworkExistence(t, dbs[1].config.NodeID, "network1", true) - if len(dbs[0].networkNodes["network1"]) != 2 { - t.Fatalf("The networkNodes list has to have be 2 instead of %d - %v", len(dbs[0].networkNodes["network1"]), dbs[0].networkNodes["network1"]) - } + dbIndex, expectNodeCount = 0, 2 + poll.WaitOn(t, dbChangeWitness, poll.WithTimeout(3*time.Second), poll.WithDelay(5*time.Millisecond)) dbs[1].verifyNetworkExistence(t, dbs[0].config.NodeID, "network1", true) - if len(dbs[1].networkNodes["network1"]) != 2 { - t.Fatalf("The networkNodes list has to have be 2 instead of %d - %v", len(dbs[1].networkNodes["network1"]), dbs[1].networkNodes["network1"]) - } + dbIndex, expectNodeCount = 1, 2 + poll.WaitOn(t, dbChangeWitness, poll.WithTimeout(3*time.Second), poll.WithDelay(5*time.Millisecond)) closeNetworkDBInstances(t, dbs) }