agent/grpc: always close the conn when dialing fails.

agent/grpc/client.go

@@ -85,12 +85,13 @@ func newDialer(servers ServerLocator, wrapper TLSWrapper) func(context.Context,
 
 		server, err := servers.ServerForAddr(addr)
 		if err != nil {
-			// TODO: should conn be closed in this case, as it is in other error cases?
+			conn.Close()
 			return nil, err
 		}
 
 		if server.UseTLS {
 			if wrapper == nil {
+				conn.Close()
 				return nil, fmt.Errorf("TLS enabled but got nil TLS wrapper")
 			}
 
@@ -111,7 +112,7 @@ func newDialer(servers ServerLocator, wrapper TLSWrapper) func(context.Context,
 
 		_, err = conn.Write([]byte{pool.RPCGRPC})
 		if err != nil {
-			// TODO: should conn be closed in this case, as it is in other error cases?
+			conn.Close()
 			return nil, err
 		}
 

agent/grpc/client_test.go

@@ -3,6 +3,7 @@ package grpc
 import (
 	"context"
 	"fmt"
+	"net"
 	"strings"
 	"testing"
 	"time"
@@ -14,11 +15,36 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestNewDialer(t *testing.T) {
-	// TODO: conn is closed on errors
-	// TODO: with TLS enabled
+func TestNewDialer_WithTLSWrapper(t *testing.T) {
+	lis, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	t.Cleanup(logError(t, lis.Close))
+
+	builder := resolver.NewServerResolverBuilder(resolver.Config{})
+	builder.AddServer(&metadata.Server{
+		Name:       "server-1",
+		ID:         "ID1",
+		Datacenter: "dc1",
+		Addr:       lis.Addr(),
+		UseTLS:     true,
+	})
+
+	var called bool
+	wrapper := func(_ string, conn net.Conn) (net.Conn, error) {
+		called = true
+		return conn, nil
+	}
+	dial := newDialer(builder, wrapper)
+	ctx := context.Background()
+	conn, err := dial(ctx, lis.Addr().String())
+	require.NoError(t, err)
+	require.NoError(t, conn.Close())
+	require.True(t, called, "expected TLSWrapper to be called")
 }
 
+// TODO: integration test TestNewDialer with TLS and rcp server, when the rpc
+// exists as an isolated component.
+
 func TestClientConnPool_IntegrationWithGRPCResolver_Failover(t *testing.T) {
 	count := 4
 	cfg := resolver.Config{Scheme: newScheme(t.Name())}