Skip to content

Commit f32eab3

Browse files
cherry-pick #8302 and #8304 to v1.72.x branch (#8303)
* resolver/delegatingresolver: wait for proxy resolver to be built in test (#8302) * resolver/delegatingresolver: wait for proxy resolver build before update in tests (#8304)
1 parent 7fcfc87 commit f32eab3

File tree

1 file changed

+81
-44
lines changed

1 file changed

+81
-44
lines changed

internal/resolver/delegatingresolver/delegatingresolver_ext_test.go

+81-44
Original file line numberDiff line numberDiff line change
@@ -131,15 +131,29 @@ func (s) TestDelegatingResolverNoProxyEnvVarsSet(t *testing.T) {
131131
// overwriting the previously registered DNS resolver. This allows the test to
132132
// mock the DNS resolution for the proxy resolver. It also registers the
133133
// original DNS resolver after the test is done.
134-
func setupDNS(t *testing.T) *manual.Resolver {
134+
func setupDNS(t *testing.T) (*manual.Resolver, chan struct{}) {
135135
t.Helper()
136136
mr := manual.NewBuilderWithScheme("dns")
137137

138138
dnsResolverBuilder := resolver.Get("dns")
139139
resolver.Register(mr)
140140

141+
resolverBuilt := make(chan struct{})
142+
mr.BuildCallback = func(resolver.Target, resolver.ClientConn, resolver.BuildOptions) {
143+
close(resolverBuilt)
144+
}
145+
141146
t.Cleanup(func() { resolver.Register(dnsResolverBuilder) })
142-
return mr
147+
return mr, resolverBuilt
148+
}
149+
150+
func mustBuildResolver(ctx context.Context, t *testing.T, buildCh chan struct{}) {
151+
t.Helper()
152+
select {
153+
case <-buildCh:
154+
case <-ctx.Done():
155+
t.Fatalf("Context timed out waiting for resolver to be built.")
156+
}
143157
}
144158

145159
// proxyAddressWithTargetAttribute creates a resolver.Address for the proxy,
@@ -181,7 +195,7 @@ func (s) TestDelegatingResolverwithDNSAndProxyWithTargetResolution(t *testing.T)
181195
targetResolver := manual.NewBuilderWithScheme("dns")
182196
target := targetResolver.Scheme() + ":///" + targetTestAddr
183197
// Set up a manual DNS resolver to control the proxy address resolution.
184-
proxyResolver := setupDNS(t)
198+
proxyResolver, proxyResolverBuilt := setupDNS(t)
185199

186200
tcc, stateCh, _ := createTestResolverClientConn(t)
187201
if _, err := delegatingresolver.New(resolver.Target{URL: *testutils.MustParseURL(target)}, tcc, resolver.BuildOptions{}, targetResolver, true); err != nil {
@@ -202,6 +216,11 @@ func (s) TestDelegatingResolverwithDNSAndProxyWithTargetResolution(t *testing.T)
202216
case <-time.After(defaultTestShortTimeout):
203217
}
204218

219+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
220+
defer cancel()
221+
222+
// Wait for the proxy resolver to be built before calling UpdateState.
223+
mustBuildResolver(ctx, t, proxyResolverBuilt)
205224
proxyResolver.UpdateState(resolver.State{
206225
Addresses: []resolver.Address{{Addr: resolvedProxyTestAddr1}},
207226
ServiceConfig: &serviceconfig.ParseResult{},
@@ -218,8 +237,8 @@ func (s) TestDelegatingResolverwithDNSAndProxyWithTargetResolution(t *testing.T)
218237
var gotState resolver.State
219238
select {
220239
case gotState = <-stateCh:
221-
case <-time.After(defaultTestTimeout):
222-
t.Fatal("Timeout when waiting for a state update from the delegating resolver")
240+
case <-ctx.Done():
241+
t.Fatal("Context timeed out when waiting for a state update from the delegating resolver")
223242
}
224243

225244
if diff := cmp.Diff(gotState, wantState); diff != "" {
@@ -257,13 +276,18 @@ func (s) TestDelegatingResolverwithDNSAndProxyWithNoTargetResolution(t *testing.
257276
targetResolver := manual.NewBuilderWithScheme("dns")
258277
target := targetResolver.Scheme() + ":///" + targetTestAddr
259278
// Set up a manual DNS resolver to control the proxy address resolution.
260-
proxyResolver := setupDNS(t)
279+
proxyResolver, proxyResolverBuilt := setupDNS(t)
261280

262281
tcc, stateCh, _ := createTestResolverClientConn(t)
263282
if _, err := delegatingresolver.New(resolver.Target{URL: *testutils.MustParseURL(target)}, tcc, resolver.BuildOptions{}, targetResolver, false); err != nil {
264283
t.Fatalf("Failed to create delegating resolver: %v", err)
265284
}
266285

286+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
287+
defer cancel()
288+
289+
// Wait for the proxy resolver to be built before calling UpdateState.
290+
mustBuildResolver(ctx, t, proxyResolverBuilt)
267291
proxyResolver.UpdateState(resolver.State{
268292
Addresses: []resolver.Address{
269293
{Addr: resolvedProxyTestAddr1},
@@ -278,8 +302,8 @@ func (s) TestDelegatingResolverwithDNSAndProxyWithNoTargetResolution(t *testing.
278302
var gotState resolver.State
279303
select {
280304
case gotState = <-stateCh:
281-
case <-time.After(defaultTestTimeout):
282-
t.Fatal("Timeout when waiting for a state update from the delegating resolver")
305+
case <-ctx.Done():
306+
t.Fatal("Context timed out when waiting for a state update from the delegating resolver")
283307
}
284308

285309
if diff := cmp.Diff(gotState, wantState); diff != "" {
@@ -319,7 +343,7 @@ func (s) TestDelegatingResolverwithCustomResolverAndProxy(t *testing.T) {
319343
targetResolver := manual.NewBuilderWithScheme("test")
320344
target := targetResolver.Scheme() + ":///" + targetTestAddr
321345
// Set up a manual DNS resolver to control the proxy address resolution.
322-
proxyResolver := setupDNS(t)
346+
proxyResolver, proxyResolverBuilt := setupDNS(t)
323347

324348
tcc, stateCh, _ := createTestResolverClientConn(t)
325349
if _, err := delegatingresolver.New(resolver.Target{URL: *testutils.MustParseURL(target)}, tcc, resolver.BuildOptions{}, targetResolver, false); err != nil {
@@ -340,6 +364,11 @@ func (s) TestDelegatingResolverwithCustomResolverAndProxy(t *testing.T) {
340364
case <-time.After(defaultTestShortTimeout):
341365
}
342366

367+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
368+
defer cancel()
369+
370+
// Wait for the proxy resolver to be built before calling UpdateState.
371+
mustBuildResolver(ctx, t, proxyResolverBuilt)
343372
proxyResolver.UpdateState(resolver.State{
344373
Addresses: []resolver.Address{{Addr: resolvedProxyTestAddr1}},
345374
ServiceConfig: &serviceconfig.ParseResult{},
@@ -355,8 +384,8 @@ func (s) TestDelegatingResolverwithCustomResolverAndProxy(t *testing.T) {
355384
var gotState resolver.State
356385
select {
357386
case gotState = <-stateCh:
358-
case <-time.After(defaultTestTimeout):
359-
t.Fatal("Timeout when waiting for a state update from the delegating resolver")
387+
case <-ctx.Done():
388+
t.Fatal("Context timed out when waiting for a state update from the delegating resolver")
360389
}
361390

362391
if diff := cmp.Diff(gotState, wantState); diff != "" {
@@ -401,7 +430,7 @@ func (s) TestDelegatingResolverForEndpointsWithProxy(t *testing.T) {
401430
targetResolver := manual.NewBuilderWithScheme("test")
402431
target := targetResolver.Scheme() + ":///" + targetTestAddr
403432
// Set up a manual DNS resolver to control the proxy address resolution.
404-
proxyResolver := setupDNS(t)
433+
proxyResolver, proxyResolverBuilt := setupDNS(t)
405434

406435
tcc, stateCh, _ := createTestResolverClientConn(t)
407436
if _, err := delegatingresolver.New(resolver.Target{URL: *testutils.MustParseURL(target)}, tcc, resolver.BuildOptions{}, targetResolver, false); err != nil {
@@ -429,6 +458,11 @@ func (s) TestDelegatingResolverForEndpointsWithProxy(t *testing.T) {
429458
case <-time.After(defaultTestShortTimeout):
430459
}
431460

461+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
462+
defer cancel()
463+
464+
// Wait for the proxy resolver to be built before calling UpdateState.
465+
mustBuildResolver(ctx, t, proxyResolverBuilt)
432466
proxyResolver.UpdateState(resolver.State{
433467
Endpoints: []resolver.Endpoint{
434468
{Addresses: []resolver.Address{{Addr: resolvedProxyTestAddr1}}},
@@ -460,8 +494,8 @@ func (s) TestDelegatingResolverForEndpointsWithProxy(t *testing.T) {
460494
var gotState resolver.State
461495
select {
462496
case gotState = <-stateCh:
463-
case <-time.After(defaultTestTimeout):
464-
t.Fatal("Timeout when waiting for a state update from the delegating resolver")
497+
case <-ctx.Done():
498+
t.Fatal("Contex timed out when waiting for a state update from the delegating resolver")
465499
}
466500

467501
if diff := cmp.Diff(gotState, wantState); diff != "" {
@@ -503,8 +537,7 @@ func (s) TestDelegatingResolverForMultipleProxyAddress(t *testing.T) {
503537
targetResolver := manual.NewBuilderWithScheme("test")
504538
target := targetResolver.Scheme() + ":///" + targetTestAddr
505539
// Set up a manual DNS resolver to control the proxy address resolution.
506-
proxyResolver := setupDNS(t)
507-
540+
proxyResolver, proxyResolverBuilt := setupDNS(t)
508541
tcc, stateCh, _ := createTestResolverClientConn(t)
509542
if _, err := delegatingresolver.New(resolver.Target{URL: *testutils.MustParseURL(target)}, tcc, resolver.BuildOptions{}, targetResolver, false); err != nil {
510543
t.Fatalf("Failed to create delegating resolver: %v", err)
@@ -524,6 +557,11 @@ func (s) TestDelegatingResolverForMultipleProxyAddress(t *testing.T) {
524557
case <-time.After(defaultTestShortTimeout):
525558
}
526559

560+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
561+
defer cancel()
562+
563+
// Wait for the proxy resolver to be built before calling UpdateState.
564+
mustBuildResolver(ctx, t, proxyResolverBuilt)
527565
proxyResolver.UpdateState(resolver.State{
528566
Addresses: []resolver.Address{
529567
{Addr: resolvedProxyTestAddr1},
@@ -542,8 +580,8 @@ func (s) TestDelegatingResolverForMultipleProxyAddress(t *testing.T) {
542580
var gotState resolver.State
543581
select {
544582
case gotState = <-stateCh:
545-
case <-time.After(defaultTestTimeout):
546-
t.Fatal("Timeout when waiting for a state update from the delegating resolver")
583+
case <-ctx.Done():
584+
t.Fatal("Context timed out when waiting for a state update from the delegating resolver")
547585
}
548586

549587
if diff := cmp.Diff(gotState, wantState); diff != "" {
@@ -586,7 +624,7 @@ func (s) TestDelegatingResolverUpdateStateDuringClose(t *testing.T) {
586624

587625
target := targetResolver.Scheme() + ":///" + "ignored"
588626
// Set up a manual DNS resolver to control the proxy address resolution.
589-
proxyResolver := setupDNS(t)
627+
proxyResolver, proxyResolverBuilt := setupDNS(t)
590628

591629
unblockProxyResolverClose := make(chan struct{}, 1)
592630
proxyResolver.CloseCallback = func() {
@@ -607,11 +645,13 @@ func (s) TestDelegatingResolverUpdateStateDuringClose(t *testing.T) {
607645
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: "1.1.1.1"}}}},
608646
})
609647

648+
// Wait for the proxy resolver to be built before calling Close.
649+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
650+
defer cancel()
651+
mustBuildResolver(ctx, t, proxyResolverBuilt)
610652
// Closing the delegating resolver will block until the test writes to the
611653
// unblockProxyResolverClose channel.
612654
go dr.Close()
613-
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
614-
defer cancel()
615655
select {
616656
case <-targetResolverCloseCalled:
617657
case <-ctx.Done():
@@ -667,7 +707,7 @@ func (s) TestDelegatingResolverUpdateStateFromResolveNow(t *testing.T) {
667707

668708
target := targetResolver.Scheme() + ":///" + "ignored"
669709
// Set up a manual DNS resolver to control the proxy address resolution.
670-
proxyResolver := setupDNS(t)
710+
proxyResolver, proxyResolverBuilt := setupDNS(t)
671711

672712
tcc, _, _ := createTestResolverClientConn(t)
673713
tcc.UpdateStateF = func(resolver.State) error {
@@ -682,14 +722,18 @@ func (s) TestDelegatingResolverUpdateStateFromResolveNow(t *testing.T) {
682722
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: "1.1.1.1"}}}},
683723
})
684724

725+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
726+
defer cancel()
727+
728+
// Wait for the proxy resolver to be built before calling UpdateState.
729+
mustBuildResolver(ctx, t, proxyResolverBuilt)
730+
685731
// Updating the channel will result in an error being returned. The
686732
// delegating resolver should call call "ResolveNow" on the target resolver.
687733
proxyResolver.UpdateState(resolver.State{
688734
Endpoints: []resolver.Endpoint{{Addresses: []resolver.Address{{Addr: "1.1.1.1"}}}},
689735
})
690736

691-
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
692-
defer cancel()
693737
select {
694738
case <-targetResolverCalled:
695739
case <-ctx.Done():
@@ -727,12 +771,9 @@ func (s) TestDelegatingResolverResolveNow(t *testing.T) {
727771

728772
target := targetResolver.Scheme() + ":///" + "ignored"
729773
// Set up a manual DNS resolver to control the proxy address resolution.
730-
proxyResolver := setupDNS(t)
774+
proxyResolver, proxyResolverBuilt := setupDNS(t)
775+
731776
proxyResolverCalled := make(chan struct{})
732-
proxyResolverBuilt := make(chan struct{})
733-
proxyResolver.BuildCallback = func(resolver.Target, resolver.ClientConn, resolver.BuildOptions) {
734-
close(proxyResolverBuilt)
735-
}
736777
proxyResolver.ResolveNowCallback = func(resolver.ResolveNowOptions) {
737778
// Updating the resolver state should not deadlock.
738779
proxyResolver.CC().UpdateState(resolver.State{
@@ -760,12 +801,7 @@ func (s) TestDelegatingResolverResolveNow(t *testing.T) {
760801
t.Fatalf("context timed out waiting for targetResolver.ResolveNow() to be called.")
761802
}
762803

763-
// Wait for proxy resolver to be built.
764-
select {
765-
case <-proxyResolverBuilt:
766-
case <-ctx.Done():
767-
t.Fatalf("Timeout when waiting for proxy resolver to be built")
768-
}
804+
mustBuildResolver(ctx, t, proxyResolverBuilt)
769805

770806
dr.ResolveNow(resolver.ResolveNowOptions{})
771807

@@ -811,11 +847,7 @@ func (s) TestDelegatingResolverForNonTCPTarget(t *testing.T) {
811847
targetResolver := manual.NewBuilderWithScheme("test")
812848
target := targetResolver.Scheme() + ":///" + targetTestAddr
813849
// Set up a manual DNS resolver to control the proxy address resolution.
814-
proxyResolver := setupDNS(t)
815-
proxyBuildCalled := make(chan struct{})
816-
proxyResolver.BuildCallback = func(resolver.Target, resolver.ClientConn, resolver.BuildOptions) {
817-
close(proxyBuildCalled)
818-
}
850+
_, proxyResolverBuilt := setupDNS(t)
819851

820852
tcc, stateCh, _ := createTestResolverClientConn(t)
821853
if _, err := delegatingresolver.New(resolver.Target{URL: *testutils.MustParseURL(target)}, tcc, resolver.BuildOptions{}, targetResolver, false); err != nil {
@@ -840,7 +872,7 @@ func (s) TestDelegatingResolverForNonTCPTarget(t *testing.T) {
840872
// Verify that the delegating resolver doesn't call proxy resolver's
841873
// UpdateState since we have no tcp address
842874
select {
843-
case <-proxyBuildCalled:
875+
case <-proxyResolverBuilt:
844876
t.Fatal("Unexpected call to proxy resolver update state")
845877
case <-time.After(defaultTestShortTimeout):
846878
}
@@ -890,7 +922,7 @@ func (s) TestDelegatingResolverForMixNetworkType(t *testing.T) {
890922
targetResolver := manual.NewBuilderWithScheme("test")
891923
target := targetResolver.Scheme() + ":///" + targetTestAddr
892924
// Set up a manual DNS resolver to control the proxy address resolution.
893-
proxyResolver := setupDNS(t)
925+
proxyResolver, proxyResolverBuilt := setupDNS(t)
894926

895927
tcc, stateCh, _ := createTestResolverClientConn(t)
896928
if _, err := delegatingresolver.New(resolver.Target{URL: *testutils.MustParseURL(target)}, tcc, resolver.BuildOptions{}, targetResolver, false); err != nil {
@@ -910,6 +942,11 @@ func (s) TestDelegatingResolverForMixNetworkType(t *testing.T) {
910942
case <-time.After(defaultTestShortTimeout):
911943
}
912944

945+
ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
946+
defer cancel()
947+
948+
// Wait for the proxy resolver to be built before calling UpdateState.
949+
mustBuildResolver(ctx, t, proxyResolverBuilt)
913950
proxyResolver.UpdateState(resolver.State{
914951
Addresses: []resolver.Address{{Addr: envProxyAddr}},
915952
ServiceConfig: &serviceconfig.ParseResult{},
@@ -918,8 +955,8 @@ func (s) TestDelegatingResolverForMixNetworkType(t *testing.T) {
918955
var gotState resolver.State
919956
select {
920957
case gotState = <-stateCh:
921-
case <-time.After(defaultTestTimeout):
922-
t.Fatal("Timeout when waiting for a state update from the delegating resolver")
958+
case <-ctx.Done():
959+
t.Fatal("Context timed out when waiting for a state update from the delegating resolver")
923960
}
924961
wantState := resolver.State{
925962
Addresses: []resolver.Address{nonTCPAddr, proxyAddressWithTargetAttribute(envProxyAddr, resolvedTargetTestAddr2)},

0 commit comments

Comments
 (0)