feat: check port use by another process or not when startup

[Anilople]

What's the purpose of this PR

Give better experience to user when run ./scripts/startup.sh

Which issue(s) this PR fixes:

Fixes #4642 #4307

Brief changelog

1. check port use by another process or not when startup, if it happened, give some suggest to user
2. if startup already, show user that app's port and pid
3. add service name to first line. for example apollo-portal in

Thu Nov 24 23:01:36 CST 2022 ==== apollo-portal Starting ====

---

An example here, there is an process use port 8080 which isn't apollo config service,
so it's alway fail to startup with default port 8080

pi@raspberrypi:~/apollo/2.2.0-SNAPSHOT/configservice $ ./scripts/startup.sh
Fri 21 Apr 15:47:59 BST 2023 ==== apollo-configservice failed to start. The port 8080 already be in use by another process
maybe you can figure out which process use port 8080 by following ways:
1. access http://change-to-this-machine-ip:8080 by browser
2. run command 'curl http://localhost:8080'
3. run command 'sudo netstat -tunlp | grep :8080'
4. run command 'sudo lsof -nP -iTCP:8080 -sTCP:LISTEN'

We change config service's port to 18080, then startup success.

pi@raspberrypi:~/apollo/2.2.0-SNAPSHOT/configservice $ ./scripts/startup.sh
Fri 21 Apr 15:49:14 BST 2023 ==== apollo-configservice Starting ====
Started [2235]
Waiting for server startup.........
....
Fri 21 Apr 15:50:21 BST 2023 Server started in 65 seconds!

Then if we want to run startup.sh again

pi@raspberrypi:~/apollo/2.2.0-SNAPSHOT/configservice $ ./scripts/startup.sh
Fri 21 Apr 15:52:36 BST 2023 ==== apollo-configservice is running already with port 18080, pid 2235

---

checklist

Follow this checklist to help us incorporate your contribution quickly and easily:

• Read the Contributing Guide before making this pull request.
• Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• Write necessary unit tests to verify the code.
• Run mvn clean test to make sure this pull request doesn't break anything.
• Update the CHANGES log.

[codecov]

Codecov Report

Merging #4656 (930b1c6) into master (b30cbe5) will decrease coverage by 0.03%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #4656      +/-   ##
============================================
- Coverage     48.41%   48.38%   -0.03%     
  Complexity     1722     1722              
============================================
  Files           346      346              
  Lines         10827    10827              
  Branches       1078     1078              
============================================
- Hits           5242     5239       -3     
- Misses         5264     5266       +2     
- Partials        321      322       +1     

see 1 file with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[stale]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 14 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[stale]

This pull request has been automatically closed because it has not had activity in the last 14 days. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

[nobodyiam]

review again

[nobodyiam]

Hello, I am a serverless review bot on flows.network. Here are my reviews of code commits in this PR.

---

The patches mentioned above relate to making improvements to the functionality of Apollo project by modifying the startup scripts and updating the CHANGES.md file. Overall, there are no potential problems identified in most of the patches, except for two. One patch raises concerns that the -sTCP:LISTEN option, which was removed, might make it more challenging to identify conflicts related to port utilization. Another patch introduces a platform-specific tool, lsof, which may not work on all supported platforms. The most essential finding is that the patches will add new features, improve code quality and reduce resource consumption. However, further testing is necessary to validate the compatibility of the patch with all platforms.

Details

Commit 1

The patch updates the CHANGES.md file by adding a new feature that checks if a port is being used by another process during startup. This feature has been added through pull request #4656. There are no potential problems identified in this patch.

Commit 2

This patch modifies the startup scripts for the Admin Service, Config Service, and Portal Service of the Apollo project. The modification changes the endpoint used to determine whether the service is up and running from /apollo/version to /health. This change should enhance the ability to check whether the service is operational.

There appear to be no potential problems associated with this patch. However, it might be worth checking to ascertain whether the update has any side effect on the expected behavior of the startup scripts or impacts other parts of the system.

Commit 3

This is a Patch to add a new feature lsof -t, which lists only the PIDs of the command that is listening on a specific port. This is done in the startup.sh script files of apollo-adminservice, apollo-configservice and apollo-portal.

There doesn't seem to be any potential problems with the patch. However, an important note is that using sudo can be dangerous if not used carefully. The user running the scripts should be knowledgeable and well-versed in the usage of sudo.

Commit 4

Key Changes:
This patch reverts a previous commit that added the option "-sTCP:LISTEN" to the "sudo lsof" command in three scripts of the Apollo project for obtaining the process ID that is using a specific port.

Potential Problems:
Since this patch reverts the previous commit, the "-sTCP:LISTEN" option is removed from the "sudo lsof" command in the three scripts. It is unclear why this option was added in the previous commit and why it is being reverted now. Without this option, the "sudo lsof" command may return more processes and provide less useful information to troubleshoot issues related to port conflicts. Further analysis is needed to ensure that this change does not introduce any problems.

