From 2424b4189b3c47d6553016a68873618959b697dc Mon Sep 17 00:00:00 2001 From: jo Date: Tue, 13 Aug 2024 14:34:10 +0200 Subject: [PATCH] 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. --- hcloud/load_balancers.go | 34 ++++++++------- hcloud/load_balancers_test.go | 82 +++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 16 deletions(-) diff --git a/hcloud/load_balancers.go b/hcloud/load_balancers.go index cf0e478aa..8393a4e6f 100644 --- 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 index 1da764dea..04bdf9bdb 100644 --- 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",