fix: Ensure SSH connections are closed after each command execution#859
fix: Ensure SSH connections are closed after each command execution#859jfanals wants to merge 1 commit intobasecamp:mainfrom
Conversation
This commit addresses an issue where SSH connections were not being properly closed after each command execution, leading to timeout errors during the `kamal deploy` process.
|
For some strange reason the test on Ruby 3.1 failed, it looks like it might have been just a glitch in the test procedures as the code should not affect only one particular version of ruby but all versions. ensure
SSHKit::Backend::Netssh.pool.close_connectionsWould it be possible to run the tests again? |
|
I've kicked that off. I'm not sure though about this change. Creating the SSH connections can be expensive especially with large numbers of servers to deploy to. We configure keepalives with an interval of 30 seconds on the connections, so that generally should stop them from timing out. Maybe you could try reducing the keepalive interval and see if that makes any difference? |
|
Thanks for the suggestion, I did try to decrease the |
|
👍 on this one, as it's pretty much similar to delano/rye#63 I faced a while ago, and I think delano/rye#38 has more context on this |
|
@jfanals - we overwrite the keepalive_interval in the Kamal config, so I'm not sure it will pick up that from Could you try: Setting the log_level to debug might also give us some useful feedback. |
I had a similar problem, which brought me to this issue. Unfortunately setting the ❯ bin/kamal config
ERROR (Kamal::ConfigurationError): ssh: unknown key: keepalive_intervalThe fix for me was to increase the |
@plattenschieber how did you apply your fix? Is it through the server? |
|
Yes, just |
Mine does not have |
I tried setting the log_level to debug and keepalive_interval to 10 by manually updating the ssh.rb at /opt/homebrew/lib/ruby/gems/3.3.0/gems/kamal-2.2.2/lib/kamal/configuration, but it's still not working. It looks like when my NextJS app is building for a long time while it streams the logs of the build, the keepalive ping is not running. |
|
This PR actually fixed my issue. It might be worth considering merging this. |
|
I'm going to close this one as we shouldn't need to re-create the connections for each command. There's something wrong here alright and it would be good to get to the bottom of it, but I don't think this brute force approach is the way to go. |
|
our team has 30% chance of closing stream as of the latest version of kamal |
This commit addresses an issue where SSH connections were not being properly closed after each command execution, leading to timeout errors during the
kamal deployprocess.Problem
When executing commands using the
executemethod inSSHKit::Runner::Parallel, SSH connections were left open, causing subsequent commands to fail with timeout errors. This issue was particularly evident when runningkamal build deliverafterkamal registry loginduring the normalkamal deploy processSolution
The
executemethod in theSSHKit::Runner::Parallel::CompleteAllmodule has been modified to ensure that all SSH connections are closed after each command execution. This is achieved by adding anensureblock to the thread creation logic, which callsSSHKit::Backend::Netssh.pool.close_connectionsafter each command, regardless of whether an exception occurs.Changes
ensureblock to theexecutemethod inSSHKit::Runner::Parallel::CompleteAllto close SSH connections after each command execution.Impact
This change ensures that SSH connections are properly closed, preventing timeout errors and improving the reliability of the
kamal deployprocess.Testing
Tested the changes by running
kamal deploycommands. Verified that the timeout errors no longer occur and the deployment process completes successfully.Closes #857