Skip to content

Commit

Permalink
Merge pull request #1766 from andreaskaris/backport-AllocateLoadBalan…
Browse files Browse the repository at this point in the history
…cerNodePortsFalse

[release-4.12] OCPBUGS-16336: Backport support AllocateLoadBalancerNodePortsFalse
  • Loading branch information
openshift-merge-robot committed Jul 30, 2023
2 parents 70d15f0 + 80dc930 commit f704c40
Show file tree
Hide file tree
Showing 5 changed files with 292 additions and 55 deletions.
53 changes: 51 additions & 2 deletions go-controller/pkg/node/gateway_iptables.go
Expand Up @@ -272,6 +272,51 @@ func getNodePortETPLocalIPTRules(svcPort kapi.ServicePort, targetIP string) []ip
}
}

func computeProbability(n, i int) string {
return fmt.Sprintf("%0.10f", 1.0/float64(n-i+1))
}

func generateIPTRulesForLoadBalancersWithoutNodePorts(svcPort kapi.ServicePort, externalIP string, service *kapi.Service, localEndpoints []string) []iptRule {
var iptRules []iptRule
if len(localEndpoints) == 0 {
// either its smart nic mode; etp&itp not implemented, OR
// fetching endpointSlices error-ed out prior to reaching here so nothing to do
return iptRules
}
numLocalEndpoints := len(localEndpoints)
for i, ip := range localEndpoints {
iptRules = append([]iptRule{
{
table: "nat",
chain: iptableETPChain,
args: []string{
"-p", string(svcPort.Protocol),
"-d", externalIP,
"--dport", fmt.Sprintf("%v", svcPort.Port),
"-j", "DNAT",
"--to-destination", util.JoinHostPortInt32(ip, int32(svcPort.TargetPort.IntValue())),
"-m", "statistic",
"--mode", "random",
"--probability", computeProbability(numLocalEndpoints, i+1),
},
protocol: getIPTablesProtocol(externalIP),
},
{
table: "nat",
chain: iptableMgmPortChain,
args: []string{
"-p", string(svcPort.Protocol),
"-d", ip,
"--dport", fmt.Sprintf("%v", int32(svcPort.TargetPort.IntValue())),
"-j", "RETURN",
},
protocol: getIPTablesProtocol(externalIP),
},
}, iptRules...)
}
return iptRules
}

