Skip to content

Commit 3786a46

Browse files
nox-404nox-404
committed
fix: duplicate backends for service with unsuported udp ports
1 parent fda1045 commit 3786a46

File tree

2 files changed

+96
-50
lines changed

2 files changed

+96
-50
lines changed

‎scaleway/loadbalancers.go‎

Lines changed: 54 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,7 +1110,7 @@ func servicePortToFrontend(service *v1.Service, loadbalancer *scwlb.LB, port v1.
11101110
}
11111111

11121112
return &scwlb.Frontend{
1113-
Name: fmt.Sprintf("%s_tcp_%d", string(service.UID), port.Port),
1113+
Name: fmt.Sprintf("%s_%s_%d", string(service.UID), strings.ToLower(string(port.Protocol)), port.Port),
11141114
InboundPort: port.Port,
11151115
TimeoutClient: &timeoutClient,
11161116
ConnectionRateLimit: connectionRateLimit,
@@ -1292,7 +1292,7 @@ func servicePortToBackend(service *v1.Service, loadbalancer *scwlb.LB, port v1.S
12921292
healthCheck.TransientCheckDelay = healthCheckTransientCheckDelay
12931293

12941294
backend := &scwlb.Backend{
1295-
Name: fmt.Sprintf("%s_tcp_%d", string(service.UID), port.NodePort),
1295+
Name: fmt.Sprintf("%s_%s_%d", string(service.UID), strings.ToLower(string(port.Protocol)), port.NodePort),
12961296
Pool: nodeIPs,
12971297
ForwardProtocol: protocol,
12981298
SslBridging: &sslBridging,
@@ -1330,25 +1330,30 @@ func servicePortToBackend(service *v1.Service, loadbalancer *scwlb.LB, port v1.S
13301330

13311331
// serviceToLB converts a service definition to a list of load balancer frontends and backends
13321332
func serviceToLB(service *v1.Service, loadbalancer *scwlb.LB, nodeIPs []string) (map[int32]*scwlb.Frontend, map[int32]*scwlb.Backend, error) {
1333-
frontends := map[int32]*scwlb.Frontend{}
1334-
backends := map[int32]*scwlb.Backend{}
1333+
tcpFrontends := map[int32]*scwlb.Frontend{}
1334+
tcpBackends := map[int32]*scwlb.Backend{}
13351335

13361336
for _, port := range service.Spec.Ports {
1337-
frontend, err := servicePortToFrontend(service, loadbalancer, port)
1337+
if port.Protocol != v1.ProtocolTCP {
1338+
klog.V(3).Infof("skipping unsupported protocol %s for port %d in service %s/%s", port.Protocol, port.Port, service.Namespace, service.Name)
1339+
continue
1340+
}
1341+
1342+
tcpFrontend, err := servicePortToFrontend(service, loadbalancer, port)
13381343
if err != nil {
13391344
return nil, nil, fmt.Errorf("failed to prepare frontend for port %d: %v", port.Port, err)
13401345
}
13411346

1342-
backend, err := servicePortToBackend(service, loadbalancer, port, nodeIPs)
1347+
tcpBackend, err := servicePortToBackend(service, loadbalancer, port, nodeIPs)
13431348
if err != nil {
13441349
return nil, nil, fmt.Errorf("failed to prepare backend for port %d: %v", port.Port, err)
13451350
}
13461351

1347-
frontends[port.Port] = frontend
1348-
backends[port.NodePort] = backend
1352+
tcpFrontends[port.Port] = tcpFrontend
1353+
tcpBackends[port.NodePort] = tcpBackend
13491354
}
13501355

1351-
return frontends, backends, nil
1356+
return tcpFrontends, tcpBackends, nil
13521357
}
13531358

13541359
// frontendEquals returns true if the two frontends configuration are equal
@@ -1482,7 +1487,7 @@ func backendEquals(got, want *scwlb.Backend) bool {
14821487
}
14831488

14841489
type frontendOps struct {
1485-
remove map[int32]*scwlb.Frontend
1490+
remove []*scwlb.Frontend
14861491
update map[int32]*scwlb.Frontend
14871492
create map[int32]*scwlb.Frontend
14881493
keep map[int32]*scwlb.Frontend
@@ -1491,42 +1496,42 @@ type frontendOps struct {
14911496
// compareFrontends returns the frontends operation to do to achieve the wanted configuration
14921497
// will ignore frontends with names not starting with the filterPrefix if provided
14931498
func compareFrontends(got []*scwlb.Frontend, want map[int32]*scwlb.Frontend, filterPrefix string) frontendOps {
1494-
remove := make(map[int32]*scwlb.Frontend)
1499+
remove := make([]*scwlb.Frontend, 0)
14951500
update := make(map[int32]*scwlb.Frontend)
14961501
create := make(map[int32]*scwlb.Frontend)
14971502
keep := make(map[int32]*scwlb.Frontend)
14981503

1499-
filteredGot := make([]*scwlb.Frontend, 0, len(got))
1504+
lbEvaluatedFrontendMap := make(map[int32]*scwlb.Frontend)
1505+
1506+
// Check for deletions and updates
15001507
for _, current := range got {
1501-
if strings.HasPrefix(current.Name, filterPrefix) {
1502-
filteredGot = append(filteredGot, current)
1508+
if !strings.HasPrefix(current.Name, filterPrefix) {
1509+
continue
1510+
}
1511+
1512+
// Will remove duplicated frontends with the same InboundPort.
1513+
if _, ok := lbEvaluatedFrontendMap[current.InboundPort]; ok {
1514+
remove = append(remove, current)
1515+
continue
1516+
} else {
1517+
lbEvaluatedFrontendMap[current.InboundPort] = current
15031518
}
1504-
}
15051519

1506-
// Check for deletions and updates
1507-
for _, current := range filteredGot {
15081520
if target, ok := want[current.InboundPort]; ok {
1509-
if !frontendEquals(current, target) {
1521+
if frontendEquals(current, target) {
1522+
keep[target.InboundPort] = current
1523+
} else {
15101524
target.ID = current.ID
15111525
update[target.InboundPort] = target
1512-
} else {
1513-
keep[target.InboundPort] = current
15141526
}
15151527
} else {
1516-
remove[current.InboundPort] = current
1528+
remove = append(remove, current)
15171529
}
15181530
}
15191531

15201532
// Check for additions
15211533
for _, target := range want {
1522-
found := false
1523-
for _, current := range filteredGot {
1524-
if current.InboundPort == target.InboundPort {
1525-
found = true
1526-
break
1527-
}
1528-
}
1529-
if !found {
1534+
if _, ok := lbEvaluatedFrontendMap[target.InboundPort]; !ok {
15301535
create[target.InboundPort] = target
15311536
}
15321537
}
@@ -1540,50 +1545,50 @@ func compareFrontends(got []*scwlb.Frontend, want map[int32]*scwlb.Frontend, fil
15401545
}
15411546

15421547
type backendOps struct {
1543-
remove map[int32]*scwlb.Backend
1548+
remove []*scwlb.Backend
15441549
update map[int32]*scwlb.Backend
15451550
create map[int32]*scwlb.Backend
15461551
keep map[int32]*scwlb.Backend
15471552
}
15481553

15491554
// compareBackends returns the backends operation to do to achieve the wanted configuration
15501555
func compareBackends(got []*scwlb.Backend, want map[int32]*scwlb.Backend, filterPrefix string) backendOps {
1551-
remove := make(map[int32]*scwlb.Backend)
1556+
remove := make([]*scwlb.Backend, 0)
15521557
update := make(map[int32]*scwlb.Backend)
15531558
create := make(map[int32]*scwlb.Backend)
15541559
keep := make(map[int32]*scwlb.Backend)
15551560

1556-
filteredGot := make([]*scwlb.Backend, 0, len(got))
1561+
lbEvaluatedBackendMap := make(map[int32]*scwlb.Backend)
1562+
1563+
// Check for deletions and updates
15571564
for _, current := range got {
1558-
if strings.HasPrefix(current.Name, filterPrefix) {
1559-
filteredGot = append(filteredGot, current)
1565+
if !strings.HasPrefix(current.Name, filterPrefix) {
1566+
continue
1567+
}
1568+
1569+
// Will remove duplicated backends with the same ForwardPort.
1570+
if _, ok := lbEvaluatedBackendMap[current.ForwardPort]; ok {
1571+
remove = append(remove, current)
1572+
continue
1573+
} else {
1574+
lbEvaluatedBackendMap[current.ForwardPort] = current
15601575
}
1561-
}
15621576

1563-
// Check for deletions and updates
1564-
for _, current := range filteredGot {
15651577
if target, ok := want[current.ForwardPort]; ok {
1566-
if !backendEquals(current, target) {
1578+
if backendEquals(current, target) {
1579+
keep[target.ForwardPort] = current
1580+
} else {
15671581
target.ID = current.ID
15681582
update[target.ForwardPort] = target
1569-
} else {
1570-
keep[target.ForwardPort] = current
15711583
}
15721584
} else {
1573-
remove[current.ForwardPort] = current
1585+
remove = append(remove, current)
15741586
}
15751587
}
15761588

15771589
// Check for additions
15781590
for _, target := range want {
1579-
found := false
1580-
for _, current := range filteredGot {
1581-
if current.ForwardPort == target.ForwardPort {
1582-
found = true
1583-
break
1584-
}
1585-
}
1586-
if !found {
1591+
if _, ok := lbEvaluatedBackendMap[target.ForwardPort]; !ok {
15871592
create[target.ForwardPort] = target
15881593
}
15891594
}

‎scaleway/loadbalancers_test.go‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ func TestServicePortToFrontend(t *testing.T) {
637637

638638
for _, tt := range matrix {
639639
t.Run(tt.name, func(t *testing.T) {
640-
got, err := servicePortToFrontend(tt.service, &scwlb.LB{ID: "lbid"}, v1.ServicePort{Port: 1234})
640+
got, err := servicePortToFrontend(tt.service, &scwlb.LB{ID: "lbid"}, v1.ServicePort{Port: 1234, Protocol: v1.ProtocolTCP})
641641
if tt.wantErr != (err != nil) {
642642
t.Errorf("got error: %s, expected: %v", err, tt.wantErr)
643643
return
@@ -2561,3 +2561,44 @@ func TestServicePortToBackendWithHealthCheckFromService(t *testing.T) {
25612561
})
25622562
}
25632563
}
2564+
2565+
func TestServiceToLB_UDPPortsIgnored(t *testing.T) {
2566+
service := &v1.Service{
2567+
ObjectMeta: metav1.ObjectMeta{
2568+
UID: "test-uid",
2569+
Annotations: map[string]string{},
2570+
},
2571+
Spec: v1.ServiceSpec{
2572+
Ports: []v1.ServicePort{
2573+
{Port: 80, NodePort: 30080, Protocol: v1.ProtocolTCP},
2574+
{Port: 53, NodePort: 30053, Protocol: v1.ProtocolUDP},
2575+
},
2576+
},
2577+
}
2578+
2579+
frontends, backends, err := serviceToLB(service, &scwlb.LB{ID: "test-lb"}, []string{"10.0.0.1"})
2580+
if err != nil {
2581+
t.Fatalf("serviceToLB() error = %v", err)
2582+
}
2583+
2584+
if _, ok := frontends[80]; !ok {
2585+
t.Error("expected frontend for TCP port 80")
2586+
}
2587+
if _, ok := frontends[53]; ok {
2588+
t.Error("unexpected frontend for UDP port 53")
2589+
}
2590+
2591+
if _, ok := backends[30080]; !ok {
2592+
t.Error("expected backend for TCP nodePort 30080")
2593+
}
2594+
if _, ok := backends[30053]; ok {
2595+
t.Error("unexpected backend for UDP nodePort 30053")
2596+
}
2597+
2598+
if len(frontends) != 1 {
2599+
t.Errorf("expected 1 frontend, got %d", len(frontends))
2600+
}
2601+
if len(backends) != 1 {
2602+
t.Errorf("expected 1 backend, got %d", len(backends))
2603+
}
2604+
}

0 commit comments

Comments
 (0)