update OverrideListenerResourceName document and simplify the test

Author: ran-su

internal/xds/server/server_options.go

@@ -55,7 +55,13 @@ func ApplyServerOptions(opts []grpc.ServerOption, so *Options) {
 }
 
 // OverrideListenerResourceName returns a server option that overrides the LDS
-// resource name selected for an xDS server listener.
+// resource name selected for an xDS server listener. The supplied function is
+// called by Serve with the address returned by the listener's Addr method, and
+// its return value is used as-is as the LDS listener resource name.
+//
+// The function is called once for each Serve invocation that gets past listener
+// validation and the server-stopped check. If Serve is called concurrently, the
+// function may be called concurrently and must be safe for concurrent use.
 func OverrideListenerResourceName(f func(net.Addr) string) grpc.ServerOption {
 	return NewServerOption(func(o *Options) {
 		o.OverrideListenerResourceName = f

xds/server_test.go

@@ -238,8 +238,7 @@ func (s) TestServer_OverrideListenerResourceNameOverridesMissingTemplate(t *test
 	ldsResourceNameReceived := grpcsync.NewEvent()
 	mgmtServer := e2e.StartManagementServer(t, e2e.ManagementServerOptions{
 		OnStreamRequest: func(_ int64, req *v3discoverypb.DiscoveryRequest) error {
-			if req.GetTypeUrl() == version.V3ListenerURL &&
-				cmp.Equal(req.GetResourceNames(), []string{wantResourceName}) {
+			if req.GetTypeUrl() == version.V3ListenerURL && cmp.Equal(req.GetResourceNames(), []string{wantResourceName}) {
 				ldsResourceNameReceived.Fire()
 			}
 			return nil
@@ -262,36 +261,28 @@ func (s) TestServer_OverrideListenerResourceNameOverridesMissingTemplate(t *test
 	newGRPCServer = func(...grpc.ServerOption) grpcServer { return fs }
 	defer func() { newGRPCServer = origNewGRPCServer }()
 
-	addrCh := make(chan net.Addr, 1)
-	srv, err := NewGRPCServer(
-		internalserver.OverrideListenerResourceName(func(addr net.Addr) string {
-			addrCh <- addr
-			return wantResourceName
-		}),
-		BootstrapContentsForTesting(bs),
-	)
+	lisAddrCh := make(chan net.Addr, 1)
+	resourceNameOpt := internalserver.OverrideListenerResourceName(func(addr net.Addr) string {
+		lisAddrCh <- addr
+		return wantResourceName
+	})
+	srv, err := NewGRPCServer(resourceNameOpt, BootstrapContentsForTesting(bs))
 	if err != nil {
 		t.Fatalf("NewGRPCServer() failed: %v", err)
 	}
-	stopped := false
-	defer func() {
-		if !stopped {
-			srv.Stop()
-		}
-	}()
+	defer srv.Stop()
 
 	lis, err := testutils.LocalTCPListener()
 	if err != nil {
 		t.Fatalf("testutils.LocalTCPListener() failed: %v", err)
 	}
-	serveErrCh := make(chan error, 1)
-	go func() { serveErrCh <- srv.Serve(lis) }()
+	go func() { _ = srv.Serve(lis) }()
 
 	ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout)
 	defer cancel()
 	// Verify that OverrideListenerResourceName receives the listener address.
 	select {
-	case gotAddr := <-addrCh:
+	case gotAddr := <-lisAddrCh:
 		if gotAddr.String() != lis.Addr().String() {
 			t.Fatalf("OverrideListenerResourceName() called with address %q, want %q", gotAddr, lis.Addr())
 		}
@@ -305,18 +296,6 @@ func (s) TestServer_OverrideListenerResourceNameOverridesMissingTemplate(t *test
 	case <-ctx.Done():
 		t.Fatal("Timeout waiting for an LDS request with the overridden resource name")
 	}
-
-	srv.Stop()
-	stopped = true
-	// Verify that Serve returns after the server is stopped.
-	select {
-	case err := <-serveErrCh:
-		if err != nil {
-			t.Fatalf("Serve() returned error: %v", err)
-		}
-	case <-ctx.Done():
-		t.Fatal("Timeout waiting for Serve() to return")
-	}
 }
 
 func (s) TestRegisterService(t *testing.T) {