diff --git a/pkg/cloudprovider/providers/oci/load_balancer_spec.go b/pkg/cloudprovider/providers/oci/load_balancer_spec.go index 2bfb0626b0..2d285e82b8 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_spec.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_spec.go @@ -585,14 +585,14 @@ func getRuleManagementMode(svc *v1.Service) (string, *ManagedNetworkSecurityGrou return secListMode, &nsg, err } - if strings.ToLower(annotationValue) == strings.ToLower(RuleManagementModeSlAll) { + if strings.EqualFold(annotationValue, RuleManagementModeSlAll) { return ManagementModeAll, &nsg, nil } - if strings.ToLower(annotationValue) == strings.ToLower(RuleManagementModeSlFrontend) { + if strings.EqualFold(annotationValue, RuleManagementModeSlFrontend) { return ManagementModeFrontend, &nsg, nil } - if strings.ToLower(annotationValue) == strings.ToLower(RuleManagementModeNsg) { + if strings.EqualFold(annotationValue, RuleManagementModeNsg) { nsg = ManagedNetworkSecurityGroup{ nsgRuleManagementMode: RuleManagementModeNsg, frontendNsgId: "", @@ -840,7 +840,7 @@ func getBackendSets(logger *zap.SugaredLogger, svc *v1.Service, provisionedNodes var sslConfiguration *client.GenericSslConfigurationDetails if sslCfg != nil && len(sslCfg.BackendSetSSLSecretName) != 0 && getLoadBalancerType(svc) == LB { secretName = sslCfg.BackendSetSSLSecretName - backendSetSSLConfig, _ := svc.Annotations[ServiceAnnotationLoadbalancerBackendSetSSLConfig] + backendSetSSLConfig := svc.Annotations[ServiceAnnotationLoadbalancerBackendSetSSLConfig] sslConfiguration, err = getSSLConfiguration(sslCfg, secretName, int(servicePort.Port), backendSetSSLConfig) if err != nil { return nil, err @@ -1112,7 +1112,7 @@ func getListenersOciLoadBalancer(svc *v1.Service, sslCfg *SSLConfig) (map[string var sslConfiguration *client.GenericSslConfigurationDetails if sslCfg != nil && len(sslCfg.ListenerSSLSecretName) != 0 { secretName = sslCfg.ListenerSSLSecretName - listenerCipherSuiteAnnotation, _ := svc.Annotations[ServiceAnnotationLoadbalancerListenerSSLConfig] + listenerCipherSuiteAnnotation := svc.Annotations[ServiceAnnotationLoadbalancerListenerSSLConfig] sslConfiguration, err = getSSLConfiguration(sslCfg, secretName, port, listenerCipherSuiteAnnotation) if err != nil { return nil, err @@ -1166,16 +1166,16 @@ func getListenersOciLoadBalancer(svc *v1.Service, sslCfg *SSLConfig) (map[string func getListenersNetworkLoadBalancer(svc *v1.Service, listenerBackendIpVersion []string) (map[string]client.GenericListener, error) { listeners := make(map[string]client.GenericListener) - portsMap := make(map[int][]string) + portsMap := make(map[int][]v1.ServicePort) + for _, servicePort := range svc.Spec.Ports { + portsMap[int(servicePort.Port)] = append(portsMap[int(servicePort.Port)], servicePort) + } + mixedProtocolsPortSet := make(map[int]bool) var enablePpv2 *bool requireIPv4, requireIPv6 := getRequireIpVersions(listenerBackendIpVersion) - for _, servicePort := range svc.Spec.Ports { - portsMap[int(servicePort.Port)] = append(portsMap[int(servicePort.Port)], string(servicePort.Protocol)) - } - if ppv2EnabledValue, ppv2AnnotationSet := svc.Annotations[ServiceAnnotationNetworkLoadBalancerIsPpv2Enabled]; ppv2AnnotationSet { if strings.ToLower(ppv2EnabledValue) == "true" { enablePpv2 = pointer.Bool(true) @@ -1195,9 +1195,25 @@ func getListenersNetworkLoadBalancer(svc *v1.Service, listenerBackendIpVersion [ return nil, fmt.Errorf("invalid backend protocol %q requested for network load balancer listener", protocol) } port := int(servicePort.Port) + useMixed := false + if len(portsMap[port]) > 1 { + firstNodePort := portsMap[port][0].NodePort + allSameNodePort := true + for _, sp := range portsMap[port] { + if sp.NodePort != firstNodePort { + allSameNodePort = false + break + } + } + if allSameNodePort { + useMixed = true + } + } + listenerName := "" backendSetName := "" - if len(portsMap[port]) > 1 { + + if useMixed { if mixedProtocolsPortSet[port] { continue } @@ -1207,7 +1223,7 @@ func getListenersNetworkLoadBalancer(svc *v1.Service, listenerBackendIpVersion [ mixedProtocolsPortSet[port] = true } else { listenerName = getListenerName(protocol, port) - backendSetName = getBackendSetName(string(servicePort.Protocol), int(servicePort.Port)) + backendSetName = getBackendSetName(string(servicePort.Protocol), port) } genericListener := client.GenericListener{ @@ -1222,8 +1238,8 @@ func getListenersNetworkLoadBalancer(svc *v1.Service, listenerBackendIpVersion [ listeners[listenerName] = genericListener } if requireIPv6 { - listenerNameIPv6 := fmt.Sprintf("%s", listenerName+"-"+IPv6) - backendSetNameIPv6 := fmt.Sprintf("%s", backendSetName+"-"+IPv6) + listenerNameIPv6 := listenerName + "-" + IPv6 + backendSetNameIPv6 := backendSetName + "-" + IPv6 genericListener.Name = common.String(listenerNameIPv6) genericListener.IpVersion = GenericIpVersion(client.GenericIPv6) genericListener.DefaultBackendSetName = common.String(backendSetNameIPv6) @@ -1546,9 +1562,9 @@ func getLoadBalancerType(svc *v1.Service) string { func getBackendSetNamePortMap(service *v1.Service) map[string]v1.ServicePort { backendSetPortMap := make(map[string]v1.ServicePort) - portsMap := make(map[int][]string) + portsMap := make(map[int][]v1.ServicePort) for _, servicePort := range service.Spec.Ports { - portsMap[int(servicePort.Port)] = append(portsMap[int(servicePort.Port)], string(servicePort.Protocol)) + portsMap[int(servicePort.Port)] = append(portsMap[int(servicePort.Port)], servicePort) } ipFamilies := getIpFamilies(service) @@ -1557,30 +1573,42 @@ func getBackendSetNamePortMap(service *v1.Service) map[string]v1.ServicePort { mixedProtocolsPortSet := make(map[int]bool) for _, servicePort := range service.Spec.Ports { port := int(servicePort.Port) + useMixed := false + if len(portsMap[port]) > 1 { + firstNodePort := portsMap[port][0].NodePort + allSameNodePort := true + for _, sp := range portsMap[port] { + if sp.NodePort != firstNodePort { + allSameNodePort = false + break + } + } + if allSameNodePort { + useMixed = true + } + } + if useMixed { + if mixedProtocolsPortSet[port] { + continue + } + mixedProtocolsPortSet[port] = true + } backendSetName := "" if requireIPv4 { - if len(portsMap[port]) > 1 { - if mixedProtocolsPortSet[port] { - continue - } + if useMixed { backendSetName = getBackendSetName(ProtocolTypeMixed, port) - mixedProtocolsPortSet[port] = true } else { - backendSetName = getBackendSetName(string(servicePort.Protocol), int(servicePort.Port)) + backendSetName = getBackendSetName(string(servicePort.Protocol), port) } backendSetPortMap[backendSetName] = servicePort } if requireIPv6 { - if len(portsMap[port]) > 1 { - if mixedProtocolsPortSet[port] { - continue - } + if useMixed { backendSetName = getBackendSetName(ProtocolTypeMixed, port) - mixedProtocolsPortSet[port] = true } else { - backendSetName = getBackendSetName(string(servicePort.Protocol), int(servicePort.Port)) + backendSetName = getBackendSetName(string(servicePort.Protocol), port) } - backendSetNameIPv6 := fmt.Sprintf("%s", backendSetName+"-"+IPv6) + backendSetNameIPv6 := backendSetName + "-" + IPv6 backendSetPortMap[backendSetNameIPv6] = servicePort } diff --git a/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go b/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go index e093df26d2..5262629bdf 100644 --- a/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go +++ b/pkg/cloudprovider/providers/oci/load_balancer_spec_test.go @@ -2888,7 +2888,7 @@ func TestNewLBSpecSuccess(t *testing.T) { Internal: false, Subnets: []string{"one", "two"}, Listeners: map[string]client.GenericListener{ - fmt.Sprintf("TCP-443"): { + "TCP-443": { Name: common.String("TCP-443"), DefaultBackendSetName: common.String("TCP-443"), Port: common.Int(443), @@ -4607,7 +4607,7 @@ func TestNewLBSpecSuccess(t *testing.T) { Internal: false, Subnets: []string{"one", "two"}, Listeners: map[string]client.GenericListener{ - fmt.Sprintf("GRPC-443"): { + "GRPC-443": { Name: common.String("GRPC-443"), DefaultBackendSetName: common.String("TCP-443"), Port: common.Int(443), @@ -9544,9 +9544,15 @@ func Test_validateService(t *testing.T) { } func Test_getListenersNetworkLoadBalancer(t *testing.T) { - testOneListenerName := "TCP_AND_UDP-67" - testOneBackendSetName := "TCP_AND_UDP-67" - testOneProtocol := "TCP_AND_UDP" + testOneCombinedListenerName := "TCP_AND_UDP-67" + testOneCombinedBackendSetName := "TCP_AND_UDP-67" + testOneCombinedProtocol := "TCP_AND_UDP" + testOneSeparateListenerNameOne := "TCP-67" + testOneSeparateListenerNameTwo := "UDP-67" + testOneSeparateBackendSetNameOne := "TCP-67" + testOneSeparateBackendSetNameTwo := "UDP-67" + testOneSeparateProtocolOne := "TCP" + testOneSeparateProtocolTwo := "UDP" testOnePort := 67 testTwoListenerNameOne := "TCP-67" @@ -9580,7 +9586,7 @@ func Test_getListenersNetworkLoadBalancer(t *testing.T) { wantListeners map[string]client.GenericListener err error }{ - "NLB_with_mixed_protocol_on_same_port": { + "NLB_with_mixed_protocol_on_same_port_and_SAME_nodeport": { service: &v1.Service{ Spec: v1.ServiceSpec{ SessionAffinity: v1.ServiceAffinityNone, @@ -9588,10 +9594,12 @@ func Test_getListenersNetworkLoadBalancer(t *testing.T) { { Protocol: v1.ProtocolTCP, Port: int32(67), + NodePort: int32(30067), }, { Protocol: v1.ProtocolUDP, Port: int32(67), + NodePort: int32(30067), }, }, }, @@ -9604,9 +9612,49 @@ func Test_getListenersNetworkLoadBalancer(t *testing.T) { listenerBackendIpVersion: []string{IPv4}, wantListeners: map[string]client.GenericListener{ "TCP_AND_UDP-67": { - Name: &testOneListenerName, - DefaultBackendSetName: common.String(testOneBackendSetName), - Protocol: &testOneProtocol, + Name: &testOneCombinedListenerName, + DefaultBackendSetName: common.String(testOneCombinedBackendSetName), + Protocol: &testOneCombinedProtocol, + Port: &testOnePort, + }, + }, + err: nil, + }, + "NLB_with_mixed_protocol_on_same_port_and_DIFFERENT_nodeports": { + service: &v1.Service{ + Spec: v1.ServiceSpec{ + SessionAffinity: v1.ServiceAffinityNone, + Ports: []v1.ServicePort{ + { + Protocol: v1.ProtocolTCP, + Port: int32(67), + NodePort: int32(30067), + }, + { + Protocol: v1.ProtocolUDP, + Port: int32(67), + NodePort: int32(30068), + }, + }, + }, + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + ServiceAnnotationLoadBalancerType: "nlb", + }, + }, + }, + listenerBackendIpVersion: []string{IPv4}, + wantListeners: map[string]client.GenericListener{ + "TCP-67": { + Name: &testOneSeparateListenerNameOne, + DefaultBackendSetName: common.String(testOneSeparateBackendSetNameOne), + Protocol: &testOneSeparateProtocolOne, + Port: &testOnePort, + }, + "UDP-67": { + Name: &testOneSeparateListenerNameTwo, + DefaultBackendSetName: common.String(testOneSeparateBackendSetNameTwo), + Protocol: &testOneSeparateProtocolTwo, Port: &testOnePort, }, }, @@ -10345,17 +10393,19 @@ func Test_getBackendSetNamePortMap(t *testing.T) { }, }, }, - "multiple ports with different protocols": { + "multiple ports with different protocols (different external ports)": { in: &v1.Service{ Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { Protocol: v1.ProtocolTCP, Port: 80, + NodePort: 30080, }, { Protocol: v1.ProtocolUDP, Port: 81, + NodePort: 30081, }, }, }, @@ -10364,32 +10414,53 @@ func Test_getBackendSetNamePortMap(t *testing.T) { "TCP-80": { Protocol: v1.ProtocolTCP, Port: 80, + NodePort: 30080, }, "UDP-81": { Protocol: v1.ProtocolUDP, Port: 81, + NodePort: 30081, }, }, }, - "multiple ports with mixed protocols": { + "multiple ports with mixed protocols (same external port, SAME NodePort)": { in: &v1.Service{ Spec: v1.ServiceSpec{ Ports: []v1.ServicePort{ { Protocol: v1.ProtocolTCP, Port: 80, + NodePort: 30000, }, { Protocol: v1.ProtocolUDP, - Port: 81, + Port: 80, + NodePort: 30000, }, + }, + }, + }, + out: map[string]v1.ServicePort{ + "TCP_AND_UDP-80": { + Protocol: v1.ProtocolTCP, + Port: 80, + NodePort: 30000, + }, + }, + }, + "multiple ports with mixed protocols (same external port, DIFFERENT NodePorts)": { + in: &v1.Service{ + Spec: v1.ServiceSpec{ + Ports: []v1.ServicePort{ { Protocol: v1.ProtocolTCP, - Port: 82, + Port: 80, + NodePort: 30000, }, { Protocol: v1.ProtocolUDP, - Port: 82, + Port: 80, + NodePort: 30001, }, }, }, @@ -10398,14 +10469,12 @@ func Test_getBackendSetNamePortMap(t *testing.T) { "TCP-80": { Protocol: v1.ProtocolTCP, Port: 80, + NodePort: 30000, }, - "UDP-81": { + "UDP-80": { Protocol: v1.ProtocolUDP, - Port: 81, - }, - "TCP_AND_UDP-82": { - Protocol: v1.ProtocolTCP, - Port: 82, + Port: 80, + NodePort: 30001, }, }, },