diff --git a/internal/ingress/status/status.go b/internal/ingress/status/status.go index ef01cdd24..626f71e02 100644 --- a/internal/ingress/status/status.go +++ b/internal/ingress/status/status.go @@ -121,12 +121,6 @@ func (s *statusSync) Shutdown() { return } - if len(addrs) > 1 { - // leave the job to the next leader - klog.InfoS("leaving status update for next leader") - return - } - if s.isRunningMultiplePods() { klog.V(2).InfoS("skipping Ingress status update (multiple pods running - another one will be elected as master)") return @@ -220,9 +214,12 @@ func (s *statusSync) runningAddresses() ([]v1.IngressLoadBalancerIngress, error) continue } - name := k8s.GetNodeIPOrName(s.Client, pod.Spec.NodeName, s.UseNodeInternalIP) - if !stringInIngresses(name, addrs) { - addrs = append(addrs, nameOrIPToLoadBalancerIngress(name)) + preferExternal := !s.UseNodeInternalIP + addresses := k8s.GetNodeAddresses(s.Client, pod.Spec.NodeName, preferExternal) + for _, addr := range addresses { + if !stringInIngresses(addr, addrs) { + addrs = append(addrs, nameOrIPToLoadBalancerIngress(addr)) + } } } diff --git a/internal/ingress/status/status_test.go b/internal/ingress/status/status_test.go index 01419708b..1cdab9c63 100644 --- a/internal/ingress/status/status_test.go +++ b/internal/ingress/status/status_test.go @@ -143,10 +143,19 @@ func buildSimpleClientSet() *testclient.Clientset { Addresses: []apiv1.NodeAddress{ { Type: apiv1.NodeInternalIP, - Address: "10.0.0.1", - }, { + Address: "10.0.1.1", + }, + { + Type: apiv1.NodeInternalIP, + Address: "fd80::1:1", + }, + { Type: apiv1.NodeExternalIP, - Address: "10.0.0.2", + Address: "11.0.1.1", + }, + { + Type: apiv1.NodeExternalIP, + Address: "2001::1:1", }, }, }, @@ -159,11 +168,19 @@ func buildSimpleClientSet() *testclient.Clientset { Addresses: []apiv1.NodeAddress{ { Type: apiv1.NodeInternalIP, - Address: "11.0.0.1", + Address: "10.0.1.2", + }, + { + Type: apiv1.NodeInternalIP, + Address: "fd80::1:2", }, { Type: apiv1.NodeExternalIP, - Address: "11.0.0.2", + Address: "11.0.1.2", + }, + { + Type: apiv1.NodeExternalIP, + Address: "2001::1:2", }, }, }, @@ -325,11 +342,12 @@ func TestStatusActions(t *testing.T) { if err := fk.sync("just-test"); err != nil { t.Errorf("unexpected error: %v", err) } - // PublishService is empty, so the running address is: ["11.0.0.2"] - // after updated, the ingress's ip should only be "11.0.0.2" - newIPs := []networking.IngressLoadBalancerIngress{{ - IP: "11.0.0.2", - }} + // PublishService is empty, so the running address is the node's: ["11.0.1.2", "2001::1:2"] + // after updated, the ingress's ip should only be "11.0.1.2", "2001::1:2" + newIPs := []networking.IngressLoadBalancerIngress{ + {IP: "11.0.1.2"}, + {IP: "2001::1:2"}, + } fooIngress1, err1 := fk.Client.NetworkingV1().Ingresses(apiv1.NamespaceDefault).Get(context.TODO(), "foo_ingress_1", metav1.GetOptions{}) if err1 != nil { t.Fatalf("unexpected error") @@ -597,13 +615,11 @@ func TestRunningAddressesWithPods(t *testing.T) { if r == nil { t.Fatalf("returned nil but expected valid []networking.IngressLoadBalancerIngress") } - rl := len(r) - if len(r) != 1 { - t.Fatalf("returned %v but expected %v", rl, 1) + er := []networking.IngressLoadBalancerIngress{ + {IP: "11.0.1.2"}, {IP: "2001::1:2"}, } - rv := r[0] - if rv.IP != "11.0.0.2" { - t.Errorf("returned %v but expected %v", rv, networking.IngressLoadBalancerIngress{IP: "11.0.0.2"}) + if !reflect.DeepEqual(r, er) { + t.Errorf("returned %v but expected %v", r, er) } } diff --git a/internal/k8s/main.go b/internal/k8s/main.go index 5e93e560d..4dcefdfc4 100644 --- a/internal/k8s/main.go +++ b/internal/k8s/main.go @@ -42,37 +42,43 @@ 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 -func GetNodeIPOrName(kubeClient clientset.Interface, name string, useInternalIP bool) string { +// GetNodeAddresses returns the IP address or name of a node in the cluster. +// If preferExternal==true AND any non-empty NodeExternalIP addresses exist, they will be returned. +// Otherwise, the node's non-empty NodeInternalIP addresses will be returned. +func GetNodeAddresses(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 "" + return nil } - defaultOrInternalIP := "" - for _, address := range node.Status.Addresses { - if address.Type == apiv1.NodeInternalIP { - if address.Address != "" { - defaultOrInternalIP = address.Address - break + addresses := make([]string, 0) + if preferExternal { + for _, address := range node.Status.Addresses { + if address.Type != apiv1.NodeExternalIP { + continue } + if address.Address == "" { + continue + } + addresses = append(addresses, address.Address) + } + if len(addresses) > 0 { + return addresses } } - if useInternalIP { - return defaultOrInternalIP - } - for _, address := range node.Status.Addresses { - if address.Type == apiv1.NodeExternalIP { - if address.Address != "" { - return address.Address - } + if address.Type != apiv1.NodeInternalIP { + continue } + if address.Address == "" { + continue + } + addresses = append(addresses, address.Address) } - return defaultOrInternalIP + return addresses } var ( diff --git a/internal/k8s/main_test.go b/internal/k8s/main_test.go index 1721c1fb2..e616dbf80 100644 --- a/internal/k8s/main_test.go +++ b/internal/k8s/main_test.go @@ -17,6 +17,7 @@ limitations under the License. package k8s import ( + "reflect" "testing" apiv1 "k8s.io/api/core/v1" @@ -55,18 +56,19 @@ func TestParseNameNS(t *testing.T) { } } -func TestGetNodeIP(t *testing.T) { +func TestGetNodeAddresses(t *testing.T) { fKNodes := []struct { - name string - cs *testclient.Clientset - nodeName string - ea string - useInternalIP bool + name string + cs *testclient.Clientset + nodeName string + preferExternal bool + ea []string }{ { "empty node list", testclient.NewSimpleClientset(), - "demo", "", true, + "demo", false, + nil, }, { "node does not exist", @@ -82,10 +84,11 @@ func TestGetNodeIP(t *testing.T) { }, }, }, - }}}), "notexistnode", "", true, + }}}), "notexistnode", false, + nil, }, { - "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 +101,8 @@ func TestGetNodeIP(t *testing.T) { }, }, }, - }}}), "demo", "10.0.0.1", false, + }}}), "demo", true, + []string{"10.0.0.1"}, }, { "node exist and only has an internal IP address", @@ -114,7 +118,8 @@ func TestGetNodeIP(t *testing.T) { }, }, }, - }}}), "demo", "10.0.0.1", true, + }}}), "demo", false, + []string{"10.0.0.1"}, }, { "node exist and only has an external IP address", @@ -130,7 +135,8 @@ func TestGetNodeIP(t *testing.T) { }, }, }, - }}}), "demo", "10.0.0.1", false, + }}}), "demo", true, + []string{"10.0.0.1"}, }, { "multiple nodes - choose the right one", @@ -162,10 +168,11 @@ func TestGetNodeIP(t *testing.T) { }, }, }}), - "demo2", "10.0.0.2", true, + "demo2", false, + []string{"10.0.0.2"}, }, { - "node with both IP internal and external IP address - returns external IP", + "node with both IP internal and external IP addresses - returns external IPs", testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ ObjectMeta: metav1.ObjectMeta{ Name: "demo", @@ -175,17 +182,27 @@ func TestGetNodeIP(t *testing.T) { { Type: apiv1.NodeInternalIP, Address: "10.0.0.1", - }, { + }, + { + Type: apiv1.NodeInternalIP, + Address: "fe80::1", + }, + { Type: apiv1.NodeExternalIP, - Address: "10.0.0.2", + Address: "8.0.0.1", + }, + { + Type: apiv1.NodeExternalIP, + Address: "2001::1", }, }, }, }}}), - "demo", "10.0.0.2", false, + "demo", true, + []string{"8.0.0.1", "2001::1"}, }, { - "node with both IP internal and external IP address - returns internal IP", + "node with both IP internal and external IP addresses - returns internal IPs", testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ ObjectMeta: metav1.ObjectMeta{ Name: "demo", @@ -195,21 +212,27 @@ func TestGetNodeIP(t *testing.T) { { Type: apiv1.NodeExternalIP, Address: "", - }, { + }, + { Type: apiv1.NodeInternalIP, - Address: "10.0.0.2", + Address: "10.0.0.1", + }, + { + Type: apiv1.NodeInternalIP, + Address: "fe80::1", }, }, }, }}}), - "demo", "10.0.0.2", true, + "demo", false, + []string{"10.0.0.1", "fe80::1"}, }, } for _, fk := range fKNodes { - address := GetNodeIPOrName(fk.cs, fk.nodeName, fk.useInternalIP) - if address != fk.ea { - t.Errorf("%v - expected %s, but returned %s", fk.name, fk.ea, address) + addresses := GetNodeAddresses(fk.cs, fk.nodeName, fk.preferExternal) + if !reflect.DeepEqual(addresses, fk.ea) { + t.Errorf("%v - expected %v, but returned %v", fk.name, fk.ea, addresses) } } }