Commit 5

The key change in this patch is to replace the use of curl with lsof to check port utilization. This change is made to the startup script of three services: apollo-adminservice, apollo-configservice, and apollo-portal. The patch can potentially reduce the resource consumption of the services since curl is a heavier tool than lsof. Additionally, there is a subjective improvement in code quality since lsof is a more direct tool for the use case, thus reducing the complexity of the code. One potential problem with this patch is that the new tool used is platform-dependent, and the script might not work on all supported environments. Therefore, further testing must be done to ensure its compatibility.

[nobodyiam]

review again

[nobodyiam]

Hello, I am a serverless review bot on flows.network. Here are my reviews of code commits in this PR.

---

The patches modify the startup scripts of three different services in the Apollo project repository to add a feature to prevent conflicts and improve the overall stability of the application. Most of the changes are straightforward and seem to have no potential problems except for a few issues with permission and platform compatibility. However, it is important to ensure that each patch is tested to avoid any unforeseen issues. The most significant finding is that the patches aim to prevent port conflicts, which are a common cause of unexpected behavior in software applications.

Details

Commit 1

This patch makes changes to the startup script of three different services: apollo-adminservice, apollo-configservice, and apollo-portal. The main change is that before running the server, the script will check if the port is already in use by another process. If it is, it will exit with an error message indicating which process is using it.

This change can prevent multiple instances of the same service from running on the same port, which can cause conflicts and unexpected behavior.

However, there are potential problems with this change. One issue is that the method used to check if the port is in use may not work in all situations. If the port is being used by a process that does not return an HTTP response, the script may not detect it. Another issue is that if the script is run as a non-root user, it may not have permission to run the pgrep command, which could lead to incorrect results.

Commit 2

This is a patch that updates the CHANGES.md file in the Apollo project repository.

The key change in this patch adds a new feature that checks if a port is being used by another process during startup. This can prevent conflicts and improve the overall stability of the application.

There don't appear to be any potential problems with this patch as it only modifies documentation. However, it is important to note that the actual implementation of the new feature must be thoroughly tested to ensure it works as intended and does not introduce any new issues.

Commit 3

The patch changes the health check call for the apollo services from /apollo/version to /health. The change addresses a potential problem where the health status could be affected by the version check, which could lead to incorrect service availability reporting.

As a reviewer, I don't see any major issues with this patch. However, depending on the implementation of the /health endpoint, there could be potential problems that cannot be identified by just looking at the patch. The endpoint could have its own issues, such as response latency, out-of-date information, or config issues that would affect the reliability of the health check. It would be wise to perform additional testing to validate the effectiveness of this modification.

Commit 4

The patch adds a new command to the startup scripts of the Apollo Admin Service, Config Service, and Portal. The new command runs 'sudo lsof -i:$SERVER_PORT -sTCP:LISTEN -t' to list out the Process ID (PID) of the process that is listening on the specified port.

The change seems harmless and can be expected to work. It improves the troubleshooting capabilities of the scripts by providing another way to find the process that is using a particular port. However, the change only works on Unix based OSes and may not work on Windows platforms. Another potential issue is that the sudo command may not be allowed on some systems, and the user might have to modify the script to remove the sudo command.

Commit 5

This patch is reverting a previous commit that added a new command to get the process ID of the application listening on a given port. The change modified the startup.sh script in the apollo-adminservice, apollo-configservice and apollo-portal modules. The new command to replace lsof with is now sudo lsof -nP -iTCP:$SERVER_PORT -sTCP:LISTEN. There are no potential problems identified in this patch.

Commit 6

Key Changes:

• The script files for Apollo adminservice, configservice, and portal have been modified.
• The existProcessUsePort function has been updated to check if the port is being used by a process using lsof command instead of curl command.

Potential Problems:

• There seem to be no potential problems with this patch. However, it is advisable to test the patch to ensure that it is working as expected.
• Additionally, since this patch is a refactoring change, it is important to ensure that all tests related to these scripts pass after applying the patch.

Commit 7

This patch is related to three different shell scripts (startup.sh) for three services in the same project. The main change involves replacing the command lsof with curl in the function existProcessUsePort(). Specifically, the patch replaces the check whether a port is already in use by another process using lsof with a check whether an HTTP response is returned from $SERVER_URL using curl.

The key change reduces the number of dependencies required for startup and improves cross-platform compatibility, as curl is more widely available than lsof, which is mostly used on Unix-based systems. Also, this solution is faster since it only takes a few milliseconds to complete a curl request versus several seconds for lsof.

There is one potential problem with this patch: if the URL $SERVER_URL is not an HTTP URL, the curl command will fail, and the function existProcessUsePort() will return false here. However, this is unlikely as $SERVER_URL is supposed to be an endpoint for the service.

[nobodyiam]

LGTM