From e95d92dbc6877914c42e401b7079330266dd6ddc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Tue, 3 Oct 2023 12:05:26 +0300 Subject: [PATCH 1/3] Close websocket heartbeat explicitly when unexpected closure received Kubernetes-commit: e1ae906048003145441fb1d4ecce4c13acf5cb19 --- tools/remotecommand/websocket.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/remotecommand/websocket.go b/tools/remotecommand/websocket.go index 9230027c..78de8af1 100644 --- a/tools/remotecommand/websocket.go +++ b/tools/remotecommand/websocket.go @@ -18,8 +18,10 @@ package remotecommand import ( "context" + "errors" "fmt" "io" + "net" "net/http" "sync" "time" @@ -476,9 +478,15 @@ func (h *heartbeat) start() { klog.V(8).Infof("Websocket Ping succeeeded") } else { klog.Errorf("Websocket Ping failed: %v", err) - // Continue, in case this is a transient failure. - // c.conn.CloseChan above will tell us when the connection is - // actually closed. + if errors.Is(err, gwebsocket.ErrCloseSent) { + continue + } else if e, ok := err.(net.Error); ok && e.Temporary() { + // Continue, in case this is a transient failure. + // c.conn.CloseChan above will tell us when the connection is + // actually closed. + continue + } + return } } } From 9a88950e38ba924e7d1d3e27145c2eca52440428 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Thu, 5 Oct 2023 12:04:06 +0300 Subject: [PATCH 2/3] revert back to IsUnexpectedCloseError check Kubernetes-commit: 914210ee9a6df6927019f200e90534edd1460fea --- tools/remotecommand/websocket.go | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/tools/remotecommand/websocket.go b/tools/remotecommand/websocket.go index 78de8af1..4d9814a8 100644 --- a/tools/remotecommand/websocket.go +++ b/tools/remotecommand/websocket.go @@ -18,10 +18,8 @@ package remotecommand import ( "context" - "errors" "fmt" "io" - "net" "net/http" "sync" "time" @@ -478,15 +476,12 @@ func (h *heartbeat) start() { klog.V(8).Infof("Websocket Ping succeeeded") } else { klog.Errorf("Websocket Ping failed: %v", err) - if errors.Is(err, gwebsocket.ErrCloseSent) { - continue - } else if e, ok := err.(net.Error); ok && e.Temporary() { - // Continue, in case this is a transient failure. - // c.conn.CloseChan above will tell us when the connection is - // actually closed. - continue + if gwebsocket.IsUnexpectedCloseError(err, gwebsocket.CloseGoingAway, gwebsocket.CloseAbnormalClosure) { + return } - return + // Continue, in case this is a transient failure. + // c.conn.CloseChan above will tell us when the connection is + // actually closed. } } } From 583e50d0087c3d9856f8c5f6324f44e2050ca93a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Arda=20G=C3=BC=C3=A7l=C3=BC?= Date: Fri, 6 Oct 2023 08:55:22 +0300 Subject: [PATCH 3/3] Use timeout function to detect transient errors Kubernetes-commit: a888fef242fd59bc3871c67099c7f5e9449873c2 --- tools/remotecommand/websocket.go | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tools/remotecommand/websocket.go b/tools/remotecommand/websocket.go index 4d9814a8..48e52092 100644 --- a/tools/remotecommand/websocket.go +++ b/tools/remotecommand/websocket.go @@ -18,8 +18,10 @@ package remotecommand import ( "context" + "errors" "fmt" "io" + "net" "net/http" "sync" "time" @@ -476,12 +478,18 @@ func (h *heartbeat) start() { klog.V(8).Infof("Websocket Ping succeeeded") } else { klog.Errorf("Websocket Ping failed: %v", err) - if gwebsocket.IsUnexpectedCloseError(err, gwebsocket.CloseGoingAway, gwebsocket.CloseAbnormalClosure) { - return + if errors.Is(err, gwebsocket.ErrCloseSent) { + // we continue because c.conn.CloseChan will manage closing the connection already + continue + } else if e, ok := err.(net.Error); ok && e.Timeout() { + // Continue, in case this is a transient failure. + // c.conn.CloseChan above will tell us when the connection is + // actually closed. + // If Temporary function hadn't been deprecated, we would have used it. + // But most of temporary errors are timeout errors anyway. + continue } - // Continue, in case this is a transient failure. - // c.conn.CloseChan above will tell us when the connection is - // actually closed. + return } } }