Merge pull request #1090 from lzhecheng/fix-enabletcpreset

Fix incorrect EnableTCPReset for non-TCP protocols
kubernetes-sigs · Feb 7, 2022 · 7146a59 · 7146a59
2 parents c129891 + fe0994b
commit 7146a59
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 6 deletions.
diff --git a/pkg/provider/azure_loadbalancer.go b/pkg/provider/azure_loadbalancer.go
@@ -1994,7 +1994,7 @@ func (az *Cloud) getExpectedLBRules(
 		ports = []v1.ServicePort{}
 	}
 
-	var enableTCPReset *bool
+	var enableTCPReset, nilTCPReset *bool
 	if az.useStandardLoadBalancer() {
 		enableTCPReset = to.BoolPtr(true)
 	}
@@ -2076,6 +2076,10 @@ func (az *Cloud) getExpectedLBRules(
 			loadDistribution = network.LoadDistributionSourceIP
 		}
 
+		tcpReset := enableTCPReset
+		if port.Protocol != v1.ProtocolTCP {
+			tcpReset = nilTCPReset
+		}
 		expectedRule := network.LoadBalancingRule{
 			Name: &lbRuleName,
 			LoadBalancingRulePropertiesFormat: &network.LoadBalancingRulePropertiesFormat{
@@ -2090,7 +2094,7 @@ func (az *Cloud) getExpectedLBRules(
 				FrontendPort:        to.Int32Ptr(port.Port),
 				BackendPort:         to.Int32Ptr(port.Port),
 				DisableOutboundSnat: to.BoolPtr(az.disableLoadBalancerOutboundSNAT()),
-				EnableTCPReset:      enableTCPReset,
+				EnableTCPReset:      tcpReset,
 				EnableFloatingIP:    to.BoolPtr(true),
 			},
 		}
@@ -2874,14 +2878,21 @@ func equalLoadBalancingRulePropertiesFormat(s *network.LoadBalancingRuleProperti
 		return false
 	}
 
-	properties := reflect.DeepEqual(s.Protocol, t.Protocol) &&
-		reflect.DeepEqual(s.FrontendIPConfiguration, t.FrontendIPConfiguration) &&
+	properties := reflect.DeepEqual(s.Protocol, t.Protocol)
+	if !properties {
+		return false
+	}
+
+	if reflect.DeepEqual(s.Protocol, network.TransportProtocolTCP) {
+		properties = properties && reflect.DeepEqual(to.Bool(s.EnableTCPReset), to.Bool(t.EnableTCPReset))
+	}
+
+	properties = properties && reflect.DeepEqual(s.FrontendIPConfiguration, t.FrontendIPConfiguration) &&
 		reflect.DeepEqual(s.BackendAddressPool, t.BackendAddressPool) &&
 		reflect.DeepEqual(s.LoadDistribution, t.LoadDistribution) &&
 		reflect.DeepEqual(s.FrontendPort, t.FrontendPort) &&
 		reflect.DeepEqual(s.BackendPort, t.BackendPort) &&
 		reflect.DeepEqual(s.EnableFloatingIP, t.EnableFloatingIP) &&
-		reflect.DeepEqual(to.Bool(s.EnableTCPReset), to.Bool(t.EnableTCPReset)) &&
 		reflect.DeepEqual(to.Bool(s.DisableOutboundSnat), to.Bool(t.DisableOutboundSnat))
 
 	if wantLB && s.IdleTimeoutInMinutes != nil && t.IdleTimeoutInMinutes != nil {

diff --git a/pkg/provider/azure_loadbalancer_test.go b/pkg/provider/azure_loadbalancer_test.go
@@ -1790,7 +1790,7 @@ func TestReconcileLoadBalancerRule(t *testing.T) {
 			}, false, 80),
 			loadBalancerSku: "standard",
 			wantLb:          true,
-			expectedRules:   getHATestRules(true, false, v1.ProtocolSCTP),
+			expectedRules:   getHATestRules(false, false, v1.ProtocolSCTP),
 		},
 		{
 			desc: "getExpectedLBRules shall return corresponding probe and lbRule (slb with HA enabled multi-ports services)",
@@ -4890,3 +4890,48 @@ func Test_getProbeIntervalInSecondsAndNumOfProbe(t *testing.T) {
 		})
 	}
 }
+
+func TestEqualLoadBalancingRulePropertiesFormat(t *testing.T) {
+	var enableTCPReset, disableTCPReset *bool = to.BoolPtr(true), to.BoolPtr(false)
+	var frontPort *int32 = to.Int32Ptr(80)
+
+	testcases := []struct {
+		s        *network.LoadBalancingRulePropertiesFormat
+		t        *network.LoadBalancingRulePropertiesFormat
+		wantLb   bool
+		expected bool
+	}{
+		{
+			s: &network.LoadBalancingRulePropertiesFormat{
+				Protocol:       network.TransportProtocolTCP,
+				EnableTCPReset: enableTCPReset,
+				FrontendPort:   frontPort,
+			},
+			t: &network.LoadBalancingRulePropertiesFormat{
+				Protocol:       network.TransportProtocolTCP,
+				EnableTCPReset: enableTCPReset,
+				FrontendPort:   frontPort,
+			},
+			wantLb:   true,
+			expected: true,
+		},
+		{
+			s: &network.LoadBalancingRulePropertiesFormat{
+				Protocol:       network.TransportProtocolUDP,
+				EnableTCPReset: disableTCPReset,
+				FrontendPort:   frontPort,
+			},
+			t: &network.LoadBalancingRulePropertiesFormat{
+				Protocol:       network.TransportProtocolUDP,
+				EnableTCPReset: enableTCPReset,
+				FrontendPort:   frontPort,
+			},
+			wantLb:   true,
+			expected: true,
+		},
+	}
+
+	for _, tc := range testcases {
+		assert.Equal(t, tc.expected, equalLoadBalancingRulePropertiesFormat(tc.s, tc.t, tc.wantLb))
+	}
+}