Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Break out of kube rm-peers loop if nothing changes #3317

Merged
merged 2 commits into from Jun 12, 2018
Merged

Conversation

bboreham
Copy link
Contributor

@bboreham bboreham commented Jun 11, 2018

Avoid getting into an infinite loop if some of the logic or underlying data is bad

#3310 has an example of one infinite loop:

DEBU: 2018/06/05 11:15:32.615919 [kube-peers] Preparing to remove disappeared peer {46:f4:4b:41:dd:11 ip-10-83-124-112.ec2.internal}
DEBU: 2018/06/05 11:15:32.615928 [kube-peers] Existing annotation 46:f4:4b:41:dd:11
DEBU: 2018/06/05 11:15:32.815841 [kube-peers] Nodes that have disappeared: map[ip-10-83-124-112.ec2.internal:{46:f4:4b:41:dd:11 ip-10-83-124-112.ec2.internal}]
DEBU: 2018/06/05 11:15:32.815867 [kube-peers] Preparing to remove disappeared peer {46:f4:4b:41:dd:11 ip-10-83-124-112.ec2.internal}
DEBU: 2018/06/05 11:15:32.815877 [kube-peers] Existing annotation 46:f4:4b:41:dd:11
DEBU: 2018/06/05 11:15:33.016143 [kube-peers] Nodes that have disappeared: map[ip-10-83-124-112.ec2.internal:{46:f4:4b:41:dd:11 ip-10-83-124-112.ec2.internal}]
DEBU: 2018/06/05 11:15:33.016172 [kube-peers] Preparing to remove disappeared peer {46:f4:4b:41:dd:11 ip-10-83-124-112.ec2.internal}

Edit: I added another commit to avoid the only way I can see to get into that situation.

@bboreham bboreham changed the base branch from master to 2.3 June 12, 2018 08:29
Avoid getting into an infinite loop if some of the logic or underlying
data is bad
@bboreham bboreham added this to the 2.3.1 milestone Jun 12, 2018
@bboreham
Copy link
Contributor Author

Moved to 2.3 branch since it's a bug users have seen for real.

Copy link
Contributor

@brb brb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just one nick.

@@ -119,6 +119,9 @@ 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.
continue

This comment was marked as abuse.

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.
@brb brb merged commit 287dab2 into 2.3 Jun 12, 2018
@brb brb modified the milestones: 2.3.1, 2.4 Jul 24, 2018
hswong3i added a commit to alvistack/kubernetes-sigs-kubespray that referenced this pull request Aug 7, 2018
Upstream Changes:

-   weave 2.4.0 (https://github.com/weaveworks/weave/releases/tag/v2.4.0)
-   Support `externalTrafficPolicy: Local` (weaveworks/weave#2924)
-   Make the ipset list size bigger (weaveworks/weave#3305)
-   Break out of kube rm-peers loop if nothing changes (weaveworks/weave#3317)

Our Changes:

-   Revamp weave-net.yml.j2 with upstream changes
-   Add more variables for customization
-   Replace WEAVE_PASSWORD with k8s secret
-   Remove hard-corded seed mode support, in favor of variables customization
rguichard pushed a commit to rguichard/kubespray that referenced this pull request Oct 5, 2018
Upstream Changes:

-   weave 2.4.0 (https://github.com/weaveworks/weave/releases/tag/v2.4.0)
-   Support `externalTrafficPolicy: Local` (weaveworks/weave#2924)
-   Make the ipset list size bigger (weaveworks/weave#3305)
-   Break out of kube rm-peers loop if nothing changes (weaveworks/weave#3317)

Our Changes:

-   Revamp weave-net.yml.j2 with upstream changes
-   Add more variables for customization
-   Replace WEAVE_PASSWORD with k8s secret
-   Remove hard-corded seed mode support, in favor of variables customization
@bboreham bboreham deleted the break-infinite-loop branch December 24, 2018 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants