diff --git a/internal/gatewayapi/route.go b/internal/gatewayapi/route.go index 16942ec64fa..611772345a3 100644 --- a/internal/gatewayapi/route.go +++ b/internal/gatewayapi/route.go @@ -369,8 +369,10 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe backendCustomRefs = append(backendCustomRefs, unstructuredRef) } - // skip backendRefs with weight 0 as they do not affect the traffic distribution - if backendDest.ds.Weight != nil && *backendDest.ds.Weight == 0 { + // A zero-weight backendRef gets no traffic but is kept, so its cluster already exists when a + // canary rollout raises its weight. A dynamic resolver has no cluster to warm, and keeping it + // would reject the rule for mixing a dynamic resolver with other backendRefs. + if backendDest.ds.IsDynamicResolver && backendDest.ds.Weight != nil && *backendDest.ds.Weight == 0 { continue } @@ -805,10 +807,6 @@ func (t *Translator) processBackendRef( if err != nil { return routeBackendRefDestination{ds: ds}, unstructuredRef, err } - // An explicitly zero-weight backendRef never contributes traffic either. - if ds.Weight != nil && *ds.Weight == 0 { - return routeBackendRefDestination{ds: ds}, unstructuredRef, err - } // Without a resolved gateway IR there's nowhere to eventually register a shared BackendCluster. if t.gatewayXdsIR(gatewayCtx, xdsIR) == nil { return routeBackendRefDestination{ds: ds}, unstructuredRef, err @@ -1586,8 +1584,10 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe } } - // skip backendRefs with weight 0 as they do not affect the traffic distribution - if backendDest.ds.Weight != nil && *backendDest.ds.Weight == 0 { + // A zero-weight backendRef gets no traffic but is kept, so its cluster already exists when a + // canary rollout raises its weight. A dynamic resolver has no cluster to warm, and keeping it + // would reject the rule for mixing a dynamic resolver with other backendRefs. + if backendDest.ds.IsDynamicResolver && backendDest.ds.Weight != nil && *backendDest.ds.Weight == 0 { continue } @@ -2947,11 +2947,6 @@ func (t *Translator) processDestination(name string, backendRefContext BackendRe } } - // Skip processing backends with 0 weight - if weight == 0 { - return emptyDS, nil, nil - } - var envoyProxy *egv1a1.EnvoyProxy if gatewayCtx != nil { envoyProxy = gatewayCtx.envoyProxy diff --git a/internal/gatewayapi/testdata/httproute-rule-with-multiple-backends-and-weights.out.yaml b/internal/gatewayapi/testdata/httproute-rule-with-multiple-backends-and-weights.out.yaml index 7a27099c266..0a43ebf1250 100644 --- a/internal/gatewayapi/testdata/httproute-rule-with-multiple-backends-and-weights.out.yaml +++ b/internal/gatewayapi/testdata/httproute-rule-with-multiple-backends-and-weights.out.yaml @@ -186,6 +186,18 @@ xdsIR: name: httproute/default/httproute-1/rule/0/backend/2 protocol: HTTP weight: 3 + - addressType: IP + endpoints: + - host: 7.7.7.7 + port: 8080 + metadata: + kind: Service + name: service-4 + namespace: default + sectionName: "8080" + name: httproute/default/httproute-1/rule/0/backend/3 + protocol: HTTP + weight: 0 hostname: '*' isHTTP2: false metadata: diff --git a/internal/gatewayapi/testdata/mergebackends-http-canary-same-service-zero-weight.in.yaml b/internal/gatewayapi/testdata/mergebackends-http-canary-same-service-zero-weight.in.yaml new file mode 100644 index 00000000000..342dbd7c352 --- /dev/null +++ b/internal/gatewayapi/testdata/mergebackends-http-canary-same-service-zero-weight.in.yaml @@ -0,0 +1,83 @@ +envoyProxyForGatewayClass: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + namespace: envoy-gateway-system + name: test + spec: + mergeBackends: {} +gateways: + - apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + namespace: envoy-gateway + name: gateway-1 + spec: + gatewayClassName: envoy-gateway-class + listeners: + - name: http + protocol: HTTP + port: 80 + allowedRoutes: + namespaces: + from: All +httpRoutes: + - apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + namespace: default + name: canary + spec: + parentRefs: + - namespace: envoy-gateway + name: gateway-1 + rules: + - matches: + - path: + type: PathPrefix + value: /canary + backendRefs: + - name: canary + port: 3000 + weight: 50 + - name: canary + port: 4000 + weight: 0 +services: + - apiVersion: v1 + kind: Service + metadata: + name: canary + namespace: default + spec: + clusterIP: 10.11.12.13 + ports: + - port: 3000 + name: http-v1 + protocol: TCP + targetPort: 3000 + - port: 4000 + name: http-v2 + protocol: TCP + targetPort: 3000 +endpointSlices: + - apiVersion: discovery.k8s.io/v1 + kind: EndpointSlice + metadata: + name: endpointslice-canary + namespace: default + labels: + kubernetes.io/service-name: canary + addressType: IPv4 + ports: + - name: http-v1 + protocol: TCP + port: 3000 + - name: http-v2 + protocol: TCP + port: 3000 + endpoints: + - addresses: + - "10.244.0.11" + conditions: + ready: true diff --git a/internal/gatewayapi/testdata/mergebackends-http-canary-same-service-zero-weight.out.yaml b/internal/gatewayapi/testdata/mergebackends-http-canary-same-service-zero-weight.out.yaml new file mode 100644 index 00000000000..95ff5f1ceff --- /dev/null +++ b/internal/gatewayapi/testdata/mergebackends-http-canary-same-service-zero-weight.out.yaml @@ -0,0 +1,214 @@ +gateways: +- apiVersion: gateway.networking.k8s.io/v1 + kind: Gateway + metadata: + name: gateway-1 + namespace: envoy-gateway + spec: + gatewayClassName: envoy-gateway-class + listeners: + - allowedRoutes: + namespaces: + from: All + name: http + port: 80 + protocol: HTTP + status: + listeners: + - attachedRoutes: 1 + conditions: + - lastTransitionTime: null + message: Sending translated listener configuration to the data plane + reason: Programmed + status: "True" + type: Programmed + - lastTransitionTime: null + message: Listener has been successfully translated + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Listener references have been resolved + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + name: http + supportedKinds: + - group: gateway.networking.k8s.io + kind: HTTPRoute + - group: gateway.networking.k8s.io + kind: GRPCRoute +httpRoutes: +- apiVersion: gateway.networking.k8s.io/v1 + kind: HTTPRoute + metadata: + name: canary + namespace: default + spec: + parentRefs: + - name: gateway-1 + namespace: envoy-gateway + rules: + - backendRefs: + - name: canary + port: 3000 + weight: 50 + - name: canary + port: 4000 + weight: 0 + matches: + - path: + type: PathPrefix + value: /canary + status: + parents: + - conditions: + - lastTransitionTime: null + message: Route is accepted + reason: Accepted + status: "True" + type: Accepted + - lastTransitionTime: null + message: Resolved all the Object references for the Route + reason: ResolvedRefs + status: "True" + type: ResolvedRefs + controllerName: gateway.envoyproxy.io/gatewayclass-controller + parentRef: + name: gateway-1 + namespace: envoy-gateway +infraIR: + envoy-gateway/gateway-1: + proxy: + config: + apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: EnvoyProxy + metadata: + name: test + namespace: envoy-gateway-system + spec: + logging: {} + mergeBackends: {} + status: {} + listeners: + - name: envoy-gateway/gateway-1/http + ports: + - containerPort: 10080 + name: http-80 + protocol: HTTP + servicePort: 80 + metadata: + labels: + gateway.envoyproxy.io/owning-gateway-name: gateway-1 + gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway + ownerReference: + kind: GatewayClass + name: envoy-gateway-class + name: envoy-gateway/gateway-1 + namespace: envoy-gateway-system +xdsIR: + envoy-gateway/gateway-1: + accessLog: + json: + - path: /dev/stdout + backendClusters: + - metadata: + kind: Service + name: canary + namespace: default + sectionName: "3000" + name: service/default/canary/3000/http + setting: + addressType: IP + endpoints: + - host: 10.244.0.11 + port: 3000 + metadata: + kind: Service + name: canary + namespace: default + sectionName: "3000" + name: service/default/canary/3000/http + protocol: HTTP + - metadata: + kind: Service + name: canary + namespace: default + sectionName: "4000" + name: service/default/canary/4000/http + setting: + addressType: IP + endpoints: + - host: 10.244.0.11 + port: 3000 + metadata: + kind: Service + name: canary + namespace: default + sectionName: "4000" + name: service/default/canary/4000/http + protocol: HTTP + globalResources: + proxyServiceCluster: + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + settings: + - addressType: IP + endpoints: + - host: 7.6.5.4 + port: 8080 + zone: zone1 + metadata: + kind: Service + name: envoy-envoy-gateway-gateway-1-196ae069 + namespace: envoy-gateway-system + sectionName: "8080" + name: envoy-gateway/gateway-1 + protocol: TCP + http: + - address: 0.0.0.0 + externalPort: 80 + hostnames: + - '*' + metadata: + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http + name: envoy-gateway/gateway-1/http + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + routes: + - destination: + backendClusterRefs: + - name: service/default/canary/3000/http + weight: 50 + - name: service/default/canary/4000/http + weight: 0 + metadata: + kind: HTTPRoute + name: canary + namespace: default + name: httproute/default/canary/rule/0 + hostname: '*' + isHTTP2: false + metadata: + kind: HTTPRoute + name: canary + namespace: default + name: httproute/default/canary/rule/0/match/0/* + pathMatch: + distinct: false + name: "" + prefix: /canary + readyListener: + address: 0.0.0.0 + ipFamily: IPv4 + path: /ready + port: 19003 diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 44994974fd1..5c699bdf604 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -2298,7 +2298,20 @@ func (r *RouteDestination) NeedsClusterPerSetting() bool { r.HasFiltersInSettings() || (len(r.Settings) > 1 && r.HasPreferLocalZone()) || r.HasMixedUpstreamProtocolRequirements() || - r.HasMixedAutoSNISettings() + r.HasMixedAutoSNISettings() || + r.HasZeroWeightSettings() +} + +// HasZeroWeightSettings returns true if any setting has an explicit zero weight. A zero locality +// weight is invalid in a shared cluster, so such a setting gets its own cluster and a zero-weight +// weighted_clusters entry. +func (r *RouteDestination) HasZeroWeightSettings() bool { + for _, s := range r.Settings { + if s.Weight != nil && *s.Weight == 0 { + return true + } + } + return false } // HasMixedEndpoints returns true if the RouteDestination has endpoints of multiple types diff --git a/internal/ir/xds_test.go b/internal/ir/xds_test.go index 7eace63e8e0..cfcb5cfa2b5 100644 --- a/internal/ir/xds_test.go +++ b/internal/ir/xds_test.go @@ -1402,6 +1402,17 @@ func TestRouteDestination_NeedsClusterPerSetting(t *testing.T) { }, expected: false, }, + { + name: "zero-weight setting", + input: RouteDestination{ + Name: "zero-weight", + Settings: []*DestinationSetting{ + {Weight: new(uint32(1))}, + {Weight: new(uint32(0))}, + }, + }, + expected: true, + }, } for _, test := range tests { diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go index d57e65dc007..7305062d4da 100644 --- a/internal/xds/translator/cluster.go +++ b/internal/xds/translator/cluster.go @@ -1191,12 +1191,11 @@ func buildWeightedLocalities(metadata *corev3.Metadata, ds *ir.DestinationSettin Metadata: buildXdsMetadata(ds.Metadata), } - // Set locality weight - var weight uint32 - if ds.Weight != nil { + // Set locality weight. A zero-weight setting has a cluster of its own (NeedsClusterPerSetting), + // and Envoy requires a locality weight of at least 1. + weight := uint32(1) + if ds.Weight != nil && *ds.Weight > 0 { weight = *ds.Weight - } else { - weight = 1 } locality.LoadBalancingWeight = &wrapperspb.UInt32Value{Value: weight} locality.Priority = ptr.Deref(ds.Priority, 0) diff --git a/internal/xds/translator/testdata/in/xds-ir/http-route-weighted-backend-zero-weight.yaml b/internal/xds/translator/testdata/in/xds-ir/http-route-weighted-backend-zero-weight.yaml new file mode 100644 index 00000000000..abae3c80260 --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/http-route-weighted-backend-zero-weight.yaml @@ -0,0 +1,53 @@ +# A zero-weight backend keeps its own cluster and a zero-weight weighted_clusters entry, +# both for route-owned settings and for merged backend clusters. +backendClusters: +- name: "canary/stable" + setting: + name: "canary/stable" + endpoints: + - host: "3.3.3.3" + port: 50003 +- name: "canary/preview" + setting: + name: "canary/preview" + endpoints: + - host: "4.4.4.4" + port: 50004 +http: +- name: "first-listener" + address: "::" + port: 10080 + hostnames: + - "*" + path: + mergeSlashes: true + escapedSlashesAction: UnescapeAndRedirect + routes: + - name: "first-route" + hostname: "*" + pathMatch: + prefix: "/first" + destination: + name: "first-route-dest" + settings: + - endpoints: + - host: "1.1.1.1" + port: 50001 + weight: 100 + name: "first-route-dest/backend/0" + - endpoints: + - host: "2.2.2.2" + port: 50002 + weight: 0 + name: "first-route-dest/backend/1" + - name: "merged-route" + hostname: "*" + pathMatch: + prefix: "/merged" + destination: + name: "merged-route-dest" + backendClusterRefs: + - name: "canary/stable" + weight: 100 + - name: "canary/preview" + weight: 0 diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.clusters.yaml new file mode 100644 index 00000000000..a3c7c1f9e10 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.clusters.yaml @@ -0,0 +1,92 @@ +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_PREFERRED + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: first-route-dest/backend/0 + ignoreHealthOnHostRemoval: true + loadBalancingPolicy: + policies: + - typedExtensionConfig: + name: envoy.load_balancing_policies.least_request + typedConfig: + '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest + localityLbConfig: + localityWeightedLbConfig: {} + name: first-route-dest/backend/0 + perConnectionBufferLimitBytes: 32768 + type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_PREFERRED + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: first-route-dest/backend/1 + ignoreHealthOnHostRemoval: true + loadBalancingPolicy: + policies: + - typedExtensionConfig: + name: envoy.load_balancing_policies.least_request + typedConfig: + '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest + localityLbConfig: + localityWeightedLbConfig: {} + name: first-route-dest/backend/1 + perConnectionBufferLimitBytes: 32768 + type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_PREFERRED + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: canary/stable + ignoreHealthOnHostRemoval: true + loadBalancingPolicy: + policies: + - typedExtensionConfig: + name: envoy.load_balancing_policies.least_request + typedConfig: + '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest + localityLbConfig: + localityWeightedLbConfig: {} + name: canary/stable + perConnectionBufferLimitBytes: 32768 + type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_PREFERRED + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: canary/preview + ignoreHealthOnHostRemoval: true + loadBalancingPolicy: + policies: + - typedExtensionConfig: + name: envoy.load_balancing_policies.least_request + typedConfig: + '@type': type.googleapis.com/envoy.extensions.load_balancing_policies.least_request.v3.LeastRequest + localityLbConfig: + localityWeightedLbConfig: {} + name: canary/preview + perConnectionBufferLimitBytes: 32768 + type: EDS diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.endpoints.yaml new file mode 100644 index 00000000000..22828885a32 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.endpoints.yaml @@ -0,0 +1,48 @@ +- clusterName: first-route-dest/backend/0 + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.1.1.1 + portValue: 50001 + loadBalancingWeight: 1 + loadBalancingWeight: 100 + locality: + region: first-route-dest/backend/0 +- clusterName: first-route-dest/backend/1 + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 2.2.2.2 + portValue: 50002 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: first-route-dest/backend/1 +- clusterName: canary/stable + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 3.3.3.3 + portValue: 50003 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: canary/stable +- clusterName: canary/preview + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 4.4.4.4 + portValue: 50004 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: canary/preview diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.listeners.yaml new file mode 100644 index 00000000000..5dd5e46e3cf --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.listeners.yaml @@ -0,0 +1,35 @@ +- address: + socketAddress: + address: '::' + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + commonHttpProtocolOptions: + headersWithUnderscoresAction: REJECT_REQUEST + http2ProtocolOptions: + initialConnectionWindowSize: 1048576 + initialStreamWindowSize: 65536 + maxConcurrentStreams: 100 + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + suppressEnvoyHeaders: true + mergeSlashes: true + normalizePath: true + pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT + rds: + configSource: + ads: {} + resourceApiVersion: V3 + routeConfigName: first-listener + serverHeaderTransformation: PASS_THROUGH + statPrefix: http-10080 + useRemoteAddress: true + name: first-listener + maxConnectionsToAcceptPerSocketEvent: 1 + name: first-listener + perConnectionBufferLimitBytes: 32768 diff --git a/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.routes.yaml new file mode 100644 index 00000000000..f5bad9ada36 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/http-route-weighted-backend-zero-weight.routes.yaml @@ -0,0 +1,33 @@ +- ignorePortInHostMatching: true + name: first-listener + virtualHosts: + - domains: + - '*' + name: first-listener/* + routes: + - match: + pathSeparatedPrefix: /first + name: first-route + route: + clusterNotFoundResponseCode: INTERNAL_SERVER_ERROR + upgradeConfigs: + - upgradeType: websocket + weightedClusters: + clusters: + - name: first-route-dest/backend/0 + weight: 100 + - name: first-route-dest/backend/1 + weight: 0 + - match: + pathSeparatedPrefix: /merged + name: merged-route + route: + clusterNotFoundResponseCode: INTERNAL_SERVER_ERROR + upgradeConfigs: + - upgradeType: websocket + weightedClusters: + clusters: + - name: canary/stable + weight: 100 + - name: canary/preview + weight: 0 diff --git a/release-notes/current/bug_fixes/10467-keep-zero-weight-backend-clusters.md b/release-notes/current/bug_fixes/10467-keep-zero-weight-backend-clusters.md new file mode 100644 index 00000000000..445806f336e --- /dev/null +++ b/release-notes/current/bug_fixes/10467-keep-zero-weight-backend-clusters.md @@ -0,0 +1 @@ +Fixed zero-weight HTTPRoute and GRPCRoute backendRefs being dropped from xDS, so their clusters now exist (with a zero-weight route entry) before a canary rollout first shifts traffic to them.