Change member removal logic to remove members only once.

[martinmr]

Currently, after removing a member, the logs print an error saying the
member cannot be removed because it's not part of the group. This
happens because the node tries to remove members more than once. It
succeeds the first time but subsequent attempts will fail.

This PR adds a check so that the node is not removed if it's already
been marked as removed in the old state and is not a current peer of the
node.

I tested this by adding logs, removing a node, and verifying the node is
only removed once and further updates don't try to remove the node
again.

Fixes #4056

---

This change is 

[manishrjain]

Couple of comments.

Reviewed 1 of 1 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @martinmr)

---

worker/groups.go, line 332 at r1 (raw file):

		// more than once.
		oldRemovedMembers := make(map[uint64]*pb.Member)
		for _, removedMember := range oldState.GetRemoved() {

I'd remove this map creation.

---

worker/groups.go, line 342 at r1 (raw file):

					// the membership state and is not a current peer of the node.
					_, isPeer := g.Node.Peer(member.GetId())
					isPeer = isPeer && member.GetId() != g.Node.RaftContext.Id

Add a comment that it should be myself.

---

worker/groups.go, line 343 at r1 (raw file):

					_, isPeer := g.Node.Peer(member.GetId())
					isPeer = isPeer && member.GetId() != g.Node.RaftContext.Id
					if oldMember, ok := oldRemovedMembers[member.GetId()]; ok &&

Just do a for loop to find the id. Simpler and good enough in this case.

---

worker/groups.go, line 344 at r1 (raw file):

					isPeer = isPeer && member.GetId() != g.Node.RaftContext.Id
					if oldMember, ok := oldRemovedMembers[member.GetId()]; ok &&
						proto.Equal(member, oldMember) && !isPeer {

Remove the proto equal.

[martinmr]

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @manishrjain)

---

worker/groups.go, line 332 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

I'd remove this map creation.

Done.

---

worker/groups.go, line 342 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Add a comment that it should be myself.

Done.

---

worker/groups.go, line 343 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Just do a for loop to find the id. Simpler and good enough in this case.

Done.

---

worker/groups.go, line 344 at r1 (raw file):

Previously, manishrjain (Manish R Jain) wrote…

Remove the proto equal.

Done.