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.

Sign up for GitHub

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

Jump to bottom

tcproute/udproute support multiple backends #3212

Merged
merged 18 commits into from
May 15, 2024
Merged

tcproute/udproute support multiple backends #3212

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
ca918c0
tcproute support multiple backend
zirain Apr 17, 2024
bf79459
updroute support multiple backend
zirain Apr 17, 2024
f6fa117
use BackendWeights
zirain May 6, 2024
1d6c561
lint
zirain May 6, 2024
52d5253
gofumpt
zirain May 6, 2024
1f1f454
fix test
zirain May 6, 2024
ce66fc5
fix gen
zirain May 6, 2024
cf36a37
Merge branch 'main' into tcproute-backendrefs
zirain May 7, 2024
1598933
Merge branch 'main' into tcproute-backendrefs
zirain May 7, 2024
cbfc3af
gen
zirain May 7, 2024
e6d5676
Merge branch 'main' into tcproute-backendrefs
zirain May 8, 2024
16c6627
comments
zirain May 13, 2024
2e7c31b
Merge branch 'main' into tcproute-backendrefs
zirain May 13, 2024
df0af7c
comments
zirain May 13, 2024
115fdb1
DirectResponse
zirain May 13, 2024
9084108
remove empty tls
zirain May 13, 2024
0b1213a
update
zirain May 15, 2024
5053054
nit
zirain May 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 1 addition & 4 deletions internal/cmd/egctl/testdata/translate/out/quickstart.all.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/envoy-gateway-system/backend/rule/0
settings:
- endpoints:
Expand Down
2 changes: 1 addition & 1 deletion internal/gatewayapi/filters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,7 +797,7 @@ func (t *Translator) processRequestMirrorFilter(
return
}

ds, _ := t.processDestination(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources)
ds := t.processDestination(mirrorBackendRef, filterContext.ParentRef, filterContext.Route, resources)

newMirror := &ir.RouteDestination{
Name: fmt.Sprintf("%s-mirror-%d", irRouteDestinationName(filterContext.Route, filterContext.RuleIdx), filterIdx),
Expand Down
119 changes: 46 additions & 73 deletions internal/gatewayapi/route.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,27 +187,27 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe

for _, backendRef := range rule.BackendRefs {
backendRef := backendRef
ds, backendWeight := t.processDestination(backendRef, parentRef, httpRoute, resources)
ds := t.processDestination(backendRef, parentRef, httpRoute, resources)
if !t.EndpointRoutingDisabled && ds != nil && len(ds.Endpoints) > 0 && ds.AddressType != nil {
dstAddrTypeMap[*ds.AddressType]++
}
if ds == nil {
continue
}

for _, route := range ruleRoutes {
// If the route already has a direct response or redirect configured, then it was from a filter so skip
// processing any destinations for this route.
if route.DirectResponse == nil && route.Redirect == nil {
if ds != nil && len(ds.Endpoints) > 0 {
if route.Destination == nil {
route.Destination = &ir.RouteDestination{
Name: irRouteDestinationName(httpRoute, ruleIdx),
}
}
route.Destination.Settings = append(route.Destination.Settings, ds)
route.BackendWeights.Valid += backendWeight
} else {
route.BackendWeights.Invalid += backendWeight
if route.DirectResponse != nil || route.Redirect != nil {
continue
}

if route.Destination == nil {
route.Destination = &ir.RouteDestination{
Name: irRouteDestinationName(httpRoute, ruleIdx),
}
}
route.Destination.Settings = append(route.Destination.Settings, ds)
}
}

Expand Down Expand Up @@ -491,24 +491,24 @@ func (t *Translator) processGRPCRouteRules(grpcRoute *GRPCRouteContext, parentRe

for _, backendRef := range rule.BackendRefs {
backendRef := backendRef
ds, backendWeight := t.processDestination(backendRef, parentRef, grpcRoute, resources)
ds := t.processDestination(backendRef, parentRef, grpcRoute, resources)
if ds == nil {
continue
}

for _, route := range ruleRoutes {
// If the route already has a direct response or redirect configured, then it was from a filter so skip
// processing any destinations for this route.
if route.DirectResponse == nil && route.Redirect == nil {
if ds != nil && len(ds.Endpoints) > 0 {
if route.Destination == nil {
route.Destination = &ir.RouteDestination{
Name: irRouteDestinationName(grpcRoute, ruleIdx),
}
}
route.Destination.Settings = append(route.Destination.Settings, ds)
route.BackendWeights.Valid += backendWeight
if route.DirectResponse != nil || route.Redirect != nil {
continue
}

} else {
route.BackendWeights.Invalid += backendWeight
if route.Destination == nil {
route.Destination = &ir.RouteDestination{
Name: irRouteDestinationName(grpcRoute, ruleIdx),
}
}
route.Destination.Settings = append(route.Destination.Settings, ds)
}
}

Expand Down Expand Up @@ -685,10 +685,6 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route
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
}
perHostRoutes = append(perHostRoutes, hostRoute)
}
}
Expand Down Expand Up @@ -746,7 +742,7 @@ func (t *Translator) processTLSRouteParentRefs(tlsRoute *TLSRouteContext, resour
for _, rule := range tlsRoute.Spec.Rules {
for _, backendRef := range rule.BackendRefs {
backendRef := backendRef
ds, _ := t.processDestination(backendRef, parentRef, tlsRoute, resources)
ds := t.processDestination(backendRef, parentRef, tlsRoute, resources)
if ds != nil {
destSettings = append(destSettings, ds)
}
Expand Down Expand Up @@ -884,27 +880,16 @@ func (t *Translator) processUDPRouteParentRefs(udpRoute *UDPRouteContext, resour
)
continue
}
if len(udpRoute.Spec.Rules[0].BackendRefs) != 1 {
routeStatus := GetRouteStatus(udpRoute)
status.SetRouteStatusCondition(routeStatus,
parentRef.routeParentStatusIdx,
udpRoute.GetGeneration(),
gwapiv1.RouteConditionResolvedRefs,
metav1.ConditionFalse,
"InvalidBackend",
"One and only one backend is supported",
)
continue
}

backendRef := udpRoute.Spec.Rules[0].BackendRefs[0]
ds, _ := t.processDestination(backendRef, parentRef, udpRoute, resources)
// Skip further processing if route destination is not valid
if ds == nil || len(ds.Endpoints) == 0 {
continue
for _, backendRef := range udpRoute.Spec.Rules[0].BackendRefs {
ds := t.processDestination(backendRef, parentRef, udpRoute, resources)
if ds == nil {
continue
}

destSettings = append(destSettings, ds)
}

destSettings = append(destSettings, ds)
// If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True"
if !parentRef.HasCondition(udpRoute, gwapiv1.RouteConditionResolvedRefs, metav1.ConditionFalse) {
routeStatus := GetRouteStatus(udpRoute)
Expand Down Expand Up @@ -1013,7 +998,6 @@ func (t *Translator) ProcessTCPRoutes(tcpRoutes []*gwapiv1a2.TCPRoute, gateways

func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resources *Resources, xdsIR XdsIRMap) {
for _, parentRef := range tcpRoute.ParentRefs {

// Need to compute Route rules within the parentRef loop because
// any conditions that come out of it have to go on each RouteParentStatus,
// not on the Route as a whole.
Expand All @@ -1032,26 +1016,17 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
)
continue
}
if len(tcpRoute.Spec.Rules[0].BackendRefs) != 1 {
routeStatus := GetRouteStatus(tcpRoute)
status.SetRouteStatusCondition(routeStatus,
parentRef.routeParentStatusIdx,
tcpRoute.GetGeneration(),
gwapiv1.RouteConditionResolvedRefs,
metav1.ConditionFalse,
"InvalidBackend",
"One and only one backend is supported",
)
continue
}

backendRef := tcpRoute.Spec.Rules[0].BackendRefs[0]
ds, _ := t.processDestination(backendRef, parentRef, tcpRoute, resources)
// Skip further processing if route destination is not valid
if ds == nil || len(ds.Endpoints) == 0 {
continue
for _, backendRef := range tcpRoute.Spec.Rules[0].BackendRefs {
backendRef := backendRef
ds := t.processDestination(backendRef, parentRef, tcpRoute, resources)
if ds == nil {
continue
}

destSettings = append(destSettings, ds)
}
destSettings = append(destSettings, ds)

// If no negative condition has been set for ResolvedRefs, set "ResolvedRefs=True"
if !parentRef.HasCondition(tcpRoute, gwapiv1.RouteConditionResolvedRefs, metav1.ConditionFalse) {
routeStatus := GetRouteStatus(tcpRoute)
Expand Down Expand Up @@ -1131,10 +1106,8 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
// returns the weight for the backend so that 500 error responses can be returned for invalid backends in
// the same proportion as the backend would have otherwise received
func (t *Translator) processDestination(backendRefContext BackendRefContext,
parentRef *RouteParentContext,
route RouteContext,
resources *Resources,
) (ds *ir.DestinationSetting, backendWeight uint32) {
parentRef *RouteParentContext, route RouteContext, resources *Resources,
) (ds *ir.DestinationSetting) {
routeType := GetRouteType(route)
weight := uint32(1)
backendRef := GetBackendRef(backendRefContext)
Expand All @@ -1144,12 +1117,12 @@ func (t *Translator) processDestination(backendRefContext BackendRefContext,

backendNamespace := NamespaceDerefOr(backendRef.Namespace, route.GetNamespace())
if !t.validateBackendRef(backendRefContext, parentRef, route, resources, backendNamespace, routeType) {
return nil, weight
return &ir.DestinationSetting{Weight: &weight}
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add a comment here highlighting that this is referring to an invalid weight

}

// Skip processing backends with 0 weight
if weight == 0 {
return nil, weight
return nil
}

var (
Expand Down Expand Up @@ -1242,7 +1215,7 @@ func (t *Translator) processDestination(backendRefContext BackendRefContext,
AddressType: addrType,
TLS: backendTLS,
}
return ds, weight
return ds
}

func inspectAppProtocolByRouteKind(kind gwapiv1.Kind) ir.AppProtocol {
Expand Down
5 changes: 1 addition & 4 deletions internal/gatewayapi/testdata/backendtlspolicy-ca-only.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/envoy-gateway/httproute-btls/rule/0
settings:
- addressType: IP
Expand Down
5 changes: 1 addition & 4 deletions internal/gatewayapi/testdata/backendtlspolicy-default-ns.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/envoy-gateway/httproute-btls/rule/0
settings:
- addressType: IP
Expand Down
5 changes: 1 addition & 4 deletions internal/gatewayapi/testdata/backendtlspolicy-invalid-ca.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/envoy-gateway/httproute-btls/rule/0
settings:
- addressType: IP
Expand Down
5 changes: 1 addition & 4 deletions internal/gatewayapi/testdata/backendtlspolicy-system-truststore.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,10 +140,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/envoy-gateway/httproute-btls/rule/0
settings:
- addressType: IP
Expand Down
5 changes: 1 addition & 4 deletions internal/gatewayapi/testdata/backendtlspolicy-without-referencegrant.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,10 +144,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/envoy-gateway/httproute-btls/rule/0
settings:
- addressType: IP
Expand Down
10 changes: 2 additions & 8 deletions internal/gatewayapi/testdata/backendtrafficpolicy-override-replace.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/default/httproute-1/rule/0
settings:
- addressType: IP
Expand All @@ -242,10 +239,7 @@ xdsIR:
distinct: false
name: ""
prefix: /foo
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/default/httproute-2/rule/0
settings:
- addressType: IP
Expand Down
18 changes: 8 additions & 10 deletions internal/gatewayapi/testdata/backendtrafficpolicy-status-conditions.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -548,11 +548,10 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 1
valid: 0
directResponse:
statusCode: 500
- destination:
name: httproute/envoy-gateway/httproute-1/rule/0
settings:
- weight: 1
hostname: '*'
isHTTP2: false
name: httproute/envoy-gateway/httproute-1/rule/0/match/0/*
Expand All @@ -575,11 +574,10 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 1
valid: 0
directResponse:
Copy link
Contributor

Choose a reason for hiding this comment

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

why is directResponse getting removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed, see 115fdb1

statusCode: 500
- destination:
name: grpcroute/envoy-gateway/grpcroute-1/rule/0
settings:
- weight: 1
headerMatches:
- distinct: false
exact: foo
Expand Down
15 changes: 3 additions & 12 deletions internal/gatewayapi/testdata/backendtrafficpolicy-status-fault-injection.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,10 +342,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: grpcroute/default/grpcroute-1/rule/0
settings:
- addressType: IP
Expand Down Expand Up @@ -379,10 +376,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/default/httproute-2/rule/0
settings:
- addressType: IP
Expand All @@ -402,10 +396,7 @@ xdsIR:
distinct: false
name: ""
prefix: /route2
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/default/httproute-1/rule/0
settings:
- addressType: IP
Expand Down
5 changes: 1 addition & 4 deletions internal/gatewayapi/testdata/backendtrafficpolicy-use-client-protocol.out.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,7 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/default/httproute-1/rule/0
settings:
- addressType: IP
Expand Down