cli: default to SPDY connection for exec #38988
Merged
Conversation
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
As of 2025-04-16 the upstream websocket executor implementation has a variety of issues due to hardcoded timeouts. See kubernetes/kubernetes@1d4be75 See https://github.com/kubernetes/kubernetes/commits/master/staging/src/k8s.io/client-go/tools/remotecommand We continue to use the fallback mechanism here to allow Websocket connections in cases where SPDY is not available. SPDY is not available when there is an HTTP-only proxy in front of the Kubernetes API server. Signed-off-by: Andrew Sauber <andrew.sauber@isovalent.com>
|
/test |
|
@asauber Do we know how long we have on the SPDY implementation before it's removed ? |
tommyp1ckles
approved these changes
Apr 16, 2025
The use of websockets by default for all cases was released with Kubernetes v1.31 (PortForward, "streaming", in addition to RemoteCommand). The implementation history can be found in the KEP. So, this indicates to me that the Kubernetes API Server will continue to support SPDY for a while, but the exact timeline isn't clear. Seems like a good question for SIG Network. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The upstream websocket executor implementation has a variety of issues due to hardcoded timeouts, and continues to cause flakes in our test suites which use a remote executor.
See kubernetes/kubernetes@1d4be75 (upstream timeout bumped from 2 seconds to 30 seconds)
See kubernetes/kubernetes@b04d117 (upstream timeout bumped from 30 seconds to 60 seconds)
See also https://github.com/kubernetes/kubernetes/commits/master/staging/src/k8s.io/client-go/tools/remotecommand
We continue to use the fallback mechanism here to allow Websocket connections in cases where SPDY is not available. SPDY is not available when there is an HTTP-only proxy in front of the Kubernetes API server.
To properly fix this issue, we could make upstream contributions to improve the websocket executor to rely less on hardcoded timeouts and also expose a mechanism for indicating to both sides of the connection when the
io.Writersfor fd 0, 1, and 2 are ready for writes.Additional context
We introduced the use of the Websocket executor with #37538 in order to address the
"Upgrade request required"error which users see when they use thecilium sysdumpsubcommand and their Kubernetes API Server is behind a proxy which does not support SPDY. As described in the PR description there, SPDY as a protocol has been deprecated since 2015, and Kubernetes has used websocket by default since v1.29.In this implementation, we included a fallback mechanism to SPDY, in the case that Websocket is not available.
After introducing websockets, we began to see at least two related errors crop up in our CI:
#37784
A change which resolved most, but not all of these errors was #37984. See the PR description there for additional details. Unfortunately, this change did not resolve the flakes that we see in CI for the cases where tcpdump writes to the stdout
io.Writerbefore the websocket connection is fully established.This PR should eliminate those flakes, and also allows users to continue using
cilium sysdumpin cases where SPDY is not available.Fixes: #38643