From 28213a00a463ca68bd32cf972ebe74280f427885 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Wed, 12 Oct 2016 17:20:47 +0100 Subject: [PATCH 1/2] Add timeouts to the probe http client --- probe/appclient/app_client.go | 3 ++- probe/appclient/probe_config.go | 26 +++++++++++++++++--------- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/probe/appclient/app_client.go b/probe/appclient/app_client.go index a99a42fde..67672bb97 100644 --- a/probe/appclient/app_client.go +++ b/probe/appclient/app_client.go @@ -82,7 +82,8 @@ func NewAppClient(pc ProbeConfig, hostname string, target url.URL, control xfer. Timeout: httpClientTimeout, }, wsDialer: websocket.Dialer{ - TLSClientConfig: httpTransport.TLSClientConfig, + TLSClientConfig: httpTransport.TLSClientConfig, + HandshakeTimeout: httpClientTimeout, }, conns: map[string]xfer.Websocket{}, readers: make(chan io.Reader, 2), diff --git a/probe/appclient/probe_config.go b/probe/appclient/probe_config.go index 04d6533c2..9c2b1d9ac 100644 --- a/probe/appclient/probe_config.go +++ b/probe/appclient/probe_config.go @@ -44,16 +44,24 @@ func (pc ProbeConfig) authorizedRequest(method string, urlStr string, body io.Re } func (pc ProbeConfig) getHTTPTransport(hostname string) (*http.Transport, error) { + var tlsConfig *tls.Config if pc.Insecure { - return &http.Transport{ - TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, - }, nil - } - - return &http.Transport{ - TLSClientConfig: &tls.Config{ + tlsConfig = &tls.Config{InsecureSkipVerify: true} + } else { + tlsConfig = &tls.Config{ RootCAs: certPool, ServerName: hostname, - }, - }, nil + } + } + return &http.Transport{ + TLSClientConfig: tlsConfig, + + DialContext: (&net.Dialer{ + Timeout: 30 * time.Second, + KeepAlive: 30 * time.Second, + }).DialContext, + IdleConnTimeout: http.DefaultTransport.IdleConnTimeout, + TLSHandshakeTimeout: http.DefaultTransport.TLSHandshakeTimeout, + ExpectContinueTimeout: http.DefaultTransport.ExpectContinueTimeout, + } } From 002770d3949b52bcc0c0dba444e74090b97b53e8 Mon Sep 17 00:00:00 2001 From: Paul Bellamy Date: Thu, 13 Oct 2016 08:31:40 +0100 Subject: [PATCH 2/2] use hashicorp/clean-http to get better http client defaults --- probe/appclient/app_client.go | 22 +++++++--------------- probe/appclient/probe_config.go | 32 +++++++++++++++++--------------- 2 files changed, 24 insertions(+), 30 deletions(-) diff --git a/probe/appclient/app_client.go b/probe/appclient/app_client.go index 67672bb97..9f8e75614 100644 --- a/probe/appclient/app_client.go +++ b/probe/appclient/app_client.go @@ -4,7 +4,6 @@ import ( "fmt" "io" "io/ioutil" - "net" "net/http" "net/rpc" "net/url" @@ -13,6 +12,7 @@ import ( log "github.com/Sirupsen/logrus" "github.com/gorilla/websocket" + "github.com/hashicorp/go-cleanhttp" "github.com/ugorji/go/codec" "github.com/weaveworks/scope/common/xfer" @@ -22,7 +22,6 @@ const ( httpClientTimeout = 4 * time.Second initialBackoff = 1 * time.Second maxBackoff = 60 * time.Second - dialTimeout = 5 * time.Second ) // AppClient is a client to an app for dealing with controls. @@ -41,7 +40,7 @@ type appClient struct { quit chan struct{} mtx sync.Mutex - client http.Client + client *http.Client wsDialer websocket.Dialer appID string hostname string @@ -63,24 +62,17 @@ type appClient struct { // NewAppClient makes a new appClient. func NewAppClient(pc ProbeConfig, hostname string, target url.URL, control xfer.ControlHandler) (AppClient, error) { - httpTransport, err := pc.getHTTPTransport(hostname) - if err != nil { - return nil, err - } - - httpTransport.Dial = func(network, addr string) (net.Conn, error) { - return net.DialTimeout(network, addr, dialTimeout) - } + httpTransport := pc.getHTTPTransport(hostname) + httpClient := cleanhttp.DefaultClient() + httpClient.Transport = httpTransport + httpClient.Timeout = httpClientTimeout return &appClient{ ProbeConfig: pc, quit: make(chan struct{}), hostname: hostname, target: target, - client: http.Client{ - Transport: httpTransport, - Timeout: httpClientTimeout, - }, + client: httpClient, wsDialer: websocket.Dialer{ TLSClientConfig: httpTransport.TLSClientConfig, HandshakeTimeout: httpClientTimeout, diff --git a/probe/appclient/probe_config.go b/probe/appclient/probe_config.go index 9c2b1d9ac..be491b463 100644 --- a/probe/appclient/probe_config.go +++ b/probe/appclient/probe_config.go @@ -5,12 +5,20 @@ import ( "crypto/x509" "fmt" "io" + "net" "net/http" + "time" "github.com/certifi/gocertifi" + "github.com/hashicorp/go-cleanhttp" + "github.com/weaveworks/scope/common/xfer" ) +const ( + dialTimeout = 5 * time.Second +) + var certPool *x509.CertPool func init() { @@ -43,25 +51,19 @@ func (pc ProbeConfig) authorizedRequest(method string, urlStr string, body io.Re return req, err } -func (pc ProbeConfig) getHTTPTransport(hostname string) (*http.Transport, error) { - var tlsConfig *tls.Config +func (pc ProbeConfig) getHTTPTransport(hostname string) *http.Transport { + transport := cleanhttp.DefaultTransport() + transport.DialContext = (&net.Dialer{ + Timeout: dialTimeout, + KeepAlive: 30 * time.Second, + }).DialContext if pc.Insecure { - tlsConfig = &tls.Config{InsecureSkipVerify: true} + transport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} } else { - tlsConfig = &tls.Config{ + transport.TLSClientConfig = &tls.Config{ RootCAs: certPool, ServerName: hostname, } } - return &http.Transport{ - TLSClientConfig: tlsConfig, - - DialContext: (&net.Dialer{ - Timeout: 30 * time.Second, - KeepAlive: 30 * time.Second, - }).DialContext, - IdleConnTimeout: http.DefaultTransport.IdleConnTimeout, - TLSHandshakeTimeout: http.DefaultTransport.TLSHandshakeTimeout, - ExpectContinueTimeout: http.DefaultTransport.ExpectContinueTimeout, - } + return transport }