Conversation
rcdailey
requested review from
ChrisJBurns,
JAORMX,
amirejaz,
blkt,
dmjb,
jhrozek and
yrobla
as code owners
|
The change LGTM. There are some linting issues which must be addressed first. Thanks a lot for your contribution! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3584 +/- ##
=======================================
Coverage 65.52% 65.52%
=======================================
Files 404 404
Lines 39781 39787 +6
=======================================
+ Hits 26068 26072 +4
- Misses 11697 11700 +3
+ Partials 2016 2015 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
The proxy could hang indefinitely when kubectl attach terminated its SPDY connection. This occurred because: - SPDY executor retained corrupted state across retry attempts - Pipe cleanup wasn't signaling EOF to the transport layer Create fresh SPDY executor per retry to avoid state corruption. Close pipes when attach goroutine exits to propagate EOF signal, allowing the transport layer to detect the failure and trigger re-attachment or exit. Fixes: stacklok#3583 Signed-off-by: Robert Dailey <[email protected]>
rcdailey
force-pushed
the
fix-spdy-eof-hang
branch
from
6219b10 to
d8113be
Compare
|
Hold off on merging please, I found an issue. Sorry for the inconvenience. I am keeping this running in my cluster and when I observe issues, I am fixing them. I just pushed up a change but let me sit on this another day. Also the lint failure appears to be a network timeout outside of my control: https://github.com/stacklok/toolhive/actions/runs/21656377704/job/62442458301?pr=3584#step:6:42 |
|
@rcdailey thanks for the diligence! Let me know when it's ready |
|
@JAORMX thanks for your patience as I took time to ensure my proposed changes were stable by running them in my homelab kubernetes cluster. I've had these services running since about 9 hours ago: So far, no restarts (I had over 100 before) and I've been actively using searxng and context7 throughout the day. Everything appears to be running well so far. I've moved the PR out of draft and I believe it's ready for merge, pending any feedback you have. |
2 participants
Summary
Fixes #3583 - Proxy does not exit on EOF from kubectl attach SPDY connection.
PR #3183 added exit-on-failure logic for retry exhaustion, but EOF errors from
k8s.io/client-goSPDY code bypassed this logic, leaving the proxy in a zombie state where HTTP health checks pass but MCP communication fails.Changes
Fresh SPDY executor per retry - The executor was created once and reused across retries. When the SPDY connection fails with EOF, the executor enters a corrupted state. Now each retry creates a fresh executor.
Pipe closure on goroutine exit - Added deferred close of stdout/stdin pipes when the attach goroutine exits, ensuring the transport layer sees EOF and can trigger re-attachment or exit.
Validation
Deployed to production cluster and verified:
Related