Merge pull request #1825 from hashicorp/f-pool-sense

Switches default for API client to pooled connections.

api/api.go

@@ -122,12 +122,34 @@ type Config struct {
 	Token string
 }
 
-// DefaultConfig returns a default configuration for the client
+// DefaultConfig returns a default configuration for the client. By default this
+// will pool and reuse idle connections to Consul. If you have a long-lived
+// client object, this is the desired behavior and should make the most efficient
+// use of the connections to Consul. If you don't reuse a client object , which
+// is not recommended, then you may notice idle connections building up over
+// time. To avoid this, use the DefaultNonPooledConfig() instead.
 func DefaultConfig() *Config {
+	return defaultConfig(cleanhttp.DefaultPooledTransport)
+}
+
+// DefaultNonPooledConfig returns a default configuration for the client which
+// does not pool connections. This isn't a recommended configuration because it
+// will reconnect to Consul on every request, but this is useful to avoid the
+// accumulation of idle connections if you make many client objects during the
+// lifetime of your application.
+func DefaultNonPooledConfig() *Config {
+	return defaultConfig(cleanhttp.DefaultTransport)
+}
+
+// defaultConfig returns the default configuration for the client, using the
+// given function to make the transport.
+func defaultConfig(transportFn func() *http.Transport) *Config {
 	config := &Config{
-		Address:    "127.0.0.1:8500",
-		Scheme:     "http",
-		HttpClient: cleanhttp.DefaultClient(),
+		Address: "127.0.0.1:8500",
+		Scheme:  "http",
+		HttpClient: &http.Client{
+			Transport: transportFn(),
+		},
 	}
 
 	if addr := os.Getenv("CONSUL_HTTP_ADDR"); addr != "" {
@@ -172,7 +194,7 @@ func DefaultConfig() *Config {
 		}
 
 		if !doVerify {
-			transport := cleanhttp.DefaultTransport()
+			transport := transportFn()
 			transport.TLSClientConfig = &tls.Config{
 				InsecureSkipVerify: true,
 			}

api/api_test.go

@@ -85,32 +85,39 @@ func TestDefaultConfig_env(t *testing.T) {
 	os.Setenv("CONSUL_HTTP_SSL_VERIFY", "0")
 	defer os.Setenv("CONSUL_HTTP_SSL_VERIFY", "")
 
-	config := DefaultConfig()
+	for i, config := range []*Config{DefaultConfig(), DefaultNonPooledConfig()} {
+		if config.Address != addr {
+			t.Errorf("expected %q to be %q", config.Address, addr)
+		}
+		if config.Token != token {
+			t.Errorf("expected %q to be %q", config.Token, token)
+		}
+		if config.HttpAuth == nil {
+			t.Fatalf("expected HttpAuth to be enabled")
+		}
+		if config.HttpAuth.Username != "username" {
+			t.Errorf("expected %q to be %q", config.HttpAuth.Username, "username")
+		}
+		if config.HttpAuth.Password != "password" {
+			t.Errorf("expected %q to be %q", config.HttpAuth.Password, "password")
+		}
+		if config.Scheme != "https" {
+			t.Errorf("expected %q to be %q", config.Scheme, "https")
+		}
+		if !config.HttpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify {
+			t.Errorf("expected SSL verification to be off")
+		}
 
-	if config.Address != addr {
-		t.Errorf("expected %q to be %q", config.Address, addr)
-	}
-
-	if config.Token != token {
-		t.Errorf("expected %q to be %q", config.Token, token)
-	}
-
-	if config.HttpAuth == nil {
-		t.Fatalf("expected HttpAuth to be enabled")
-	}
-	if config.HttpAuth.Username != "username" {
-		t.Errorf("expected %q to be %q", config.HttpAuth.Username, "username")
-	}
-	if config.HttpAuth.Password != "password" {
-		t.Errorf("expected %q to be %q", config.HttpAuth.Password, "password")
-	}
-
-	if config.Scheme != "https" {
-		t.Errorf("expected %q to be %q", config.Scheme, "https")
-	}
-
-	if !config.HttpClient.Transport.(*http.Transport).TLSClientConfig.InsecureSkipVerify {
-		t.Errorf("expected SSL verification to be off")
+		// Use keep alives as a check for whether pooling is on or off.
+		if pooled := i == 0; pooled {
+			if config.HttpClient.Transport.(*http.Transport).DisableKeepAlives != false {
+				t.Errorf("expected keep alives to be enabled")
+			}
+		} else {
+			if config.HttpClient.Transport.(*http.Transport).DisableKeepAlives != true {
+				t.Errorf("expected keep alives to be disabled")
+			}
+		}
 	}
 }