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

tcproute/udproute support multiple backends #3212

Merged
merged 18 commits into from
May 15, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,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
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
136 changes: 58 additions & 78 deletions internal/gatewayapi/route.go
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 All @@ -225,7 +225,8 @@ func (t *Translator) processHTTPRouteRules(httpRoute *HTTPRouteContext, parentRe

// If the route has no valid backends then just use a direct response and don't fuss with weighted responses
for _, ruleRoute := range ruleRoutes {
if ruleRoute.Destination == nil && ruleRoute.Redirect == nil {
noValidBackends := ruleRoute.Destination == nil || ruleRoute.Destination.ToBackendWeights().Valid == 0
if noValidBackends && ruleRoute.Redirect == nil {
ruleRoute.DirectResponse = &ir.DirectResponse{
StatusCode: 500,
}
Expand Down Expand Up @@ -493,30 +494,31 @@ 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)
}
}

// If the route has no valid backends then just use a direct response and don't fuss with weighted responses
for _, ruleRoute := range ruleRoutes {
if ruleRoute.Destination == nil && ruleRoute.Redirect == nil {
noValidBackends := ruleRoute.Destination == nil || ruleRoute.Destination.ToBackendWeights().Valid == 0
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldnt this be invalidBackendsExist instead of noValidBackends i.e. some instead of all ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

noValidBackends = invalidBackendsExist | emptyBackends

Copy link
Contributor

Choose a reason for hiding this comment

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

but noValidBackends also implies validBackends == 0 which is not true here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in current code validBackends == 0 equals emptyBackends

if noValidBackends && ruleRoute.Redirect == nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

isnt something like this needed in processHTTPRouteRules as well ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch

ruleRoute.DirectResponse = &ir.DirectResponse{
StatusCode: 500,
}
Expand Down Expand Up @@ -685,10 +687,6 @@ func (t *Translator) processHTTPRouteParentRefListener(route RouteContext, route
ExtensionRefs: routeRoute.ExtensionRefs,
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,
Expand Down Expand Up @@ -752,7 +750,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 @@ -890,27 +888,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 @@ -1019,7 +1006,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 @@ -1038,26 +1024,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 @@ -1088,9 +1065,13 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
accepted = true
irKey := t.getIRKey(listener.gateway)

tls := ir.TLS{
Terminate: irTLSConfigs(listener.tlsSecrets),
var tls *ir.TLS
if len(listener.tlsSecrets) > 0 {
tls = &ir.TLS{
Terminate: irTLSConfigs(listener.tlsSecrets),
}
}

if listener.Hostname != nil {
tls.TLSInspectorConfig = &ir.TLSInspectorConfig{
SNIs: []string{string(*listener.Hostname)},
Expand All @@ -1105,7 +1086,7 @@ func (t *Translator) processTCPRouteParentRefs(tcpRoute *TCPRouteContext, resour
Name: irRouteDestinationName(tcpRoute, -1 /*rule index*/),
Settings: destSettings,
},
TLS: &tls,
TLS: tls,
}
irListener.Routes = append(irListener.Routes, irRoute)

Expand Down Expand Up @@ -1145,10 +1126,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 @@ -1158,12 +1137,13 @@ 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 with empty endpoint means the backend is invalid
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 @@ -1256,7 +1236,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
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,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
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,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
Original file line number Diff line number Diff line change
Expand Up @@ -142,10 +142,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
Original file line number Diff line number Diff line change
Expand Up @@ -139,10 +139,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
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
Original file line number Diff line number Diff line change
Expand Up @@ -218,10 +218,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 @@ -241,10 +238,7 @@ xdsIR:
loadBalancer:
consistentHash:
sourceIP: true
- backendWeights:
invalid: 0
valid: 0
destination:
- destination:
name: httproute/default/httproute-2/rule/0
settings:
- addressType: IP
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,9 +545,10 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 1
valid: 0
- destination:
name: httproute/envoy-gateway/httproute-1/rule/0
settings:
- weight: 1
directResponse:
statusCode: 500
hostname: '*'
Expand All @@ -573,9 +574,10 @@ xdsIR:
mergeSlashes: true
port: 10080
routes:
- backendWeights:
invalid: 1
valid: 0
- destination:
name: grpcroute/envoy-gateway/grpcroute-1/rule/0
settings:
- weight: 1
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
headerMatches:
Expand Down
Loading
Loading