[fix]: Fix getNodeBalancerByStatus bug when IPv6 ingress is enabled (#386)

* Refactor getNodeBalancerByIPv4 to support IPv6

* Add tests for getNodeBalancerByStatus

Author: cbbond

cloud/linode/loadbalancers.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net"
 	"net/http"
+	"net/netip"
 	"os"
 	"reflect"
 	"strconv"
@@ -99,12 +100,17 @@ func (l *loadbalancers) getLatestServiceLoadBalancerStatus(ctx context.Context,
 	return service.Status.LoadBalancer, nil
 }
 
-// getNodeBalancerByStatus attempts to get the NodeBalancer from the IPv4 specified in the
+// getNodeBalancerByStatus attempts to get the NodeBalancer from the IP or hostname specified in the
 // most recent LoadBalancer status.
 func (l *loadbalancers) getNodeBalancerByStatus(ctx context.Context, service *v1.Service) (nb *linodego.NodeBalancer, err error) {
 	for _, ingress := range service.Status.LoadBalancer.Ingress {
 		if ingress.IP != "" {
-			return l.getNodeBalancerByIPv4(ctx, service, ingress.IP)
+			address, err := netip.ParseAddr(ingress.IP)
+			if err != nil {
+				klog.Warningf("failed to parse IP address %s from service %s/%s status, error: %s", ingress.IP, service.Namespace, service.Name, err)
+			} else {
+				return l.getNodeBalancerByIP(ctx, service, address)
+			}
 		}
 		if ingress.Hostname != "" {
 			return l.getNodeBalancerByHostname(ctx, service, ingress.Hostname)
@@ -603,16 +609,22 @@ func (l *loadbalancers) getNodeBalancerByHostname(ctx context.Context, service *
 	return nil, lbNotFoundError{serviceNn: getServiceNn(service)}
 }
 
-func (l *loadbalancers) getNodeBalancerByIPv4(ctx context.Context, service *v1.Service, ipv4 string) (*linodego.NodeBalancer, error) {
-	filter := fmt.Sprintf(`{"ipv4": "%v"}`, ipv4)
+func (l *loadbalancers) getNodeBalancerByIP(ctx context.Context, service *v1.Service, ip netip.Addr) (*linodego.NodeBalancer, error) {
+	var filter string
+	if ip.Is6() {
+		filter = fmt.Sprintf(`{"ipv6": "%v"}`, ip.String())
+	} else {
+		filter = fmt.Sprintf(`{"ipv4": "%v"}`, ip.String())
+	}
+
 	lbs, err := l.client.ListNodeBalancers(ctx, &linodego.ListOptions{Filter: filter})
 	if err != nil {
 		return nil, err
 	}
 	if len(lbs) == 0 {
 		return nil, lbNotFoundError{serviceNn: getServiceNn(service)}
 	}
-	klog.V(2).Infof("found NodeBalancer (%d) for service (%s) via IPv4 (%s)", lbs[0].ID, getServiceNn(service), ipv4)
+	klog.V(2).Infof("found NodeBalancer (%d) for service (%s) via IP (%s)", lbs[0].ID, getServiceNn(service), ip.String())
 	return &lbs[0], nil
 }
 

cloud/linode/loadbalancers_test.go

@@ -282,6 +282,10 @@ func TestCCMLoadBalancers(t *testing.T) {
 			name: "Create Load Balancer - Very long Service name",
 			f:    testVeryLongServiceName,
 		},
+		{
+			name: "getNodeBalancerByStatus with IPv4 and IPv6 addresses",
+			f:    testGetNodeBalancerByStatus,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -3660,6 +3664,70 @@ func testUpdateLoadBalancerNoNodes(t *testing.T, client *linodego.Client, _ *fak
 	}
 }
 
+func testGetNodeBalancerByStatus(t *testing.T, client *linodego.Client, _ *fakeAPI) {
+	t.Helper()
+
+	lb, assertion := newLoadbalancers(client, "us-west").(*loadbalancers)
+	if !assertion {
+		t.Error("type assertion failed")
+	}
+	fakeClientset := fake.NewSimpleClientset()
+	lb.kubeClient = fakeClientset
+
+	for _, test := range []struct {
+		name    string
+		service *v1.Service
+	}{
+		{
+			name: "hostname only",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "hostname-ingress-" + randString(),
+					Annotations: map[string]string{annotations.AnnLinodeHostnameOnlyIngress: "true"},
+				},
+			},
+		},
+		{
+			name: "ipv4",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "ipv4-ingress-" + randString(),
+				},
+			},
+		},
+		{
+			name: "ipv4 and ipv6",
+			service: &v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:        "ipv6-ingress-" + randString(),
+					Annotations: map[string]string{annotations.AnnLinodeEnableIPv6Ingress: "true"},
+				},
+			},
+		},
+	} {
+		t.Run(test.name, func(t *testing.T) {
+			expectedNB, err := lb.createNodeBalancer(t.Context(), "linodelb", test.service, []*linodego.NodeBalancerConfigCreateOptions{})
+			if err != nil {
+				t.Fatal(err)
+			}
+			test.service.Status.LoadBalancer = *makeLoadBalancerStatus(test.service, expectedNB)
+
+			actualNB, err := lb.getNodeBalancerByStatus(t.Context(), test.service)
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			if expectedNB.ID != actualNB.ID {
+				t.Error("unexpected nodebalancer ID")
+				t.Logf("expected: %v", expectedNB.ID)
+				t.Logf("actual: %v", actualNB.ID)
+			}
+
+			_ = lb.EnsureLoadBalancerDeleted(t.Context(), "linodelb", test.service)
+		})
+	}
+}
+
 func testGetNodeBalancerForServiceIDDoesNotExist(t *testing.T, client *linodego.Client, _ *fakeAPI) {
 	t.Helper()