Skip to content

Commit 0800ec7

Browse files
xds/clusterimpl: update TestChildPolicyChangeOnConfigUpdate to use custom lb policy. (#8730)
Fixes #8703 ### Description This PR fixes the data race in `TestChildPolicyChangeOnConfigUpdate`. ### Changes * **Isolated Policy:** Instead of overwriting the global `pick_first` policy (and triggering a race condition via `Unregister`), the test now registers a unique custom stub policy named `test_pick_first`. * **TypedStruct Configuration:** Updated the management server resource update to use `v3xdsxdstypepb.TypedStruct`and specify the custom `type.googleapis.com/test_pick_first` policy via TypeURL. RELEASE NOTES: N/A
1 parent 6553ea1 commit 0800ec7

File tree

1 file changed

+32
-19
lines changed

1 file changed

+32
-19
lines changed

internal/xds/balancer/clusterimpl/tests/balancer_test.go

Lines changed: 32 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ import (
5555
"google.golang.org/protobuf/types/known/durationpb"
5656
"google.golang.org/protobuf/types/known/wrapperspb"
5757

58+
v3xdsxdstypepb "github.com/cncf/xds/go/xds/type/v3"
5859
v3clusterpb "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
5960
v3corepb "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
6061
v3endpointpb "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
@@ -64,6 +65,7 @@ import (
6465
v3lrspb "github.com/envoyproxy/go-control-plane/envoy/service/load_stats/v3"
6566
testgrpc "google.golang.org/grpc/interop/grpc_testing"
6667
testpb "google.golang.org/grpc/interop/grpc_testing"
68+
"google.golang.org/protobuf/types/known/structpb"
6769

6870
_ "google.golang.org/grpc/xds"
6971
)
@@ -1124,6 +1126,8 @@ func (s) TestUpdateLRSServerToNil(t *testing.T) {
11241126
// Test verifies that child policy was updated on receipt of
11251127
// configuration update.
11261128
func (s) TestChildPolicyChangeOnConfigUpdate(t *testing.T) {
1129+
const customLBPolicy = "test_custom_lb_policy"
1130+
11271131
// Create an xDS management server.
11281132
mgmtServer := e2e.StartManagementServer(t, e2e.ManagementServerOptions{AllowResourceSubset: true})
11291133
defer mgmtServer.Stop()
@@ -1175,21 +1179,21 @@ func (s) TestChildPolicyChangeOnConfigUpdate(t *testing.T) {
11751179
t.Fatalf("client.EmptyCall() failed: %v", err)
11761180
}
11771181

1178-
// Register stub pickfirst LB policy so that we can catch config changes.
1182+
// Register stub customLBPolicy LB policy so that we can catch config changes.
11791183
pfBuilder := balancer.Get(pickfirst.Name)
1180-
internal.BalancerUnregister(pfBuilder.Name())
11811184
lbCfgCh := make(chan serviceconfig.LoadBalancingConfig, 1)
1182-
var updatedChildPolicy atomic.Pointer[string]
1183-
stub.Register(pfBuilder.Name(), stub.BalancerFuncs{
1185+
var updatedChildPolicy atomic.Value
1186+
1187+
stub.Register(customLBPolicy, stub.BalancerFuncs{
11841188
ParseConfig: func(lbCfg json.RawMessage) (serviceconfig.LoadBalancingConfig, error) {
11851189
return pfBuilder.(balancer.ConfigParser).ParseConfig(lbCfg)
11861190
},
11871191
Init: func(bd *stub.BalancerData) {
11881192
bd.ChildBalancer = pfBuilder.Build(bd.ClientConn, bd.BuildOptions)
11891193
},
11901194
UpdateClientConnState: func(bd *stub.BalancerData, ccs balancer.ClientConnState) error {
1191-
name := pfBuilder.Name()
1192-
updatedChildPolicy.Store(&name)
1195+
// name := customLBPolicy
1196+
updatedChildPolicy.Store(customLBPolicy)
11931197
select {
11941198
case lbCfgCh <- ccs.BalancerConfig:
11951199
case <-ctx.Done():
@@ -1201,13 +1205,17 @@ func (s) TestChildPolicyChangeOnConfigUpdate(t *testing.T) {
12011205
bd.ChildBalancer.Close()
12021206
},
12031207
})
1204-
defer balancer.Register(pfBuilder)
12051208

1206-
// Now update the cluster to use "pick_first" as the endpoint picking policy.
1209+
defer internal.BalancerUnregister(customLBPolicy)
1210+
1211+
// Update the cluster to use customLBPolicy as the endpoint picking policy.
12071212
resources.Clusters[0].LoadBalancingPolicy = &v3clusterpb.LoadBalancingPolicy{
12081213
Policies: []*v3clusterpb.LoadBalancingPolicy_Policy{{
12091214
TypedExtensionConfig: &v3corepb.TypedExtensionConfig{
1210-
TypedConfig: testutils.MarshalAny(t, &v3pickfirstpb.PickFirst{}),
1215+
TypedConfig: testutils.MarshalAny(t, &v3xdsxdstypepb.TypedStruct{
1216+
TypeUrl: "type.googleapis.com/" + customLBPolicy,
1217+
Value: &structpb.Struct{},
1218+
}),
12111219
},
12121220
}},
12131221
}
@@ -1217,21 +1225,26 @@ func (s) TestChildPolicyChangeOnConfigUpdate(t *testing.T) {
12171225

12181226
select {
12191227
case <-ctx.Done():
1220-
t.Fatalf("Timeout waiting for pickfirst child policy config")
1228+
t.Fatalf("Timeout waiting for child policy config")
12211229
case <-lbCfgCh:
12221230
}
12231231

1224-
if p := updatedChildPolicy.Load(); p == nil || *p != pfBuilder.Name() {
1225-
var got string
1226-
if p != nil {
1227-
got = *p
1228-
}
1229-
t.Fatalf("Unexpected child policy after config update, got %q, want %q", got, pfBuilder.Name())
1232+
if p, ok := updatedChildPolicy.Load().(string); !ok || p != customLBPolicy {
1233+
t.Fatalf("Unexpected child policy after config update, got %q (ok: %v), want %q", p, ok, customLBPolicy)
12301234
}
12311235

1232-
// New RPC should still be routed successfully
1233-
if _, err := client.EmptyCall(ctx, &testpb.Empty{}); err != nil {
1234-
t.Errorf("EmptyCall() failed after policy update: %v", err)
1236+
ticker := time.NewTicker(10 * time.Millisecond)
1237+
defer ticker.Stop()
1238+
1239+
for {
1240+
select {
1241+
case <-ctx.Done():
1242+
t.Fatalf("Timeout waiting for successful RPC after policy update.")
1243+
case <-ticker.C:
1244+
if _, err := client.EmptyCall(ctx, &testpb.Empty{}); err == nil {
1245+
return
1246+
}
1247+
}
12351248
}
12361249
}
12371250

0 commit comments

Comments
 (0)