Skip to content

Commit

Permalink
refactor: group xds backend traffic features for BackendTrafficPolicy (…
Browse files Browse the repository at this point in the history 
…envoyproxy#3189)

* group xds backend traffic features for BackendTrafficPolicy

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* rm Empty method for SP

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* rename `BackendTraffic` to `Traffic` for HTTPRoute in xds

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* resolve conflicts and add more tests

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* fix lint

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

* fix lint

Signed-off-by: shawnh2 <shawnhxh@outlook.com>

---------

Signed-off-by: shawnh2 <shawnhxh@outlook.com>
  • Loading branch information
@shawnh2
shawnh2 authored May 13, 2024
1 parent 3bc7cf1 commit 6f6d868
Show file tree
Hide file tree
Showing 58 changed files with 1,175 additions and 1,015 deletions.
100 changes: 42 additions & 58 deletions internal/gatewayapi/backendtrafficpolicy.go
View file
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
View file
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
View file
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
24 changes: 13 additions & 11 deletions internal/gatewayapi/testdata/backendtrafficpolicy-override-replace.out.yaml
View file
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
2 changes: 2 additions & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-status-conditions.out.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,7 @@ xdsIR:
distinct: false
name: ""
prefix: /
traffic: {}
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
39 changes: 21 additions & 18 deletions internal/gatewayapi/testdata/backendtrafficpolicy-status-fault-injection.out.yaml
View file
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
1 change: 1 addition & 0 deletions internal/gatewayapi/testdata/backendtrafficpolicy-use-client-protocol.out.yaml
View file
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

0 comments on commit 6f6d868

Please sign in to comment.