From 3d8bbb2bec0d584fba3736d31f2bf3abac2bded8 Mon Sep 17 00:00:00 2001
From: Russell Bryant <rbryant@redhat.com>
Date: Tue, 14 May 2019 11:33:33 -0400
Subject: [PATCH] nodelink-controller: Support multiple internal IPs.

In a baremetal environment, we may have servers with multiple network
interfaces.  In that case, the Machine may have multiple internal IPs
listed for the different network interfaces.

This patch makes the nodelink-controller keep track of all Machine
internal IPs so that it can successfully match a Node's internal IP
against whichever IP is being used by the kubelet.

The patch also includes unit test coverage for
add/update/deleteMachine() methods.
---
 cmd/nodelink-controller/controller_test.go | 98 +++++++++++++++++++++-
 cmd/nodelink-controller/main.go            |  3 -
 2 files changed, 94 insertions(+), 7 deletions(-)

diff --git a/cmd/nodelink-controller/controller_test.go b/cmd/nodelink-controller/controller_test.go
index e424ae9be..5df82d98c 100644
--- a/cmd/nodelink-controller/controller_test.go
+++ b/cmd/nodelink-controller/controller_test.go
@@ -20,7 +20,7 @@ import (
 	"reflect"
 	"testing"
 
-	mapiv1alpha1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
+	mapiv1 "github.com/openshift/cluster-api/pkg/apis/machine/v1beta1"
 	corev1 "k8s.io/api/core/v1"
 )
 
@@ -32,9 +32,9 @@ func node(taints *[]corev1.Taint) *corev1.Node {
 	}
 }
 
-func machine(taints *[]corev1.Taint) *mapiv1alpha1.Machine {
-	return &mapiv1alpha1.Machine{
-		Spec: mapiv1alpha1.MachineSpec{
+func machine(taints *[]corev1.Taint) *mapiv1.Machine {
+	return &mapiv1.Machine{
+		Spec: mapiv1.MachineSpec{
 			Taints: *taints,
 		},
 	}
@@ -83,3 +83,93 @@ func TestAddTaintsToNode(t *testing.T) {
 		}
 	}
 }
+
+func fakeController() *Controller {
+	c := Controller{}
+	c.machineAddress = make(map[string]*mapiv1.Machine)
+	return &c
+}
+
+func TestAddUpdateDeleteMachine(t *testing.T) {
+	testCases := []struct {
+		description  string
+		machine      mapiv1.Machine
+		numAddresses int
+	}{
+		{
+			description:  "Machine with no addresses",
+			machine:      mapiv1.Machine{},
+			numAddresses: 0,
+		},
+		{
+			description: "Machine with one address",
+			machine: mapiv1.Machine{
+				Status: mapiv1.MachineStatus{
+					Addresses: []corev1.NodeAddress{
+						corev1.NodeAddress{
+							Address: "192.168.1.1",
+							Type:    "InternalIP",
+						},
+					},
+				},
+			},
+			numAddresses: 1,
+		},
+		{
+			description: "Machine with two addresses",
+			machine: mapiv1.Machine{
+				Status: mapiv1.MachineStatus{
+					Addresses: []corev1.NodeAddress{
+						corev1.NodeAddress{
+							Address: "192.168.1.1",
+							Type:    "InternalIP",
+						},
+						corev1.NodeAddress{
+							Address: "172.0.20.2",
+							Type:    "InternalIP",
+						},
+					},
+				},
+			},
+			numAddresses: 2,
+		},
+		{
+			description: "Use InternalIP only",
+			machine: mapiv1.Machine{
+				Status: mapiv1.MachineStatus{
+					Addresses: []corev1.NodeAddress{
+						corev1.NodeAddress{
+							Address: "192.168.1.1",
+							Type:    "InternalIP",
+						},
+						corev1.NodeAddress{
+							Address: "10.0.20.2",
+							Type:    "ExternalIP",
+						},
+						corev1.NodeAddress{
+							Address: "host.example.com",
+							Type:    "Hostname",
+						},
+					},
+				},
+			},
+			numAddresses: 1,
+		},
+	}
+
+	for _, test := range testCases {
+		c := fakeController()
+		c.addMachine(&test.machine)
+		if len(c.machineAddress) != test.numAddresses {
+			t.Errorf("Test case: %s, after addMachine(), Expected %d addresses, got %d", test.description, test.numAddresses, len(c.machineAddress))
+		}
+		c.updateMachine(mapiv1.Machine{}, &test.machine)
+		if len(c.machineAddress) != test.numAddresses {
+			t.Errorf("Test case: %s, after updateMachine(), Expected %d addresses, got %d", test.description, test.numAddresses, len(c.machineAddress))
+		}
+		c.deleteMachine(&test.machine)
+		if len(c.machineAddress) > 0 {
+			t.Errorf("Test case: %s, after deleteMachine(), Expected 0 addresses, got %d", test.description, len(c.machineAddress))
+		}
+	}
+}
diff --git a/cmd/nodelink-controller/main.go b/cmd/nodelink-controller/main.go
index c2b66e272..113688834 100644
--- a/cmd/nodelink-controller/main.go
+++ b/cmd/nodelink-controller/main.go
@@ -179,7 +179,6 @@ func (c *Controller) addMachine(obj interface{}) {
 		if a.Type == corev1.NodeInternalIP {
 			glog.V(3).Infof("Adding machine %q into machineAddress list for %q", machine.Name, a.Address)
 			c.machineAddress[a.Address] = machine
-			break
 		}
 	}
 }
@@ -195,7 +194,6 @@ func (c *Controller) updateMachine(old, cur interface{}) {
 		if a.Type == corev1.NodeInternalIP {
 			c.machineAddress[a.Address] = machine
 			glog.V(3).Infof("Updating machine addresses list. Machine: %q, address: %q", machine.Name, a.Address)
-			break
 		}
 	}
 }
@@ -210,7 +208,6 @@ func (c *Controller) deleteMachine(obj interface{}) {
 		// Use the internal IP to look for matches:
 		if a.Type == corev1.NodeInternalIP {
 			delete(c.machineAddress, a.Address)
-			break
 		}
 	}
 	glog.V(3).Infof("Delete obsolete machines from machine addresses list")