// getExternalIPTRules returns the IPTable DNAT rules for a service of type LB or ExternalIP
// `svcPort` corresponds to port details for this service as specified in the service object
// `externalIP` can either be the externalIP or LB.status.ingressIP
Expand Down Expand Up @@ -445,7 +490,7 @@ func recreateIPTRules(table, chain string, keepIPTRules []iptRule) error {
// case3: if svcHasLocalHostNetEndPnt and svcTypeIsITPLocal, rule that redirects clusterIP traffic to host targetPort is added.
// if !svcHasLocalHostNetEndPnt and svcTypeIsITPLocal, rule that marks clusterIP traffic to steer it to ovn-k8s-mp0 is added.
//
func getGatewayIPTRules(service *kapi.Service, svcHasLocalHostNetEndPnt bool) []iptRule {
func getGatewayIPTRules(service *kapi.Service, localEndpoints []string, svcHasLocalHostNetEndPnt bool) []iptRule {
rules := make([]iptRule, 0)
clusterIPs := util.GetClusterIPs(service)
svcTypeIsETPLocal := util.ServiceExternalTrafficPolicyLocal(service)
Expand Down Expand Up @@ -488,7 +533,11 @@ func getGatewayIPTRules(service *kapi.Service, svcHasLocalHostNetEndPnt bool) []
// case1 (see function description for details)
// DNAT traffic to masqueradeIP:nodePort instead of clusterIP:Port. We are leveraging the existing rules for NODEPORT
// service so no need to add skip SNAT rule to OVN-KUBE-SNAT-MGMTPORT since the corresponding nodePort svc would have one.
rules = append(rules, getExternalIPTRules(svcPort, externalIP, "", svcHasLocalHostNetEndPnt, svcTypeIsETPLocal)...)
if !util.ServiceTypeHasNodePort(service) {
rules = append(rules, generateIPTRulesForLoadBalancersWithoutNodePorts(svcPort, externalIP, service, localEndpoints)...)
} else {
rules = append(rules, getExternalIPTRules(svcPort, externalIP, "", svcHasLocalHostNetEndPnt, svcTypeIsETPLocal)...)
}
}
// case2 (see function description for details)
rules = append(rules, getExternalIPTRules(svcPort, externalIP, clusterIP, svcHasLocalHostNetEndPnt, false)...)
Expand Down
136 changes: 136 additions & 0 deletions go-controller/pkg/node/gateway_localnet_linux_test.go
Expand Up @@ -148,6 +148,14 @@ func newService(name, namespace, ip string, ports []v1.ServicePort, serviceType
}
}

func newServiceWithoutNodePortAllocation(name, namespace, ip string, ports []v1.ServicePort, serviceType v1.ServiceType,
externalIPs []string, serviceStatus v1.ServiceStatus, isETPLocal, isITPLocal bool) *v1.Service {
doNotAllocateNodePorts := false
service := newService(name, namespace, ip, ports, serviceType, externalIPs, serviceStatus, isETPLocal, isITPLocal)
service.Spec.AllocateLoadBalancerNodePorts = &doNotAllocateNodePorts
return service
}

func newEndpointSlice(svcName, namespace string, endpoints []discovery.Endpoint, endpointPort []discovery.EndpointPort) *discovery.EndpointSlice {
return &discovery.EndpointSlice{
ObjectMeta: metav1.ObjectMeta{
Expand Down Expand Up @@ -830,6 +838,134 @@ var _ = Describe("Node Operations", func() {
Expect(err).NotTo(HaveOccurred())
})

It("inits iptables rules and openflows with LoadBalancer where AllocateLoadBalancerNodePorts=False, ETP=local, LGW mode", func() {
app.Action = func(ctx *cli.Context) error {
externalIP := "1.1.1.1"
config.Gateway.Mode = config.GatewayModeLocal
fakeOvnNode.fakeExec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-ofctl show ",
Err: fmt.Errorf("deliberate error to fall back to output:LOCAL"),
})
fakeOvnNode.fakeExec.AddFakeCmd(&ovntest.ExpectedCmd{
Cmd: "ovs-ofctl show ",
Err: fmt.Errorf("deliberate error to fall back to output:LOCAL"),
})
service := *newServiceWithoutNodePortAllocation("service1", "namespace1", "10.129.0.2",
[]v1.ServicePort{
{
Protocol: v1.ProtocolTCP,
Port: int32(80),
TargetPort: intstr.FromInt(int(int32(8080))),
},
},
v1.ServiceTypeLoadBalancer,
[]string{externalIP},
v1.ServiceStatus{
LoadBalancer: v1.LoadBalancerStatus{
Ingress: []v1.LoadBalancerIngress{{
IP: "5.5.5.5",
}},
},
},
true, false,
)
ep1 := discovery.Endpoint{
Addresses: []string{"10.244.0.3"},
NodeName: &fakeNodeName,
}
otherNodeName := "node2"
nonLocalEndpoint := discovery.Endpoint{
Addresses: []string{"10.244.1.3"}, // is not picked since its not local to the node
NodeName: &otherNodeName,
}
ep2 := discovery.Endpoint{
Addresses: []string{"10.244.0.4"},
NodeName: &fakeNodeName,
}
epPortName := "http"
epPortValue := int32(8080)
epPort1 := discovery.EndpointPort{
Name: &epPortName,
Port: &epPortValue,
}
// endpointSlice.Endpoints is ovn-networked so this will
// come under !hasLocalHostNetEp case
endpointSlice := *newEndpointSlice(
"service1",
"namespace1",
[]discovery.Endpoint{ep1, ep2, nonLocalEndpoint},
[]discovery.EndpointPort{epPort1})

fakeOvnNode.start(ctx,
&v1.ServiceList{
Items: []v1.Service{
service,
},
},
&endpointSlice,
)

fNPW.watchFactory = fakeOvnNode.watcher
Expect(startNodePortWatcher(fNPW, fakeOvnNode.fakeClient, &fakeMgmtPortConfig)).To(Succeed())
fNPW.AddService(&service)

expectedTables := map[string]util.FakeTable{
"nat": {
"PREROUTING": []string{
"-j OVN-KUBE-ETP",
"-j OVN-KUBE-EXTERNALIP",
"-j OVN-KUBE-NODEPORT",
},
"OUTPUT": []string{
"-j OVN-KUBE-EXTERNALIP",
"-j OVN-KUBE-NODEPORT",
"-j OVN-KUBE-ITP",
},
"POSTROUTING": []string{
"-j OVN-KUBE-EGRESS-SVC",
},
"OVN-KUBE-NODEPORT": []string{},
"OVN-KUBE-SNAT-MGMTPORT": []string{
fmt.Sprintf("-p TCP -d %s --dport %d -j RETURN", ep1.Addresses[0], int32(service.Spec.Ports[0].TargetPort.IntValue())),
fmt.Sprintf("-p TCP -d %s --dport %d -j RETURN", ep2.Addresses[0], int32(service.Spec.Ports[0].TargetPort.IntValue())),
},
"OVN-KUBE-EXTERNALIP": []string{
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%v", service.Spec.Ports[0].Protocol, service.Status.LoadBalancer.Ingress[0].IP, service.Spec.Ports[0].Port, service.Spec.ClusterIP, service.Spec.Ports[0].Port),
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%v", service.Spec.Ports[0].Protocol, externalIP, service.Spec.Ports[0].Port, service.Spec.ClusterIP, service.Spec.Ports[0].Port),
},
"OVN-KUBE-ETP": []string{
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%d -m statistic --mode random --probability 0.5000000000", service.Spec.Ports[0].Protocol, service.Status.LoadBalancer.Ingress[0].IP, service.Spec.Ports[0].Port, ep1.Addresses[0], int32(service.Spec.Ports[0].TargetPort.IntValue())),
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%d -m statistic --mode random --probability 1.0000000000", service.Spec.Ports[0].Protocol, service.Status.LoadBalancer.Ingress[0].IP, service.Spec.Ports[0].Port, ep2.Addresses[0], int32(service.Spec.Ports[0].TargetPort.IntValue())),
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%d -m statistic --mode random --probability 0.5000000000", service.Spec.Ports[0].Protocol, externalIP, service.Spec.Ports[0].Port, ep1.Addresses[0], int32(service.Spec.Ports[0].TargetPort.IntValue())),
fmt.Sprintf("-p %s -d %s --dport %v -j DNAT --to-destination %s:%d -m statistic --mode random --probability 1.0000000000", service.Spec.Ports[0].Protocol, externalIP, service.Spec.Ports[0].Port, ep2.Addresses[0], int32(service.Spec.Ports[0].TargetPort.IntValue())),
},
"OVN-KUBE-ITP": []string{},
"OVN-KUBE-EGRESS-SVC": []string{},
},
"filter": {},
"mangle": {
"OUTPUT": []string{
"-j OVN-KUBE-ITP",
},
"OVN-KUBE-ITP": []string{},
},
}
expectedLBIngressFlows := []string{
"cookie=0xd8c1fe514f305bc1, priority=110, in_port=eth0, arp, arp_op=1, arp_tpa=5.5.5.5, actions=output:LOCAL",
}
expectedLBExternalIPFlows := []string{
"cookie=0x799e0efe5404e9a1, priority=110, in_port=eth0, arp, arp_op=1, arp_tpa=1.1.1.1, actions=output:LOCAL",
}

f4 := iptV4.(*util.FakeIPTables)
Expect(f4.MatchState(expectedTables)).To(Succeed())
Expect(fNPW.ofm.flowCache["Ingress_namespace1_service1_5.5.5.5_80"]).To(Equal(expectedLBIngressFlows))
Expect(fNPW.ofm.flowCache["External_namespace1_service1_1.1.1.1_80"]).To(Equal(expectedLBExternalIPFlows))
return nil
}
Expect(app.Run([]string{app.Name})).To(Succeed())
})

It("inits iptables rules and openflows with LoadBalancer where ETP=cluster, LGW mode", func() {
app.Action = func(ctx *cli.Context) error {
externalIP := "1.1.1.1"
Expand Down

0 comments on commit f704c40

Please sign in to comment.