Allow connect and mesh gateway to have 0 for EnforcingConsecutive5xx

hashicorp · Dec 12, 2022 · 5d6d9a4 · 5d6d9a4
1 parent ec19956
commit 5d6d9a4
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 18 deletions.
diff --git a/agent/xds/clusters.go b/agent/xds/clusters.go
@@ -855,7 +855,7 @@ func (s *ResourceGenerator) configIngressUpstreamCluster(c *envoy_cluster_v3.Clu
 	if svc != nil {
 		override = svc.PassiveHealthCheck
 	}
-	outlierDetection := ToOutlierDetection(cfgSnap.IngressGateway.Defaults.PassiveHealthCheck, override)
+	outlierDetection := ToOutlierDetection(cfgSnap.IngressGateway.Defaults.PassiveHealthCheck, override, false)
 
 	// Specail handling for failover peering service, which has set MaxEjectionPercent
 	if c.OutlierDetection != nil && c.OutlierDetection.MaxEjectionPercent != nil {
@@ -963,7 +963,7 @@ func (s *ResourceGenerator) makeUpstreamClusterForPeerService(
 
 	clusterName := generatePeeredClusterName(uid, tbs)
 
-	outlierDetection := ToOutlierDetection(upstreamConfig.PassiveHealthCheck, nil)
+	outlierDetection := ToOutlierDetection(upstreamConfig.PassiveHealthCheck, nil, true)
 	// We can't rely on health checks for services on cluster peers because they
 	// don't take into account service resolvers, splitters and routers. Setting
 	// MaxEjectionPercent too 100% gives outlier detection the power to eject the
@@ -1098,7 +1098,7 @@ func (s *ResourceGenerator) makeUpstreamClusterForPreparedQuery(upstream structs
 			CircuitBreakers: &envoy_cluster_v3.CircuitBreakers{
 				Thresholds: makeThresholdsIfNeeded(cfg.Limits),
 			},
-			OutlierDetection: ToOutlierDetection(cfg.PassiveHealthCheck, nil),
+			OutlierDetection: ToOutlierDetection(cfg.PassiveHealthCheck, nil, true),
 		}
 		if cfg.Protocol == "http2" || cfg.Protocol == "grpc" {
 			if err := s.setHttp2ProtocolOptions(c); err != nil {
@@ -1341,7 +1341,7 @@ func (s *ResourceGenerator) makeUpstreamClustersForDiscoveryChain(
 				CircuitBreakers: &envoy_cluster_v3.CircuitBreakers{
 					Thresholds: makeThresholdsIfNeeded(upstreamConfig.Limits),
 				},
-				OutlierDetection: ToOutlierDetection(upstreamConfig.PassiveHealthCheck, nil),
+				OutlierDetection: ToOutlierDetection(upstreamConfig.PassiveHealthCheck, nil, true),
 			}
 
 			var lb *structs.LoadBalancer

diff --git a/agent/xds/config.go b/agent/xds/config.go
@@ -178,8 +178,11 @@ func ParseGatewayConfig(m map[string]interface{}) (GatewayConfig, error) {
 // Return an envoy.OutlierDetection populated by the values from structs.PassiveHealthChec.
 // If all values are zero a default empty OutlierDetection will be returned to
 // enable outlier detection with default values.
-// If override is not nil, it will overwrite the values from p, e.g., ingress gateway defaults
-func ToOutlierDetection(p *structs.PassiveHealthCheck, override *structs.PassiveHealthCheck) *envoy_cluster_v3.OutlierDetection {
+//   - If override is not nil, it will overwrite the values from p, e.g., ingress gateway defaults
+//   - allowZero is added to handle the legacy case where connec-proxy and mesh gateway can set 0
+//     for EnforcingConsecutive5xx. Due to the definitino of proto of PassiveHealthCheck, ingress
+//     gateway's EnforcingConsecutive5xx must be > 0.
+func ToOutlierDetection(p *structs.PassiveHealthCheck, override *structs.PassiveHealthCheck, allowZero bool) *envoy_cluster_v3.OutlierDetection {
 	od := &envoy_cluster_v3.OutlierDetection{}
 	if p != nil {
 
@@ -190,23 +193,33 @@ func ToOutlierDetection(p *structs.PassiveHealthCheck, override *structs.Passive
 			od.Consecutive_5Xx = &wrappers.UInt32Value{Value: p.MaxFailures}
 		}
 
-		// NOTE: EnforcingConsecutive5xx must be great than 0
-		if p.EnforcingConsecutive5xx != nil && *p.EnforcingConsecutive5xx > 0 {
-			od.EnforcingConsecutive_5Xx = &wrappers.UInt32Value{Value: *p.EnforcingConsecutive5xx}
+		if p.EnforcingConsecutive5xx != nil {
+			// NOTE: EnforcingConsecutive5xx must be great than 0 for ingress-gateway
+			if *p.EnforcingConsecutive5xx != 0 {
+				od.EnforcingConsecutive_5Xx = &wrappers.UInt32Value{Value: *p.EnforcingConsecutive5xx}
+			} else if allowZero {
+				od.EnforcingConsecutive_5Xx = &wrappers.UInt32Value{Value: *p.EnforcingConsecutive5xx}
+			}
 		}
 	}
 
+	if override == nil {
+		return od
+	}
+
 	// override the default outlier detection value
-	if override != nil {
-		if override.Interval != 0 {
-			od.Interval = durationpb.New(override.Interval)
-		}
-		if override.MaxFailures != 0 {
-			od.Consecutive_5Xx = &wrappers.UInt32Value{Value: override.MaxFailures}
-		}
+	if override.Interval != 0 {
+		od.Interval = durationpb.New(override.Interval)
+	}
+	if override.MaxFailures != 0 {
+		od.Consecutive_5Xx = &wrappers.UInt32Value{Value: override.MaxFailures}
+	}
 
-		// NOTE: EnforcingConsecutive5xx must be great than 0
-		if override.EnforcingConsecutive5xx != nil && *override.EnforcingConsecutive5xx > 0 {
+	if override.EnforcingConsecutive5xx != nil {
+		// NOTE: EnforcingConsecutive5xx must be great than 0 for ingress-gateway
+		if *override.EnforcingConsecutive5xx != 0 {
+			od.EnforcingConsecutive_5Xx = &wrappers.UInt32Value{Value: *override.EnforcingConsecutive5xx}
+		} else if allowZero {
 			od.EnforcingConsecutive_5Xx = &wrappers.UInt32Value{Value: *override.EnforcingConsecutive5xx}
 		}
 	}