fix: populate ingress ip when disable-public-network is set

Closes #553 Refactor getting the ingresses for a load-balacencer for both get load-balancer and ensure load-balancer, into using a shared function. This ensure we are consistent between the 2 methods.
hetznercloud · Aug 13, 2024 · ff61181 · ff61181
1 parent 5742d63
commit ff61181
Show file tree

Hide file tree

Showing 2 changed files with 100 additions and 16 deletions.
diff --git a/hcloud/load_balancers.go b/hcloud/load_balancers.go
@@ -86,23 +86,12 @@ func (l *loadBalancers) GetLoadBalancer(
 		}, true, nil
 	}
 
-	ingresses := []corev1.LoadBalancerIngress{
-		{
-			IP: lb.PublicNet.IPv4.IP.String(),
-		},
-	}
-
-	disableIPV6, err := l.getDisableIPv6(service)
+	ingress, err := l.buildLoadBalancerStatusIngress(lb, service)
 	if err != nil {
 		return nil, false, fmt.Errorf("%s: %v", op, err)
 	}
-	if !disableIPV6 {
-		ingresses = append(ingresses, corev1.LoadBalancerIngress{
-			IP: lb.PublicNet.IPv6.IP.String(),
-		})
-	}
 
-	return &corev1.LoadBalancerStatus{Ingress: ingresses}, true, nil
+	return &corev1.LoadBalancerStatus{Ingress: ingress}, true, nil
 }
 
 func (l *loadBalancers) GetLoadBalancerName(_ context.Context, _ string, service *corev1.Service) string {
@@ -203,6 +192,18 @@ func (l *loadBalancers) EnsureLoadBalancer(
 		}, nil
 	}
 
+	ingress, err := l.buildLoadBalancerStatusIngress(lb, svc)
+	if err != nil {
+		return nil, fmt.Errorf("%s: %w", op, err)
+	}
+
+	return &corev1.LoadBalancerStatus{Ingress: ingress}, nil
+}
+
+func (l *loadBalancers) buildLoadBalancerStatusIngress(lb *hcloud.LoadBalancer, svc *corev1.Service) ([]corev1.LoadBalancerIngress, error) {
+	const op = "hcloud/loadBalancers.getLoadBalancerStatusIngress"
+	metrics.OperationCalled.WithLabelValues(op).Inc()
+
 	var ingress []corev1.LoadBalancerIngress
 
 	disablePubNet, err := annotation.LBDisablePublicNetwork.BoolFromService(svc)
@@ -226,13 +227,14 @@ func (l *loadBalancers) EnsureLoadBalancer(
 	if err != nil {
 		return nil, fmt.Errorf("%s: %w", op, err)
 	}
+
 	if !disablePrivIngress {
-		for _, nw := range lb.PrivateNet {
-			ingress = append(ingress, corev1.LoadBalancerIngress{IP: nw.IP.String()})
+		for _, privateNet := range lb.PrivateNet {
+			ingress = append(ingress, corev1.LoadBalancerIngress{IP: privateNet.IP.String()})
 		}
 	}
 
-	return &corev1.LoadBalancerStatus{Ingress: ingress}, nil
+	return ingress, nil
 }
 
 func (l *loadBalancers) getDisablePrivateIngress(svc *corev1.Service) (bool, error) {

diff --git a/hcloud/load_balancers_test.go b/hcloud/load_balancers_test.go
@@ -105,6 +105,88 @@ func TestLoadBalancers_GetLoadBalancer(t *testing.T) {
 				assert.Equal(t, "hostname", status.Ingress[0].Hostname)
 			},
 		},
+		{
+			Name:       "get load balancer with private network",
+			ServiceUID: "1",
+			LB: &hcloud.LoadBalancer{
+				ID:               1,
+				Name:             "with-priv-net",
+				LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"},
+				Location:         &hcloud.Location{Name: "nbg1", NetworkZone: hcloud.NetworkZoneEUCentral},
+				PublicNet: hcloud.LoadBalancerPublicNet{
+					Enabled: true,
+					IPv4:    hcloud.LoadBalancerPublicNetIPv4{IP: net.ParseIP("1.2.3.4")},
+					IPv6:    hcloud.LoadBalancerPublicNetIPv6{IP: net.ParseIP("fe80::1")},
+				},
+				PrivateNet: []hcloud.LoadBalancerPrivateNet{
+					{
+						Network: &hcloud.Network{
+							ID:   4711,
+							Name: "priv-net",
+						},
+						IP: net.ParseIP("10.10.10.2"),
+					},
+				},
+			},
+			Mock: func(_ *testing.T, tt *LoadBalancerTestCase) {
+				tt.LBOps.
+					On("GetByK8SServiceUID", tt.Ctx, tt.Service).
+					Return(tt.LB, nil)
+			},
+			Perform: func(t *testing.T, tt *LoadBalancerTestCase) {
+				status, exists, err := tt.LoadBalancers.GetLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service)
+				assert.NoError(t, err)
+				assert.True(t, exists)
+
+				if !assert.Len(t, status.Ingress, 3) {
+					return
+				}
+				assert.Equal(t, tt.LB.PublicNet.IPv4.IP.String(), status.Ingress[0].IP)
+				assert.Equal(t, tt.LB.PublicNet.IPv6.IP.String(), status.Ingress[1].IP)
+				assert.Equal(t, tt.LB.PrivateNet[0].IP.String(), status.Ingress[2].IP)
+			},
+		},
+		{
+			Name:                         "get load balancer with private network and without private ingress",
+			ServiceUID:                   "1",
+			DisablePrivateIngressDefault: true,
+			LB: &hcloud.LoadBalancer{
+				ID:               1,
+				Name:             "with-priv-net-without-private-ingress",
+				LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"},
+				Location:         &hcloud.Location{Name: "nbg1", NetworkZone: hcloud.NetworkZoneEUCentral},
+				PublicNet: hcloud.LoadBalancerPublicNet{
+					Enabled: true,
+					IPv4:    hcloud.LoadBalancerPublicNetIPv4{IP: net.ParseIP("1.2.3.4")},
+					IPv6:    hcloud.LoadBalancerPublicNetIPv6{IP: net.ParseIP("fe80::1")},
+				},
+				PrivateNet: []hcloud.LoadBalancerPrivateNet{
+					{
+						Network: &hcloud.Network{
+							ID:   4711,
+							Name: "priv-net",
+						},
+						IP: net.ParseIP("10.10.10.2"),
+					},
+				},
+			},
+			Mock: func(_ *testing.T, tt *LoadBalancerTestCase) {
+				tt.LBOps.
+					On("GetByK8SServiceUID", tt.Ctx, tt.Service).
+					Return(tt.LB, nil)
+			},
+			Perform: func(t *testing.T, tt *LoadBalancerTestCase) {
+				status, exists, err := tt.LoadBalancers.GetLoadBalancer(tt.Ctx, tt.ClusterName, tt.Service)
+				assert.NoError(t, err)
+				assert.True(t, exists)
+
+				if !assert.Len(t, status.Ingress, 2) {
+					return
+				}
+				assert.Equal(t, tt.LB.PublicNet.IPv4.IP.String(), status.Ingress[0].IP)
+				assert.Equal(t, tt.LB.PublicNet.IPv6.IP.String(), status.Ingress[1].IP)
+			},
+		},
 		{
 			Name:       "load balancer not found",
 			ServiceUID: "3",