Skip to content

Commit

Permalink
feat(load-balancer): handle planned targets exceedings max targets (#570
Browse files Browse the repository at this point in the history 
)

Catches the error condition before we even make a request to the API.
This causes less requests to be sent to the Hetzner Cloud API.

This does not change the behavior, as we just log & continue right now
if we encounter a `ErrorCodeResourceLimitExceeded` while adding the
target. Both cases are just visible in the HCCM logs (see #569 for
improvements in that regard).

Co-authored-by: Janis Kemper <[email protected]>
  • Loading branch information
@apricote @janiskemper
apricote and janiskemper authored Nov 29, 2023
1 parent 65dea11 commit 8bb131f
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 0 deletions.
16 changes: 16 additions & 0 deletions internal/hcops/load_balancer.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -627,6 +627,8 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
}
}

numberOfTargets := len(lb.Targets)

// Extract IDs of the hc Load Balancer's server targets. Along the way,
// Remove all server targets from the HC Load Balancer which are currently
// not assigned as nodes to the K8S Load Balancer.
Expand All @@ -650,6 +652,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
return changed, fmt.Errorf("%s: target: %s: %w", op, k8sNodeNames[id], err)
}
changed = true
numberOfTargets--
}

// Cleanup of IP Targets happens whether Robot Support is enabled or not.
Expand Down Expand Up @@ -684,6 +687,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
return changed, e
}
changed = true
numberOfTargets--
}
}

Expand All @@ -696,6 +700,11 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
continue
}

if numberOfTargets >= lb.LoadBalancerType.MaxTargets {
klog.InfoS("cannot add server target because max number of targets have been reached", "op", op, "service", svc.ObjectMeta.Name, "targetName", k8sNodeNames[id])
continue
}

klog.InfoS("add target", "op", op, "service", svc.ObjectMeta.Name, "targetName", k8sNodeNames[id])
opts := hcloud.LoadBalancerAddServerTargetOpts{
Server: &hcloud.Server{ID: id},
Expand All @@ -713,6 +722,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
return changed, fmt.Errorf("%s: target %s: %w", op, k8sNodeNames[id], err)
}
changed = true
numberOfTargets++
}

if l.Cfg.Robot.Enabled {
Expand All @@ -731,6 +741,11 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
continue
}

if numberOfTargets >= lb.LoadBalancerType.MaxTargets {
klog.InfoS("cannot add ip target because max number of targets have been reached", "op", op, "service", svc.ObjectMeta.Name, "targetName", k8sNodeNames[int64(id)])
continue
}

klog.InfoS("add target", "op", op, "service", svc.ObjectMeta.Name, "targetName", k8sNodeNames[int64(id)], "ip", ip)
opts := hcloud.LoadBalancerAddIPTargetOpts{
IP: net.ParseIP(ip),
Expand All @@ -747,6 +762,7 @@ func (l *LoadBalancerOps) ReconcileHCLBTargets(
return changed, fmt.Errorf("%s: target %s: %w", op, k8sNodeNames[int64(id)], err)
}
changed = true
numberOfTargets++
}
}

Expand Down
50 changes: 50 additions & 0 deletions internal/hcops/load_balancer_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,9 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
},
initialLB: &hcloud.LoadBalancer{
ID: 1,
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 25,
},
},
robotServers: []hrobotmodels.Server{
{
Expand Down Expand Up @@ -1253,6 +1256,35 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
},
cfg: config.HCCMConfiguration{LoadBalancer: config.LoadBalancerConfiguration{DisableIPv6: true}},
},
{
name: "too many targets",
k8sNodes: []*corev1.Node{
{Spec: corev1.NodeSpec{ProviderID: "hcloud://1"}},
{Spec: corev1.NodeSpec{ProviderID: "hcloud://2"}},
},
initialLB: &hcloud.LoadBalancer{
ID: 2,
Targets: []hcloud.LoadBalancerTarget{
{
Type: hcloud.LoadBalancerTargetTypeServer,
Server: &hcloud.LoadBalancerTargetServer{Server: &hcloud.Server{ID: 1}},
},
},
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 1,
},
},
mock: func(t *testing.T, tt *LBReconcilementTestCase) {
// Nothing to mock because no action will be taken besides logging an info message,
// will fail if an action would be taken instead.
},
perform: func(t *testing.T, tt *LBReconcilementTestCase) {
changed, err := tt.fx.LBOps.ReconcileHCLBTargets(tt.fx.Ctx, tt.initialLB, tt.service, tt.k8sNodes)
assert.NoError(t, err)
assert.False(t, changed)
},
cfg: config.HCCMConfiguration{LoadBalancer: config.LoadBalancerConfiguration{DisableIPv6: true}},
},
{
name: "enable use of private network via default",
cfg: config.HCCMConfiguration{
Expand All @@ -1268,6 +1300,9 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
},
initialLB: &hcloud.LoadBalancer{
ID: 3,
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 25,
},
},
mock: func(t *testing.T, tt *LBReconcilementTestCase) {
tt.fx.LBOps.NetworkID = 4711
Expand Down Expand Up @@ -1306,6 +1341,9 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
},
initialLB: &hcloud.LoadBalancer{
ID: 3,
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 25,
},
},
mock: func(t *testing.T, tt *LBReconcilementTestCase) {
tt.fx.LBOps.NetworkID = 4711
Expand Down Expand Up @@ -1350,6 +1388,9 @@ func TestLoadBalancerOps_ReconcileHCLBTargets(t *testing.T) {
UsePrivateIP: true,
},
},
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 25,
},
},
mock: func(t *testing.T, tt *LBReconcilementTestCase) {
action := tt.fx.MockRemoveServerTarget(tt.initialLB, &hcloud.Server{ID: 1}, nil)
Expand Down Expand Up @@ -1388,6 +1429,9 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
},
initialLB: &hcloud.LoadBalancer{
ID: 4,
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 25,
},
},
mock: func(t *testing.T, tt *LBReconcilementTestCase) {
opts := hcloud.LoadBalancerAddServiceOpts{
Expand Down Expand Up @@ -1427,6 +1471,9 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
},
initialLB: &hcloud.LoadBalancer{
ID: 10,
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 25,
},
},
serviceAnnotations: map[annotation.Name]interface{}{
annotation.LBSvcHTTPCertificates: []string{"1"},
Expand Down Expand Up @@ -1462,6 +1509,9 @@ func TestLoadBalancerOps_ReconcileHCLBServices(t *testing.T) {
},
initialLB: &hcloud.LoadBalancer{
ID: 10,
LoadBalancerType: &hcloud.LoadBalancerType{
MaxTargets: 25,
},
},
serviceAnnotations: map[annotation.Name]interface{}{
annotation.LBSvcHTTPCertificates: []string{"some-cert"},
Expand Down

0 comments on commit 8bb131f

Please sign in to comment.