prework: refactor GetNodeIPOrName

this is done to make the intention of current behaviour clearer
before I go changing it.
this should have no effect on current behaviour.

internal/ingress/status/status.go

@@ -220,7 +220,8 @@ func (s *statusSync) runningAddresses() ([]v1.IngressLoadBalancerIngress, error)
 			continue
 		}
 
-		name := k8s.GetNodeIPOrName(s.Client, pod.Spec.NodeName, s.UseNodeInternalIP)
+		preferExternal := !s.UseNodeInternalIP
+		name := k8s.GetNodeIPOrName(s.Client, pod.Spec.NodeName, preferExternal)
 		if !stringInIngresses(name, addrs) {
 			addrs = append(addrs, nameOrIPToLoadBalancerIngress(name))
 		}

internal/k8s/main.go

@@ -42,37 +42,39 @@ func ParseNameNS(input string) (ns, name string, err error) {
 	return nsName[0], nsName[1], nil
 }
 
-// GetNodeIPOrName returns the IP address or the name of a node in the cluster
+// GetNodeIPOrName returns the IP address or name of a node in the cluster.
-func GetNodeIPOrName(kubeClient clientset.Interface, name string, useInternalIP bool) string {
+// If preferExternal==true AND any non-empty NodeExternalIP addresses exist, the first one will be returned.
+// Otherwise, the node's first non-empty NodeInternalIP address will be returned.
+func GetNodeIPOrName(kubeClient clientset.Interface, name string, preferExternal bool) string {
 	node, err := kubeClient.CoreV1().Nodes().Get(context.TODO(), name, metav1.GetOptions{})
 	if err != nil {
 		klog.ErrorS(err, "Error getting node", "name", name)
 		return ""
 	}
 
-	defaultOrInternalIP := ""
+	if preferExternal {
-	for _, address := range node.Status.Addresses {
+		for _, address := range node.Status.Addresses {
-		if address.Type == apiv1.NodeInternalIP {
+			if address.Type != apiv1.NodeExternalIP {
-			if address.Address != "" {
+				continue
-				defaultOrInternalIP = address.Address
-				break
 			}
+			if address.Address == "" {
+				continue
+			}
+			return address.Address
 		}
 	}
 
-	if useInternalIP {
-		return defaultOrInternalIP
-	}
-
 	for _, address := range node.Status.Addresses {
-		if address.Type == apiv1.NodeExternalIP {
+		if address.Type != apiv1.NodeInternalIP {
-			if address.Address != "" {
+			continue
-				return address.Address
-			}
 		}
+		if address.Address == "" {
+			continue
+		}
+		return address.Address
 	}
 
-	return defaultOrInternalIP
+	return ""
 }
 
 var (

internal/k8s/main_test.go

@@ -57,16 +57,17 @@ func TestParseNameNS(t *testing.T) {
 
 func TestGetNodeIP(t *testing.T) {
 	fKNodes := []struct {
-		name          string
+		name           string
-		cs            *testclient.Clientset
+		cs             *testclient.Clientset
-		nodeName      string
+		nodeName       string
-		ea            string
+		preferExternal bool
-		useInternalIP bool
+		ea             string
 	}{
 		{
 			"empty node list",
 			testclient.NewSimpleClientset(),
-			"demo", "", true,
+			"demo", false,
+			"",
 		},
 		{
 			"node does not exist",
@@ -82,10 +83,11 @@ func TestGetNodeIP(t *testing.T) {
 						},
 					},
 				},
-			}}}), "notexistnode", "", true,
+			}}}), "notexistnode", false,
+			"",
 		},
 		{
-			"node exist and only has an internal IP address (useInternalIP=false)",
+			"node exist and only has an internal IP address (preferExternal=true)",
 			testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{
 				ObjectMeta: metav1.ObjectMeta{
 					Name: "demo",
@@ -98,7 +100,8 @@ func TestGetNodeIP(t *testing.T) {
 						},
 					},
 				},
-			}}}), "demo", "10.0.0.1", false,
+			}}}), "demo", true,
+			"10.0.0.1",
 		},
 		{
 			"node exist and only has an internal IP address",
@@ -114,7 +117,8 @@ func TestGetNodeIP(t *testing.T) {
 						},
 					},
 				},
-			}}}), "demo", "10.0.0.1", true,
+			}}}), "demo", false,
+			"10.0.0.1",
 		},
 		{
 			"node exist and only has an external IP address",
@@ -130,7 +134,8 @@ func TestGetNodeIP(t *testing.T) {
 						},
 					},
 				},
-			}}}), "demo", "10.0.0.1", false,
+			}}}), "demo", true,
+			"10.0.0.1",
 		},
 		{
 			"multiple nodes - choose the right one",
@@ -162,7 +167,8 @@ func TestGetNodeIP(t *testing.T) {
 					},
 				},
 			}}),
-			"demo2", "10.0.0.2", true,
+			"demo2", false,
+			"10.0.0.2",
 		},
 		{
 			"node with both IP internal and external IP address - returns external IP",
@@ -182,7 +188,8 @@ func TestGetNodeIP(t *testing.T) {
 					},
 				},
 			}}}),
-			"demo", "10.0.0.2", false,
+			"demo", true,
+			"10.0.0.2",
 		},
 		{
 			"node with both IP internal and external IP address - returns internal IP",
@@ -202,12 +209,13 @@ func TestGetNodeIP(t *testing.T) {
 					},
 				},
 			}}}),
-			"demo", "10.0.0.2", true,
+			"demo", false,
+			"10.0.0.2",
 		},
 	}
 
 	for _, fk := range fKNodes {
-		address := GetNodeIPOrName(fk.cs, fk.nodeName, fk.useInternalIP)
+		address := GetNodeIPOrName(fk.cs, fk.nodeName, fk.preferExternal)
 		if address != fk.ea {
 			t.Errorf("%v - expected %s, but returned %s", fk.name, fk.ea, address)
 		}