Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor IoUtil.getFreeRandomPort to always generate a valid free random port #2501

Closed
1 task done
rohanKanojia opened this issue Dec 19, 2023 · 1 comment · Fixed by #2641
Closed
1 task done

Refactor IoUtil.getFreeRandomPort to always generate a valid free random port #2501

rohanKanojia opened this issue Dec 19, 2023 · 1 comment · Fixed by #2641
Assignees
Milestone

Comments

@rohanKanojia
Copy link
Member

rohanKanojia commented Dec 19, 2023

Component

JKube Kit

Task description

Description

I noticed this while working on #2495

IoUtil.getFreeRandomPort seems to be generating already allocated ports. We often see these failures while running tests that involve TestHttpStaticServer (that uses this method to generate a free random port):

java.lang.RuntimeException: java.util.concurrent.ExecutionException: java.net.BindException: Address already in use

	at org.eclipse.jkube.kit.common.TestHttpStaticServer.getServer(TestHttpStaticServer.java:101)

Expected Behavior

Acceptance Criteria

  • IoUtil.getFreeRandomPort should guarantee to generate a valid port even on multiple invocations.
@rohanKanojia
Copy link
Member Author

It looks like ConnectException is thrown by new Socket("localhost", port) even when socket is being used but is not ready to accept connections:

https://github.com/eclipse/jkube/blob/a616fe68e3d348f5f5c4f94844c50ba3ecb12ede/jkube-kit/common/src/main/java/org/eclipse/jkube/kit/common/util/IoUtil.java#L131-L134

I added debug logs to see for which ports it's throwing BindException. While running test multiple times, I noticed it failed for a port 36266.

On checking status of this port using netstat, it shows connection is already established.

it : $ netstat -antup | grep 36266
(Not all processes could be identified, non-owned process info
 will not be shown, you would have to be root to see it all.)
tcp        0      0 192.168.29.207:36266    198.252.206.25:443      ESTABLISHED 6017/firefox        

rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Feb 8, 2024
…always generate a valid free random port (eclipse-jkube#2501)

+ Use ServerSocket to establish a connection instead of Socket which might
  also throw ConnectException for a socket already in use.
+ Modify IoUtilTes.findOpenPortWithSmallAttemptsCount to use
  `@RepeatedTest` annotation to run it 500 times

Signed-off-by: Rohan Kumar <[email protected]>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Feb 8, 2024
…always generate a valid free random port (eclipse-jkube#2501)

+ Use ServerSocket to establish a connection instead of Socket which might
  also throw ConnectException for a socket already in use.
+ Modify IoUtilTes.findOpenPortWithSmallAttemptsCount to use
  `@RepeatedTest` annotation to run it 500 times

Signed-off-by: Rohan Kumar <[email protected]>
rohanKanojia added a commit to rohanKanojia/jkube that referenced this issue Feb 8, 2024
…always generate a valid free random port (eclipse-jkube#2501)

+ Use ServerSocket to establish a connection instead of Socket which might
  also throw ConnectException for a socket already in use.
+ Modify IoUtilTes.findOpenPortWithSmallAttemptsCount to use
  `@RepeatedTest` annotation to run it 500 times

Signed-off-by: Rohan Kumar <[email protected]>
@rohanKanojia rohanKanojia moved this from In Progress to Review in Eclipse JKube Feb 9, 2024
@manusa manusa added this to the 1.16.0 milestone Feb 9, 2024
manusa pushed a commit that referenced this issue Feb 9, 2024
…always generate a valid free random port (#2501)

+ Use ServerSocket to establish a connection instead of Socket which might
  also throw ConnectException for a socket already in use.
+ Modify IoUtilTes.findOpenPortWithSmallAttemptsCount to use
  `@RepeatedTest` annotation to run it 500 times

Signed-off-by: Rohan Kumar <[email protected]>
@github-project-automation github-project-automation bot moved this from Review to Done in Eclipse JKube Feb 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants