Merge pull request kubernetes#104149 from aojea/automated-cherry-pick…

…-of-#104009-upstream-release-1.22 Automated cherry pick of kubernetes#104009: delete stale UDP conntrack entries for loadbalancer IPs
openshift · Aug 16, 2021 · 807ed8e · 807ed8e
2 parents f304aa4 + 5edc5e8
commit 807ed8e
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 12 deletions.
diff --git a/pkg/proxy/iptables/proxier.go b/pkg/proxy/iptables/proxier.go
@@ -831,6 +831,9 @@ func (proxier *Proxier) syncProxyRules() {
 			for _, extIP := range svcInfo.ExternalIPStrings() {
 				conntrackCleanupServiceIPs.Insert(extIP)
 			}
+			for _, lbIP := range svcInfo.LoadBalancerIPStrings() {
+				conntrackCleanupServiceIPs.Insert(lbIP)
+			}
 			nodePort := svcInfo.NodePort()
 			if svcInfo.Protocol() == v1.ProtocolUDP && nodePort != 0 {
 				klog.V(2).Infof("Stale %s service NodePort %v -> %d", strings.ToLower(string(svcInfo.Protocol())), svcPortName, nodePort)

diff --git a/pkg/proxy/iptables/proxier_test.go b/pkg/proxy/iptables/proxier_test.go
@@ -3178,6 +3178,12 @@ func TestProxierDeleteNodePortStaleUDP(t *testing.T) {
 	// Delete ClusterIP entries
 	fcmd.CombinedOutputScript = append(fcmd.CombinedOutputScript, cmdFunc)
 	fexec.CommandScript = append(fexec.CommandScript, execFunc)
+	// Delete ExternalIP entries
+	fcmd.CombinedOutputScript = append(fcmd.CombinedOutputScript, cmdFunc)
+	fexec.CommandScript = append(fexec.CommandScript, execFunc)
+	// Delete LoadBalancerIP entries
+	fcmd.CombinedOutputScript = append(fcmd.CombinedOutputScript, cmdFunc)
+	fexec.CommandScript = append(fexec.CommandScript, execFunc)
 	// Delete NodePort entries
 	fcmd.CombinedOutputScript = append(fcmd.CombinedOutputScript, cmdFunc)
 	fexec.CommandScript = append(fexec.CommandScript, execFunc)
@@ -3187,6 +3193,8 @@ func TestProxierDeleteNodePortStaleUDP(t *testing.T) {
 	fp.exec = &fexec
 
 	svcIP := "10.20.30.41"
+	extIP := "1.1.1.1"
+	lbIngressIP := "2.2.2.2"
 	svcPort := 80
 	nodePort := 31201
 	svcPortName := proxy.ServicePortName{
@@ -3198,12 +3206,17 @@ func TestProxierDeleteNodePortStaleUDP(t *testing.T) {
 	makeServiceMap(fp,
 		makeTestService(svcPortName.Namespace, svcPortName.Name, func(svc *v1.Service) {
 			svc.Spec.ClusterIP = svcIP
+			svc.Spec.ExternalIPs = []string{extIP}
+			svc.Spec.Type = "LoadBalancer"
 			svc.Spec.Ports = []v1.ServicePort{{
 				Name:     svcPortName.Port,
 				Port:     int32(svcPort),
 				Protocol: v1.ProtocolUDP,
 				NodePort: int32(nodePort),
 			}}
+			svc.Status.LoadBalancer.Ingress = []v1.LoadBalancerIngress{{
+				IP: lbIngressIP,
+			}}
 		}),
 	)
 
@@ -3229,21 +3242,33 @@ func TestProxierDeleteNodePortStaleUDP(t *testing.T) {
 	)
 
 	fp.syncProxyRules()
-	if fexec.CommandCalls != 2 {
-		t.Fatalf("Updated UDP service with new endpoints must clear UDP entries")
+
+	if fexec.CommandCalls != 4 {
+		t.Fatalf("Updated UDP service with new endpoints must clear UDP entries 4 times: ClusterIP, NodePort, ExternalIP and LB")
 	}
 
-	// Delete ClusterIP Conntrack entries
-	expectCommand := fmt.Sprintf("conntrack -D --orig-dst %s -p %s", svcIP, strings.ToLower(string((v1.ProtocolUDP))))
-	actualCommand := strings.Join(fcmd.CombinedOutputLog[0], " ")
-	if actualCommand != expectCommand {
-		t.Errorf("Expected command: %s, but executed %s", expectCommand, actualCommand)
+	// the order is not guaranteed so we have to compare the strings in any order
+	expectedCommands := []string{
+		// Delete ClusterIP Conntrack entries
+		fmt.Sprintf("conntrack -D --orig-dst %s -p %s", svcIP, strings.ToLower(string((v1.ProtocolUDP)))),
+		// Delete ExternalIP Conntrack entries
+		fmt.Sprintf("conntrack -D --orig-dst %s -p %s", extIP, strings.ToLower(string((v1.ProtocolUDP)))),
+		// Delete LoadBalancerIP Conntrack entries
+		fmt.Sprintf("conntrack -D --orig-dst %s -p %s", lbIngressIP, strings.ToLower(string((v1.ProtocolUDP)))),
+		// Delete NodePort Conntrack entrie
+		fmt.Sprintf("conntrack -D -p %s --dport %d", strings.ToLower(string((v1.ProtocolUDP))), nodePort),
 	}
-	// Delete NodePort Conntrack entrie
-	expectCommand = fmt.Sprintf("conntrack -D -p %s --dport %d", strings.ToLower(string((v1.ProtocolUDP))), nodePort)
-	actualCommand = strings.Join(fcmd.CombinedOutputLog[1], " ")
-	if actualCommand != expectCommand {
-		t.Errorf("Expected command: %s, but executed %s", expectCommand, actualCommand)
+	actualCommands := []string{
+		strings.Join(fcmd.CombinedOutputLog[0], " "),
+		strings.Join(fcmd.CombinedOutputLog[1], " "),
+		strings.Join(fcmd.CombinedOutputLog[2], " "),
+		strings.Join(fcmd.CombinedOutputLog[3], " "),
+	}
+	sort.Strings(expectedCommands)
+	sort.Strings(actualCommands)
+
+	if !reflect.DeepEqual(expectedCommands, actualCommands) {
+		t.Errorf("Expected commands: %v, but executed %v", expectedCommands, actualCommands)
 	}
 }