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

xds RDS weighted_cluster totalWeight default 100 #4432

Closed
alpha-baby opened this issue May 13, 2021 · 1 comment · Fixed by #4439
Closed

xds RDS weighted_cluster totalWeight default 100 #4432

alpha-baby opened this issue May 13, 2021 · 1 comment · Fixed by #4439

Comments

@alpha-baby
Copy link
Contributor

alpha-baby commented May 13, 2021

NOTE: if you are reporting is a potential security vulnerability or a crash,
please follow our CVE process at
https://github.com/grpc/proposal/blob/master/P4-grpc-cve-process.md instead of
filing an issue here.

Please see the FAQ in our main README.md, then answer the questions below
before submitting your issue.

What version of gRPC are you using?

latest version

https://github.com/grpc/grpc-go/blob/master/xds/internal/client/xds.go#L525

What version of Go are you using (go version)?

1.16

What operating system (Linux, Windows, …) and version?

macOS

What did you do?

What did you expect to see?

not return NACK

xds weighted_cluster DOC:

https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto.html?highlight=weighted_clusters#config-route-v3-weightedcluster

What did you see instead?

WARNING: 2021/05/13 14:57:03 [xds] [xds-client 0xc0002a6d00] Sending NACK for response type: RouteConfigResource, version: 2021-05-13T14:54:33+08:00/9, nonce: tR5YMAcCo6M=1b2e0689-8aa9-43af-9fa5-abdf340a0864, reason: error parsing "RDS" response: xxx... route:{weighted_clusters:{clusters:{name:"outbound|50002|aaaaaasssss|xxx" weight:{value:36}} clusters:{name:"outbound|50002|efwewef|xxx" weight:{value:13}} clusters:{name:"outbound|50002|efsegwsefwefwe|xxx" weight:{value:14}} clusters:{name:"outbound|50002|sdrgsfsefewf|xxx" weight:{value:31}} clusters:{name:"outbound|50002|dfefwewe|xxx" weight:{value:6}}} timeout:{} retry_policy:{retry_on:"connect-failure,refused-stream,unavailable,cancelled,retriable-status-codes" num_retries:{value:2} retry_host_predicate:{name:"envoy.retry_host_predicates.previous_hosts"} host_selection_retry_max_attempts:5 retriable_status_codes:503}} metadata:{filter_metadata:{key:"istio" value:{fields:{key:"config" value:{string_value:"/apis/networking.istio.io/v1alpha3/namespaces/jsfns/virtual-service/xxx"}}}}} decorator:{operation:"xxx/*"}, action &{WeightedClusters:clusters:{name:"outbound|50002|aaaaaasssss|xxx" weight:{value:36}} clusters:{name:"outbound|50002|efwewef|xxx" weight:{value:13}} clusters:{name:"outbound|50002|efsegwsefwefwe|xxx" weight:{value:14}} clusters:{name:"outbound|50002|sdrgsfsefewf|xxx" weight:{value:31}} clusters:{name:"outbound|50002|dfefwewe|xxx" weight:{value:6}}}, weights of clusters do not add up to total total weight, got: 0, want 100

If you also think this is a bug, I'm happy to commit a PR to fix it

@menghanl
Copy link
Contributor

Yes, this looks like a bug. We should check for nil and use default value 100:

if totalWeight != wcs.GetTotalWeight().GetValue() {
return nil, fmt.Errorf("route %+v, action %+v, weights of clusters do not add up to total total weight, got: %v, want %v", r, a, wcs.GetTotalWeight().GetValue(), totalWeight)
}

I'll be great if you can send a PR (if you do, please also add a test). Thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants