Merge pull request #1519 from hashicorp/revert-1517-f-api-pooling

Revert "Adds client and transport pooling in the API so we don't leak connections."

api/api.go

@@ -13,7 +13,6 @@ import (
 	"os"
 	"strconv"
 	"strings"
-	"sync"
 	"time"
 
 	"github.com/hashicorp/go-cleanhttp"
@@ -123,28 +122,12 @@ type Config struct {
 	Token string
 }
 
-// defaultHttpClient is a shared client instance that is used to prevent apps
-// that create multiple clients from opening multiple connections, which would
-// leak file descriptors.
-var defaultHttpClient = cleanhttp.DefaultClient()
-
-// defaultInsecureTransport is a shared transport that will get injected into
-// the defaultHttpClient if the CONSUL_HTTP_SSL_VERIFY environment variable is
-// set to true.
-var defaultInsecureTransport = func() *http.Transport {
-	trans := cleanhttp.DefaultTransport()
-	trans.TLSClientConfig = &tls.Config{
-		InsecureSkipVerify: true,
-	}
-	return trans
-}()
-
 // DefaultConfig returns a default configuration for the client
 func DefaultConfig() *Config {
 	config := &Config{
 		Address:    "127.0.0.1:8500",
 		Scheme:     "http",
-		HttpClient: defaultHttpClient,
+		HttpClient: cleanhttp.DefaultClient(),
 	}
 
 	if addr := os.Getenv("CONSUL_HTTP_ADDR"); addr != "" {
@@ -189,7 +172,11 @@ func DefaultConfig() *Config {
 		}
 
 		if !doVerify {
-			config.HttpClient.Transport = defaultInsecureTransport
+			config.HttpClient.Transport = &http.Transport{
+				TLSClientConfig: &tls.Config{
+					InsecureSkipVerify: true,
+				},
+			}
 		}
 	}
 
@@ -201,15 +188,6 @@ type Client struct {
 	config Config
 }
 
-// unixClients contains a set of shared UNIX socket clients, indexed by address.
-// These shared instances are used to prevent apps that create multiple clients
-// from opening multiple connections, which would leak file descriptors.
-var unixClients = make(map[string]*http.Client)
-
-// unixClientsLock serializes access to the unixClients map, since most users
-// would expect NewClient to be thread-safe.
-var unixClientsLock sync.Mutex
-
 // NewClient returns a new client
 func NewClient(config *Config) (*Client, error) {
 	// bootstrap the config
@@ -228,22 +206,14 @@ func NewClient(config *Config) (*Client, error) {
 	}
 
 	if parts := strings.SplitN(config.Address, "unix://", 2); len(parts) == 2 {
-		config.Address = parts[1]
-
-		unixClientsLock.Lock()
-		if client, ok := unixClients[config.Address]; ok {
-			config.HttpClient = client
-		} else {
-			trans := cleanhttp.DefaultTransport()
-			trans.Dial = func(_, _ string) (net.Conn, error) {
-				return net.Dial("unix", config.Address)
-			}
-			config.HttpClient = &http.Client{
-				Transport: trans,
-			}
-			unixClients[config.Address] = config.HttpClient
+		trans := cleanhttp.DefaultTransport()
+		trans.Dial = func(_, _ string) (net.Conn, error) {
+			return net.Dial("unix", parts[1])
+		}
+		config.HttpClient = &http.Client{
+			Transport: trans,
 		}
-		unixClientsLock.Unlock()
+		config.Address = parts[1]
 	}
 
 	client := &Client{

api/api_test.go

@@ -244,16 +244,13 @@ func TestAPI_UnixSocket(t *testing.T) {
 	})
 	defer s.Stop()
 
-	// Run a few iterations to test the path where we use the pooled
-	// connection.
-	for i := 0; i < 3; i++ {
-		agent := c.Agent()
-		info, err := agent.Self()
-		if err != nil {
-			t.Fatalf("err: %s", err)
-		}
-		if info["Config"]["NodeName"] == "" {
-			t.Fatalf("bad: %v", info)
-		}
+	agent := c.Agent()
+
+	info, err := agent.Self()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	if info["Config"]["NodeName"] == "" {
+		t.Fatalf("bad: %v", info)
 	}
 }