Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: group xds backend traffic features for BackendTrafficPolicy #3189

Merged
merged 10 commits into from
May 13, 2024
100 changes: 42 additions & 58 deletions internal/gatewayapi/backendtrafficpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
}
}

for _, ir := range xdsIR {
for _, tcp := range ir.TCP {
for _, x := range xdsIR {
for _, tcp := range x.TCP {
for _, r := range tcp.Routes {
if strings.HasPrefix(r.Destination.Name, prefix) {
r.LoadBalancer = lb
Expand All @@ -353,34 +353,37 @@ func (t *Translator) translateBackendTrafficPolicyForRoute(policy *egv1a1.Backen
}
}

for _, udp := range ir.UDP {
for _, udp := range x.UDP {
if strings.HasPrefix(udp.Destination.Name, prefix) {
udp.LoadBalancer = lb
udp.Timeout = to
}
}

for _, http := range ir.HTTP {
for _, http := range x.HTTP {
for _, r := range http.Routes {
// Apply if there is a match
if strings.HasPrefix(r.Name, prefix) {
r.RateLimit = rl
r.LoadBalancer = lb
r.ProxyProtocol = pp
r.HealthCheck = hc
r.Traffic = &ir.TrafficFeatures{
RateLimit: rl,
LoadBalancer: lb,
ProxyProtocol: pp,
HealthCheck: hc,
CircuitBreaker: cb,
FaultInjection: fi,
TCPKeepalive: ka,
Retry: rt,
}

// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
r.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)
r.CircuitBreaker = cb
r.FaultInjection = fi
r.TCPKeepalive = ka
r.Retry = rt
r.Traffic.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)

// some timeout setting originate from the route
// Some timeout setting originate from the route.
if policy.Spec.Timeout != nil {
if to, err = t.buildTimeout(policy, r); err != nil {
return errors.Wrap(err, "Timeout")
}
r.Timeout = to
r.Traffic.Timeout = to
}

if policy.Spec.UseClientProtocol != nil {
Expand Down Expand Up @@ -445,7 +448,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
// set by a policy attaching to the route
irKey := t.getIRKey(gateway.Gateway)
// Should exist since we've validated this
ir := xdsIR[irKey]
x := xdsIR[irKey]

policyTarget := irStringKey(policy.Namespace, string(policy.Spec.TargetRef.Name))

Expand All @@ -455,7 +458,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
}
}

for _, tcp := range ir.TCP {
for _, tcp := range x.TCP {
gatewayName := tcp.Name[0:strings.LastIndex(tcp.Name, "/")]
if t.MergeGateways && gatewayName != policyTarget {
continue
Expand All @@ -481,7 +484,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
}
}

for _, udp := range ir.UDP {
for _, udp := range x.UDP {
gatewayName := udp.Name[0:strings.LastIndex(udp.Name, "/")]
if t.MergeGateways && gatewayName != policyTarget {
continue
Expand All @@ -498,7 +501,7 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
}
}

for _, http := range ir.HTTP {
for _, http := range x.HTTP {
gatewayName := http.Name[0:strings.LastIndex(http.Name, "/")]
if t.MergeGateways && gatewayName != policyTarget {
continue
Expand All @@ -509,52 +512,29 @@ func (t *Translator) translateBackendTrafficPolicyForGateway(policy *egv1a1.Back
for _, r := range http.Routes {
// If any of the features are already set, it means that a more specific
// policy(targeting xRoute) has already set it, so we skip it.
// TODO: zhaohuabing group the features into a struct and check if all of them are set
if r.RateLimit != nil || r.LoadBalancer != nil ||
r.ProxyProtocol != nil || r.HealthCheck != nil ||
r.CircuitBreaker != nil || r.FaultInjection != nil ||
r.TCPKeepalive != nil || r.Retry != nil ||
r.Timeout != nil {
if r.Traffic != nil {
continue
}

// Apply if not already set
if r.RateLimit == nil {
r.RateLimit = rl
}
if r.LoadBalancer == nil {
r.LoadBalancer = lb
}
if r.ProxyProtocol == nil {
r.ProxyProtocol = pp
}
if r.HealthCheck == nil {
r.HealthCheck = hc
// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
r.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)
r.Traffic = &ir.TrafficFeatures{
RateLimit: rl,
LoadBalancer: lb,
ProxyProtocol: pp,
HealthCheck: hc,
CircuitBreaker: cb,
FaultInjection: fi,
TCPKeepalive: ka,
Retry: rt,
}

if r.CircuitBreaker == nil {
r.CircuitBreaker = cb
}
if r.FaultInjection == nil {
r.FaultInjection = fi
}
if r.TCPKeepalive == nil {
r.TCPKeepalive = ka
}
if r.Retry == nil {
r.Retry = rt
}
// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
r.Traffic.HealthCheck.SetHTTPHostIfAbsent(r.Hostname)

if policy.Spec.Timeout != nil {
if ct, err = t.buildTimeout(policy, r); err != nil {
return errors.Wrap(err, "Timeout")
}

if r.Timeout == nil {
r.Timeout = ct
}
r.Traffic.Timeout = ct
}

if policy.Spec.UseClientProtocol != nil {
Expand Down Expand Up @@ -1066,13 +1046,17 @@ func (t *Translator) buildTimeout(policy *egv1a1.BackendTrafficPolicy, r *ir.HTT

// http request timeout is translated during the gateway-api route resource translation
// merge route timeout setting with backendtrafficpolicy timeout settings
if r != nil && r.Timeout != nil && r.Timeout.HTTP != nil && r.Timeout.HTTP.RequestTimeout != nil {
if r != nil &&
r.Traffic != nil &&
r.Traffic.Timeout != nil &&
r.Traffic.Timeout.HTTP != nil &&
r.Traffic.Timeout.HTTP.RequestTimeout != nil {
if hto == nil {
hto = &ir.HTTPTimeout{
RequestTimeout: r.Timeout.HTTP.RequestTimeout,
RequestTimeout: r.Traffic.Timeout.HTTP.RequestTimeout,
}
} else {
hto.RequestTimeout = r.Timeout.HTTP.RequestTimeout
hto.RequestTimeout = r.Traffic.Timeout.HTTP.RequestTimeout
}
}

Expand Down
16 changes: 11 additions & 5 deletions internal/gatewayapi/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,8 +248,8 @@ func processTimeout(irRoute *ir.HTTPRoute, rule gwapiv1.HTTPRouteRule) {
var rto *ir.Timeout

// Timeout is translated from multiple resources and may already be partially set
if irRoute.Timeout != nil {
rto = irRoute.Timeout.DeepCopy()
if irRoute.Traffic != nil && irRoute.Traffic.Timeout != nil {
rto = irRoute.Traffic.Timeout.DeepCopy()
} else {
rto = &ir.Timeout{}
}
Expand All @@ -272,7 +272,9 @@ func processTimeout(irRoute *ir.HTTPRoute, rule gwapiv1.HTTPRouteRule) {
setRequestTimeout(rto, metav1.Duration{Duration: d})
}

irRoute.Timeout = rto
irRoute.Traffic = &ir.TrafficFeatures{
Timeout: rto,
}
}
}

Expand Down Expand Up @@ -681,14 +683,18 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route
URLRewrite: routeRoute.URLRewrite,
Mirrors: routeRoute.Mirrors,
ExtensionRefs: routeRoute.ExtensionRefs,
Timeout: routeRoute.Timeout,
Retry: routeRoute.Retry,
IsHTTP2: routeRoute.IsHTTP2,
}
// Don't bother copying over the weights unless the route has invalid backends.
if routeRoute.BackendWeights.Invalid > 0 {
hostRoute.BackendWeights = routeRoute.BackendWeights
}
if routeRoute.Traffic != nil {
hostRoute.Traffic = &ir.TrafficFeatures{
Timeout: routeRoute.Traffic.Timeout,
Retry: routeRoute.Traffic.Retry,
}
}
perHostRoutes = append(perHostRoutes, hostRoute)
}
}
Expand Down
2 changes: 1 addition & 1 deletion internal/gatewayapi/securitypolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ func (t *Translator) translateSecurityPolicyForGateway(
for _, r := range h.Routes {
// If any of the features are already set, it means that a more specific
// policy(targeting xRoute) has already set it, so we skip it.
if !r.Security.Empty() {
if r.Security != nil {
continue
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,14 +232,15 @@ xdsIR:
weight: 1
hostname: gateway.envoyproxy.io
isHTTP2: false
loadBalancer:
consistentHash:
sourceIP: true
name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io
pathMatch:
distinct: false
name: ""
prefix: /foo
traffic:
loadBalancer:
consistentHash:
sourceIP: true
- backendWeights:
invalid: 0
valid: 0
Expand All @@ -254,16 +255,17 @@ xdsIR:
weight: 1
hostname: gateway.envoyproxy.io
isHTTP2: false
loadBalancer:
random: {}
name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io
pathMatch:
distinct: false
name: ""
prefix: /bar
timeout:
http:
connectionIdleTimeout: 21s
maxConnectionDuration: 22s
tcp:
connectTimeout: 20s
traffic:
loadBalancer:
random: {}
timeout:
http:
connectionIdleTimeout: 21s
maxConnectionDuration: 22s
tcp:
connectTimeout: 20s
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ xdsIR:
distinct: false
name: ""
prefix: /
traffic: {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty, is this expected ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed is empty

envoy-gateway/gateway-2:
accessLog:
text:
Expand Down Expand Up @@ -584,6 +585,7 @@ xdsIR:
hostname: '*'
isHTTP2: true
name: grpcroute/envoy-gateway/grpcroute-1/rule/0/match/0/*
traffic: {}
tcp:
- address: 0.0.0.0
name: envoy-gateway/gateway-2/tcp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,16 +351,17 @@ xdsIR:
port: 8080
protocol: GRPC
weight: 1
faultInjection:
abort:
grpcStatus: 14
percentage: 100
delay:
fixedDelay: 5.4s
percentage: 80
hostname: '*'
isHTTP2: true
name: grpcroute/default/grpcroute-1/rule/0/match/-1/*
traffic:
faultInjection:
abort:
grpcStatus: 14
percentage: 100
delay:
fixedDelay: 5.4s
percentage: 80
envoy-gateway/gateway-2:
accessLog:
text:
Expand Down Expand Up @@ -388,17 +389,18 @@ xdsIR:
port: 8080
protocol: HTTP
weight: 1
faultInjection:
abort:
httpStatus: 14
percentage: 0.01
hostname: gateway.envoyproxy.io
isHTTP2: false
name: httproute/default/httproute-2/rule/0/match/0/gateway_envoyproxy_io
pathMatch:
distinct: false
name: ""
prefix: /route2
traffic:
faultInjection:
abort:
httpStatus: 14
percentage: 0.01
- backendWeights:
invalid: 0
valid: 0
Expand All @@ -411,17 +413,18 @@ xdsIR:
port: 8080
protocol: HTTP
weight: 1
faultInjection:
abort:
httpStatus: 500
percentage: 100
delay:
fixedDelay: 5.4s
percentage: 80
hostname: gateway.envoyproxy.io
isHTTP2: false
name: httproute/default/httproute-1/rule/0/match/0/gateway_envoyproxy_io
pathMatch:
distinct: false
name: ""
prefix: /
traffic:
faultInjection:
abort:
httpStatus: 500
percentage: 100
delay:
fixedDelay: 5.4s
percentage: 80
Original file line number Diff line number Diff line change
Expand Up @@ -156,4 +156,5 @@ xdsIR:
distinct: false
name: ""
prefix: /
traffic: {}
useClientProtocol: true
Loading
Loading