Browse Source

fix flaky TestReplication_FederationStates test due to race conditions (#7612)

The test had two racy bugs related to memdb references.

The first was when we initially populated data and retained the FederationState objects in a slice. Due to how the `inmemCodec` works these were actually the identical objects passed into memdb.

The second was that the `checkSame` assertion function was reading from memdb and setting the RaftIndexes to zeros to aid in equality checks. This was mutating the contents of memdb which is a no-no.

With this fix, the command:
```
i=0; while /usr/local/bin/go test -count=1 -timeout 30s github.com/hashicorp/consul/agent/consul -run '^(TestReplication_FederationStates)$'; do i=$((i + 1)); printf "$i "; done
```
That used to break on my machine in less than 20 runs is now running 150+ times without any issue.

Might also fix #7575
pull/7633/head
Pierre Souchay 5 years ago committed by GitHub
parent
commit
1b4218a068
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
  1. 22
      agent/consul/federation_state_replication_test.go

22
agent/consul/federation_state_replication_test.go

@ -43,7 +43,7 @@ func TestReplication_FederationStates(t *testing.T) {
testrpc.WaitForLeader(t, s1.RPC, "dc2") testrpc.WaitForLeader(t, s1.RPC, "dc2")
// Create some new federation states (weird because we're having dc1 update it for the other 50) // Create some new federation states (weird because we're having dc1 update it for the other 50)
var fedStates []*structs.FederationState var fedStateDCs []string
for i := 0; i < 50; i++ { for i := 0; i < 50; i++ {
dc := fmt.Sprintf("alt-dc%d", i+1) dc := fmt.Sprintf("alt-dc%d", i+1)
ip1 := fmt.Sprintf("1.2.3.%d", i+1) ip1 := fmt.Sprintf("1.2.3.%d", i+1)
@ -67,7 +67,7 @@ func TestReplication_FederationStates(t *testing.T) {
out := false out := false
require.NoError(t, s1.RPC("FederationState.Apply", &arg, &out)) require.NoError(t, s1.RPC("FederationState.Apply", &arg, &out))
fedStates = append(fedStates, arg.State) fedStateDCs = append(fedStateDCs, dc)
} }
checkSame := func(t *retry.R) error { checkSame := func(t *retry.R) error {
@ -77,11 +77,15 @@ func TestReplication_FederationStates(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
require.Len(t, local, len(remote)) require.Len(t, local, len(remote))
for i, _ := range remote { for i := range remote {
// Make lightweight copies so we can zero out the raft fields
// without mutating the copies in memdb.
remoteCopy := *remote[i]
localCopy := *local[i]
// zero out the raft data for future comparisons // zero out the raft data for future comparisons
remote[i].RaftIndex = structs.RaftIndex{} remoteCopy.RaftIndex = structs.RaftIndex{}
local[i].RaftIndex = structs.RaftIndex{} localCopy.RaftIndex = structs.RaftIndex{}
require.Equal(t, remote[i], local[i]) require.Equal(t, remoteCopy, localCopy)
} }
return nil return nil
} }
@ -126,11 +130,13 @@ func TestReplication_FederationStates(t *testing.T) {
checkSame(r) checkSame(r)
}) })
for _, fedState := range fedStates { for _, fedStateDC := range fedStateDCs {
arg := structs.FederationStateRequest{ arg := structs.FederationStateRequest{
Datacenter: "dc1", Datacenter: "dc1",
Op: structs.FederationStateDelete, Op: structs.FederationStateDelete,
State: fedState, State: &structs.FederationState{
Datacenter: fedStateDC,
},
} }
out := false out := false

Loading…
Cancel
Save