Fix issue with conflicting ports when starting multiple Dev sessions on Podman #6660
Conversation
…on Podman Due to regression on 6.1+ kernels [1], binding on 127.0.0.1:$port after some process listens on 0.0.0.0:$port does pass, which is not expected. Until this issue is fixed [2], a possible workaround is to try to bind on 0.0.0.0:$port. This works correctly on Podman, and also on Kubernetes (because we do port-forwarding on Kubernetes on 127.0.0.1:$port). The unit test added should allow us to detect similar issues faster in the future, should it happen again. [1] https://lore.kernel.org/stable/e21bf153-80b0-9ec0-15ba-e04a4ad42c34@redhat.com/T/ [2] https://lore.kernel.org/netdev/20230312031904.4674-3-kuniyu@amazon.com/T/
✅ Deploy Preview for odo-docusaurus-preview canceled.
|
openshift-ci
bot
added
kind/bug
| listener, err := net.Listen("tcp", address) | ||
| if err != nil { | ||
| return false | ||
| } | ||
| _ = listener.Addr().(*net.TCPAddr).Port |
There was a problem hiding this comment.
It can be used to determine the port value on which we are listening (by casting listener.Addr() into *net.TCPAddr). But the return value is not used at all. And it is useless to me since we are already passing the input port to net.Listen. It would have been useful for example if we were letting the kernel bind to a random port and we wanted to get that random port value.
| @@ -782,12 +782,11 @@ func SafeGetBool(b *bool) bool { | |||
|
|
|||
| // IsPortFree checks if the port on localhost is free to use | |||
| func IsPortFree(port int) bool { | |||
| address := fmt.Sprintf("localhost:%d", port) | |||
| address := fmt.Sprintf("0.0.0.0:%d", port) | |||
There was a problem hiding this comment.
Should this change be reverted once the regression has been fixed? If so, can you add a todo about it?
There was a problem hiding this comment.
To me, this is still valid even after the regression is fixed in the kernel. Podman binds by default to 0.0.0.0, so it makes sense to check on all addresses. As for Kubernetes, port-forwarding currently binds to 127.0.0.1 by default, but it makes sense to me to still check on all addresses.
As part of #6479, we would need to properly control the address on which to bind and pass the right address to this function.
|
Kudos, SonarCloud Quality Gate passed!
|
openshift-ci
bot
added
the
lgtm
What type of PR is this:
/kind bug
/area odo-on-podman
What does this PR do / why we need it:
Due to regression on 6.1+ kernels [1], binding on 127.0.0.1:$port after some process listens on 0.0.0.0:$port does pass, which is not expected. Until this issue is fixed [2], a possible workaround is to try to bind on 0.0.0.0:$port. This works correctly on Podman, and also on Kubernetes (because we do port-forwarding on Kubernetes on 127.0.0.1:$port).
The unit test added should allow us to detect similar issues faster in the future, should it happen again.
[1] https://lore.kernel.org/stable/e21bf153-80b0-9ec0-15ba-e04a4ad42c34@redhat.com/T/
[2] https://lore.kernel.org/netdev/20230312031904.4674-3-kuniyu@amazon.com/T/
Which issue(s) this PR fixes:
Fixes #6612
PR acceptance criteria:
Unit test
Integration test
Documentation
How to test changes / Special notes to the reviewer:
See the repro steps provided in #6612