From 02ad9d75c2c5e26e090043bd7e7e4d6ea35a0922 Mon Sep 17 00:00:00 2001 From: alpha-baby Date: Fri, 14 May 2021 10:51:07 +0800 Subject: [PATCH 1/3] [fix bug] weighted_cluster totalWeight default 100 #4432 Signed-off-by: alpha-baby --- xds/internal/client/rds_test.go | 65 +++++++++++++++++++++++++++++++++ xds/internal/client/xds.go | 2 +- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/xds/internal/client/rds_test.go b/xds/internal/client/rds_test.go index 10745e9e97da..d069bcd4dd30 100644 --- a/xds/internal/client/rds_test.go +++ b/xds/internal/client/rds_test.go @@ -1077,6 +1077,71 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { }, wantErr: true, }, + { + name: "totalWeight is nil in weighted clusters action", + routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"}, + }, + Action: &v3routepb.Route_Route{ + Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{ + WeightedClusters: &v3routepb.WeightedCluster{ + Clusters: []*v3routepb.WeightedCluster_ClusterWeight{ + {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 0}}, + {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 0}}, + }, + }}}}, + }, + }, + wantErr: true, + }, + { + name: "The sum of all weighted clusters is not equal totalWeight", + routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"}, + }, + Action: &v3routepb.Route_Route{ + Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{ + WeightedClusters: &v3routepb.WeightedCluster{ + Clusters: []*v3routepb.WeightedCluster_ClusterWeight{ + {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 50}}, + {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 20}}, + }, + TotalWeight: &wrapperspb.UInt32Value{Value: 100}, + }}}}, + }, + }, + wantErr: true, + }, + { + name: "default totalWeight is 100 in weighted clusters action", + routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"}, + }, + Action: &v3routepb.Route_Route{ + Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{ + WeightedClusters: &v3routepb.WeightedCluster{ + Clusters: []*v3routepb.WeightedCluster_ClusterWeight{ + {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 60}}, + {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 40}}, + }, + }}}}, + }, + }, + wantRoutes: []*Route{{ + Prefix: newStringP("/a/"), + WeightedClusters: map[string]WeightedCluster{"A": {Weight: 40}, "B": {Weight: 60}}, + }}, + wantErr: false, + }, { name: "with custom HTTP filter config", routes: goodRouteWithFilterConfigs(map[string]*anypb.Any{"foo": customFilterConfig}), diff --git a/xds/internal/client/xds.go b/xds/internal/client/xds.go index 781eeb47a062..0664feddcbbd 100644 --- a/xds/internal/client/xds.go +++ b/xds/internal/client/xds.go @@ -522,7 +522,7 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger, route.WeightedClusters[c.GetName()] = wc totalWeight += w } - if totalWeight != wcs.GetTotalWeight().GetValue() { + if totalWeight != 100 && (wcs.GetTotalWeight() == nil || 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) } if totalWeight == 0 { From 4cd109f74b7bc9478772fd4710d1b1f4c3089b01 Mon Sep 17 00:00:00 2001 From: alpha-baby Date: Sat, 15 May 2021 15:37:35 +0800 Subject: [PATCH 2/3] xds_client/rds: weighted_cluster totalWeight default to 100 Signed-off-by: alpha-baby --- xds/internal/client/rds_test.go | 29 +++++++++++++++++++++++++++-- xds/internal/client/xds.go | 9 +++++++-- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/xds/internal/client/rds_test.go b/xds/internal/client/rds_test.go index d069bcd4dd30..33bcc2ad13f8 100644 --- a/xds/internal/client/rds_test.go +++ b/xds/internal/client/rds_test.go @@ -1089,8 +1089,8 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{ WeightedClusters: &v3routepb.WeightedCluster{ Clusters: []*v3routepb.WeightedCluster_ClusterWeight{ - {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 0}}, - {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 0}}, + {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 20}}, + {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 30}}, }, }}}}, }, @@ -1142,6 +1142,31 @@ func (s) TestRoutesProtoToSlice(t *testing.T) { }}, wantErr: false, }, + { + name: "default totalWeight is 100 in weighted clusters action", + routes: []*v3routepb.Route{ + { + Match: &v3routepb.RouteMatch{ + PathSpecifier: &v3routepb.RouteMatch_Prefix{Prefix: "/a/"}, + }, + Action: &v3routepb.Route_Route{ + Route: &v3routepb.RouteAction{ + ClusterSpecifier: &v3routepb.RouteAction_WeightedClusters{ + WeightedClusters: &v3routepb.WeightedCluster{ + Clusters: []*v3routepb.WeightedCluster_ClusterWeight{ + {Name: "B", Weight: &wrapperspb.UInt32Value{Value: 30}}, + {Name: "A", Weight: &wrapperspb.UInt32Value{Value: 20}}, + }, + TotalWeight: &wrapperspb.UInt32Value{Value: 50}, + }}}}, + }, + }, + wantRoutes: []*Route{{ + Prefix: newStringP("/a/"), + WeightedClusters: map[string]WeightedCluster{"A": {Weight: 20}, "B": {Weight: 30}}, + }}, + wantErr: false, + }, { name: "with custom HTTP filter config", routes: goodRouteWithFilterConfigs(map[string]*anypb.Any{"foo": customFilterConfig}), diff --git a/xds/internal/client/xds.go b/xds/internal/client/xds.go index 0664feddcbbd..8806565f88ee 100644 --- a/xds/internal/client/xds.go +++ b/xds/internal/client/xds.go @@ -522,8 +522,13 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger, route.WeightedClusters[c.GetName()] = wc totalWeight += w } - if totalWeight != 100 && (wcs.GetTotalWeight() == nil || 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) + // default TotalWeight https://github.com/grpc/grpc-go/issues/4432 + wantTotalWeight := uint32(100) + if tw := wcs.GetTotalWeight(); tw != nil { + wantTotalWeight = tw.GetValue() + } + if totalWeight != wantTotalWeight { + return nil, fmt.Errorf("route %+v, action %+v, weights of clusters do not add up to total total weight, got: %v, expected total weight from response: %v", r, a, totalWeight, wantTotalWeight) } if totalWeight == 0 { return nil, fmt.Errorf("route %+v, action %+v, has no valid cluster in WeightedCluster action", r, a) From 267f1eac5f4e5a0968f27260fbc193a0681613e2 Mon Sep 17 00:00:00 2001 From: alpha-baby Date: Wed, 19 May 2021 09:41:06 +0800 Subject: [PATCH 3/3] add envoy totalWeight doc into comment Signed-off-by: alpha-baby --- xds/internal/client/xds.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/xds/internal/client/xds.go b/xds/internal/client/xds.go index 8806565f88ee..12f0e6fe8aaa 100644 --- a/xds/internal/client/xds.go +++ b/xds/internal/client/xds.go @@ -522,7 +522,8 @@ func routesProtoToSlice(routes []*v3routepb.Route, logger *grpclog.PrefixLogger, route.WeightedClusters[c.GetName()] = wc totalWeight += w } - // default TotalWeight https://github.com/grpc/grpc-go/issues/4432 + // envoy xds doc + // default TotalWeight https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto.html#envoy-v3-api-field-config-route-v3-weightedcluster-total-weight wantTotalWeight := uint32(100) if tw := wcs.GetTotalWeight(); tw != nil { wantTotalWeight = tw.GetValue()