From cc81ffe96b058f5d1094d749d3694e8ae245a455 Mon Sep 17 00:00:00 2001
From: Hector Sanjuan <hector@protocol.ai>
Date: Tue, 14 Nov 2017 23:54:23 +0100
Subject: [PATCH] cluster: peerAdd: try to return an up-to-date new peer ID

Sometimes tests fail because the returned api.ID for a new peer
does not include the current cluster peers. This is because the
new peerset has not yet be commited in the new peer at the time
of the request.

This commit retries obtaining the request until the correct peerset
comes in, or gives up after two seconds retrying.

Rather than the tests failing, note that the ID returned it is very
user-facing and should contain the current cluster peers after adding,
and not the former peerset, at least while peerAdd operation is allowed.

License: MIT
Signed-off-by: Hector Sanjuan <hector@protocol.ai>
---
 cluster.go | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 5 deletions(-)

diff --git a/cluster.go b/cluster.go
index a5edd76a..df4c460e 100644
--- a/cluster.go
+++ b/cluster.go
@@ -373,10 +373,9 @@ func (c *Cluster) watchPeers() {
 			if len(peers) != len(lastPeers) {
 				save = true
 			} else {
-				for i := range peers {
-					if peers[i] != lastPeers[i] {
-						save = true
-					}
+				added, removed := diffPeers(lastPeers, peers)
+				if len(added) != 0 || len(removed) != 0 {
+					save = true
 				}
 			}
 
@@ -692,7 +691,27 @@ func (c *Cluster) PeerAdd(addr ma.Multiaddr) (api.ID, error) {
 		logger.Error(err)
 	}
 
-	id, err := c.getIDForPeer(pid)
+	id := api.ID{}
+
+	// wait up to 2 seconds for new peer to catch up
+	// and return an up to date api.ID object.
+	// otherwise it might not contain the current cluster peers
+	// as it should.
+	for i := 0; i < 20; i++ {
+		id, _ = c.getIDForPeer(pid)
+		ownPeers, err := c.consensus.Peers()
+		if err != nil {
+			break
+		}
+		newNodePeers := id.ClusterPeers
+		added, removed := diffPeers(ownPeers, newNodePeers)
+		if len(added) == 0 && len(removed) == 0 {
+			break // the new peer has fully joined
+		}
+		time.Sleep(200 * time.Millisecond)
+		logger.Debugf("%s addPeer: retrying to get ID from %s",
+			c.id.Pretty(), pid.Pretty())
+	}
 	return id, nil
 }
 
@@ -1352,3 +1371,43 @@ func (c *Cluster) backupState() {
 		return
 	}
 }
+
+// diffPeers returns the peerIDs added and removed from peers2 in relation to
+// peers1
+func diffPeers(peers1, peers2 []peer.ID) (added, removed []peer.ID) {
+	m1 := make(map[peer.ID]struct{})
+	m2 := make(map[peer.ID]struct{})
+	added = make([]peer.ID, 0)
+	removed = make([]peer.ID, 0)
+	if peers1 == nil && peers2 == nil {
+		return
+	}
+	if peers1 == nil {
+		added = peers2
+		return
+	}
+	if peers2 == nil {
+		removed = peers1
+		return
+	}
+
+	for _, p := range peers1 {
+		m1[p] = struct{}{}
+	}
+	for _, p := range peers2 {
+		m2[p] = struct{}{}
+	}
+	for k, _ := range m1 {
+		_, ok := m2[k]
+		if !ok {
+			removed = append(removed, k)
+		}
+	}
+	for k, _ := range m2 {
+		_, ok := m1[k]
+		if !ok {
+			added = append(added, k)
+		}
+	}
+	return
+}