Skip to content

Conversation

@dmytrotkk
Copy link
Collaborator

This pull request introduces improvements to the SSL health check logic in the SKALE Node CLI, enhancing reliability and error handling. The changes mainly focus on making the SSL connection verification process more robust and updating subprocess management.

SSL Health Check Improvements

  • The check_ssl_connection function now waits for the OpenSSL subprocess to finish with a timeout, raising a clear error if the check takes too long or fails, instead of relying on a fixed sleep and poll approach. This results in more reliable and informative error handling.
  • The subprocess module is now explicitly imported in node_cli/core/ssl/check.py to support the new timeout logic.

Subprocess Management

  • The detached_subprocess function in node_cli/core/ssl/utils.py now sets stdin=subprocess.DEVNULL to prevent the subprocess from waiting for input, improving process isolation and avoiding potential hangs.

Documentation

  • The README.md was updated to remove the version restriction on docker-compose, clarifying that any version is acceptable.

kucharskim and others added 6 commits June 23, 2025 20:07
Function doesn't wait for `openssl s_client ...` to finish.
It assumes that when the command is still running that is the
successful condition. However the function should wait for
exit code from the binary. We saw in production intermittent
and very often `skale ssl upload` failures. This change
should fix this problem and underlying race condition.
Replace for loop and dp.poll() with more straightforward
dp.wait() with a timeout, as requested during diff review.
Redirect the child's standard input to subprocess.DEVNULL,
so it starts with no stdin attached. This prevents the OpenSSL
health-check process from reading from, or blocking on, the
parent's terminal or execution environment stdin stream.
Fix check_ssl_connection() function
@dmytrotkk dmytrotkk requested review from a team, badrogger and kladkogex as code owners December 16, 2025 19:46
@dmytrotkk dmytrotkk self-assigned this Dec 16, 2025
@DmytroNazarenko DmytroNazarenko merged commit 768417e into beta Dec 17, 2025
2 of 3 checks passed
@DmytroNazarenko DmytroNazarenko deleted the pre-pr-947 branch December 17, 2025 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants