From 250b421e7c2fe84e031485d47c42f61ff6895d80 Mon Sep 17 00:00:00 2001 From: Arvind Bright Date: Tue, 18 Oct 2022 15:40:20 -0700 Subject: [PATCH] test: refactor test to use testutils helpers --- .../weightedtarget/weightedtarget_test.go | 91 +++++++------ internal/balancergroup/balancergroup_test.go | 21 +-- internal/testutils/balancer.go | 7 + .../balancer/clusterresolver/priority_test.go | 122 ++++++++++-------- .../balancer/clusterresolver/testutil_test.go | 71 ---------- 5 files changed, 135 insertions(+), 177 deletions(-) diff --git a/balancer/weightedtarget/weightedtarget_test.go b/balancer/weightedtarget/weightedtarget_test.go index ea76ea1297cb..6a15bd4a5ee0 100644 --- a/balancer/weightedtarget/weightedtarget_test.go +++ b/balancer/weightedtarget/weightedtarget_test.go @@ -19,6 +19,7 @@ package weightedtarget import ( + "context" "encoding/json" "errors" "fmt" @@ -40,6 +41,10 @@ import ( "google.golang.org/grpc/serviceconfig" ) +const ( + defaultTestTimeout = 5 * time.Second +) + type s struct { grpctest.Tester } @@ -58,6 +63,17 @@ func newTestConfigBalancerBuilder() *testConfigBalancerBuilder { } } +func makeRPCsAndAssertContainsError(rpcCount int, want error) func(balancer.Picker) error { + return func(p balancer.Picker) error { + for i := 0; i < rpcCount; i++ { + if _, err := p.Pick(balancer.PickInfo{}); !strings.Contains(err.Error(), want.Error()) { + return fmt.Errorf("want pick error to contain %q, got %q", want, err) + } + } + return nil + } +} + func (t *testConfigBalancerBuilder) Build(cc balancer.ClientConn, opts balancer.BuildOptions) balancer.Balancer { rr := t.Builder.Build(cc, opts) return &testConfigBalancer{ @@ -289,13 +305,6 @@ func (s) TestWeightedTarget(t *testing.T) { } } -func subConnFromPicker(p balancer.Picker) func() balancer.SubConn { - return func() balancer.SubConn { - scst, _ := p.Pick(balancer.PickInfo{}) - return scst.SubConn - } -} - // TestWeightedTarget_OneSubBalancer_AddRemoveBackend tests the case where we // have a weighted target balancer will one sub-balancer, and we add and remove // backends from the subBalancer. @@ -366,7 +375,7 @@ func (s) TestWeightedTarget_OneSubBalancer_AddRemoveBackend(t *testing.T) { // Test round robin pick. want := []balancer.SubConn{sc1, sc2} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -455,7 +464,7 @@ func (s) TestWeightedTarget_TwoSubBalancers_OneBackend(t *testing.T) { // Test roundrobin on the last picker. want := []balancer.SubConn{sc1, sc2} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } } @@ -536,7 +545,7 @@ func (s) TestWeightedTarget_TwoSubBalancers_MoreBackends(t *testing.T) { // Test roundrobin on the last picker. RPCs should be sent equally to all // backends. want := []balancer.SubConn{sc1, sc2, sc3, sc4} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -544,7 +553,7 @@ func (s) TestWeightedTarget_TwoSubBalancers_MoreBackends(t *testing.T) { wtb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.TransientFailure}) p = <-cc.NewPickerCh want = []balancer.SubConn{sc1, sc1, sc3, sc4} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -566,19 +575,19 @@ func (s) TestWeightedTarget_TwoSubBalancers_MoreBackends(t *testing.T) { wtb.UpdateSubConnState(scRemoved, balancer.SubConnState{ConnectivityState: connectivity.Shutdown}) p = <-cc.NewPickerCh want = []balancer.SubConn{sc1, sc4} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } // Turn sc1's connection down. - scConnErr := errors.New("subConn connection error") + wantSubConnErr := errors.New("subConn connection error") wtb.UpdateSubConnState(sc1, balancer.SubConnState{ ConnectivityState: connectivity.TransientFailure, - ConnectionError: scConnErr, + ConnectionError: wantSubConnErr, }) p = <-cc.NewPickerCh want = []balancer.SubConn{sc4} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -594,13 +603,14 @@ func (s) TestWeightedTarget_TwoSubBalancers_MoreBackends(t *testing.T) { // Turn all connections down. wtb.UpdateSubConnState(sc4, balancer.SubConnState{ ConnectivityState: connectivity.TransientFailure, - ConnectionError: scConnErr, + ConnectionError: wantSubConnErr, }) - p = <-cc.NewPickerCh - for i := 0; i < 5; i++ { - if _, err := p.Pick(balancer.PickInfo{}); err == nil || !strings.Contains(err.Error(), scConnErr.Error()) { - t.Fatalf("want pick error %q, got error %q", scConnErr, err) - } + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + + if err := cc.WaitForPicker(ctx, makeRPCsAndAssertContainsError(5, wantSubConnErr)); err != nil { + t.Fatal(err.Error()) } } @@ -680,7 +690,7 @@ func (s) TestWeightedTarget_TwoSubBalancers_DifferentWeight_MoreBackends(t *test // Test roundrobin on the last picker. Twice the number of RPCs should be // sent to cluster_1 when compared to cluster_2. want := []balancer.SubConn{sc1, sc1, sc2, sc2, sc3, sc4} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } } @@ -757,7 +767,7 @@ func (s) TestWeightedTarget_ThreeSubBalancers_RemoveBalancer(t *testing.T) { p := <-cc.NewPickerCh want := []balancer.SubConn{sc1, sc2, sc3} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -797,15 +807,15 @@ func (s) TestWeightedTarget_ThreeSubBalancers_RemoveBalancer(t *testing.T) { t.Fatalf("RemoveSubConn, want %v, got %v", sc2, scRemoved) } want = []balancer.SubConn{sc1, sc3} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } // Move balancer 3 into transient failure. - scConnErr := errors.New("subConn connection error") + wantSubConnErr := errors.New("subConn connection error") wtb.UpdateSubConnState(sc3, balancer.SubConnState{ ConnectivityState: connectivity.TransientFailure, - ConnectionError: scConnErr, + ConnectionError: wantSubConnErr, }) <-cc.NewPickerCh @@ -833,16 +843,17 @@ func (s) TestWeightedTarget_ThreeSubBalancers_RemoveBalancer(t *testing.T) { // Removing a subBalancer causes the weighted target LB policy to push a new // picker which ensures that the removed subBalancer is not picked for RPCs. - p = <-cc.NewPickerCh scRemoved = <-cc.RemoveSubConnCh if !cmp.Equal(scRemoved, sc1, cmp.AllowUnexported(testutils.TestSubConn{})) { t.Fatalf("RemoveSubConn, want %v, got %v", sc1, scRemoved) } - for i := 0; i < 5; i++ { - if _, err := p.Pick(balancer.PickInfo{}); err == nil || !strings.Contains(err.Error(), scConnErr.Error()) { - t.Fatalf("want pick error %q, got error %q", scConnErr, err) - } + + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + + if err := cc.WaitForPicker(ctx, makeRPCsAndAssertContainsError(5, wantSubConnErr)); err != nil { + t.Fatal(err.Error()) } } @@ -922,7 +933,7 @@ func (s) TestWeightedTarget_TwoSubBalancers_ChangeWeight_MoreBackends(t *testing // Test roundrobin on the last picker. Twice the number of RPCs should be // sent to cluster_1 when compared to cluster_2. want := []balancer.SubConn{sc1, sc1, sc2, sc2, sc3, sc4} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -958,7 +969,7 @@ func (s) TestWeightedTarget_TwoSubBalancers_ChangeWeight_MoreBackends(t *testing // Weight change causes a new picker to be pushed to the channel. p = <-cc.NewPickerCh want = []balancer.SubConn{sc1, sc1, sc1, sc2, sc2, sc2, sc3, sc4} - if err := testutils.IsRoundRobin(want, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p)); err != nil { t.Fatalf("want %v, got %v", want, err) } } @@ -1077,21 +1088,21 @@ func (s) TestBalancerGroup_SubBalancerTurnsConnectingFromTransientFailure(t *tes // Set both subconn to TransientFailure, this will put both sub-balancers in // transient failure. - scConnErr := errors.New("subConn connection error") + wantSubConnErr := errors.New("subConn connection error") wtb.UpdateSubConnState(sc1, balancer.SubConnState{ ConnectivityState: connectivity.TransientFailure, - ConnectionError: scConnErr, + ConnectionError: wantSubConnErr, }) <-cc.NewPickerCh wtb.UpdateSubConnState(sc2, balancer.SubConnState{ ConnectivityState: connectivity.TransientFailure, - ConnectionError: scConnErr, + ConnectionError: wantSubConnErr, }) p := <-cc.NewPickerCh for i := 0; i < 5; i++ { - if _, err := p.Pick(balancer.PickInfo{}); err == nil || !strings.Contains(err.Error(), scConnErr.Error()) { - t.Fatalf("want pick error %q, got error %q", scConnErr, err) + if _, err := p.Pick(balancer.PickInfo{}); err == nil || !strings.Contains(err.Error(), wantSubConnErr.Error()) { + t.Fatalf("want pick error %q, got error %q", wantSubConnErr, err) } } @@ -1105,8 +1116,8 @@ func (s) TestBalancerGroup_SubBalancerTurnsConnectingFromTransientFailure(t *tes for i := 0; i < 5; i++ { r, err := p.Pick(balancer.PickInfo{}) - if err == nil || !strings.Contains(err.Error(), scConnErr.Error()) { - t.Fatalf("want pick error %q, got result %v, err %q", scConnErr, r, err) + if err == nil || !strings.Contains(err.Error(), wantSubConnErr.Error()) { + t.Fatalf("want pick error %q, got result %v, err %q", wantSubConnErr, r, err) } } } diff --git a/internal/balancergroup/balancergroup_test.go b/internal/balancergroup/balancergroup_test.go index 566ffc386c04..1228f85ad986 100644 --- a/internal/balancergroup/balancergroup_test.go +++ b/internal/balancergroup/balancergroup_test.go @@ -67,13 +67,6 @@ func Test(t *testing.T) { grpctest.RunSubTests(t, s{}) } -func subConnFromPicker(p balancer.Picker) func() balancer.SubConn { - return func() balancer.SubConn { - scst, _ := p.Pick(balancer.PickInfo{}) - return scst.SubConn - } -} - // Create a new balancer group, add balancer and backends, but not start. // - b1, weight 2, backends [0,1] // - b2, weight 1, backends [2,3] @@ -116,7 +109,7 @@ func (s) TestBalancerGroup_start_close(t *testing.T) { m1[testBackendAddrs[1]], m1[testBackendAddrs[1]], m1[testBackendAddrs[2]], m1[testBackendAddrs[3]], } - if err := testutils.IsRoundRobin(want, subConnFromPicker(p1)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p1)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -158,7 +151,7 @@ func (s) TestBalancerGroup_start_close(t *testing.T) { m2[testBackendAddrs[3]], m2[testBackendAddrs[3]], m2[testBackendAddrs[3]], m2[testBackendAddrs[1]], m2[testBackendAddrs[2]], } - if err := testutils.IsRoundRobin(want, subConnFromPicker(p2)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p2)); err != nil { t.Fatalf("want %v, got %v", want, err) } } @@ -241,7 +234,7 @@ func initBalancerGroupForCachingTest(t *testing.T) (*weightedaggregator.Aggregat m1[testBackendAddrs[1]], m1[testBackendAddrs[1]], m1[testBackendAddrs[2]], m1[testBackendAddrs[3]], } - if err := testutils.IsRoundRobin(want, subConnFromPicker(p1)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p1)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -263,7 +256,7 @@ func initBalancerGroupForCachingTest(t *testing.T) (*weightedaggregator.Aggregat want = []balancer.SubConn{ m1[testBackendAddrs[0]], m1[testBackendAddrs[1]], } - if err := testutils.IsRoundRobin(want, subConnFromPicker(p2)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p2)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -304,7 +297,7 @@ func (s) TestBalancerGroup_locality_caching(t *testing.T) { // addr2 is down, b2 only has addr3 in READY state. addrToSC[testBackendAddrs[3]], addrToSC[testBackendAddrs[3]], } - if err := testutils.IsRoundRobin(want, subConnFromPicker(p3)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p3)); err != nil { t.Fatalf("want %v, got %v", want, err) } @@ -452,7 +445,7 @@ func (s) TestBalancerGroup_locality_caching_readd_with_different_builder(t *test addrToSC[testBackendAddrs[1]], addrToSC[testBackendAddrs[1]], addrToSC[testBackendAddrs[4]], addrToSC[testBackendAddrs[5]], } - if err := testutils.IsRoundRobin(want, subConnFromPicker(p3)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p3)); err != nil { t.Fatalf("want %v, got %v", want, err) } } @@ -583,7 +576,7 @@ func (s) TestBalancerGracefulSwitch(t *testing.T) { want := []balancer.SubConn{ m1[testBackendAddrs[0]], m1[testBackendAddrs[1]], } - if err := testutils.IsRoundRobin(want, subConnFromPicker(p1)); err != nil { + if err := testutils.IsRoundRobin(want, testutils.SubConnFromPicker(p1)); err != nil { t.Fatalf("want %v, got %v", want, err) } diff --git a/internal/testutils/balancer.go b/internal/testutils/balancer.go index d45486abd251..55c2495fa13e 100644 --- a/internal/testutils/balancer.go +++ b/internal/testutils/balancer.go @@ -339,6 +339,13 @@ func IsRoundRobin(want []balancer.SubConn, f func() balancer.SubConn) error { return nil } +func SubConnFromPicker(p balancer.Picker) func() balancer.SubConn { + return func() balancer.SubConn { + scst, _ := p.Pick(balancer.PickInfo{}) + return scst.SubConn + } +} + // ErrTestConstPicker is error returned by test const picker. var ErrTestConstPicker = fmt.Errorf("const picker error") diff --git a/xds/internal/balancer/clusterresolver/priority_test.go b/xds/internal/balancer/clusterresolver/priority_test.go index 0ba5e1e80946..be98642e8d66 100644 --- a/xds/internal/balancer/clusterresolver/priority_test.go +++ b/xds/internal/balancer/clusterresolver/priority_test.go @@ -117,8 +117,10 @@ func (s) TestEDSPriority_HighPriorityReady(t *testing.T) { edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc1}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil { + t.Fatal(err.Error()) } // Add p2, it shouldn't cause any updates. @@ -139,7 +141,7 @@ func (s) TestEDSPriority_HighPriorityReady(t *testing.T) { select { case p := <-cc.NewPickerCh: // If we do get a new picker, ensure it is still a p1 picker. - if err := testutils.IsRoundRobin([]balancer.SubConn{sc1}, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin([]balancer.SubConn{sc1}, testutils.SubConnFromPicker(p)); err != nil { t.Fatal(err.Error()) } default: @@ -165,7 +167,7 @@ func (s) TestEDSPriority_HighPriorityReady(t *testing.T) { select { case p := <-cc.NewPickerCh: // If we do get a new picker, ensure it is still a p1 picker. - if err := testutils.IsRoundRobin([]balancer.SubConn{sc1}, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin([]balancer.SubConn{sc1}, testutils.SubConnFromPicker(p)); err != nil { t.Fatal(err.Error()) } default: @@ -201,8 +203,10 @@ func (s) TestEDSPriority_SwitchPriority(t *testing.T) { edsb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc0}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc0); err != nil { + t.Fatal(err.Error()) } // Turn down 0, 1 is used. @@ -216,8 +220,8 @@ func (s) TestEDSPriority_SwitchPriority(t *testing.T) { edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test pick with 1. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc1}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil { + t.Fatal(err.Error()) } // Add p2, it shouldn't cause any updates. @@ -238,7 +242,7 @@ func (s) TestEDSPriority_SwitchPriority(t *testing.T) { select { case p := <-cc.NewPickerCh: // If we do get a new picker, ensure it is still a p1 picker. - if err := testutils.IsRoundRobin([]balancer.SubConn{sc1}, subConnFromPicker(p)); err != nil { + if err := testutils.IsRoundRobin([]balancer.SubConn{sc1}, testutils.SubConnFromPicker(p)); err != nil { t.Fatal(err.Error()) } default: @@ -262,8 +266,8 @@ func (s) TestEDSPriority_SwitchPriority(t *testing.T) { edsb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test pick with 2. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc2}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc2); err != nil { + t.Fatal(err.Error()) } // Remove 2, use 1. @@ -280,8 +284,8 @@ func (s) TestEDSPriority_SwitchPriority(t *testing.T) { // Should get an update with 1's old picker, to override 2's old picker. want := errors.New("last connection error: subConn connection error") - if err := testErrPickerFromCh(cc.NewPickerCh, want); err != nil { - t.Fatal(err) + if err := cc.WaitForPickerWithErr(ctx, want); err != nil { + t.Fatal(err.Error()) } } @@ -318,8 +322,10 @@ func (s) TestEDSPriority_HigherDownWhileAddingLower(t *testing.T) { }) // Test pick failure. + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() want := errors.New("last connection error: subConn connection error") - if err := testErrPickerFromCh(cc.NewPickerCh, want); err != nil { + if err := cc.WaitForPickerWithErr(ctx, want); err != nil { t.Fatal(err) } @@ -338,8 +344,8 @@ func (s) TestEDSPriority_HigherDownWhileAddingLower(t *testing.T) { edsb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test pick with 2. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc2}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc2); err != nil { + t.Fatal(err.Error()) } } @@ -380,8 +386,10 @@ func (s) TestEDSPriority_HigherReadyCloseAllLower(t *testing.T) { edsb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test pick with 2. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc2}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc2); err != nil { + t.Fatal(err.Error()) } // When 0 becomes ready, 0 should be used, 1 and 2 should all be closed. @@ -415,8 +423,8 @@ func (s) TestEDSPriority_HigherReadyCloseAllLower(t *testing.T) { } // Test pick with 0. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc0}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc0); err != nil { + t.Fatal(err.Error()) } } @@ -467,8 +475,10 @@ func (s) TestEDSPriority_InitTimeout(t *testing.T) { edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test pick with 1. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc1}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil { + t.Fatal(err.Error()) } } @@ -493,8 +503,10 @@ func (s) TestEDSPriority_MultipleLocalities(t *testing.T) { edsb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc0}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc0); err != nil { + t.Fatal(err.Error()) } // Turn down p0 subconns, p1 subconns will be created. @@ -509,8 +521,8 @@ func (s) TestEDSPriority_MultipleLocalities(t *testing.T) { edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p1 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc1}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil { + t.Fatal(err.Error()) } // Reconnect p0 subconns, p1 subconn will be closed. @@ -522,8 +534,8 @@ func (s) TestEDSPriority_MultipleLocalities(t *testing.T) { } // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc0}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc0); err != nil { + t.Fatal(err.Error()) } // Add two localities, with two priorities, with one backend. @@ -542,8 +554,8 @@ func (s) TestEDSPriority_MultipleLocalities(t *testing.T) { edsb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only two p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc0, sc2}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc0, sc2); err != nil { + t.Fatal(err.Error()) } // Turn down p0 subconns, p1 subconns will be created. @@ -558,8 +570,8 @@ func (s) TestEDSPriority_MultipleLocalities(t *testing.T) { edsb.UpdateSubConnState(sc4, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p1 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc3, sc4}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc3, sc4); err != nil { + t.Fatal(err.Error()) } } @@ -590,8 +602,10 @@ func (s) TestEDSPriority_RemovesAllLocalities(t *testing.T) { edsb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc0}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc0); err != nil { + t.Fatal(err.Error()) } // Remove all priorities. @@ -606,7 +620,7 @@ func (s) TestEDSPriority_RemovesAllLocalities(t *testing.T) { // time.Sleep(time.Second) // Test pick return TransientFailure. - if err := testErrPickerFromCh(cc.NewPickerCh, priority.ErrAllPrioritiesRemoved); err != nil { + if err := cc.WaitForPickerWithErr(ctx, priority.ErrAllPrioritiesRemoved); err != nil { t.Fatal(err) } @@ -635,8 +649,8 @@ func (s) TestEDSPriority_RemovesAllLocalities(t *testing.T) { edsb.UpdateSubConnState(sc11, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p1 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc11}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc11); err != nil { + t.Fatal(err.Error()) } // Remove p1 from EDS, to fallback to p0. @@ -651,7 +665,7 @@ func (s) TestEDSPriority_RemovesAllLocalities(t *testing.T) { } // Test pick return TransientFailure. - if err := testErrPickerFromCh(cc.NewPickerCh, balancer.ErrNoSubConnAvailable); err != nil { + if err := cc.WaitForPickerWithErr(ctx, balancer.ErrNoSubConnAvailable); err != nil { t.Fatal(err) } @@ -661,8 +675,8 @@ func (s) TestEDSPriority_RemovesAllLocalities(t *testing.T) { edsb.UpdateSubConnState(sc01, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc01}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc01); err != nil { + t.Fatal(err.Error()) } select { @@ -697,8 +711,10 @@ func (s) TestEDSPriority_HighPriorityNoEndpoints(t *testing.T) { edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc1}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil { + t.Fatal(err.Error()) } // Remove addresses from priority 0, should use p1. @@ -722,8 +738,8 @@ func (s) TestEDSPriority_HighPriorityNoEndpoints(t *testing.T) { edsb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p1 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc2}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc2); err != nil { + t.Fatal(err.Error()) } } @@ -748,8 +764,10 @@ func (s) TestEDSPriority_HighPriorityAllUnhealthy(t *testing.T) { edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc1}); err != nil { - t.Fatal(err) + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() + if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil { + t.Fatal(err.Error()) } // Set priority 0 endpoints to all unhealthy, should use p1. @@ -775,8 +793,8 @@ func (s) TestEDSPriority_HighPriorityAllUnhealthy(t *testing.T) { edsb.UpdateSubConnState(sc2, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p1 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc2}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc2); err != nil { + t.Fatal(err.Error()) } } @@ -865,8 +883,8 @@ func (s) TestFallbackToDNS(t *testing.T) { edsb.UpdateSubConnState(sc0, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test roundrobin with only p0 subconns. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc0}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc0); err != nil { + t.Fatal(err.Error()) } // Turn down 0, p1 (DNS) will be used. @@ -893,8 +911,8 @@ func (s) TestFallbackToDNS(t *testing.T) { edsb.UpdateSubConnState(sc1, balancer.SubConnState{ConnectivityState: connectivity.Ready}) // Test pick with 1. - if err := testRoundRobinPickerFromCh(cc.NewPickerCh, []balancer.SubConn{sc1}); err != nil { - t.Fatal(err) + if err := cc.WaitForRoundRobinPicker(ctx, sc1); err != nil { + t.Fatal(err.Error()) } // Turn down the DNS endpoint, this should trigger an re-resolve in the DNS diff --git a/xds/internal/balancer/clusterresolver/testutil_test.go b/xds/internal/balancer/clusterresolver/testutil_test.go index 9c51db49f921..2792f802b258 100644 --- a/xds/internal/balancer/clusterresolver/testutil_test.go +++ b/xds/internal/balancer/clusterresolver/testutil_test.go @@ -19,16 +19,12 @@ package clusterresolver import ( "fmt" "net" - "reflect" "strconv" - "time" xdspb "github.com/envoyproxy/go-control-plane/envoy/api/v2" corepb "github.com/envoyproxy/go-control-plane/envoy/api/v2/core" endpointpb "github.com/envoyproxy/go-control-plane/envoy/api/v2/endpoint" typepb "github.com/envoyproxy/go-control-plane/envoy/type" - "google.golang.org/grpc/balancer" - "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/xds/internal" "google.golang.org/grpc/xds/internal/xdsclient/xdsresource" ) @@ -115,70 +111,3 @@ func parseEndpoints(lbEndpoints []*endpointpb.LbEndpoint) []xdsresource.Endpoint } return endpoints } - -// testPickerFromCh receives pickers from the channel, and check if their -// behaviors are as expected (that the given function returns nil err). -// -// It returns nil if one picker has the correct behavior. -// -// It returns error when there's no picker from channel after 1 second timeout, -// and the error returned is the mismatch error from the previous picker. -func testPickerFromCh(ch chan balancer.Picker, f func(balancer.Picker) error) error { - var ( - p balancer.Picker - err error - ) - for { - select { - case p = <-ch: - case <-time.After(defaultTestTimeout): - // TODO: this function should take a context, and use the context - // here, instead of making a new timer. - return fmt.Errorf("timeout waiting for picker with expected behavior, error from previous picker: %v", err) - } - - err = f(p) - if err == nil { - return nil - } - } -} - -func subConnFromPicker(p balancer.Picker) func() balancer.SubConn { - return func() balancer.SubConn { - scst, _ := p.Pick(balancer.PickInfo{}) - return scst.SubConn - } -} - -// testRoundRobinPickerFromCh receives pickers from the channel, and check if -// their behaviors are round-robin of want. -// -// It returns nil if one picker has the correct behavior. -// -// It returns error when there's no picker from channel after 1 second timeout, -// and the error returned is the mismatch error from the previous picker. -func testRoundRobinPickerFromCh(ch chan balancer.Picker, want []balancer.SubConn) error { - return testPickerFromCh(ch, func(p balancer.Picker) error { - return testutils.IsRoundRobin(want, subConnFromPicker(p)) - }) -} - -// testErrPickerFromCh receives pickers from the channel, and check if they -// return the wanted error. -// -// It returns nil if one picker has the correct behavior. -// -// It returns error when there's no picker from channel after 1 second timeout, -// and the error returned is the mismatch error from the previous picker. -func testErrPickerFromCh(ch chan balancer.Picker, want error) error { - return testPickerFromCh(ch, func(p balancer.Picker) error { - for i := 0; i < 5; i++ { - _, err := p.Pick(balancer.PickInfo{}) - if !reflect.DeepEqual(err, want) { - return fmt.Errorf("picker.Pick, got err %q, want err %q", err, want) - } - } - return nil - }) -}