-
Notifications
You must be signed in to change notification settings - Fork 360
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
Changes from 15 commits
ca918c0
bf79459
f6fa117
1d6c561
52d5253
1f1f454
ce66fc5
cf36a37
1598933
cbfc3af
e6d5676
16c6627
2e7c31b
df0af7c
115fdb1
9084108
0b1213a
5053054
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
} | ||
} | ||
|
||
|
@@ -493,30 +493,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 | ||
if noValidBackends && ruleRoute.Redirect == nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isnt something like this needed in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch |
||
ruleRoute.DirectResponse = &ir.DirectResponse{ | ||
StatusCode: 500, | ||
} | ||
|
@@ -685,10 +686,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, | ||
|
@@ -752,7 +749,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) | ||
} | ||
|
@@ -890,27 +887,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) | ||
|
@@ -1019,7 +1005,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. | ||
|
@@ -1038,26 +1023,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) | ||
|
@@ -1145,10 +1121,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) | ||
|
@@ -1158,12 +1132,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} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( | ||
|
@@ -1256,7 +1230,7 @@ func (t *Translator) processDestination(backendRefContext BackendRefContext, | |
AddressType: addrType, | ||
TLS: backendTLS, | ||
} | ||
return ds, weight | ||
return ds | ||
} | ||
|
||
func inspectAppProtocolByRouteKind(kind gwapiv1.Kind) ir.AppProtocol { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -545,11 +545,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/* | ||
|
@@ -573,9 +572,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: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed, see 115fdb1 |
||
statusCode: 500 | ||
headerMatches: | ||
|
There was a problem hiding this comment.
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 ofnoValidBackends
i.e. some instead of all ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
noValidBackends = invalidBackendsExist | emptyBackends
There was a problem hiding this comment.
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 hereThere was a problem hiding this comment.
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