From f364803cb460b8083c535fe20ec3035cbdd35fcf Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Wed, 28 Oct 2020 23:31:15 -0700 Subject: [PATCH 1/2] configure the ReadIdleTimeout and PingTimeout of the h2 transport Kubernetes-commit: 8ee10ce6aa57e85268c276406b038eabfbcf14ec --- pkg/util/net/http.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/util/net/http.go b/pkg/util/net/http.go index 945886c43..28553d64c 100644 --- a/pkg/util/net/http.go +++ b/pkg/util/net/http.go @@ -33,6 +33,7 @@ import ( "regexp" "strconv" "strings" + "time" "unicode" "unicode/utf8" @@ -132,13 +133,31 @@ func SetTransportDefaults(t *http.Transport) *http.Transport { if s := os.Getenv("DISABLE_HTTP2"); len(s) > 0 { klog.Infof("HTTP2 has been explicitly disabled") } else if allowsHTTP2(t) { - if err := http2.ConfigureTransport(t); err != nil { + if err := configureHTTP2Transport(t); err != nil { klog.Warningf("Transport failed http2 configuration: %v", err) } } return t } +func configureHTTP2Transport(t *http.Transport) error { + t2, err := http2.ConfigureTransports(t) + if err != nil { + return err + } + // The following enables the HTTP/2 connection health check added in + // https://github.com/golang/net/pull/55. The health check detects and + // closes broken transport layer connections. Without the health check, + // a broken connection can linger too long, e.g., a broken TCP + // connection will be closed by the Linux kernel after 13 to 30 minutes + // by default, which caused + // https://github.com/kubernetes/client-go/issues/374 and + // https://github.com/kubernetes/kubernetes/issues/87615. + t2.ReadIdleTimeout = 30 * time.Second + t2.PingTimeout = 15 * time.Second + return nil +} + func allowsHTTP2(t *http.Transport) bool { if t.TLSClientConfig == nil || len(t.TLSClientConfig.NextProtos) == 0 { // the transport expressed no NextProto preference, allow From 5ff866257092272718fb97fec1f1b37a3f5375ee Mon Sep 17 00:00:00 2001 From: Chao Xu Date: Fri, 30 Oct 2020 00:56:12 -0700 Subject: [PATCH 2/2] allow configuring ReadIdelTimeout and PingTimeout via env var Kubernetes-commit: 15648f1a7b51584df96dd5d6a422545b7f0f146b --- pkg/util/net/http.go | 34 ++++++++++++++++++++++++++++++++-- pkg/util/net/http_test.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 2 deletions(-) diff --git a/pkg/util/net/http.go b/pkg/util/net/http.go index 28553d64c..ba63d02df 100644 --- a/pkg/util/net/http.go +++ b/pkg/util/net/http.go @@ -140,6 +140,36 @@ func SetTransportDefaults(t *http.Transport) *http.Transport { return t } +func readIdleTimeoutSeconds() int { + ret := 30 + // User can set the readIdleTimeout to 0 to disable the HTTP/2 + // connection health check. + if s := os.Getenv("HTTP2_READ_IDLE_TIMEOUT_SECONDS"); len(s) > 0 { + i, err := strconv.Atoi(s) + if err != nil { + klog.Warningf("Illegal HTTP2_READ_IDLE_TIMEOUT_SECONDS(%q): %v."+ + " Default value %d is used", s, err, ret) + return ret + } + ret = i + } + return ret +} + +func pingTimeoutSeconds() int { + ret := 15 + if s := os.Getenv("HTTP2_PING_TIMEOUT_SECONDS"); len(s) > 0 { + i, err := strconv.Atoi(s) + if err != nil { + klog.Warningf("Illegal HTTP2_PING_TIMEOUT_SECONDS(%q): %v."+ + " Default value %d is used", s, err, ret) + return ret + } + ret = i + } + return ret +} + func configureHTTP2Transport(t *http.Transport) error { t2, err := http2.ConfigureTransports(t) if err != nil { @@ -153,8 +183,8 @@ func configureHTTP2Transport(t *http.Transport) error { // by default, which caused // https://github.com/kubernetes/client-go/issues/374 and // https://github.com/kubernetes/kubernetes/issues/87615. - t2.ReadIdleTimeout = 30 * time.Second - t2.PingTimeout = 15 * time.Second + t2.ReadIdleTimeout = time.Duration(readIdleTimeoutSeconds()) * time.Second + t2.PingTimeout = time.Duration(pingTimeoutSeconds()) * time.Second return nil } diff --git a/pkg/util/net/http_test.go b/pkg/util/net/http_test.go index a13cd96a4..a43161b88 100644 --- a/pkg/util/net/http_test.go +++ b/pkg/util/net/http_test.go @@ -1071,3 +1071,39 @@ func TestIsProbableEOF(t *testing.T) { }) } } + +func setEnv(key, value string) func() { + originalValue := os.Getenv(key) + os.Setenv(key, value) + return func() { + os.Setenv(key, originalValue) + } +} + +func TestReadIdleTimeoutSeconds(t *testing.T) { + reset := setEnv("HTTP2_READ_IDLE_TIMEOUT_SECONDS", "60") + if e, a := 60, readIdleTimeoutSeconds(); e != a { + t.Errorf("expected %d, got %d", e, a) + } + reset() + + reset = setEnv("HTTP2_READ_IDLE_TIMEOUT_SECONDS", "illegal value") + if e, a := 30, readIdleTimeoutSeconds(); e != a { + t.Errorf("expected %d, got %d", e, a) + } + reset() +} + +func TestPingTimeoutSeconds(t *testing.T) { + reset := setEnv("HTTP2_PING_TIMEOUT_SECONDS", "60") + if e, a := 60, pingTimeoutSeconds(); e != a { + t.Errorf("expected %d, got %d", e, a) + } + reset() + + reset = setEnv("HTTP2_PING_TIMEOUT_SECONDS", "illegal value") + if e, a := 15, pingTimeoutSeconds(); e != a { + t.Errorf("expected %d, got %d", e, a) + } + reset() +}