Skip to content

Commit 4801f0e

Browse files
lhp-nemliglhp-nemlig
authored
feat(service): Specify private ip for loadbalancer (#724)
Allows users to specify the IPv4 of the load balancer in the private network via the annotation "load-balancer.hetzner.cloud/private-ipv4" Removes network logic from main load balancer creation, since the API does not support specifying the IP address there, and moves network logic to exclusively happen in AttachToNetwork and DetachFromNetwork. I also added an extra LoadBalancer refresh in the main reconciliation loop, since changing the IP address of an SVC object causes network detach and subsequent attach, which in turn needs to cause the targets to be re-added to loadbalancer if they were private network targets. Unit tests pass and i have tested this in a few scenarios on a cluster of my own.
1 parent 176990a commit 4801f0e

File tree

4 files changed

+114
-79
lines changed

4 files changed

+114
-79
lines changed

‎hcloud/load_balancers.go‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,21 @@ func (l *loadBalancers) EnsureLoadBalancer(
160160
}
161161
reload = reload || lbChanged
162162

163+
// Reload early here if reload is true.
164+
// If the load balancer private network ip changed,
165+
// the load balancer would be detached and re-attached to the network
166+
// As a result all of the private network targets would have been
167+
// removed and we should make sure the lb state here matches the actual
168+
// lb state so that we can re-attach the targets if needed
169+
if reload {
170+
klog.InfoS("reload HC Load Balancer", "op", op, "loadBalancerID", lb.ID)
171+
lb, err = l.lbOps.GetByID(ctx, lb.ID)
172+
if err != nil {
173+
return nil, fmt.Errorf("%s: %w", op, err)
174+
}
175+
reload = false
176+
}
177+
163178
servicesChanged, err := l.lbOps.ReconcileHCLBServices(ctx, lb, svc)
164179
if err != nil {
165180
return nil, fmt.Errorf("%s: %w", op, err)

‎internal/annotation/load_balancer.go‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,10 @@ const (
5454
// Load Balancer server targets.
5555
LBUsePrivateIP Name = "load-balancer.hetzner.cloud/use-private-ip"
5656

57+
// LBPrivateIPv4 specifies the IPv4 address to assign to the load balancer in the
58+
// private network that it's attached to.
59+
LBPrivateIPv4 Name = "load-balancer.hetzner.cloud/private-ipv4"
60+
5761
// LBHostname specifies the hostname of the Load Balancer. This will be
5862
// used as ingress address instead of the Load Balancer IP addresses if
5963
// specified.

‎internal/hcops/load_balancer.go‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -203,16 +203,6 @@ func (l *LoadBalancerOps) Create(
203203
opts.Algorithm = &hcloud.LoadBalancerAlgorithm{Type: algType}
204204
}
205205

206-
if l.NetworkID > 0 {
207-
nw, _, err := l.NetworkClient.GetByID(ctx, l.NetworkID)
208-
if err != nil {
209-
return nil, fmt.Errorf("%s: get network %d: %w", op, l.NetworkID, err)
210-
}
211-
if nw == nil {
212-
return nil, fmt.Errorf("%s: get network %d: %w", op, l.NetworkID, ErrNotFound)
213-
}
214-
opts.Network = nw
215-
}
216206
disablePubIface, err := annotation.LBDisablePublicNetwork.BoolFromService(svc)
217207
if err != nil && !errors.Is(err, annotation.ErrNotSet) {
218208
return nil, fmt.Errorf("%s: %w", op, err)
@@ -289,13 +279,13 @@ func (l *LoadBalancerOps) ReconcileHCLB(ctx context.Context, lb *hcloud.LoadBala
289279
}
290280
changed = changed || typeChanged
291281

292-
networkDetached, err := l.detachFromNetwork(ctx, lb)
282+
networkDetached, err := l.detachFromNetwork(ctx, lb, svc)
293283
if err != nil {
294284
return changed, fmt.Errorf("%s: %w", op, err)
295285
}
296286
changed = changed || networkDetached
297287

298-
networkAttached, err := l.attachToNetwork(ctx, lb)
288+
networkAttached, err := l.attachToNetwork(ctx, lb, svc)
299289
if err != nil {
300290
return changed, fmt.Errorf("%s: %w", op, err)
301291
}
@@ -457,19 +447,21 @@ func (l *LoadBalancerOps) changeType(ctx context.Context, lb *hcloud.LoadBalance
457447
return true, nil
458448
}
459449

460-
func (l *LoadBalancerOps) detachFromNetwork(ctx context.Context, lb *hcloud.LoadBalancer) (bool, error) {
450+
func (l *LoadBalancerOps) detachFromNetwork(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) {
461451
const op = "hcops/LoadBalancerOps.detachFromNetwork"
462452
metrics.OperationCalled.WithLabelValues(op).Inc()
463453

464454
var changed bool
465455

456+
privateIPv4, privateIPv4configured := annotation.LBPrivateIPv4.StringFromService(svc)
466457
for _, lbpn := range lb.PrivateNet {
467458
// Don't detach the Load Balancer from the network it is supposed to
468-
// be attached to.
469-
if l.NetworkID == lbpn.Network.ID {
459+
// be attached to and the current private IP of the load balancer matches
460+
// the one configured by the user, if one is configured.
461+
if l.NetworkID == lbpn.Network.ID && (!privateIPv4configured || privateIPv4 == lbpn.IP.String()) {
470462
continue
471463
}
472-
klog.InfoS("detach from network", "op", op, "loadBalancerID", lb.ID, "networkID", lbpn.Network.ID)
464+
klog.InfoS("detach from network", "op", op, "loadBalancerID", lb.ID, "networkID", lbpn.Network.ID, "privateIPv4", lbpn.IP.String())
473465

474466
opts := hcloud.LoadBalancerDetachFromNetworkOpts{Network: lbpn.Network}
475467
a, _, err := l.LBClient.DetachFromNetwork(ctx, lb, opts)
@@ -484,16 +476,30 @@ func (l *LoadBalancerOps) detachFromNetwork(ctx context.Context, lb *hcloud.Load
484476
return changed, nil
485477
}
486478

487-
func (l *LoadBalancerOps) attachToNetwork(ctx context.Context, lb *hcloud.LoadBalancer) (bool, error) {
479+
func (l *LoadBalancerOps) attachToNetwork(ctx context.Context, lb *hcloud.LoadBalancer, svc *corev1.Service) (bool, error) {
488480
const op = "hcops/LoadBalancerOps.attachToNetwork"
489481
metrics.OperationCalled.WithLabelValues(op).Inc()
490482

483+
privateIPv4String, privateIPv4configured := annotation.LBPrivateIPv4.StringFromService(svc)
491484
// Don't attach the Load Balancer if network is not set, or the load
492485
// balancer is already attached.
493-
if l.NetworkID == 0 || lbAttached(lb, l.NetworkID) {
486+
if l.NetworkID == 0 || lbAttached(lb, l.NetworkID, privateIPv4String) {
494487
return false, nil
495488
}
496-
klog.InfoS("attach to network", "op", op, "loadBalancerID", lb.ID, "networkID", l.NetworkID)
489+
490+
var privateIPv4 net.IP
491+
if privateIPv4configured {
492+
privateIPv4 = net.ParseIP(privateIPv4String)
493+
if privateIPv4 == nil {
494+
return false, fmt.Errorf("%s: %w", op, fmt.Errorf("could not parse private IPv4 '%s'", privateIPv4))
495+
}
496+
}
497+
498+
if privateIPv4 != nil {
499+
klog.InfoS("attach to network", "op", op, "loadBalancerID", lb.ID, "networkID", l.NetworkID, "privateIP", privateIPv4)
500+
} else {
501+
klog.InfoS("attach to network", "op", op, "loadBalancerID", lb.ID, "networkID", l.NetworkID)
502+
}
497503

498504
nw, _, err := l.NetworkClient.GetByID(ctx, l.NetworkID)
499505
if err != nil {
@@ -508,6 +514,9 @@ func (l *LoadBalancerOps) attachToNetwork(ctx context.Context, lb *hcloud.LoadBa
508514
retryDelay = time.Second
509515
}
510516
opts := hcloud.LoadBalancerAttachToNetworkOpts{Network: nw}
517+
if privateIPv4 != nil {
518+
opts.IP = privateIPv4
519+
}
511520
a, _, err := l.LBClient.AttachToNetwork(ctx, lb, opts)
512521
if hcloud.IsError(err, hcloud.ErrorCodeConflict, hcloud.ErrorCodeLocked) {
513522
klog.InfoS("retry due to conflict or lock",
@@ -1342,9 +1351,9 @@ func (b *hclbServiceOptsBuilder) buildUpdateServiceOpts() (hcloud.LoadBalancerUp
13421351
return opts, nil
13431352
}
13441353

1345-
func lbAttached(lb *hcloud.LoadBalancer, nwID int64) bool {
1354+
func lbAttached(lb *hcloud.LoadBalancer, nwID int64, privateIPv4 string) bool {
13461355
for _, nw := range lb.PrivateNet {
1347-
if nw.Network.ID == nwID {
1356+
if nw.Network.ID == nwID && (privateIPv4 == "" || privateIPv4 == nw.IP.String()) {
13481357
return true
13491358
}
13501359
}

‎internal/hcops/load_balancer_test.go‎

Lines changed: 65 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -438,56 +438,6 @@ func TestLoadBalancerOps_Create(t *testing.T) {
438438
},
439439
err: fmt.Errorf("hcops/LoadBalancerOps.Create: annotation/Name.LBAlgorithmTypeFromService: annotation/validateAlgorithmType: invalid: invalidtype"),
440440
},
441-
{
442-
name: "attach Load Balancer to private network",
443-
serviceAnnotations: map[annotation.Name]interface{}{
444-
annotation.LBLocation: "nbg1",
445-
},
446-
createOpts: hcloud.LoadBalancerCreateOpts{
447-
Name: "lb-with-priv",
448-
LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"},
449-
Location: &hcloud.Location{Name: "nbg1"},
450-
Network: &hcloud.Network{
451-
ID: 4711,
452-
Name: "some-network",
453-
},
454-
Labels: map[string]string{
455-
hcops.LabelServiceUID: "lb-with-priv-uid",
456-
},
457-
},
458-
mock: func(_ *testing.T, tt *testCase, fx *hcops.LoadBalancerOpsFixture) {
459-
fx.LBOps.NetworkID = tt.createOpts.Network.ID
460-
fx.NetworkClient.
461-
On("GetByID", fx.Ctx, fx.LBOps.NetworkID).
462-
Return(tt.createOpts.Network, nil, nil)
463-
action := fx.MockCreate(tt.createOpts, tt.lb, nil)
464-
fx.MockGetByID(tt.lb, nil)
465-
fx.ActionClient.On("WaitFor", fx.Ctx, action).Return(nil)
466-
},
467-
lb: &hcloud.LoadBalancer{ID: 5},
468-
},
469-
{
470-
name: "fail if network could not be found",
471-
serviceAnnotations: map[annotation.Name]interface{}{
472-
annotation.LBLocation: "nbg1",
473-
},
474-
mock: func(_ *testing.T, _ *testCase, fx *hcops.LoadBalancerOpsFixture) {
475-
fx.LBOps.NetworkID = 4711
476-
fx.NetworkClient.On("GetByID", fx.Ctx, fx.LBOps.NetworkID).Return(nil, nil, nil)
477-
},
478-
err: fmt.Errorf("hcops/LoadBalancerOps.Create: get network %d: %w", 4711, hcops.ErrNotFound),
479-
},
480-
{
481-
name: "fail if looking for network returns an error",
482-
serviceAnnotations: map[annotation.Name]interface{}{
483-
annotation.LBLocation: "nbg1",
484-
},
485-
mock: func(_ *testing.T, _ *testCase, fx *hcops.LoadBalancerOpsFixture) {
486-
fx.LBOps.NetworkID = 4712
487-
fx.NetworkClient.On("GetByID", fx.Ctx, fx.LBOps.NetworkID).Return(nil, nil, errTestLbClient)
488-
},
489-
err: fmt.Errorf("hcops/LoadBalancerOps.Create: get network %d: %w", 4712, errTestLbClient),
490-
},
491441
{
492442
name: "disable public interface",
493443
serviceAnnotations: map[annotation.Name]interface{}{
@@ -499,19 +449,11 @@ func TestLoadBalancerOps_Create(t *testing.T) {
499449
LoadBalancerType: &hcloud.LoadBalancerType{Name: "lb11"},
500450
Location: &hcloud.Location{Name: "nbg1"},
501451
PublicInterface: hcloud.Ptr(false),
502-
Network: &hcloud.Network{
503-
ID: 4711,
504-
Name: "some-network",
505-
},
506452
Labels: map[string]string{
507453
hcops.LabelServiceUID: "lb-with-priv-uid",
508454
},
509455
},
510456
mock: func(_ *testing.T, tt *testCase, fx *hcops.LoadBalancerOpsFixture) {
511-
fx.LBOps.NetworkID = tt.createOpts.Network.ID
512-
513-
fx.NetworkClient.On("GetByID", fx.Ctx, fx.LBOps.NetworkID).Return(tt.createOpts.Network, nil, nil)
514-
515457
action := fx.MockCreate(tt.createOpts, tt.lb, nil)
516458
fx.MockGetByID(tt.lb, nil)
517459
fx.ActionClient.On("WaitFor", fx.Ctx, action).Return(nil)
@@ -868,6 +810,48 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) {
868810
assert.True(t, changed)
869811
},
870812
},
813+
{
814+
name: "reattach Load Balancer to network because private ipv4 annotation changed",
815+
initialLB: &hcloud.LoadBalancer{
816+
ID: 4,
817+
PrivateNet: []hcloud.LoadBalancerPrivateNet{
818+
{
819+
Network: &hcloud.Network{ID: 14, Name: "some-network"},
820+
IP: net.ParseIP("10.10.10.3"),
821+
},
822+
},
823+
},
824+
serviceAnnotations: map[annotation.Name]interface{}{
825+
annotation.LBPrivateIPv4: "10.10.10.2",
826+
},
827+
mock: func(_ *testing.T, tt *LBReconcilementTestCase) {
828+
nw := &hcloud.Network{ID: 14, Name: "some-network"}
829+
detachOpts := hcloud.LoadBalancerDetachFromNetworkOpts{
830+
Network: nw,
831+
}
832+
833+
tt.fx.LBOps.NetworkID = 14
834+
835+
attachOpts := hcloud.LoadBalancerAttachToNetworkOpts{
836+
Network: nw,
837+
IP: net.ParseIP("10.10.10.2"),
838+
}
839+
840+
detachAction := &hcloud.Action{ID: rand.Int63()}
841+
tt.fx.LBClient.On("DetachFromNetwork", tt.fx.Ctx, tt.initialLB, detachOpts).Return(detachAction, nil, nil)
842+
tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, detachAction).Return(nil)
843+
844+
tt.fx.NetworkClient.On("GetByID", tt.fx.Ctx, nw.ID).Return(nw, nil, nil)
845+
attachAction := &hcloud.Action{ID: rand.Int63()}
846+
tt.fx.LBClient.On("AttachToNetwork", tt.fx.Ctx, tt.initialLB, attachOpts).Return(attachAction, nil, nil)
847+
tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, attachAction).Return(nil)
848+
},
849+
perform: func(t *testing.T, tt *LBReconcilementTestCase) {
850+
changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service)
851+
assert.NoError(t, err)
852+
assert.True(t, changed)
853+
},
854+
},
871855
{
872856
name: "don't detach Load Balancer from current network",
873857
initialLB: &hcloud.LoadBalancer{
@@ -907,6 +891,29 @@ func TestLoadBalancerOps_ReconcileHCLB(t *testing.T) {
907891
assert.True(t, changed)
908892
},
909893
},
894+
{
895+
name: "attach Load Balancer to network with specific IP",
896+
initialLB: &hcloud.LoadBalancer{ID: 4},
897+
serviceAnnotations: map[annotation.Name]interface{}{
898+
annotation.LBPrivateIPv4: "10.10.10.2",
899+
},
900+
mock: func(_ *testing.T, tt *LBReconcilementTestCase) {
901+
nw := &hcloud.Network{ID: 15, Name: "some-network"}
902+
tt.fx.NetworkClient.On("GetByID", tt.fx.Ctx, nw.ID).Return(nw, nil, nil)
903+
904+
tt.fx.LBOps.NetworkID = nw.ID
905+
906+
opts := hcloud.LoadBalancerAttachToNetworkOpts{Network: nw, IP: net.ParseIP("10.10.10.2")}
907+
action := &hcloud.Action{ID: rand.Int63()}
908+
tt.fx.LBClient.On("AttachToNetwork", tt.fx.Ctx, tt.initialLB, opts).Return(action, nil, nil)
909+
tt.fx.ActionClient.On("WaitFor", tt.fx.Ctx, action).Return(nil)
910+
},
911+
perform: func(t *testing.T, tt *LBReconcilementTestCase) {
912+
changed, err := tt.fx.LBOps.ReconcileHCLB(tt.fx.Ctx, tt.initialLB, tt.service)
913+
assert.NoError(t, err)
914+
assert.True(t, changed)
915+
},
916+
},
910917
{
911918
name: "re-try attach to network on conflict",
912919
initialLB: &hcloud.LoadBalancer{ID: 5},

0 commit comments

Comments
 (0)