From da99f238e8b9ff5cd9e733960d57e066547552e8 Mon Sep 17 00:00:00 2001 From: Dawid Wysocki Date: Thu, 21 Nov 2024 09:47:00 +0000 Subject: [PATCH 1/2] Add forwardingrules.GetILBProtocol func for mixed protocol This will be used behind MixedProtocol feature flag for ILBs only. --- pkg/forwardingrules/protocol.go | 42 ++++++++++++++++ pkg/forwardingrules/protocol_test.go | 75 ++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+) create mode 100644 pkg/forwardingrules/protocol.go create mode 100644 pkg/forwardingrules/protocol_test.go diff --git a/pkg/forwardingrules/protocol.go b/pkg/forwardingrules/protocol.go new file mode 100644 index 000000000..f5efe005e --- /dev/null +++ b/pkg/forwardingrules/protocol.go @@ -0,0 +1,42 @@ +package forwardingrules + +import api_v1 "k8s.io/api/core/v1" + +const ( + // ProtocolTCP is the value used for Protocol when creating TCP Forwarding Rule + ProtocolTCP = "TCP" + // ProtocolUDP is the value used for Protocol when creating UDP Forwarding Rule + ProtocolUDP = "UDP" + // ProtocolL3 is the value used for Protocol when creating L3 Forwarding Rule. + // L3 Forwarding rule allows all the traffic to flow through it regardless of ports + // and protocol. When using ProtocolL3 AllPorts option should be used as well. + ProtocolL3 = "L3_DEFAULT" +) + +// GetILBProtocol returns the protocol to be used for a Forwarding Rule for an ILB. +// For ELB please use forwardingrules.NeedsTCP or forwardingrules.NeedsUDP functions. +func GetILBProtocol(svcPorts []api_v1.ServicePort) string { + protocolSet := make(map[api_v1.Protocol]struct{}) + for _, port := range svcPorts { + protocol := port.Protocol + if protocol == "" { + // Protocol is optional, defaults to TCP + protocol = api_v1.ProtocolTCP + } + protocolSet[protocol] = struct{}{} + } + + _, okTCP := protocolSet[api_v1.ProtocolTCP] + _, okUDP := protocolSet[api_v1.ProtocolUDP] + + switch { + case okTCP && okUDP: + return ProtocolL3 + case okUDP: + return ProtocolUDP + case okTCP: + return ProtocolTCP + default: + return ProtocolTCP + } +} diff --git a/pkg/forwardingrules/protocol_test.go b/pkg/forwardingrules/protocol_test.go new file mode 100644 index 000000000..f34a79a4c --- /dev/null +++ b/pkg/forwardingrules/protocol_test.go @@ -0,0 +1,75 @@ +package forwardingrules_test + +import ( + "testing" + + api_v1 "k8s.io/api/core/v1" + "k8s.io/ingress-gce/pkg/forwardingrules" +) + +func TestGetProtocol(t *testing.T) { + tcpPort := api_v1.ServicePort{ + Protocol: api_v1.ProtocolTCP, + } + udpPort := api_v1.ServicePort{ + Protocol: api_v1.ProtocolUDP, + } + emptyPort := api_v1.ServicePort{} + + testCases := []struct { + ports []api_v1.ServicePort + want string + desc string + }{ + { + desc: "no ports", + ports: []api_v1.ServicePort{}, + want: forwardingrules.ProtocolTCP, + }, + { + desc: "udp single", + ports: []api_v1.ServicePort{udpPort}, + want: forwardingrules.ProtocolUDP, + }, + { + desc: "udp multiple", + ports: []api_v1.ServicePort{udpPort, udpPort}, + want: forwardingrules.ProtocolUDP, + }, + { + desc: "tcp single", + ports: []api_v1.ServicePort{tcpPort}, + want: forwardingrules.ProtocolTCP, + }, + { + desc: "tcp multiple", + ports: []api_v1.ServicePort{tcpPort, tcpPort}, + want: forwardingrules.ProtocolTCP, + }, + { + desc: "tcp default", + ports: []api_v1.ServicePort{emptyPort}, + want: forwardingrules.ProtocolTCP, + }, + { + desc: "mixed, udp first", + ports: []api_v1.ServicePort{udpPort, tcpPort}, + want: forwardingrules.ProtocolL3, + }, + { + desc: "mixed, tcp first", + ports: []api_v1.ServicePort{tcpPort, udpPort}, + want: forwardingrules.ProtocolL3, + }, + } + + for _, tC := range testCases { + t.Run(tC.desc, func(t *testing.T) { + got := forwardingrules.GetILBProtocol(tC.ports) + + if got != tC.want { + t.Errorf("GetProtocol(_) = %v, want %v", got, tC.want) + } + }) + } +} From 46aab782636251aac5ae5b57da3ea7d91e60c4e7 Mon Sep 17 00:00:00 2001 From: Dawid Wysocki Date: Mon, 25 Nov 2024 15:22:08 +0000 Subject: [PATCH 2/2] Use GetILBProtocol in ensureIPv4ForwardingRule --- pkg/loadbalancers/forwarding_rules.go | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/pkg/loadbalancers/forwarding_rules.go b/pkg/loadbalancers/forwarding_rules.go index 9714288ee..5901dec5b 100644 --- a/pkg/loadbalancers/forwarding_rules.go +++ b/pkg/loadbalancers/forwarding_rules.go @@ -227,7 +227,20 @@ func (l4 *L4) ensureIPv4ForwardingRule(bsLink string, options gce.ILBOptions, ex servicePorts := l4.Service.Spec.Ports ports := utils.GetPorts(servicePorts) - protocol := utils.GetProtocol(servicePorts) + protocol := string(utils.GetProtocol(servicePorts)) + allPorts := false + if l4.enableMixedProtocol { + protocol = forwardingrules.GetILBProtocol(servicePorts) + if protocol == forwardingrules.ProtocolL3 { + allPorts = true + ports = nil + } + } + if len(ports) > maxForwardedPorts { + allPorts = true + ports = nil + } + // Create the forwarding rule frDesc, err := utils.MakeL4LBServiceDescription(utils.ServiceKeyFunc(l4.Service.Namespace, l4.Service.Name), ipToUse, version, false, utils.ILB) @@ -240,7 +253,8 @@ func (l4 *L4) ensureIPv4ForwardingRule(bsLink string, options gce.ILBOptions, ex Name: frName, IPAddress: ipToUse, Ports: ports, - IPProtocol: string(protocol), + AllPorts: allPorts, + IPProtocol: protocol, LoadBalancingScheme: string(cloud.SchemeInternal), Subnetwork: subnetworkURL, Network: l4.network.NetworkURL, @@ -250,10 +264,6 @@ func (l4 *L4) ensureIPv4ForwardingRule(bsLink string, options gce.ILBOptions, ex AllowGlobalAccess: options.AllowGlobalAccess, Description: frDesc, } - if len(ports) > maxForwardedPorts { - newFwdRule.Ports = nil - newFwdRule.AllPorts = true - } if existingFwdRule != nil { equal, err := forwardingrules.EqualIPv4(existingFwdRule, newFwdRule)