Skip to content

Commit 96707c5

Browse files
committed
ci: fix TestNomad_BootstrapExpect_NonVoter test
PR #12130 refactored the test to use the `wantPeers` helper, but this function only returns the number of voting peers, which in this test should be equal to 2. I think the tests were passing back them because of a bug in Raft (hashicorp/raft#483) where a non-voting server was able to transition to candidate state. One possible evidence of this is that a successful test run would have the following log line: ``` [email protected]/raft.go:1058: nomad.raft: updating configuration: command=AddVoter server-id=127.0.0.1:9101 server-addr=127.0.0.1:9101 servers="[{Suffrage:Voter ID:127.0.0.1:9107 Address:127.0.0.1:9107} {Suffrage:Voter ID:127.0.0.1:9105 Address:127.0.0.1:9105} {Suffrage:Voter ID:127.0.0.1:9103 Address:127.0.0.1:9103} {Suffrage:Voter ID:127.0.0.1:9101 Address:127.0.0.1:9101}]" ``` This commit reverts the test logic to check for peer count, regardless of voting status.
1 parent 13bc6d6 commit 96707c5

File tree

1 file changed

+25
-7
lines changed

1 file changed

+25
-7
lines changed

nomad/serf_test.go

+25-7
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/hashicorp/nomad/testutil"
1212
"github.com/hashicorp/raft"
1313
"github.com/hashicorp/serf/serf"
14-
"github.com/hashicorp/serf/testutil/retry"
1514
"github.com/stretchr/testify/require"
1615
)
1716

@@ -327,16 +326,35 @@ func TestNomad_BootstrapExpect_NonVoter(t *testing.T) {
327326
})
328327
defer cleanupS4()
329328

329+
servers := []*Server{s1, s2, s3, s4}
330+
330331
// Join with fourth server (now have quorum)
331332
// Start with 4th server for higher chance of success
332-
TestJoin(t, s4, s3, s2, s1)
333+
TestJoin(t, servers...)
333334

334335
// Assert leadership with 4 peers
335-
servers := []*Server{s1, s2, s3, s4}
336-
for _, s := range servers {
337-
testutil.WaitForLeader(t, s.RPC)
338-
retry.Run(t, func(r *retry.R) { r.Check(wantPeers(s, 4)) })
339-
}
336+
expect := len(servers)
337+
testutil.WaitForLeader(t, servers[0].RPC)
338+
testutil.WaitForResult(func() (bool, error) {
339+
// Retry the join to decrease flakiness
340+
TestJoin(t, servers...)
341+
for _, s := range servers {
342+
peers, err := s.numPeers()
343+
if err != nil {
344+
return false, fmt.Errorf("failed to get number of peers: %v", err)
345+
}
346+
if peers != expect {
347+
return false, fmt.Errorf("expected %d peers, got %d", expect, peers)
348+
}
349+
if len(s.localPeers) != expect {
350+
return false, fmt.Errorf("expected %d local peers, got %d: %#v", expect, len(s.localPeers), s.localPeers)
351+
}
352+
353+
}
354+
return true, nil
355+
}, func(err error) {
356+
require.NoError(t, err)
357+
})
340358
}
341359

342360
func TestNomad_BadExpect(t *testing.T) {

0 commit comments

Comments
 (0)