Skip to content

Commit

Permalink
k8s: Use kubelet's logic to close all idle connections
Browse files Browse the repository at this point in the history
Following up from #19259, this
commit implements the same logic as kubelet [1] to close all idle
connections. We can rely on HTTP/2 to determine which connections are
idle (HTTP/2 pings), instead of closing all connections indiscriminately
(when the heartbeat fails) via the custom dialer in setDialer().

We can assume HTTP/2 support as it has been supported since K8s 1.20
[2]. Users can disable HTTP/2 by setting the DISABLE_HTTP2 environment
variable, however this only really meant from very advanced users, hence
no documentation. Kubernetes has been running with HTTP/2 as the default
for a long time (~since Dec 2020) without major issues.

CloseIdleConnectionsFor() was introduced in K8s 1.23 [3], hence this commit
cannot be backported to any earlier version of Cilium which doesn't
support K8s 1.23.

[1]: kubernetes/kubernetes#104844
[2]: kubernetes/kubernetes#95981
[3]: kubernetes/kubernetes@b9d865a

Suggested-by: André Martins <andre@cilium.io>
Signed-off-by: Chris Tarazi <chris@isovalent.com>
  • Loading branch information
christarazi authored and pchaigno committed Apr 4, 2022
1 parent 55d7916 commit 365c788
Showing 1 changed file with 17 additions and 2 deletions.
19 changes: 17 additions & 2 deletions pkg/k8s/init.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ package k8s
import (
"context"
"fmt"
"os"
"time"

"github.com/sirupsen/logrus"
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
utilnet "k8s.io/apimachinery/pkg/util/net"
"k8s.io/client-go/rest"

"github.com/cilium/cilium/pkg/backoff"
Expand Down Expand Up @@ -139,7 +141,9 @@ func Init(conf k8sconfig.Configuration) error {
if err != nil {
return fmt.Errorf("unable to create k8s client rest configuration: %s", err)
}
closeAllDefaultClientConns := setDialer(restConfig)

defaultCloseAllConns := setDialer(restConfig)

// Use the same http client for all k8s connections. It does not matter that
// we are using a restConfig for the HTTP client that differs from each
// individual client since the rest.HTTPClientFor only does not use fields
Expand All @@ -164,6 +168,17 @@ func Init(conf k8sconfig.Configuration) error {
return fmt.Errorf("unable to create k8s apiextensions client: %s", err)
}

// We are implementing the same logic as Kubelet, see
// https://github.com/kubernetes/kubernetes/blob/v1.24.0-beta.0/cmd/kubelet/app/server.go#L852.
var closeAllConns func()
if s := os.Getenv("DISABLE_HTTP2"); len(s) > 0 {
closeAllConns = defaultCloseAllConns
} else {
closeAllConns = func() {
utilnet.CloseIdleConnectionsFor(restConfig.Transport)
}
}

heartBeat := func(ctx context.Context) error {
// Kubernetes does a get node of the node that kubelet is running [0]. This seems excessive in
// our case because the amount of data transferred is bigger than doing a Get of /healthz.
Expand All @@ -181,7 +196,7 @@ func Init(conf k8sconfig.Configuration) error {
runHeartbeat(
heartBeat,
option.Config.K8sHeartbeatTimeout,
closeAllDefaultClientConns,
closeAllConns,
)
return nil
},
Expand Down

0 comments on commit 365c788

Please sign in to comment.