From 9e5a421221606982dac234781af7ab08b3a7b794 Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Sun, 29 Dec 2019 21:40:21 -0300 Subject: [PATCH] Fix ingress status regression introduced in #4490 --- internal/k8s/main.go | 18 ++-- internal/k8s/main_test.go | 197 +++++++++++++++++++++++--------------- 2 files changed, 131 insertions(+), 84 deletions(-) diff --git a/internal/k8s/main.go b/internal/k8s/main.go index 85f1c15c29..e5901f92f3 100644 --- a/internal/k8s/main.go +++ b/internal/k8s/main.go @@ -48,16 +48,20 @@ func GetNodeIPOrName(kubeClient clientset.Interface, name string, useInternalIP return "" } - if useInternalIP { - for _, address := range node.Status.Addresses { - if address.Type == apiv1.NodeInternalIP { - if address.Address != "" { - return address.Address - } + defaultOrInternalIP := "" + for _, address := range node.Status.Addresses { + if address.Type == apiv1.NodeInternalIP { + if address.Address != "" { + defaultOrInternalIP = address.Address + break } } } + if useInternalIP { + return defaultOrInternalIP + } + for _, address := range node.Status.Addresses { if address.Type == apiv1.NodeExternalIP { if address.Address != "" { @@ -66,7 +70,7 @@ func GetNodeIPOrName(kubeClient clientset.Interface, name string, useInternalIP } } - return "" + return defaultOrInternalIP } // PodInfo contains runtime information about the pod running the Ingres controller diff --git a/internal/k8s/main_test.go b/internal/k8s/main_test.go index 7283ebc5e9..c723b56689 100644 --- a/internal/k8s/main_test.go +++ b/internal/k8s/main_test.go @@ -58,49 +58,54 @@ func TestParseNameNS(t *testing.T) { func TestGetNodeIP(t *testing.T) { fKNodes := []struct { - cs *testclient.Clientset - n string - ea string - i bool + name string + cs *testclient.Clientset + nodeName string + ea string + useInternalIP bool }{ - // empty node list - {testclient.NewSimpleClientset(), "demo", "", true}, - - // node not exist - {testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ - ObjectMeta: metav1.ObjectMeta{ - Name: "demo", - }, - Status: apiv1.NodeStatus{ - Addresses: []apiv1.NodeAddress{ - { - Type: apiv1.NodeInternalIP, - Address: "10.0.0.1", + { + "empty node list", + testclient.NewSimpleClientset(), + "demo", "", true, + }, + { + "node does not exist", + testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + }, + Status: apiv1.NodeStatus{ + Addresses: []apiv1.NodeAddress{ + { + Type: apiv1.NodeInternalIP, + Address: "10.0.0.1", + }, }, }, - }, - }}}), "notexistnode", "", true}, - - // node exist - {testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ - ObjectMeta: metav1.ObjectMeta{ - Name: "demo", - }, - Status: apiv1.NodeStatus{ - Addresses: []apiv1.NodeAddress{ - { - Type: apiv1.NodeInternalIP, - Address: "10.0.0.1", + }}}), "notexistnode", "", true, + }, + { + "node exist and only has an internal IP address (useInternalIP=false)", + testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + }, + Status: apiv1.NodeStatus{ + Addresses: []apiv1.NodeAddress{ + { + Type: apiv1.NodeInternalIP, + Address: "10.0.0.1", + }, }, }, - }, - }}}), "demo", "10.0.0.1", true}, - - // search the correct node - {testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{ - { + }}}), "demo", "10.0.0.1", false, + }, + { + "node exist and only has an internal IP address", + testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ ObjectMeta: metav1.ObjectMeta{ - Name: "demo1", + Name: "demo", }, Status: apiv1.NodeStatus{ Addresses: []apiv1.NodeAddress{ @@ -110,63 +115,101 @@ func TestGetNodeIP(t *testing.T) { }, }, }, - }, - { + }}}), "demo", "10.0.0.1", true, + }, + { + "node exist and only has an external IP address", + testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", + }, + Status: apiv1.NodeStatus{ + Addresses: []apiv1.NodeAddress{ + { + Type: apiv1.NodeExternalIP, + Address: "10.0.0.1", + }, + }, + }, + }}}), "demo", "10.0.0.1", false, + }, + { + "multiple nodes - choose the right one", + testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{ + { + ObjectMeta: metav1.ObjectMeta{ + Name: "demo1", + }, + Status: apiv1.NodeStatus{ + Addresses: []apiv1.NodeAddress{ + { + Type: apiv1.NodeInternalIP, + Address: "10.0.0.1", + }, + }, + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Name: "demo2", + }, + Status: apiv1.NodeStatus{ + Addresses: []apiv1.NodeAddress{ + { + Type: apiv1.NodeInternalIP, + Address: "10.0.0.2", + }, + }, + }, + }, + }}), + "demo2", "10.0.0.2", true, + }, + { + "node with both IP internal and external IP address - returns external IP", + testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ ObjectMeta: metav1.ObjectMeta{ - Name: "demo2", + Name: "demo", }, Status: apiv1.NodeStatus{ Addresses: []apiv1.NodeAddress{ { Type: apiv1.NodeInternalIP, + Address: "10.0.0.1", + }, { + Type: apiv1.NodeExternalIP, Address: "10.0.0.2", }, }, }, - }, - }}), "demo2", "10.0.0.2", true}, - - // get NodeExternalIP - {testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ - ObjectMeta: metav1.ObjectMeta{ - Name: "demo", - }, - Status: apiv1.NodeStatus{ - Addresses: []apiv1.NodeAddress{ - { - Type: apiv1.NodeInternalIP, - Address: "10.0.0.1", - }, { - Type: apiv1.NodeExternalIP, - Address: "10.0.0.2", - }, + }}}), + "demo", "10.0.0.2", false, + }, + { + "node with both IP internal and external IP address - returns internal IP", + testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ + ObjectMeta: metav1.ObjectMeta{ + Name: "demo", }, - }, - }}}), "demo", "10.0.0.2", false}, - - // get NodeInternalIP - {testclient.NewSimpleClientset(&apiv1.NodeList{Items: []apiv1.Node{{ - ObjectMeta: metav1.ObjectMeta{ - Name: "demo", - }, - Status: apiv1.NodeStatus{ - Addresses: []apiv1.NodeAddress{ - { - Type: apiv1.NodeExternalIP, - Address: "", - }, { - Type: apiv1.NodeInternalIP, - Address: "10.0.0.2", + Status: apiv1.NodeStatus{ + Addresses: []apiv1.NodeAddress{ + { + Type: apiv1.NodeExternalIP, + Address: "", + }, { + Type: apiv1.NodeInternalIP, + Address: "10.0.0.2", + }, }, }, - }, - }}}), "demo", "10.0.0.2", true}, + }}}), + "demo", "10.0.0.2", true}, } for _, fk := range fKNodes { - address := GetNodeIPOrName(fk.cs, fk.n, fk.i) + address := GetNodeIPOrName(fk.cs, fk.nodeName, fk.useInternalIP) if address != fk.ea { - t.Errorf("expected %s, but returned %s", fk.ea, address) + t.Errorf("%v - expected %s, but returned %s", fk.name, fk.ea, address) } } }