From 1b4218a068a1fa76118dc35b9d34447fd5dc396e Mon Sep 17 00:00:00 2001
From: Pierre Souchay <pierresouchay@users.noreply.github.com>
Date: Thu, 9 Apr 2020 22:42:41 +0200
Subject: [PATCH] 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
---
 .../federation_state_replication_test.go      | 22 ++++++++++++-------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/agent/consul/federation_state_replication_test.go b/agent/consul/federation_state_replication_test.go
index ac047e0dbf..d83d3f2713 100644
--- a/agent/consul/federation_state_replication_test.go
+++ b/agent/consul/federation_state_replication_test.go
@@ -43,7 +43,7 @@ func TestReplication_FederationStates(t *testing.T) {
 	testrpc.WaitForLeader(t, s1.RPC, "dc2")
 
 	// 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++ {
 		dc := fmt.Sprintf("alt-dc%d", i+1)
 		ip1 := fmt.Sprintf("1.2.3.%d", i+1)
@@ -67,7 +67,7 @@ func TestReplication_FederationStates(t *testing.T) {
 
 		out := false
 		require.NoError(t, s1.RPC("FederationState.Apply", &arg, &out))
-		fedStates = append(fedStates, arg.State)
+		fedStateDCs = append(fedStateDCs, dc)
 	}
 
 	checkSame := func(t *retry.R) error {
@@ -77,11 +77,15 @@ func TestReplication_FederationStates(t *testing.T) {
 		require.NoError(t, err)
 
 		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
-			remote[i].RaftIndex = structs.RaftIndex{}
-			local[i].RaftIndex = structs.RaftIndex{}
-			require.Equal(t, remote[i], local[i])
+			remoteCopy.RaftIndex = structs.RaftIndex{}
+			localCopy.RaftIndex = structs.RaftIndex{}
+			require.Equal(t, remoteCopy, localCopy)
 		}
 		return nil
 	}
@@ -126,11 +130,13 @@ func TestReplication_FederationStates(t *testing.T) {
 		checkSame(r)
 	})
 
-	for _, fedState := range fedStates {
+	for _, fedStateDC := range fedStateDCs {
 		arg := structs.FederationStateRequest{
 			Datacenter: "dc1",
 			Op:         structs.FederationStateDelete,
-			State:      fedState,
+			State: &structs.FederationState{
+				Datacenter: fedStateDC,
+			},
 		}
 
 		out := false