From e13e8c580c86fcee1db17af0b9465cd4a0213fd6 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Wed, 24 May 2023 11:30:35 -0700 Subject: [PATCH 1/6] xds: support pick_first custom load balancing policy --- go.mod | 2 +- go.sum | 6 +- xds/internal/balancer/wrrlocality/balancer.go | 2 +- .../xdslbregistry/converter/converter.go | 32 +++++-- .../xdslbregistry/xdslbregistry_test.go | 94 +++++++++---------- 5 files changed, 73 insertions(+), 63 deletions(-) diff --git a/go.mod b/go.mod index 75ea83d9309c..088c703575da 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/cespare/xxhash/v2 v2.2.0 github.com/cncf/udpa/go v0.0.0-20220112060539-c52dc94e7fbe github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195 - github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596 + github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f github.com/golang/glog v1.1.0 github.com/golang/protobuf v1.5.3 github.com/google/go-cmp v0.5.9 diff --git a/go.sum b/go.sum index bd4e7e729e2d..4e7adc822040 100644 --- a/go.sum +++ b/go.sum @@ -17,8 +17,8 @@ github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195 h1:58f1tJ1ra+zFINPlwLW github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596 h1:MDgbDqe1rWfGBa+yCcthuqDSHvXFyenZI1U7f1IbWI8= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f h1:7T++XKzy4xg7PKy+bM+Sa9/oe1OC88yz2hXQUISoXfA= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.10.1 h1:c0g45+xCJhdgFGw7a5QAfdS4byAbud7miNWJ1WwEVf8= github.com/envoyproxy/protoc-gen-validate v0.10.1/go.mod h1:DRjgyB0I43LtJapqN6NiRwroiAU2PaFuvk/vjgh61ss= @@ -40,7 +40,7 @@ github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= -github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.3 h1:RP3t2pwF7cMEbC1dqtB6poj3niw/9gnV4Cjg5oW5gtY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/xds/internal/balancer/wrrlocality/balancer.go b/xds/internal/balancer/wrrlocality/balancer.go index ac63e84e62fb..4df2e4ed0086 100644 --- a/xds/internal/balancer/wrrlocality/balancer.go +++ b/xds/internal/balancer/wrrlocality/balancer.go @@ -51,7 +51,7 @@ func (bb) Name() string { // LBConfig is the config for the wrr locality balancer. type LBConfig struct { - serviceconfig.LoadBalancingConfig + serviceconfig.LoadBalancingConfig `json:"-"` // ChildPolicy is the config for the child policy. ChildPolicy *internalserviceconfig.BalancerConfig `json:"childPolicy,omitempty"` } diff --git a/xds/internal/xdsclient/xdslbregistry/converter/converter.go b/xds/internal/xdsclient/xdslbregistry/converter/converter.go index 27dc6533087b..63323f5c2a04 100644 --- a/xds/internal/xdsclient/xdslbregistry/converter/converter.go +++ b/xds/internal/xdsclient/xdslbregistry/converter/converter.go @@ -39,18 +39,20 @@ import ( v1xdsudpatypepb "github.com/cncf/xds/go/udpa/type/v1" v3xdsxdstypepb "github.com/cncf/xds/go/xds/type/v3" v3clientsideweightedroundrobinpb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/client_side_weighted_round_robin/v3" + v3pickfirstpb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/pick_first/v3" v3ringhashpb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/ring_hash/v3" v3wrrlocalitypb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/wrr_locality/v3" structpb "github.com/golang/protobuf/ptypes/struct" ) func init() { + xdslbregistry.Register("type.googleapis.com/envoy.extensions.load_balancing_policies.client_side_weighted_round_robin.v3.ClientSideWeightedRoundRobin", convertWeightedRoundRobinProtoToServiceConfig) xdslbregistry.Register("type.googleapis.com/envoy.extensions.load_balancing_policies.ring_hash.v3.RingHash", convertRingHashProtoToServiceConfig) + xdslbregistry.Register("type.googleapis.com/envoy.extensions.load_balancing_policies.pick_first.v3.PickFirst", convertPickFirstProtoToServiceConfig) xdslbregistry.Register("type.googleapis.com/envoy.extensions.load_balancing_policies.round_robin.v3.RoundRobin", convertRoundRobinProtoToServiceConfig) xdslbregistry.Register("type.googleapis.com/envoy.extensions.load_balancing_policies.wrr_locality.v3.WrrLocality", convertWRRLocalityProtoToServiceConfig) - xdslbregistry.Register("type.googleapis.com/envoy.extensions.load_balancing_policies.client_side_weighted_round_robin.v3.ClientSideWeightedRoundRobin", convertWeightedRoundRobinProtoToServiceConfig) - xdslbregistry.Register("type.googleapis.com/xds.type.v3.TypedStruct", convertV3TypedStructToServiceConfig) xdslbregistry.Register("type.googleapis.com/udpa.type.v1.TypedStruct", convertV1TypedStructToServiceConfig) + xdslbregistry.Register("type.googleapis.com/xds.type.v3.TypedStruct", convertV3TypedStructToServiceConfig) } const ( @@ -58,7 +60,7 @@ const ( defaultRingHashMaxSize = 8 * 1024 * 1024 // 8M ) -func convertRingHashProtoToServiceConfig(rawProto []byte, depth int) (json.RawMessage, error) { +func convertRingHashProtoToServiceConfig(rawProto []byte, _ int) (json.RawMessage, error) { if !envconfig.XDSRingHash { return nil, nil } @@ -90,6 +92,24 @@ func convertRingHashProtoToServiceConfig(rawProto []byte, depth int) (json.RawMe return makeBalancerConfigJSON(ringhash.Name, rhCfgJSON), nil } +type pfConfig struct { + ShuffleAddressList bool `json:"shuffleAddressList"` +} + +func convertPickFirstProtoToServiceConfig(rawProto []byte, _ int) (json.RawMessage, error) { + pfProto := &v3pickfirstpb.PickFirst{} + if err := proto.Unmarshal(rawProto, pfProto); err != nil { + return nil, fmt.Errorf("failed to unmarshal resource: %v", err) + } + + pfCfg := &pfConfig{ShuffleAddressList: pfProto.GetShuffleAddressList()} + js, err := json.Marshal(pfCfg) + if err != nil { + return nil, fmt.Errorf("error marshaling JSON for type %T: %v", pfCfg, err) + } + return makeBalancerConfigJSON("pick_first", js), nil +} + func convertRoundRobinProtoToServiceConfig([]byte, int) (json.RawMessage, error) { return makeBalancerConfigJSON("round_robin", json.RawMessage("{}")), nil } @@ -118,7 +138,7 @@ func convertWRRLocalityProtoToServiceConfig(rawProto []byte, depth int) (json.Ra return makeBalancerConfigJSON(wrrlocality.Name, lbCfgJSON), nil } -func convertWeightedRoundRobinProtoToServiceConfig(rawProto []byte, depth int) (json.RawMessage, error) { +func convertWeightedRoundRobinProtoToServiceConfig(rawProto []byte, _ int) (json.RawMessage, error) { cswrrProto := &v3clientsideweightedroundrobinpb.ClientSideWeightedRoundRobin{} if err := proto.Unmarshal(rawProto, cswrrProto); err != nil { return nil, fmt.Errorf("failed to unmarshal resource: %v", err) @@ -152,7 +172,7 @@ func convertWeightedRoundRobinProtoToServiceConfig(rawProto []byte, depth int) ( return makeBalancerConfigJSON(weightedroundrobin.Name, lbCfgJSON), nil } -func convertV1TypedStructToServiceConfig(rawProto []byte, depth int) (json.RawMessage, error) { +func convertV1TypedStructToServiceConfig(rawProto []byte, _ int) (json.RawMessage, error) { tsProto := &v1xdsudpatypepb.TypedStruct{} if err := proto.Unmarshal(rawProto, tsProto); err != nil { return nil, fmt.Errorf("failed to unmarshal resource: %v", err) @@ -160,7 +180,7 @@ func convertV1TypedStructToServiceConfig(rawProto []byte, depth int) (json.RawMe return convertCustomPolicy(tsProto.GetTypeUrl(), tsProto.GetValue()) } -func convertV3TypedStructToServiceConfig(rawProto []byte, depth int) (json.RawMessage, error) { +func convertV3TypedStructToServiceConfig(rawProto []byte, _ int) (json.RawMessage, error) { tsProto := &v3xdsxdstypepb.TypedStruct{} if err := proto.Unmarshal(rawProto, tsProto); err != nil { return nil, fmt.Errorf("failed to unmarshal resource: %v", err) diff --git a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go index b3f19c2e5953..c14e2fcb715d 100644 --- a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go +++ b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go @@ -35,7 +35,6 @@ import ( "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/serviceconfig" _ "google.golang.org/grpc/xds" // Register the xDS LB Registry Converters. - "google.golang.org/grpc/xds/internal/balancer/ringhash" "google.golang.org/grpc/xds/internal/balancer/wrrlocality" "google.golang.org/grpc/xds/internal/xdsclient/xdslbregistry" "google.golang.org/protobuf/types/known/anypb" @@ -46,6 +45,7 @@ import ( v3clusterpb "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3" v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" v3leastrequestpb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/least_request/v3" + v3pickfirstpb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/pick_first/v3" v3ringhashpb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/ring_hash/v3" v3roundrobinpb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/round_robin/v3" v3wrrlocalitypb "github.com/envoyproxy/go-control-plane/envoy/extensions/load_balancing_policies/wrr_locality/v3" @@ -84,7 +84,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { tests := []struct { name string policy *v3clusterpb.LoadBalancingPolicy - wantConfig *internalserviceconfig.BalancerConfig + wantConfig string // JSON config rhDisabled bool }{ { @@ -102,13 +102,22 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: &internalserviceconfig.BalancerConfig{ - Name: "ring_hash_experimental", - Config: &ringhash.LBConfig{ - MinRingSize: 10, - MaxRingSize: 100, + wantConfig: `[{"ring_hash_experimental": { "minRingSize": 10, "maxRingSize": 100 }}]`, + }, + { + name: "pick_first", + policy: &v3clusterpb.LoadBalancingPolicy{ + Policies: []*v3clusterpb.LoadBalancingPolicy_Policy{ + { + TypedExtensionConfig: &v3corepb.TypedExtensionConfig{ + TypedConfig: testutils.MarshalAny(&v3pickfirstpb.PickFirst{ + ShuffleAddressList: true, + }), + }, + }, }, }, + wantConfig: `[{"pick_first": { "shuffleAddressList": true }}]`, }, { name: "round_robin", @@ -121,9 +130,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: &internalserviceconfig.BalancerConfig{ - Name: "round_robin", - }, + wantConfig: `[{"round_robin": {}}]`, }, { name: "round_robin_ring_hash_use_first_supported", @@ -145,9 +152,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: &internalserviceconfig.BalancerConfig{ - Name: "round_robin", - }, + wantConfig: `[{"round_robin": {}}]`, }, { name: "ring_hash_disabled_rh_rr_use_first_supported", @@ -169,9 +174,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: &internalserviceconfig.BalancerConfig{ - Name: "round_robin", - }, + wantConfig: `[{"round_robin": {}}]`, rhDisabled: true, }, { @@ -198,10 +201,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: &internalserviceconfig.BalancerConfig{ - Name: "myorg.MyCustomLeastRequestPolicy", - Config: customLBConfig{}, - }, + wantConfig: `[{"myorg.MyCustomLeastRequestPolicy": {}}]`, }, { name: "custom_lb_type_v1_struct", @@ -217,10 +217,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: &internalserviceconfig.BalancerConfig{ - Name: "myorg.MyCustomLeastRequestPolicy", - Config: customLBConfig{}, - }, + wantConfig: `[{"myorg.MyCustomLeastRequestPolicy": {}}]`, }, { name: "wrr_locality_child_round_robin", @@ -233,9 +230,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: wrrLocalityBalancerConfig(&internalserviceconfig.BalancerConfig{ - Name: "round_robin", - }), + wantConfig: `[{"xds_wrr_locality_experimental": { "childPolicy": [{"round_robin": {}}] }}]`, }, { name: "wrr_locality_child_custom_lb_type_v3_struct", @@ -251,10 +246,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: wrrLocalityBalancerConfig(&internalserviceconfig.BalancerConfig{ - Name: "myorg.MyCustomLeastRequestPolicy", - Config: customLBConfig{}, - }), + wantConfig: `[{"xds_wrr_locality_experimental": { "childPolicy": [{"myorg.MyCustomLeastRequestPolicy": {}}] }}]`, }, { name: "on-the-boundary-of-recursive-limit", @@ -267,9 +259,9 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, }, }, - wantConfig: wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(&internalserviceconfig.BalancerConfig{ + wantConfig: jsonMarshal(t, wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(wrrLocalityBalancerConfig(&internalserviceconfig.BalancerConfig{ Name: "round_robin", - }))))))))))))))), + })))))))))))))))), }, } @@ -286,32 +278,30 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { if err != nil { t.Fatalf("ConvertToServiceConfig(%s) failed: %v", pretty.ToJSON(test.policy), err) } - bc := &internalserviceconfig.BalancerConfig{} - // The converter registry is not guaranteed to emit json that is - // valid. It's scope is to simply convert from a proto message to - // internal gRPC JSON format. Thus, the tests cause valid JSON to - // eventually be emitted from ConvertToServiceConfig(), but this - // leaves this test brittle over time in case balancer validations - // change over time and add more failure cases. The simplicity of - // using this type (to get rid of non determinism in JSON strings) - // outweighs this brittleness, and also there are plans on - // decoupling the unmarshalling and validation step both present in - // this function in the future. In the future if balancer - // validations change, any configurations in this test that become - // invalid will need to be fixed. (need to make sure emissions above - // are valid configuration). Also, once this Unmarshal call is - // partitioned into Unmarshal vs. Validation in separate operations, - // the brittleness of this test will go away. - if err := json.Unmarshal(rawJSON, bc); err != nil { - t.Fatalf("failed to unmarshal JSON: %v", err) + var got []map[string]interface{} + if err := json.Unmarshal(rawJSON, &got); err != nil { + t.Fatalf("Error unmarshalling rawJSON (%q): %v", rawJSON, err) + } + var want []map[string]interface{} + if err := json.Unmarshal(json.RawMessage(test.wantConfig), &want); err != nil { + t.Fatalf("Error unmarshalling wantConfig (%q): %v", test.wantConfig, err) } - if diff := cmp.Diff(bc, test.wantConfig); diff != "" { + if diff := cmp.Diff(got, want); diff != "" { t.Fatalf("ConvertToServiceConfig() got unexpected output, diff (-got +want): %v", diff) } }) } } +func jsonMarshal(t *testing.T, x interface{}) string { + t.Helper() + js, err := json.Marshal(x) + if err != nil { + t.Fatalf("Error marshalling to JSON (%+v): %v", x, err) + } + return string(js) +} + // TestConvertToServiceConfigFailure tests failure cases of the xDS LB registry // of converting proto configuration to JSON configuration. func (s) TestConvertToServiceConfigFailure(t *testing.T) { From 1f3f8d6b0797deafffc36a351f3fdff9a719a1ec Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Wed, 24 May 2023 11:47:50 -0700 Subject: [PATCH 2/6] envconfig knob --- internal/envconfig/envconfig.go | 4 ++ pickfirst.go | 3 +- test/pickfirst_test.go | 58 +++++++++++++++++++ .../xdslbregistry/converter/converter.go | 3 + .../xdslbregistry/xdslbregistry_test.go | 31 ++++++++-- 5 files changed, 94 insertions(+), 5 deletions(-) diff --git a/internal/envconfig/envconfig.go b/internal/envconfig/envconfig.go index 5ba9d94d49c2..80fd5c7d2a4f 100644 --- a/internal/envconfig/envconfig.go +++ b/internal/envconfig/envconfig.go @@ -36,6 +36,10 @@ var ( // "GRPC_RING_HASH_CAP". This does not override the default bounds // checking which NACKs configs specifying ring sizes > 8*1024*1024 (~8M). RingHashCap = uint64FromEnv("GRPC_RING_HASH_CAP", 4096, 1, 8*1024*1024) + // PickFirstLBConfig is set if we should support configuration of the + // pick_first LB policy, which can be enabled by setting the environment + // variable "GRPC_EXPERIMENTAL_PICKFIRST_LB_CONFIG" to "true". + PickFirstLBConfig = boolFromEnv("GRPC_EXPERIMENTAL_PICKFIRST_LB_CONFIG", false) ) func boolFromEnv(envVar string, def bool) bool { diff --git a/pickfirst.go b/pickfirst.go index 611bef7995cd..abe266b021d2 100644 --- a/pickfirst.go +++ b/pickfirst.go @@ -25,6 +25,7 @@ import ( "google.golang.org/grpc/balancer" "google.golang.org/grpc/connectivity" + "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/grpcrand" "google.golang.org/grpc/serviceconfig" ) @@ -112,7 +113,7 @@ func (b *pickfirstBalancer) UpdateClientConnState(state balancer.ClientConnState b.cfg = cfg } - if b.cfg != nil && b.cfg.ShuffleAddressList { + if envconfig.PickFirstLBConfig && b.cfg != nil && b.cfg.ShuffleAddressList { grpcrand.Shuffle(len(addrs), func(i, j int) { addrs[i], addrs[j] = addrs[j], addrs[i] }) } if b.subConn != nil { diff --git a/test/pickfirst_test.go b/test/pickfirst_test.go index 62310d4d330e..7f36ed400c8b 100644 --- a/test/pickfirst_test.go +++ b/test/pickfirst_test.go @@ -30,6 +30,7 @@ import ( "google.golang.org/grpc/connectivity" "google.golang.org/grpc/credentials/insecure" "google.golang.org/grpc/internal/channelz" + "google.golang.org/grpc/internal/envconfig" "google.golang.org/grpc/internal/grpcrand" "google.golang.org/grpc/internal/stubserver" "google.golang.org/grpc/internal/testutils" @@ -382,6 +383,8 @@ func (s) TestPickFirst_StickyTransientFailure(t *testing.T) { } func (s) TestPickFirst_ShuffleAddressList(t *testing.T) { + defer func(old bool) { envconfig.PickFirstLBConfig = old }(envconfig.PickFirstLBConfig) + envconfig.PickFirstLBConfig = true const serviceConfig = `{"loadBalancingConfig": [{"pick_first":{ "shuffleAddressList": true }}]}` // Install a shuffler that always reverses two entries. @@ -431,3 +434,58 @@ func (s) TestPickFirst_ShuffleAddressList(t *testing.T) { t.Fatal(err) } } + +func (s) TestPickFirst_ShuffleAddressListDisabled(t *testing.T) { + defer func(old bool) { envconfig.PickFirstLBConfig = old }(envconfig.PickFirstLBConfig) + envconfig.PickFirstLBConfig = false + const serviceConfig = `{"loadBalancingConfig": [{"pick_first":{ "shuffleAddressList": true }}]}` + + // Install a shuffler that always reverses two entries. + origShuf := grpcrand.Shuffle + defer func() { grpcrand.Shuffle = origShuf }() + grpcrand.Shuffle = func(n int, f func(int, int)) { + if n != 2 { + t.Errorf("Shuffle called with n=%v; want 2", n) + } + f(0, 1) // reverse the two addresses + } + + // Set up our backends. + cc, r, backends := setupPickFirst(t, 2) + addrs := stubBackendsToResolverAddrs(backends) + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + + // Push an update with both addresses and shuffling disabled. We should + // connect to backend 0. + r.UpdateState(resolver.State{Addresses: []resolver.Address{addrs[0], addrs[1]}}) + if err := pickfirst.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil { + t.Fatal(err) + } + + // Send a config with shuffling enabled. This will reverse the addresses, + // but the channel should still be connected to backend 0. + shufState := resolver.State{ + ServiceConfig: parseServiceConfig(t, r, serviceConfig), + Addresses: []resolver.Address{addrs[0], addrs[1]}, + } + r.UpdateState(shufState) + if err := pickfirst.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil { + t.Fatal(err) + } + + // Send a resolver update with no addresses. This should push the channel + // into TransientFailure. + r.UpdateState(resolver.State{}) + awaitState(ctx, t, cc, connectivity.TransientFailure) + + // Send the same config as last time with shuffling enabled. Since we are + // not connected to backend 0, we should connect to backend 1 if shuffling + // is supported. However with it disabled at the start of the test, we + // will connect to backend 0 instead. + r.UpdateState(shufState) + if err := pickfirst.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil { + t.Fatal(err) + } +} diff --git a/xds/internal/xdsclient/xdslbregistry/converter/converter.go b/xds/internal/xdsclient/xdslbregistry/converter/converter.go index 63323f5c2a04..6bafa647794b 100644 --- a/xds/internal/xdsclient/xdslbregistry/converter/converter.go +++ b/xds/internal/xdsclient/xdslbregistry/converter/converter.go @@ -97,6 +97,9 @@ type pfConfig struct { } func convertPickFirstProtoToServiceConfig(rawProto []byte, _ int) (json.RawMessage, error) { + if !envconfig.PickFirstLBConfig { + return nil, nil + } pfProto := &v3pickfirstpb.PickFirst{} if err := proto.Unmarshal(rawProto, pfProto); err != nil { return nil, fmt.Errorf("failed to unmarshal resource: %v", err) diff --git a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go index c14e2fcb715d..9a7f5be53108 100644 --- a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go +++ b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go @@ -86,6 +86,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { policy *v3clusterpb.LoadBalancingPolicy wantConfig string // JSON config rhDisabled bool + pfDisabled bool }{ { name: "ring_hash", @@ -177,6 +178,27 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { wantConfig: `[{"round_robin": {}}]`, rhDisabled: true, }, + { + name: "pick_first_disabled_pf_rr_use_first_supported", + policy: &v3clusterpb.LoadBalancingPolicy{ + Policies: []*v3clusterpb.LoadBalancingPolicy_Policy{ + { + TypedExtensionConfig: &v3corepb.TypedExtensionConfig{ + TypedConfig: testutils.MarshalAny(&v3pickfirstpb.PickFirst{ + ShuffleAddressList: true, + }), + }, + }, + { + TypedExtensionConfig: &v3corepb.TypedExtensionConfig{ + TypedConfig: testutils.MarshalAny(&v3roundrobinpb.RoundRobin{}), + }, + }, + }, + }, + wantConfig: `[{"round_robin": {}}]`, + pfDisabled: true, + }, { name: "custom_lb_type_v3_struct", policy: &v3clusterpb.LoadBalancingPolicy{ @@ -268,11 +290,12 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { if test.rhDisabled { - oldRingHashSupport := envconfig.XDSRingHash + defer func(old bool) { envconfig.XDSRingHash = old }(envconfig.XDSRingHash) envconfig.XDSRingHash = false - defer func() { - envconfig.XDSRingHash = oldRingHashSupport - }() + } + if !test.pfDisabled { + defer func(old bool) { envconfig.PickFirstLBConfig = old }(envconfig.PickFirstLBConfig) + envconfig.PickFirstLBConfig = true } rawJSON, err := xdslbregistry.ConvertToServiceConfig(test.policy, 0) if err != nil { From 6c64630998d156fba3818e42607ddc87fbb01734 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Wed, 24 May 2023 13:12:17 -0700 Subject: [PATCH 3/6] go mod tidy everywhere --- examples/go.mod | 2 +- examples/go.sum | 5 +++-- gcp/observability/go.sum | 3 ++- interop/observability/go.sum | 3 ++- stats/opencensus/go.sum | 3 ++- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/examples/go.mod b/examples/go.mod index 7e49c3bce4c9..c631aae7da39 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -17,7 +17,7 @@ require ( github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/cncf/udpa/go v0.0.0-20220112060539-c52dc94e7fbe // indirect - github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596 // indirect + github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f // indirect github.com/envoyproxy/protoc-gen-validate v0.10.1 // indirect golang.org/x/net v0.9.0 // indirect golang.org/x/sys v0.7.0 // indirect diff --git a/examples/go.sum b/examples/go.sum index 8006bf69fef6..d257912f6717 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -636,8 +636,8 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596 h1:MDgbDqe1rWfGBa+yCcthuqDSHvXFyenZI1U7f1IbWI8= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f h1:7T++XKzy4xg7PKy+bM+Sa9/oe1OC88yz2hXQUISoXfA= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -821,6 +821,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/gcp/observability/go.sum b/gcp/observability/go.sum index bb5535fab90b..4e70d82da940 100644 --- a/gcp/observability/go.sum +++ b/gcp/observability/go.sum @@ -647,7 +647,7 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -841,6 +841,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/interop/observability/go.sum b/interop/observability/go.sum index 167fb14bc0ce..b21857e14a68 100644 --- a/interop/observability/go.sum +++ b/interop/observability/go.sum @@ -648,7 +648,7 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -843,6 +843,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/stats/opencensus/go.sum b/stats/opencensus/go.sum index 43f540fb5667..48faad4b66dd 100644 --- a/stats/opencensus/go.sum +++ b/stats/opencensus/go.sum @@ -630,7 +630,7 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230406144219-ba92d50b6596/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -814,6 +814,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= From 8d2f69a2ba8ab45dbbedb1e9dd93654332544db4 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Thu, 25 May 2023 15:49:43 -0700 Subject: [PATCH 4/6] review comments --- test/pickfirst_test.go | 30 +++++-------------- .../xdslbregistry/converter/converter.go | 6 ++-- .../xdslbregistry/xdslbregistry_test.go | 17 ++++++++++- 3 files changed, 28 insertions(+), 25 deletions(-) diff --git a/test/pickfirst_test.go b/test/pickfirst_test.go index 7f36ed400c8b..55659b928a57 100644 --- a/test/pickfirst_test.go +++ b/test/pickfirst_test.go @@ -382,6 +382,7 @@ func (s) TestPickFirst_StickyTransientFailure(t *testing.T) { wg.Wait() } +// Tests the PF LB policy with shuffling enabled. func (s) TestPickFirst_ShuffleAddressList(t *testing.T) { defer func(old bool) { envconfig.PickFirstLBConfig = old }(envconfig.PickFirstLBConfig) envconfig.PickFirstLBConfig = true @@ -393,6 +394,7 @@ func (s) TestPickFirst_ShuffleAddressList(t *testing.T) { grpcrand.Shuffle = func(n int, f func(int, int)) { if n != 2 { t.Errorf("Shuffle called with n=%v; want 2", n) + return } f(0, 1) // reverse the two addresses } @@ -435,6 +437,8 @@ func (s) TestPickFirst_ShuffleAddressList(t *testing.T) { } } +// Tests the PF LB policy with the environment variable support of address list +// shuffling disabled. func (s) TestPickFirst_ShuffleAddressListDisabled(t *testing.T) { defer func(old bool) { envconfig.PickFirstLBConfig = old }(envconfig.PickFirstLBConfig) envconfig.PickFirstLBConfig = false @@ -446,6 +450,7 @@ func (s) TestPickFirst_ShuffleAddressListDisabled(t *testing.T) { grpcrand.Shuffle = func(n int, f func(int, int)) { if n != 2 { t.Errorf("Shuffle called with n=%v; want 2", n) + return } f(0, 1) // reverse the two addresses } @@ -457,15 +462,10 @@ func (s) TestPickFirst_ShuffleAddressListDisabled(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() - // Push an update with both addresses and shuffling disabled. We should - // connect to backend 0. - r.UpdateState(resolver.State{Addresses: []resolver.Address{addrs[0], addrs[1]}}) - if err := pickfirst.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil { - t.Fatal(err) - } - // Send a config with shuffling enabled. This will reverse the addresses, - // but the channel should still be connected to backend 0. + // so we should connect to backend 1 if shuffling is supported. However + // with it disabled at the start of the test, we will connect to backend 0 + // instead. shufState := resolver.State{ ServiceConfig: parseServiceConfig(t, r, serviceConfig), Addresses: []resolver.Address{addrs[0], addrs[1]}, @@ -474,18 +474,4 @@ func (s) TestPickFirst_ShuffleAddressListDisabled(t *testing.T) { if err := pickfirst.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil { t.Fatal(err) } - - // Send a resolver update with no addresses. This should push the channel - // into TransientFailure. - r.UpdateState(resolver.State{}) - awaitState(ctx, t, cc, connectivity.TransientFailure) - - // Send the same config as last time with shuffling enabled. Since we are - // not connected to backend 0, we should connect to backend 1 if shuffling - // is supported. However with it disabled at the start of the test, we - // will connect to backend 0 instead. - r.UpdateState(shufState) - if err := pickfirst.CheckRPCsToBackend(ctx, cc, addrs[0]); err != nil { - t.Fatal(err) - } } diff --git a/xds/internal/xdsclient/xdslbregistry/converter/converter.go b/xds/internal/xdsclient/xdslbregistry/converter/converter.go index 6bafa647794b..c5d5afe4ebdc 100644 --- a/xds/internal/xdsclient/xdslbregistry/converter/converter.go +++ b/xds/internal/xdsclient/xdslbregistry/converter/converter.go @@ -28,7 +28,9 @@ import ( "strings" "github.com/golang/protobuf/proto" + "google.golang.org/grpc" "google.golang.org/grpc/balancer" + "google.golang.org/grpc/balancer/roundrobin" "google.golang.org/grpc/balancer/weightedroundrobin" "google.golang.org/grpc/internal/envconfig" internalserviceconfig "google.golang.org/grpc/internal/serviceconfig" @@ -110,11 +112,11 @@ func convertPickFirstProtoToServiceConfig(rawProto []byte, _ int) (json.RawMessa if err != nil { return nil, fmt.Errorf("error marshaling JSON for type %T: %v", pfCfg, err) } - return makeBalancerConfigJSON("pick_first", js), nil + return makeBalancerConfigJSON(grpc.PickFirstBalancerName, js), nil } func convertRoundRobinProtoToServiceConfig([]byte, int) (json.RawMessage, error) { - return makeBalancerConfigJSON("round_robin", json.RawMessage("{}")), nil + return makeBalancerConfigJSON(roundrobin.Name, json.RawMessage("{}")), nil } type wrrLocalityLBConfig struct { diff --git a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go index 9a7f5be53108..8ffb64114244 100644 --- a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go +++ b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go @@ -106,7 +106,7 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { wantConfig: `[{"ring_hash_experimental": { "minRingSize": 10, "maxRingSize": 100 }}]`, }, { - name: "pick_first", + name: "pick_first_shuffle", policy: &v3clusterpb.LoadBalancingPolicy{ Policies: []*v3clusterpb.LoadBalancingPolicy_Policy{ { @@ -120,6 +120,19 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { }, wantConfig: `[{"pick_first": { "shuffleAddressList": true }}]`, }, + { + name: "pick_first", + policy: &v3clusterpb.LoadBalancingPolicy{ + Policies: []*v3clusterpb.LoadBalancingPolicy_Policy{ + { + TypedExtensionConfig: &v3corepb.TypedExtensionConfig{ + TypedConfig: testutils.MarshalAny(&v3pickfirstpb.PickFirst{}), + }, + }, + }, + }, + wantConfig: `[{"pick_first": { "shuffleAddressList": false }}]`, + }, { name: "round_robin", policy: &v3clusterpb.LoadBalancingPolicy{ @@ -301,6 +314,8 @@ func (s) TestConvertToServiceConfigSuccess(t *testing.T) { if err != nil { t.Fatalf("ConvertToServiceConfig(%s) failed: %v", pretty.ToJSON(test.policy), err) } + // got and want must be unmarshalled since JSON strings shouldn't + // generally be directly compared. var got []map[string]interface{} if err := json.Unmarshal(rawJSON, &got); err != nil { t.Fatalf("Error unmarshalling rawJSON (%q): %v", rawJSON, err) From 57a3f29753eb5257c62c10e161c9fdfce4672cba Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Fri, 26 May 2023 11:11:25 -0700 Subject: [PATCH 5/6] small extra cleanup --- .../xdsclient/xdslbregistry/xdslbregistry_test.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go index 8ffb64114244..f1ce5496b794 100644 --- a/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go +++ b/xds/internal/xdsclient/xdslbregistry/xdslbregistry_test.go @@ -33,7 +33,6 @@ import ( "google.golang.org/grpc/internal/pretty" internalserviceconfig "google.golang.org/grpc/internal/serviceconfig" "google.golang.org/grpc/internal/testutils" - "google.golang.org/grpc/serviceconfig" _ "google.golang.org/grpc/xds" // Register the xDS LB Registry Converters. "google.golang.org/grpc/xds/internal/balancer/wrrlocality" "google.golang.org/grpc/xds/internal/xdsclient/xdslbregistry" @@ -60,10 +59,6 @@ func Test(t *testing.T) { grpctest.RunSubTests(t, s{}) } -type customLBConfig struct { - serviceconfig.LoadBalancingConfig -} - func wrrLocalityBalancerConfig(childPolicy *internalserviceconfig.BalancerConfig) *internalserviceconfig.BalancerConfig { return &internalserviceconfig.BalancerConfig{ Name: wrrlocality.Name, @@ -75,11 +70,7 @@ func wrrLocalityBalancerConfig(childPolicy *internalserviceconfig.BalancerConfig func (s) TestConvertToServiceConfigSuccess(t *testing.T) { const customLBPolicyName = "myorg.MyCustomLeastRequestPolicy" - stub.Register(customLBPolicyName, stub.BalancerFuncs{ - ParseConfig: func(json.RawMessage) (serviceconfig.LoadBalancingConfig, error) { - return customLBConfig{}, nil - }, - }) + stub.Register(customLBPolicyName, stub.BalancerFuncs{}) tests := []struct { name string From 90ca8aad916379540296fce3c434a5959c63cda8 Mon Sep 17 00:00:00 2001 From: Doug Fawley Date: Fri, 26 May 2023 13:15:42 -0700 Subject: [PATCH 6/6] re-update go-control-plane to repair after merge --- examples/go.mod | 2 +- examples/go.sum | 5 +++-- gcp/observability/go.sum | 3 ++- go.mod | 2 +- go.sum | 6 +++--- interop/observability/go.sum | 3 ++- stats/opencensus/go.sum | 3 ++- 7 files changed, 14 insertions(+), 10 deletions(-) diff --git a/examples/go.mod b/examples/go.mod index 0c75c14493b0..c631aae7da39 100644 --- a/examples/go.mod +++ b/examples/go.mod @@ -17,7 +17,7 @@ require ( github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/cncf/udpa/go v0.0.0-20220112060539-c52dc94e7fbe // indirect - github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41 // indirect + github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f // indirect github.com/envoyproxy/protoc-gen-validate v0.10.1 // indirect golang.org/x/net v0.9.0 // indirect golang.org/x/sys v0.7.0 // indirect diff --git a/examples/go.sum b/examples/go.sum index 7bc1a3576a60..d257912f6717 100644 --- a/examples/go.sum +++ b/examples/go.sum @@ -636,8 +636,8 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41 h1:TNyxMch3whemmD2xddvlcYav9UR0hUvFeWnMUMSdhHA= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f h1:7T++XKzy4xg7PKy+bM+Sa9/oe1OC88yz2hXQUISoXfA= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -821,6 +821,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/gcp/observability/go.sum b/gcp/observability/go.sum index 2b1c8b61771f..4e70d82da940 100644 --- a/gcp/observability/go.sum +++ b/gcp/observability/go.sum @@ -647,7 +647,7 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -841,6 +841,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/go.mod b/go.mod index ecff5ff74e01..088c703575da 100644 --- a/go.mod +++ b/go.mod @@ -6,7 +6,7 @@ require ( github.com/cespare/xxhash/v2 v2.2.0 github.com/cncf/udpa/go v0.0.0-20220112060539-c52dc94e7fbe github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195 - github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41 + github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f github.com/golang/glog v1.1.0 github.com/golang/protobuf v1.5.3 github.com/google/go-cmp v0.5.9 diff --git a/go.sum b/go.sum index 80188b3fda93..4e7adc822040 100644 --- a/go.sum +++ b/go.sum @@ -17,8 +17,8 @@ github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195 h1:58f1tJ1ra+zFINPlwLW github.com/cncf/xds/go v0.0.0-20230310173818-32f1caf87195/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/envoyproxy/go-control-plane v0.9.0/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41 h1:TNyxMch3whemmD2xddvlcYav9UR0hUvFeWnMUMSdhHA= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f h1:7T++XKzy4xg7PKy+bM+Sa9/oe1OC88yz2hXQUISoXfA= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.10.1 h1:c0g45+xCJhdgFGw7a5QAfdS4byAbud7miNWJ1WwEVf8= github.com/envoyproxy/protoc-gen-validate v0.10.1/go.mod h1:DRjgyB0I43LtJapqN6NiRwroiAU2PaFuvk/vjgh61ss= @@ -40,7 +40,7 @@ github.com/google/uuid v1.3.0 h1:t6JiXgmwXMjEs8VusXIJk2BXHsn+wx8BZdTaoZ5fu7I= github.com/google/uuid v1.3.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA= -github.com/stretchr/testify v1.8.1 h1:w7B6lhMri9wdJUVmEZPGGhZzrYTPvgJArz7wNPgYKsk= +github.com/stretchr/testify v1.8.3 h1:RP3t2pwF7cMEbC1dqtB6poj3niw/9gnV4Cjg5oW5gtY= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= diff --git a/interop/observability/go.sum b/interop/observability/go.sum index 8c008e984f8c..b21857e14a68 100644 --- a/interop/observability/go.sum +++ b/interop/observability/go.sum @@ -648,7 +648,7 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -843,6 +843,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= diff --git a/stats/opencensus/go.sum b/stats/opencensus/go.sum index 7bdc3927073a..48faad4b66dd 100644 --- a/stats/opencensus/go.sum +++ b/stats/opencensus/go.sum @@ -630,7 +630,7 @@ github.com/docopt/docopt-go v0.0.0-20180111231733-ee0de3bc6815/go.mod h1:WwZ+bS3 github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/envoyproxy/go-control-plane v0.10.2-0.20220325020618-49ff273808a1/go.mod h1:KJwIaB5Mv44NWtYuAOFCVOjcI94vtpEz2JU/D2v6IjE= github.com/envoyproxy/go-control-plane v0.10.3/go.mod h1:fJJn/j26vwOu972OllsvAgJJM//w9BV6Fxbg2LuVd34= -github.com/envoyproxy/go-control-plane v0.11.1-0.20230517004634-d1c5e72e4c41/go.mod h1:84cjSkVxFD9Pi/gvI5AOq5NPhGsmS8oPsJLtCON6eK8= +github.com/envoyproxy/go-control-plane v0.11.1-0.20230524094728-9239064ad72f/go.mod h1:sfYdkwUW4BA3PbKjySwjJy+O4Pu0h62rlqCMHNk+K+Q= github.com/envoyproxy/protoc-gen-validate v0.1.0/go.mod h1:iSmxcyjqTsJpI2R4NaDN7+kN2VEUnK/pcBlmesArF7c= github.com/envoyproxy/protoc-gen-validate v0.6.7/go.mod h1:dyJXwwfPK2VSqiB9Klm1J6romD608Ba7Hij42vrOBCo= github.com/envoyproxy/protoc-gen-validate v0.9.1/go.mod h1:OKNgG7TCp5pF4d6XftA0++PMirau2/yoOwVac3AbF2w= @@ -814,6 +814,7 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= +github.com/stretchr/testify v1.8.3/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= github.com/yuin/goldmark v1.1.25/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.27/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74=