diff --git a/changelogs/unreleased/7525-SAY-5-small.md b/changelogs/unreleased/7525-SAY-5-small.md new file mode 100644 index 00000000000..3aa3299a5ad --- /dev/null +++ b/changelogs/unreleased/7525-SAY-5-small.md @@ -0,0 +1 @@ +Fix RetryPolicy numRetries=0 being silently coerced to 1 by populating num_retries explicitly when set, so HTTPProxy retry policies that explicitly request zero retries are honored. diff --git a/internal/envoy/v3/route.go b/internal/envoy/v3/route.go index f35a19f600e..df082fd0547 100644 --- a/internal/envoy/v3/route.go +++ b/internal/envoy/v3/route.go @@ -547,9 +547,14 @@ func retryPolicy(r *dag.Route) *envoy_config_route_v3.RetryPolicy { RetryOn: r.RetryPolicy.RetryOn, RetriableStatusCodes: r.RetryPolicy.RetriableStatusCodes, } - if r.RetryPolicy.NumRetries > 0 { - rp.NumRetries = wrapperspb.UInt32(r.RetryPolicy.NumRetries) - } + // HTTPProxy documents numRetries: -1 as "disable retries". The DAG + // translates -1 to 0 (internal/dag/policy.go), so the DAG value 0 + // here uniquely means "user asked for no retries". Previously we + // only set NumRetries when > 0, and Envoy then defaulted to 1 -- + // giving exactly one retry instead of none (projectcontour#5944). + // Always set NumRetries (including 0) so Envoy respects the + // documented contract. + rp.NumRetries = wrapperspb.UInt32(r.RetryPolicy.NumRetries) rp.PerTryTimeout = envoy.Timeout(r.RetryPolicy.PerTryTimeout) return rp diff --git a/internal/featuretests/v3/envoy.go b/internal/featuretests/v3/envoy.go index 04544a93e68..5cccfd4ef5e 100644 --- a/internal/featuretests/v3/envoy.go +++ b/internal/featuretests/v3/envoy.go @@ -313,10 +313,8 @@ func withPrefixRewrite(route *envoy_config_route_v3.Route_Route, replacement str func withRetryPolicy(route *envoy_config_route_v3.Route_Route, retryOn string, numRetries uint32, perTryTimeout time.Duration) *envoy_config_route_v3.Route_Route { route.Route.RetryPolicy = &envoy_config_route_v3.RetryPolicy{ - RetryOn: retryOn, - } - if numRetries > 0 { - route.Route.RetryPolicy.NumRetries = wrapperspb.UInt32(numRetries) + RetryOn: retryOn, + NumRetries: wrapperspb.UInt32(numRetries), } if perTryTimeout > 0 { route.Route.RetryPolicy.PerTryTimeout = durationpb.New(perTryTimeout) diff --git a/internal/xdscache/v3/route_test.go b/internal/xdscache/v3/route_test.go index 4f01441bbdd..364cd09917c 100644 --- a/internal/xdscache/v3/route_test.go +++ b/internal/xdscache/v3/route_test.go @@ -3896,10 +3896,8 @@ func routetimeout(cluster string, timeout time.Duration) *envoy_config_route_v3. func routeretry(cluster, retryOn string, numRetries uint32, perTryTimeout time.Duration) *envoy_config_route_v3.Route_Route { r := routecluster(cluster) r.Route.RetryPolicy = &envoy_config_route_v3.RetryPolicy{ - RetryOn: retryOn, - } - if numRetries > 0 { - r.Route.RetryPolicy.NumRetries = wrapperspb.UInt32(numRetries) + RetryOn: retryOn, + NumRetries: wrapperspb.UInt32(numRetries), } if perTryTimeout > 0 { r.Route.RetryPolicy.PerTryTimeout = durationpb.New(perTryTimeout)