From 4604af6aa5a6af116b2617f8cef36cdc1823c200 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Thu, 17 Dec 2015 10:56:50 -0500 Subject: [PATCH] Revert "Adds client and transport pooling in the API so we don't leak connections." --- api/api.go | 56 ++++++++++++------------------------------------- api/api_test.go | 19 +++++++---------- 2 files changed, 21 insertions(+), 54 deletions(-) diff --git a/api/api.go b/api/api.go index f34fafbf63..6736aecd2e 100644 --- a/api/api.go +++ b/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]) } - unixClientsLock.Unlock() + config.HttpClient = &http.Client{ + Transport: trans, + } + config.Address = parts[1] } client := &Client{ diff --git a/api/api_test.go b/api/api_test.go index 4f13d0ae2c..314a89b146 100644 --- a/api/api_test.go +++ b/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) } }