Updates k8s ParseNode() to Handle Multiple IPs

Co-authored-by: Nikhil Sharma <nikhilsharma230303@gmail.com> Signed-off-by: Daneyon Hansen <daneyon.hansen@solo.io>
cilium · May 23, 2023 · ceb3d66 · ceb3d66
1 parent 7755f39
commit ceb3d66
Show file tree

Hide file tree

Showing 3 changed files with 113 additions and 3 deletions.
diff --git a/pkg/k8s/node.go b/pkg/k8s/node.go
@@ -36,8 +36,14 @@ func ParseNodeAddressType(k8sAddress slim_corev1.NodeAddressType) (addressing.Ad
 	return convertedAddr, err
 }
 
+type nodeAddressGroup struct {
+	typ    slim_corev1.NodeAddressType
+	family slim_corev1.IPFamily
+}
+
 // ParseNode parses a kubernetes node to a cilium node
 func ParseNode(k8sNode *slim_corev1.Node, source source.Source) *nodeTypes.Node {
+	addrGroups := make(map[nodeAddressGroup]struct{})
 	scopedLog := log.WithFields(logrus.Fields{
 		logfields.NodeName:  k8sNode.Name,
 		logfields.K8sNodeID: k8sNode.UID,
@@ -56,17 +62,33 @@ func ParseNode(k8sNode *slim_corev1.Node, source source.Source) *nodeTypes.Node
 		if addr.Address == "" {
 			continue
 		}
+		addrGroup := nodeAddressGroup{
+			typ: addr.Type,
+		}
 		ip := net.ParseIP(addr.Address)
-		if ip == nil {
+		switch {
+		case ip != nil && ip.To4() != nil:
+			addrGroup.family = slim_corev1.IPv4Protocol
+		case ip != nil && ip.To16() != nil:
+			addrGroup.family = slim_corev1.IPv6Protocol
+		default:
 			scopedLog.WithFields(logrus.Fields{
 				logfields.IPAddr: addr.Address,
-				"type":           addr.Type,
+				logfields.Type:   addr.Type,
 			}).Warn("Ignoring invalid node IP")
 			continue
 		}
+		_, groupFound := addrGroups[addrGroup]
+		if groupFound {
+			scopedLog.WithFields(logrus.Fields{
+				logfields.Node: k8sNode.Name,
+				logfields.Type: addr.Type,
+			}).Warn("Detected multiple IPs of the same address type and family, Cilium will only consider the first IP in the Node resource")
+			continue
+		}
+		addrGroups[addrGroup] = struct{}{}
 
 		addressType, err := ParseNodeAddressType(addr.Type)
-
 		if err != nil {
 			scopedLog.WithError(err).Warn("invalid address type for node")
 		}

diff --git a/pkg/k8s/node_test.go b/pkg/k8s/node_test.go
@@ -87,6 +87,91 @@ func (s *K8sSuite) TestParseNode(c *C) {
 	c.Assert(n.IPv4AllocCIDR.String(), Equals, "10.254.0.0/16")
 	c.Assert(n.IPv6AllocCIDR, NotNil)
 	c.Assert(n.IPv6AllocCIDR.String(), Equals, "f00d:aaaa:bbbb:cccc:dddd:eeee::/112")
+
+	// No IPv4/IPv6 annotations but PodCIDRs with IPv4/IPv6
+	k8sNode = &slim_corev1.Node{
+		ObjectMeta: slim_metav1.ObjectMeta{
+			Name: "node2",
+			Annotations: map[string]string{
+				annotation.V4CIDRName: "10.254.0.0/16",
+			},
+		},
+		Spec: slim_corev1.NodeSpec{
+			PodCIDR:  "10.1.0.0/16",
+			PodCIDRs: []string{"10.1.0.0/16", "f00d:aaaa:bbbb:cccc:dddd:eeee::/112"},
+		},
+	}
+
+	n = ParseNode(k8sNode, source.Local)
+	c.Assert(n.Name, Equals, "node2")
+	c.Assert(n.IPv4AllocCIDR, NotNil)
+	c.Assert(n.IPv4AllocCIDR.String(), Equals, "10.1.0.0/16")
+	c.Assert(n.IPv6AllocCIDR, NotNil)
+	c.Assert(n.IPv6AllocCIDR.String(), Equals, "f00d:aaaa:bbbb:cccc:dddd:eeee::/112")
+
+	// Node with multiple status addresses of the same type and family
+	expected := []string{"1.2.3.4", "f00d:aaaa:bbbb:cccc:dddd:eeee:0:1", "4.3.2.1", "f00d:aaaa:bbbb:cccc:dddd:eeef:0:1"}
+	notExpected := []string{"5.6.7.8", "f00d:aaaa:bbbb:cccc:dddd:aaaa::1", "8.7.6.5", "f00d:aaaa:bbbb:cccc:dddd:aaab::1"}
+	k8sNode = &slim_corev1.Node{
+		ObjectMeta: slim_metav1.ObjectMeta{
+			Name:        "node2",
+			Annotations: map[string]string{},
+		},
+		Spec: slim_corev1.NodeSpec{
+			PodCIDR: "10.1.0.0/16",
+		},
+		Status: slim_corev1.NodeStatus{
+			Addresses: []slim_corev1.NodeAddress{
+				{
+					Type:    slim_corev1.NodeInternalIP,
+					Address: expected[0],
+				},
+				{
+					Type:    slim_corev1.NodeInternalIP,
+					Address: notExpected[0],
+				},
+				{
+					Type:    slim_corev1.NodeInternalIP,
+					Address: expected[1],
+				},
+				{
+					Type:    slim_corev1.NodeInternalIP,
+					Address: notExpected[1],
+				},
+				{
+					Type:    slim_corev1.NodeExternalIP,
+					Address: expected[2],
+				},
+				{
+					Type:    slim_corev1.NodeExternalIP,
+					Address: notExpected[2],
+				},
+				{
+					Type:    slim_corev1.NodeExternalIP,
+					Address: expected[3],
+				},
+				{
+					Type:    slim_corev1.NodeExternalIP,
+					Address: notExpected[3],
+				},
+			},
+		},
+	}
+
+	n = ParseNode(k8sNode, source.Local)
+	c.Assert(n.Name, Equals, "node2")
+	c.Assert(n.IPv4AllocCIDR, NotNil)
+	c.Assert(n.IPv4AllocCIDR.String(), Equals, "10.1.0.0/16")
+	c.Assert(len(n.IPAddresses), Equals, len(expected))
+	addrsFound := 0
+	for _, addr := range n.IPAddresses {
+		for _, expect := range expected {
+			if addr.IP.String() == expect {
+				addrsFound++
+			}
+		}
+	}
+	c.Assert(addrsFound, Equals, len(expected))
 }
 
 func (s *K8sSuite) TestParseNodeWithoutAnnotations(c *C) {

diff --git a/pkg/logging/logfields/logfields.go b/pkg/logging/logfields/logfields.go
@@ -372,6 +372,9 @@ const (
 	// with TunnelPeer
 	ConflictingTunnelPeer = "conflictingTunnelPeer"
 
+	// Type is the address type
+	Type = "type"
+
 	// Selector is a selector of any sort: endpoint, CIDR, toFQDNs
 	Selector = "Selector"