return multiple IPs for nodes when updating status, not just the first

the main consideration I had for doing this was to make sure both IPv4
and IPv6 addresses are attached to my ingress, however the approach here
has nothing to do with IP Families: the change is to return all matching
addresses instead of just the first, regardless of family.
This commit is contained in:
Jarrad Whitaker 2023-12-14 11:21:26 +11:00
parent 5f939ee97c
commit e518ae45c7
No known key found for this signature in database
4 changed files with 61 additions and 41 deletions

View file

@ -221,9 +221,11 @@ func (s *statusSync) runningAddresses() ([]v1.IngressLoadBalancerIngress, error)
}
preferExternal := !s.UseNodeInternalIP
name := k8s.GetNodeIPOrName(s.Client, pod.Spec.NodeName, preferExternal)
if !stringInIngresses(name, addrs) {
addrs = append(addrs, nameOrIPToLoadBalancerIngress(name))
addresses := k8s.GetNodeAddresses(s.Client, pod.Spec.NodeName, preferExternal)
for _, addr := range addresses {
if !stringInIngresses(addr, addrs) {
addrs = append(addrs, nameOrIPToLoadBalancerIngress(addr))
}
}
}

View file

@ -342,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 the node's: ["11.0.1.2"]
// after updated, the ingress's ip should only be "11.0.1.2"
newIPs := []networking.IngressLoadBalancerIngress{{
IP: "11.0.1.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")
@ -614,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.1.2" {
t.Errorf("returned %v but expected %v", rv, networking.IngressLoadBalancerIngress{IP: "11.0.1.2"})
if !reflect.DeepEqual(r, er) {
t.Errorf("returned %v but expected %v", r, er)
}
}

View file

@ -42,16 +42,17 @@ func ParseNameNS(input string) (ns, name string, err error) {
return nsName[0], nsName[1], nil
}
// GetNodeIPOrName returns the IP address or name of a node in the cluster.
// 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 {
// 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
}
addresses := make([]string, 0)
if preferExternal {
for _, address := range node.Status.Addresses {
if address.Type != apiv1.NodeExternalIP {
@ -60,7 +61,10 @@ func GetNodeIPOrName(kubeClient clientset.Interface, name string, preferExternal
if address.Address == "" {
continue
}
return address.Address
addresses = append(addresses, address.Address)
}
if len(addresses) > 0 {
return addresses
}
}
@ -71,10 +75,10 @@ func GetNodeIPOrName(kubeClient clientset.Interface, name string, preferExternal
if address.Address == "" {
continue
}
return address.Address
addresses = append(addresses, address.Address)
}
return ""
return addresses
}
var (

View file

@ -17,6 +17,7 @@ limitations under the License.
package k8s
import (
"reflect"
"testing"
apiv1 "k8s.io/api/core/v1"
@ -55,19 +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
preferExternal bool
ea string
ea []string
}{
{
"empty node list",
testclient.NewSimpleClientset(),
"demo", false,
"",
nil,
},
{
"node does not exist",
@ -84,7 +85,7 @@ func TestGetNodeIP(t *testing.T) {
},
},
}}}), "notexistnode", false,
"",
nil,
},
{
"node exist and only has an internal IP address (preferExternal=true)",
@ -101,7 +102,7 @@ func TestGetNodeIP(t *testing.T) {
},
},
}}}), "demo", true,
"10.0.0.1",
[]string{"10.0.0.1"},
},
{
"node exist and only has an internal IP address",
@ -118,7 +119,7 @@ func TestGetNodeIP(t *testing.T) {
},
},
}}}), "demo", false,
"10.0.0.1",
[]string{"10.0.0.1"},
},
{
"node exist and only has an external IP address",
@ -135,7 +136,7 @@ func TestGetNodeIP(t *testing.T) {
},
},
}}}), "demo", true,
"10.0.0.1",
[]string{"10.0.0.1"},
},
{
"multiple nodes - choose the right one",
@ -168,10 +169,10 @@ func TestGetNodeIP(t *testing.T) {
},
}}),
"demo2", false,
"10.0.0.2",
[]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",
@ -181,18 +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", true,
"10.0.0.2",
[]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",
@ -202,22 +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", false,
"10.0.0.2",
[]string{"10.0.0.1", "fe80::1"},
},
}
for _, fk := range fKNodes {
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)
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)
}
}
}