omrikiei / ktunnel

A cli that exposes your local resources to kubernetes
GNU General Public License v3.0
913 stars 71 forks source link

fix: resolve shutdown deadlock on port forwarding when port is already in use #130

Closed jboewer closed 2 weeks ago

jboewer commented 3 weeks ago

Concerning https://github.com/omrikiei/ktunnel/issues/129

Problem The ktunnel expose command encounters a deadlock, ignoring SIGINTs, and requiring a manual process kill.

Root Cause In the k8s.PortForward function, the fwdWaitGroup counter is not decremented when readyChan is not closed (see common.go#L281). This results in an unresolved wg.Wait() in the expose command (see expose.go#L137).

Solution To maintain the current function signature of k8s.PortForward, the function should defer its return until all ready signals are received or an error occurs on one of the forwarders.

If changing the function signature is permissible, consider removing fwdWaitGroup as a parameter and creating it within the function itself, as the waiting is already handled internally.

I noticed another open PR (https://github.com/omrikiei/ktunnel/pull/127) that addresses this issue but introduces the side effect of calling os.Exit(1). That could break applications that depend on ktunnel as a module.

omrikiei commented 2 weeks ago

Thank you for your contribution!

jboewer commented 2 weeks ago

Thank you for your contribution!

Thank you for ktunnel and your time reviewing this PR :)