Merge pull request #228 from nox-404/fix-unsupported-udp

fix: duplicate backends for service with unsuported udp ports

Author: pablo-ruth

scaleway/loadbalancers.go

@@ -1112,7 +1112,7 @@ func servicePortToFrontend(service *v1.Service, loadbalancer *scwlb.LB, port v1.
 	}
 
 	return &scwlb.Frontend{
-		Name:                fmt.Sprintf("%s_tcp_%d", string(service.UID), port.Port),
+		Name:                fmt.Sprintf("%s_%s_%d", string(service.UID), strings.ToLower(string(port.Protocol)), port.Port),
 		InboundPort:         port.Port,
 		TimeoutClient:       &timeoutClient,
 		ConnectionRateLimit: connectionRateLimit,
@@ -1294,7 +1294,7 @@ func servicePortToBackend(service *v1.Service, loadbalancer *scwlb.LB, port v1.S
 	healthCheck.TransientCheckDelay = healthCheckTransientCheckDelay
 
 	backend := &scwlb.Backend{
-		Name:                   fmt.Sprintf("%s_tcp_%d", string(service.UID), port.NodePort),
+		Name:                   fmt.Sprintf("%s_%s_%d", string(service.UID), strings.ToLower(string(port.Protocol)), port.NodePort),
 		Pool:                   nodeIPs,
 		ForwardProtocol:        protocol,
 		SslBridging:            &sslBridging,
@@ -1332,25 +1332,30 @@ func servicePortToBackend(service *v1.Service, loadbalancer *scwlb.LB, port v1.S
 
 // serviceToLB converts a service definition to a list of load balancer frontends and backends
 func serviceToLB(service *v1.Service, loadbalancer *scwlb.LB, nodeIPs []string) (map[int32]*scwlb.Frontend, map[int32]*scwlb.Backend, error) {
-	frontends := map[int32]*scwlb.Frontend{}
-	backends := map[int32]*scwlb.Backend{}
+	tcpFrontends := map[int32]*scwlb.Frontend{}
+	tcpBackends := map[int32]*scwlb.Backend{}
 
 	for _, port := range service.Spec.Ports {
-		frontend, err := servicePortToFrontend(service, loadbalancer, port)
+		if port.Protocol != v1.ProtocolTCP {
+			klog.V(3).Infof("skipping unsupported protocol %s for port %d in service %s/%s", port.Protocol, port.Port, service.Namespace, service.Name)
+			continue
+		}
+
+		tcpFrontend, err := servicePortToFrontend(service, loadbalancer, port)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to prepare frontend for port %d: %v", port.Port, err)
 		}
 
-		backend, err := servicePortToBackend(service, loadbalancer, port, nodeIPs)
+		tcpBackend, err := servicePortToBackend(service, loadbalancer, port, nodeIPs)
 		if err != nil {
 			return nil, nil, fmt.Errorf("failed to prepare backend for port %d: %v", port.Port, err)
 		}
 
-		frontends[port.Port] = frontend
-		backends[port.NodePort] = backend
+		tcpFrontends[port.Port] = tcpFrontend
+		tcpBackends[port.NodePort] = tcpBackend
 	}
 
-	return frontends, backends, nil
+	return tcpFrontends, tcpBackends, nil
 }
 
 // frontendEquals returns true if the two frontends configuration are equal
@@ -1484,7 +1489,7 @@ func backendEquals(got, want *scwlb.Backend) bool {
 }
 
 type frontendOps struct {
-	remove map[int32]*scwlb.Frontend
+	remove []*scwlb.Frontend
 	update map[int32]*scwlb.Frontend
 	create map[int32]*scwlb.Frontend
 	keep   map[int32]*scwlb.Frontend
@@ -1493,42 +1498,42 @@ type frontendOps struct {
 // compareFrontends returns the frontends operation to do to achieve the wanted configuration
 // will ignore frontends with names not starting with the filterPrefix if provided
 func compareFrontends(got []*scwlb.Frontend, want map[int32]*scwlb.Frontend, filterPrefix string) frontendOps {
-	remove := make(map[int32]*scwlb.Frontend)
+	remove := make([]*scwlb.Frontend, 0)
 	update := make(map[int32]*scwlb.Frontend)
 	create := make(map[int32]*scwlb.Frontend)
 	keep := make(map[int32]*scwlb.Frontend)
 
-	filteredGot := make([]*scwlb.Frontend, 0, len(got))
+	lbEvaluatedFrontendMap := make(map[int32]*scwlb.Frontend)
+
+	// Check for deletions and updates
 	for _, current := range got {
-		if strings.HasPrefix(current.Name, filterPrefix) {
-			filteredGot = append(filteredGot, current)
+		if !strings.HasPrefix(current.Name, filterPrefix) {
+			continue
+		}
+
+		// Will remove duplicated frontends with the same InboundPort.
+		if _, ok := lbEvaluatedFrontendMap[current.InboundPort]; ok {
+			remove = append(remove, current)
+			continue
+		} else {
+			lbEvaluatedFrontendMap[current.InboundPort] = current
 		}
-	}
 
-	// Check for deletions and updates
-	for _, current := range filteredGot {
 		if target, ok := want[current.InboundPort]; ok {
-			if !frontendEquals(current, target) {
+			if frontendEquals(current, target) {
+				keep[target.InboundPort] = current
+			} else {
 				target.ID = current.ID
 				update[target.InboundPort] = target
-			} else {
-				keep[target.InboundPort] = current
 			}
 		} else {
-			remove[current.InboundPort] = current
+			remove = append(remove, current)
 		}
 	}
 
 	// Check for additions
 	for _, target := range want {
-		found := false
-		for _, current := range filteredGot {
-			if current.InboundPort == target.InboundPort {
-				found = true
-				break
-			}
-		}
-		if !found {
+		if _, ok := lbEvaluatedFrontendMap[target.InboundPort]; !ok {
 			create[target.InboundPort] = target
 		}
 	}
@@ -1542,50 +1547,50 @@ func compareFrontends(got []*scwlb.Frontend, want map[int32]*scwlb.Frontend, fil
 }
 
 type backendOps struct {
-	remove map[int32]*scwlb.Backend
+	remove []*scwlb.Backend
 	update map[int32]*scwlb.Backend
 	create map[int32]*scwlb.Backend
 	keep   map[int32]*scwlb.Backend
 }
 
 // compareBackends returns the backends operation to do to achieve the wanted configuration
 func compareBackends(got []*scwlb.Backend, want map[int32]*scwlb.Backend, filterPrefix string) backendOps {
-	remove := make(map[int32]*scwlb.Backend)
+	remove := make([]*scwlb.Backend, 0)
 	update := make(map[int32]*scwlb.Backend)
 	create := make(map[int32]*scwlb.Backend)
 	keep := make(map[int32]*scwlb.Backend)
 
-	filteredGot := make([]*scwlb.Backend, 0, len(got))
+	lbEvaluatedBackendMap := make(map[int32]*scwlb.Backend)
+
+	// Check for deletions and updates
 	for _, current := range got {
-		if strings.HasPrefix(current.Name, filterPrefix) {
-			filteredGot = append(filteredGot, current)
+		if !strings.HasPrefix(current.Name, filterPrefix) {
+			continue
+		}
+
+		// Will remove duplicated backends with the same ForwardPort.
+		if _, ok := lbEvaluatedBackendMap[current.ForwardPort]; ok {
+			remove = append(remove, current)
+			continue
+		} else {
+			lbEvaluatedBackendMap[current.ForwardPort] = current
 		}
-	}
 
-	// Check for deletions and updates
-	for _, current := range filteredGot {
 		if target, ok := want[current.ForwardPort]; ok {
-			if !backendEquals(current, target) {
+			if backendEquals(current, target) {
+				keep[target.ForwardPort] = current
+			} else {
 				target.ID = current.ID
 				update[target.ForwardPort] = target
-			} else {
-				keep[target.ForwardPort] = current
 			}
 		} else {
-			remove[current.ForwardPort] = current
+			remove = append(remove, current)
 		}
 	}
 
 	// Check for additions
 	for _, target := range want {
-		found := false
-		for _, current := range filteredGot {
-			if current.ForwardPort == target.ForwardPort {
-				found = true
-				break
-			}
-		}
-		if !found {
+		if _, ok := lbEvaluatedBackendMap[target.ForwardPort]; !ok {
 			create[target.ForwardPort] = target
 		}
 	}

scaleway/loadbalancers_test.go

@@ -637,7 +637,7 @@ func TestServicePortToFrontend(t *testing.T) {
 
 	for _, tt := range matrix {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := servicePortToFrontend(tt.service, &scwlb.LB{ID: "lbid"}, v1.ServicePort{Port: 1234})
+			got, err := servicePortToFrontend(tt.service, &scwlb.LB{ID: "lbid"}, v1.ServicePort{Port: 1234, Protocol: v1.ProtocolTCP})
 			if tt.wantErr != (err != nil) {
 				t.Errorf("got error: %s, expected: %v", err, tt.wantErr)
 				return
@@ -2607,3 +2607,44 @@ func TestServicePortToBackendWithHealthCheckFromService(t *testing.T) {
 		})
 	}
 }
+
+func TestServiceToLB_UDPPortsIgnored(t *testing.T) {
+	service := &v1.Service{
+		ObjectMeta: metav1.ObjectMeta{
+			UID:         "test-uid",
+			Annotations: map[string]string{},
+		},
+		Spec: v1.ServiceSpec{
+			Ports: []v1.ServicePort{
+				{Port: 80, NodePort: 30080, Protocol: v1.ProtocolTCP},
+				{Port: 53, NodePort: 30053, Protocol: v1.ProtocolUDP},
+			},
+		},
+	}
+
+	frontends, backends, err := serviceToLB(service, &scwlb.LB{ID: "test-lb"}, []string{"10.0.0.1"})
+	if err != nil {
+		t.Fatalf("serviceToLB() error = %v", err)
+	}
+
+	if _, ok := frontends[80]; !ok {
+		t.Error("expected frontend for TCP port 80")
+	}
+	if _, ok := frontends[53]; ok {
+		t.Error("unexpected frontend for UDP port 53")
+	}
+
+	if _, ok := backends[30080]; !ok {
+		t.Error("expected backend for TCP nodePort 30080")
+	}
+	if _, ok := backends[30053]; ok {
+		t.Error("unexpected backend for UDP nodePort 30053")
+	}
+
+	if len(frontends) != 1 {
+		t.Errorf("expected 1 frontend, got %d", len(frontends))
+	}
+	if len(backends) != 1 {
+		t.Errorf("expected 1 backend, got %d", len(backends))
+	}
+}