Skip to content

Commit bf41455

Browse files
soltyshk8s-publishing-bot
authored andcommitted
Clean error handling in port-forward
This commit introduces: 1. Cleanups in port-forwarding error handling code, which ensures that we only compare lowercased text always. 2. E2E verifying that when a pod is removed a port-forward is stopped. Signed-off-by: Maciej Szulik <[email protected]> Kubernetes-commit: 0b1617ccefbc6ea61c0e7c2b0b4052703f11c51c
1 parent d6404c8 commit bf41455

File tree

1 file changed

+14
-8
lines changed

1 file changed

+14
-8
lines changed

tools/portforward/portforward.go

+14-8
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ import (
3737
// TODO move to API machinery and re-unify with kubelet/server/portfoward
3838
const PortForwardProtocolV1Name = "portforward.k8s.io"
3939

40-
var ErrLostConnectionToPod = errors.New("lost connection to pod")
40+
var (
41+
// error returned whenever we lost connection to a pod
42+
ErrLostConnectionToPod = errors.New("lost connection to pod")
43+
44+
// set of error we're expecting during port-forwarding
45+
networkClosedError = "use of closed network connection"
46+
)
4147

4248
// PortForwarder knows how to listen for local connections and forward them to
4349
// a remote pod via an upgraded HTTP request.
@@ -312,7 +318,7 @@ func (pf *PortForwarder) waitForConnection(listener net.Listener, port Forwarded
312318
conn, err := listener.Accept()
313319
if err != nil {
314320
// TODO consider using something like https://github.com/hydrogen18/stoppableListener?
315-
if !strings.Contains(strings.ToLower(err.Error()), "use of closed network connection") {
321+
if !strings.Contains(strings.ToLower(err.Error()), networkClosedError) {
316322
runtime.HandleError(fmt.Errorf("error accepting connection on port %d: %v", port.Local, err))
317323
}
318324
return
@@ -381,7 +387,7 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) {
381387

382388
go func() {
383389
// Copy from the remote side to the local port.
384-
if _, err := io.Copy(conn, dataStream); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
390+
if _, err := io.Copy(conn, dataStream); err != nil && !strings.Contains(strings.ToLower(err.Error()), networkClosedError) {
385391
runtime.HandleError(fmt.Errorf("error copying from remote stream to local connection: %v", err))
386392
}
387393

@@ -394,7 +400,7 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) {
394400
defer dataStream.Close()
395401

396402
// Copy from the local port to the remote side.
397-
if _, err := io.Copy(dataStream, conn); err != nil && !strings.Contains(err.Error(), "use of closed network connection") {
403+
if _, err := io.Copy(dataStream, conn); err != nil && !strings.Contains(strings.ToLower(err.Error()), networkClosedError) {
398404
runtime.HandleError(fmt.Errorf("error copying from local connection to remote stream: %v", err))
399405
// break out of the select below without waiting for the other copy to finish
400406
close(localError)
@@ -406,10 +412,10 @@ func (pf *PortForwarder) handleConnection(conn net.Conn, port ForwardedPort) {
406412
case <-remoteDone:
407413
case <-localError:
408414
}
409-
/*
410-
reset dataStream to discard any unsent data, preventing port forwarding from being blocked.
411-
we must reset dataStream before waiting on errorChan, otherwise, the blocking data will affect errorStream and cause <-errorChan to block indefinitely.
412-
*/
415+
416+
// reset dataStream to discard any unsent data, preventing port forwarding from being blocked.
417+
// we must reset dataStream before waiting on errorChan, otherwise,
418+
// the blocking data will affect errorStream and cause <-errorChan to block indefinitely.
413419
_ = dataStream.Reset()
414420

415421
// always expect something on errorChan (it may be nil)

0 commit comments

Comments
 (0)