Add port conflict exception by gfukushima · Pull Request #4565 · hyperledger/besu

gfukushima · 2022-10-27T04:51:41Z

PR description

Added methods to check if port is available and call before starting websockets and rpc services.

Fixed Issue(s)

Fixes #4176

Documentation

I thought about documentation and added the doc-change-required label to this PR if
updates are required.

Changelog

I thought about the changelog and included a changelog update if required.

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

…o add_port_conflict_exception

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

fab-10

just a comment on the method name, then a general question, this covers listening ports of the RPC server, are the other listening ports already covered?

fab-10 · 2022-10-27T10:02:37Z

util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java

+    return false;
+  }
+
+  public static void checkPortsAvailable(final int tcpPort) {


then method name says Ports so suggests that more than one port as parameter like a var arg, otherwise use the singular

Changed the name of the method, thanks for raising that @fab-10

Changed the approach to cover the ports we will require to initialise all the services defined in the config. So we can have a more general solution that loops through the ports and check if they are available.

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcService.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

jframe · 2022-10-31T05:04:22Z

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

+    if (!unavailablePorts.isEmpty()) {
+      throw new InvalidConfigurationException(
+          "Port(s) '"
+              + unavailablePorts.toString()


nit: toString on unavailablePorts isn't necessary

jframe · 2022-10-31T05:04:44Z

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

  }

+  private void checkIfRequiredPortsAreAvailable() {
+    List<Integer> unavailablePorts = new ArrayList<>();


jframe · 2022-10-31T05:05:58Z

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java


    final CompletableFuture<?> resultFuture = new CompletableFuture<>();
    try {
+


any reason to add the extra empty line?

util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java

changes have been made to check port conflicts for other services

siladu · 2022-10-31T06:11:20Z

util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java

    }
  }
+
+  public static boolean isPortAvailableForTcp(final int port) {


Could be private?

siladu · 2022-10-31T06:11:29Z

util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java

+    return false;
+  }
+
+  public static boolean isPortAvailableForUdp(final int port) {


could be private?

util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

jframe · 2022-11-02T02:55:10Z

util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java

+    return false;
+  }
+
+  public static boolean isPortAvailable(final int port) {


nit: consider renaming to make to clear this is checking both udp and tcp. maybe rename to isPortAvailableForTcpAndUdp

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com> Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima added 7 commits October 27, 2022 15:43

Add method to check port availability

c504b13

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Check port availability

3ec630e

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add tests to check port availability

de1afc7

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge branch 'hyperledger:main' into add_port_conflict_exception

4a69660

Fix broken test

feeaf2a

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge remote-tracking branch 'origin/add_port_conflict_exception' int…

69961ba

…o add_port_conflict_exception

Spotless

b91487f

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima marked this pull request as ready for review October 27, 2022 09:22

fab-10 requested changes Oct 27, 2022

View reviewed changes

Rename method

b749733

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

jframe previously requested changes Oct 28, 2022

View reviewed changes

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcHttpService.java Outdated Show resolved Hide resolved

ethereum/api/src/main/java/org/hyperledger/besu/ethereum/api/jsonrpc/JsonRpcService.java Outdated Show resolved Hide resolved

gfukushima added 3 commits October 28, 2022 17:24

Rollback changes

8752282

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add general port verification

817286e

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Fix test

d1cb28c

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

fab-10 approved these changes Oct 28, 2022

View reviewed changes

jframe reviewed Oct 31, 2022

View reviewed changes

siladu reviewed Oct 31, 2022

View reviewed changes

jframe reviewed Oct 31, 2022

View reviewed changes

util/src/main/java/org/hyperledger/besu/util/NetworkUtility.java Outdated Show resolved Hide resolved

gfukushima added 4 commits November 1, 2022 00:35

Add unit test

7ba61a7

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Refactor method

489ce6f

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add more specific validation for ports/services

49555f5

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge branch 'main' into add_port_conflict_exception

896ebf2

jframe approved these changes Nov 2, 2022

View reviewed changes

Merge branch 'main' into add_port_conflict_exception

9229afc

jframe merged commit 42260fd into hyperledger:main Nov 2, 2022

macfarla pushed a commit to jflo/besu that referenced this pull request Jan 10, 2023

Add port conflict exception (hyperledger#4565)

9b275db

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com> Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

gfukushima deleted the add_port_conflict_exception branch January 13, 2023 07:08

gfukushima added the mainnet label Jan 24, 2023

garyschulte mentioned this pull request Apr 4, 2023

propose Gabriel Fukushima protocolguild/documentation#56

Merged

eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023

Add port conflict exception (hyperledger#4565)

ac2fbb2

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>


		final CompletableFuture<?> resultFuture = new CompletableFuture<>();
		try {

Comments

Conversation

gfukushima commented Oct 27, 2022

PR description

Fixed Issue(s)

Documentation

Changelog

Uh oh!

fab-10 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants