Repository navigation
Conversation
…n cncf-kubernetes In urllib3 2.x, HTTPConnection.__init__ binds socket_options to default_socket_options as a default argument at import time, so reassigning the class attribute later never reaches new connections and [kubernetes_executor] enable_tcp_keepalive was silently a no-op. Pass the options through the Kubernetes client's Configuration.socket_options, which RESTClientObject forwards to urllib3.PoolManager. Closes: apache#68396 Supersedes: apache#68557
- Mark the google cncf.kubernetes floor with 'use next version'. - enable_tcp_keepalive: drop the urllib3 monkey-patch fallback; warn and return when Configuration has no socket_options. - Merge keepalive options into existing socket_options (or urllib3 defaults) without duplicates. - Deprecated _enable_tcp_keepalive() now sets the default Configuration. - Tidy docstrings and revert the spelling_wordlist additions. - Add tests for merging, idempotency, unsupported clients, get_kube_client, _TimeoutK8sApiClient, the deprecated helper and the hook; use autospec in hook tests.
shahar1
requested review from
hussein-awala,
jedcunningham and
jscheffl
as code owners
October 17, 2026 13:45
Configuration.socket_options, the only way to apply keepalive options with urllib3 2.x, does not exist in kubernetes 35.0.0, so on that floor keepalive could only ever be logged as unavailable and the lowest-dependency tests failed on it.
Cover custom tcp_keep_idle/intvl/cnt values, and exercise _TimeoutK8sApiClient with and without a supplied configuration and disable_verify_ssl, asserting the global default stays unchanged.
SameerMesiah97
approved these changes
Oct 17, 2026
SameerMesiah97
left a comment
Contributor
There was a problem hiding this comment.
Approved pending green CI.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes: #68396
replaces: #68557
This PR makes
[kubernetes_executor] enable_tcp_keepaliveeffective for all synchronous Kubernetes clients by settingConfiguration.socket_options, preserving existing options with urllib3 2.x.AI Summary
[kubernetes_executor] enable_tcp_keepalivedefaults toTrue, but it has had no effect since urllib3 2.x. The provider changedHTTPConnection.default_socket_optionsafter import, but urllib3 2.x binds that list as a default argument ofHTTPConnection.__init__when the module is imported, so new connections never see the change. When a cloud load balancer or firewall silently drops idle connections to the Kubernetes API server, they stay in the pool inCLOSE_WAIT(see #68396).This sets the options on the Kubernetes client's
Configuration.socket_optionsinstead. The Kubernetes client passes them to urllib3's pool manager (including the proxy path). Every sync client gets them:get_kube_client()(KubernetesExecutor and the pod watcher), every branch ofKubernetesHook.get_conn(), and the GKE hook.socket_optionsthe user already set, or into urllib3's defaults, soTCP_NODELAYis kept. Calling it twice adds no duplicates.enable_tcp_keepalive(configuration)is now public because the google provider imports it._enable_tcp_keepalive()stays as a deprecated wrapper. The google provider's dependency on cncf.kubernetes is marked# use next version.Configuration.socket_options. There a warning is logged and keepalive stays off, as it effectively already was.kubernetes_asyncio) clients are not changed.I checked this on a local kubeadm cluster by reading
getsockopton the pooled sockets after a real API call. Onmain, every path above hasSO_KEEPALIVE=0with kernel defaults (TCP_KEEPIDLE=7200). With this change,SO_KEEPALIVE=1and the configuredtcp_keep_idle/tcp_keep_intvl/tcp_keep_cntare used.disable_tcp_keepalive/enable_tcp_keepalive=Falsestill leave them off.This takes over #68557, which became inactive. The original commit and its authorship are kept, with review fixes in a follow-up commit.
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5.5) following the guidelines