From e5cdafc07504d992a47edbab016bd95168ecc151 Mon Sep 17 00:00:00 2001 From: Lucas Manuel Rodriguez Date: Thu, 9 May 2024 08:54:11 -0300 Subject: [PATCH] Add cleanup of idle connections in fleetd (#18857) #18783 - [X] Changes file added for user-visible changes in `changes/`, `orbit/changes/` or `ee/fleetd-chrome/changes`. See [Changes files](https://fleetdm.com/docs/contributing/committing-changes#changes-files) for more information. - [x] Manual QA for all new/changed functionality - For Orbit and Fleet Desktop changes: - [x] Manual QA must be performed in the three main OSs, macOS, Windows and Linux. - [x] Auto-update manual QA, from released version of component to new version (see [tools/tuf/test](../tools/tuf/test/README.md)). --- .../18783-fleetd-cleanup-idle-conn-every-10m | 1 + server/service/orbit_client.go | 82 +++++++++++++++++-- tools/tuf/test/gen_pkgs.sh | 21 +++-- 3 files changed, 87 insertions(+), 17 deletions(-) create mode 100644 orbit/changes/18783-fleetd-cleanup-idle-conn-every-10m diff --git a/orbit/changes/18783-fleetd-cleanup-idle-conn-every-10m b/orbit/changes/18783-fleetd-cleanup-idle-conn-every-10m new file mode 100644 index 0000000000..1eeb93b96e --- /dev/null +++ b/orbit/changes/18783-fleetd-cleanup-idle-conn-every-10m @@ -0,0 +1 @@ +* Close idle connections every 10 minutes to prevent load balancers (like AWS ELB) from forcefully terminating long lived connections. diff --git a/server/service/orbit_client.go b/server/service/orbit_client.go index 0203319b10..48d03d0cd9 100644 --- a/server/service/orbit_client.go +++ b/server/service/orbit_client.go @@ -2,6 +2,7 @@ package service import ( "bytes" + "context" "crypto/tls" "encoding/json" "errors" @@ -9,6 +10,7 @@ import ( "io/fs" "net" "net/http" + "net/http/httptrace" "os" "path/filepath" "runtime" @@ -40,6 +42,9 @@ type OrbitClient struct { onGetConfigErrFns *OnGetConfigErrFuncs lastNetErrOnGetConfigLogged time.Time + lastIdleConnectionsCleanupMu sync.Mutex + lastIdleConnectionsCleanup time.Time + // TestNodeKey is used for testing only. TestNodeKey string } @@ -64,7 +69,15 @@ func (oc *OrbitClient) request(verb string, path string, params interface{}, res } } - request, err := http.NewRequest( + oc.closeIdleConnections() + + ctx := context.Background() + if os.Getenv("FLEETD_TEST_HTTPTRACE") == "1" { + ctx = httptrace.WithClientTrace(ctx, testStdoutHTTPTracer) + } + + request, err := http.NewRequestWithContext( + ctx, verb, oc.url(path, "").String(), bytes.NewBuffer(bodyBytes), @@ -124,14 +137,48 @@ func NewOrbitClient( } nodeKeyFilePath := filepath.Join(rootDir, constant.OrbitNodeKeyFileName) - return &OrbitClient{ - nodeKeyFilePath: nodeKeyFilePath, - baseClient: bc, - enrollSecret: enrollSecret, - hostInfo: orbitHostInfo, - enrolled: false, - onGetConfigErrFns: onGetConfigErrFns, - }, nil + oc := &OrbitClient{ + nodeKeyFilePath: nodeKeyFilePath, + baseClient: bc, + enrollSecret: enrollSecret, + hostInfo: orbitHostInfo, + enrolled: false, + onGetConfigErrFns: onGetConfigErrFns, + lastIdleConnectionsCleanup: time.Now(), + } + + return oc, nil +} + +// closeIdleConnections attempts to close idle connections from the pool +// every 55 minutes. +// +// Some load balancers (e.g. AWS ELB) have a maximum lifetime for a connection +// (no matter if the connection is active or not) and will forcefully close the +// connection causing errors in the client (e.g. https://github.com/fleetdm/fleet/issues/18783). +// To prevent these errors, we will attempt to cleanup idle connections every 55 +// minutes to not let these connection grow too old. (AWS ELB's default value for maximum +// lifetime of a connection is 3600 seconds.) +func (oc *OrbitClient) closeIdleConnections() { + oc.lastIdleConnectionsCleanupMu.Lock() + defer oc.lastIdleConnectionsCleanupMu.Unlock() + + if time.Since(oc.lastIdleConnectionsCleanup) < 55*time.Minute { + return + } + + oc.lastIdleConnectionsCleanup = time.Now() + + c, ok := oc.baseClient.http.(*http.Client) + if !ok { + return + } + t, ok := c.Transport.(*http.Transport) + if !ok { + return + } + + t.CloseIdleConnections() } // GetConfig returns the Orbit config fetched from Fleet server for this instance of OrbitClient. @@ -431,3 +478,20 @@ func (oc *OrbitClient) SetOrUpdateDiskEncryptionKey(diskEncryptionStatus fleet.O } return nil } + +const httpTraceTimeFormat = "2006-01-02T15:04:05Z" + +var testStdoutHTTPTracer = &httptrace.ClientTrace{ + ConnectStart: func(network, addr string) { + fmt.Printf( + "httptrace: %s: ConnectStart: %s, %s\n", + time.Now().UTC().Format(httpTraceTimeFormat), network, addr, + ) + }, + ConnectDone: func(network, addr string, err error) { + fmt.Printf( + "httptrace: %s: ConnectDone: %s, %s, err='%s'\n", + time.Now().UTC().Format(httpTraceTimeFormat), network, addr, err, + ) + }, +} diff --git a/tools/tuf/test/gen_pkgs.sh b/tools/tuf/test/gen_pkgs.sh index fc23f43c18..517357a1df 100755 --- a/tools/tuf/test/gen_pkgs.sh +++ b/tools/tuf/test/gen_pkgs.sh @@ -31,6 +31,11 @@ set -ex # FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST: Alternative host:port to use for the Fleet Desktop browser URLs. # DEBUG: Whether or not to build the package with --debug. +ENABLE_SCRIPTS="1" +if [[ -n $DISABLE_SCRIPTS ]]; then + ENABLE_SCRIPTS="" +fi + if [ -n "$GENERATE_PKG" ]; then echo "Generating pkg..." ./build/fleetctl package \ @@ -51,8 +56,8 @@ if [ -n "$GENERATE_PKG" ]; then ${USE_UPDATE_CLIENT_CERTIFICATE:+--update-tls-client-key=./tools/test-orbit-mtls/client.key} \ ${FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST:+--fleet-desktop-alternative-browser-host=$FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST} \ --update-url=$PKG_TUF_URL \ - --disable-keystore \ - --enable-scripts + ${ENABLE_SCRIPTS:+--enable-scripts} \ + --disable-keystore fi if [ -n "$GENERATE_DEB" ]; then @@ -74,8 +79,8 @@ if [ -n "$GENERATE_DEB" ]; then ${USE_UPDATE_CLIENT_CERTIFICATE:+--update-tls-client-certificate=./tools/test-orbit-mtls/client.crt} \ ${USE_UPDATE_CLIENT_CERTIFICATE:+--update-tls-client-key=./tools/test-orbit-mtls/client.key} \ ${FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST:+--fleet-desktop-alternative-browser-host=$FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST} \ - --update-url=$DEB_TUF_URL \ - --enable-scripts + ${ENABLE_SCRIPTS:+--enable-scripts} \ + --update-url=$DEB_TUF_URL fi if [ -n "$GENERATE_RPM" ]; then @@ -97,8 +102,8 @@ if [ -n "$GENERATE_RPM" ]; then ${USE_UPDATE_CLIENT_CERTIFICATE:+--update-tls-client-certificate=./tools/test-orbit-mtls/client.crt} \ ${USE_UPDATE_CLIENT_CERTIFICATE:+--update-tls-client-key=./tools/test-orbit-mtls/client.key} \ ${FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST:+--fleet-desktop-alternative-browser-host=$FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST} \ - --update-url=$RPM_TUF_URL \ - --enable-scripts + ${ENABLE_SCRIPTS:+--enable-scripts} \ + --update-url=$RPM_TUF_URL fi if [ -n "$GENERATE_MSI" ]; then @@ -120,8 +125,8 @@ if [ -n "$GENERATE_MSI" ]; then ${USE_UPDATE_CLIENT_CERTIFICATE:+--update-tls-client-certificate=./tools/test-orbit-mtls/client.crt} \ ${USE_UPDATE_CLIENT_CERTIFICATE:+--update-tls-client-key=./tools/test-orbit-mtls/client.key} \ ${FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST:+--fleet-desktop-alternative-browser-host=$FLEET_DESKTOP_ALTERNATIVE_BROWSER_HOST} \ - --update-url=$MSI_TUF_URL \ - --enable-scripts + ${ENABLE_SCRIPTS:+--enable-scripts} \ + --update-url=$MSI_TUF_URL fi echo "Packages generated."