grpc-go
grpc-go copied to clipboard
xds: report more descriptive error when matched route on client is not of type `RouteActionRoute`
In the config selector's SelectConfig
method, we look for a matching route, and if we don't find one or the matched route does not contain clusters, we fail the RPC: https://github.com/grpc/grpc-go/blob/f2fbb0e07ebf3dd46aa641ee89c9f17e8083eaf6/xds/internal/resolver/serviceconfig.go#L158
It would be better if we check the route action type on the matched route and return a more meaningful error, when the route action type is not RouteActionRoute
which is the only supported one on the client. https://github.com/grpc/grpc-go/blob/master/xds/internal/xdsclient/xdsresource/type_rds.go#L101-L115
hi @easwars and @arvindbr8, can I work on this?
@Aditya-Sood : Thank you for your offer. I'm assigning this to you.
The ideal fix should look something like this:
- When parsing an incoming
RouteConfiguration
resource in the xDS client, we store the route action type in this field. - When creating the
configSelector
in the xDS resolver here, we are currently not plumbing the route action type from [Route])https://github.com/grpc/grpc-go/blob/da1a5eb25d2a3be62c5419c7004f9c0d669ba8bf/xds/internal/xdsclient/xdsresource/type_rds.go#L119) into the internals of theconfigSelector
. We should do that. - And once we have route action type in the internal representation of the route in the
configSelector
, at RPC time in SelectConfig, we should report a more meaningful error when the action type is anything other than Route.
Hope this helps. Please let us know if you have more questions before/when you start working on this.
thank you @easwars, I will keep updating here on the progress
hi @easwars, I have raised PR #6248 to receive your review of the changes, please let me know your inputs on the same thank you!
An issue me and Doug just discussed is that we don't actually plumb in the full enum to our internal representation. Right now, there's a bucket of RouteActionUnsupported that encapsulates all the route actions we don't support client or sever side. We would have to scale this up as well alongside passing this information to the config selector.
update: have asked for clarification on the remaining types of route actions to cover for the returned error - link to comment