Skip to content

Commit

Permalink
fix(network_routes): missing node ip -> error log
Browse files Browse the repository at this point in the history
Before we used to raise an error when a node was missing an IP, but it
turns out that this is not a required attribute of a node. And while it
is rare that a node would be missing an IP address, a node doesn't
require an IP address or a name or really much of anything in order to
exist.

This brings us to stronger conformance with the Kubernetes API and makes
it so that kube-router logs errors rather than changing it's health
status and potentially causing cascading failures across the fleet if a
user adds a node like this.
  • Loading branch information
aauren committed May 25, 2020
1 parent d2178da commit d001b7a
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 7 deletions.
22 changes: 18 additions & 4 deletions pkg/controllers/routing/bgp_peers.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,11 @@ func (nrc *NetworkRoutingController) syncInternalPeers() {
currentNodes := make([]string, 0)
for _, obj := range nodes {
node := obj.(*v1core.Node)
nodeIP, _ := utils.GetNodeIP(node)
nodeIP, err := utils.GetNodeIP(node)
if err != nil {
glog.Errorf("Failed to find a node IP and therefore cannot sync internal BGP Peer: %v", err)
continue
}

// skip self
if nodeIP.String() == nrc.nodeIP.String() {
Expand Down Expand Up @@ -321,7 +325,11 @@ func (nrc *NetworkRoutingController) newNodeEventHandler() cache.ResourceEventHa
return cache.ResourceEventHandlerFuncs{
AddFunc: func(obj interface{}) {
node := obj.(*v1core.Node)
nodeIP, _ := utils.GetNodeIP(node)
nodeIP, err := utils.GetNodeIP(node)
if err != nil {
glog.Errorf("New node received, but we were unable to add it as we were couldn't find it's node IP: %v", err)
return
}

glog.V(2).Infof("Received node %s added update from watch API so peer with new node", nodeIP)
nrc.OnNodeUpdate(obj)
Expand All @@ -344,9 +352,15 @@ func (nrc *NetworkRoutingController) newNodeEventHandler() cache.ResourceEventHa
return
}
}
nodeIP, _ := utils.GetNodeIP(node)
nodeIP, err := utils.GetNodeIP(node)
// In this case even if we can't get the NodeIP that's alright as the node is being removed anyway and
// future node lister operations that happen in OnNodeUpdate won't be affected as the node won't be returned
if err == nil {
glog.Infof("Received node %s removed update from watch API, so remove node from peer", nodeIP)
} else {
glog.Infof("Received node (IP unavailable) removed update from watch API, so remove node from peer")
}

glog.Infof("Received node %s removed update from watch API, so remove node from peer", nodeIP)
nrc.OnNodeUpdate(obj)
},
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/controllers/routing/bgp_policies.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ package routing

import (
"errors"
"fmt"
"github.com/golang/glog"

"github.com/cloudnativelabs/kube-router/pkg/utils"
"github.com/osrg/gobgp/config"
Expand Down Expand Up @@ -56,7 +56,8 @@ func (nrc *NetworkRoutingController) AddPolicies() error {
nodeObj := node.(*v1core.Node)
nodeIP, err := utils.GetNodeIP(nodeObj)
if err != nil {
return fmt.Errorf("Failed to find a node IP: %s", err)
glog.Errorf("Failed to find a node IP and therefore cannot add internal BGP Peer: %v", err)
continue
}
iBGPPeers = append(iBGPPeers, nodeIP.String())
}
Expand Down
3 changes: 2 additions & 1 deletion pkg/controllers/routing/network_routes_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,8 @@ func (nrc *NetworkRoutingController) syncNodeIPSets() error {
currentPodCidrs = append(currentPodCidrs, podCIDR)
nodeIP, err := utils.GetNodeIP(node)
if err != nil {
return fmt.Errorf("Failed to find a node IP: %s", err)
glog.Errorf("Failed to find a node IP, cannot add to node ipset which could affect routing: %v", err)
continue
}
currentNodeIPs = append(currentNodeIPs, nodeIP.String())
}
Expand Down

0 comments on commit d001b7a

Please sign in to comment.