should override PORT #606
should override PORT #606
Conversation
|
I also add arg |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the KasmVNC module where the PORT configuration wasn't being properly applied to the kasmvncserver command. The changes ensure the server starts on the correct port and adds improved error detection by checking if the service is actually accessible.
Key changes:
- Adds
-websocketPort "${PORT}"parameter to the kasmvncserver command to explicitly override the port configuration - Enhances error handling to verify server availability via HTTP health check before reporting failure
| set -e | ||
|
|
||
| if [[ $RETVAL -ne 0 ]]; then | ||
| if [[ $RETVAL -ne 0 ]] && ! curl -s http://127.0.0.1:"${PORT}"/app; then |
There was a problem hiding this comment.
The curl command is used without checking if it's available. The script already has a download_file function that handles multiple download tools (curl, wget, busybox) as fallbacks. If curl is not available on the system, this health check will fail even if the server started successfully. Consider using a similar fallback pattern or checking for curl availability before using it.
| set -e | ||
|
|
||
| if [[ $RETVAL -ne 0 ]]; then | ||
| if [[ $RETVAL -ne 0 ]] && ! curl -s http://127.0.0.1:"${PORT}"/app; then |
There was a problem hiding this comment.
The curl health check is performed immediately after starting the kasmvncserver. There's a potential race condition where the server might need a moment to fully start and bind to the port. If the curl request happens before the server is ready to accept connections, this could result in false negatives. Consider adding a brief sleep or implementing a retry mechanism before checking the health endpoint.
| if [[ $RETVAL -ne 0 ]] && ! curl -s http://127.0.0.1:"${PORT}"/app; then | |
| # Retry health check up to 10 times with 1s delay | |
| health_check_with_retries() { | |
| local url=$1 | |
| local max_attempts=10 | |
| local attempt=1 | |
| while (( attempt <= max_attempts )); do | |
| if curl -s "$url" > /dev/null; then | |
| return 0 | |
| fi | |
| sleep 1 | |
| ((attempt++)) | |
| done | |
| return 1 | |
| } | |
| if [[ $RETVAL -ne 0 ]] && ! health_check_with_retries "http://127.0.0.1:${PORT}/app"; then |
1 participant
Description
Type of Change
Module Information
Path:
registry/[namespace]/modules/[module-name]New version:
v1.0.0Breaking change: [ ] Yes [ ] No
Template Information
Path:
registry/[namespace]/templates/[template-name]Testing & Validation
bun test)bun fmt)Related Issues