From ad6148f0f68fa22e9093bb51c2a1b4e987921bf8 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 11 Jun 2018 17:02:21 +0000 Subject: [PATCH 1/2] Break out of kube rm peers loop if nothing changes Avoid getting into an infinite loop if some of the logic or underlying data is bad --- prog/kube-peers/main.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/prog/kube-peers/main.go b/prog/kube-peers/main.go index 53708d03..1b17a88f 100644 --- a/prog/kube-peers/main.go +++ b/prog/kube-peers/main.go @@ -97,7 +97,7 @@ func checkIamInPeerList(cml *configMapAnnotations, c *kubernetes.Clientset, peer // For each of those peers that is no longer listed as a node by // Kubernetes, remove it from Weave IPAM func reclaimRemovedPeers(weave *weaveapi.Client, cml *configMapAnnotations, nodes []nodeInfo, myPeerName string) error { - for { + for loopsWhenNothingChanged := 0; loopsWhenNothingChanged < 3; loopsWhenNothingChanged++ { if err := cml.Init(); err != nil { return err } @@ -144,6 +144,7 @@ func reclaimRemovedPeers(weave *weaveapi.Client, cml *configMapAnnotations, node if err != nil { return err } + loopsWhenNothingChanged = 0 cml.LoopUpdate(func() error { // 7aa. Remove any annotations Z* that have contents X if err := cml.RemoveAnnotationsWithValue(peer.PeerName); err != nil { @@ -168,6 +169,7 @@ func reclaimRemovedPeers(weave *weaveapi.Client, cml *configMapAnnotations, node } // 9. Go back to step 1 until there is no difference between the two sets + // (or we hit the counter that says we've been round the loop 3 times and nothing happened) } // Question: Should we narrow step 2 by checking against Weave Net IPAM? // i.e. If peer X owns any address space and is marked unreachable, we want to rmpeer X From cf66c8d50743cddc9b20efda6e87c253c00c8493 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Mon, 11 Jun 2018 17:12:57 +0000 Subject: [PATCH 2/2] Don't remove myself when reclaiming Some logs from users show an entry of X with annotation X, which suggests we have gone down the path of removing ourselves. This change avoids that possibility. --- prog/kube-peers/main.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/prog/kube-peers/main.go b/prog/kube-peers/main.go index 1b17a88f..a4a22222 100644 --- a/prog/kube-peers/main.go +++ b/prog/kube-peers/main.go @@ -119,6 +119,10 @@ func reclaimRemovedPeers(weave *weaveapi.Client, cml *configMapAnnotations, node } // 2. Loop for each X in the first set and not in the second - we wish to remove X from our data structures for _, peer := range peerMap { + if peer.PeerName == myPeerName { // Don't remove myself. + common.Log.Warnln("[kube-peers] not removing myself", peer) + continue + } common.Log.Debugln("[kube-peers] Preparing to remove disappeared peer", peer) okToRemove := false // 3. Check if there is an existing annotation with key X