From f64b5969074f21e702857e056dd27efa6b683b8e Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Thu, 22 Jun 2023 16:09:25 +0100 Subject: [PATCH] Prevent SSH connection restarts Set a high idle timeout on the sshkit connection pool. This will reduce the incidence of re-connection storms when a deployment has been idle for a while (e.g. when waiting for a docker build). The default timeout was 30 seconds, so we'll enable keepalives at a 30s interval to match. This is to help prevent connections from being killed during long idle periods. --- lib/mrsk/commander.rb | 1 + lib/mrsk/configuration.rb | 11 ++++++++++- test/configuration_test.rb | 25 +++++++++++++++++++++++-- test/integration/main_test.rb | 2 +- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/lib/mrsk/commander.rb b/lib/mrsk/commander.rb index 7b3af31f..94c0aff2 100644 --- a/lib/mrsk/commander.rb +++ b/lib/mrsk/commander.rb @@ -143,6 +143,7 @@ class Mrsk::Commander private # Lazy setup of SSHKit def configure_sshkit_with(config) + SSHKit::Backend::Netssh.pool.idle_timeout = config.sshkit_pool_idle_timeout SSHKit::Backend::Netssh.configure do |sshkit| sshkit.max_concurrent_starts = config.sshkit_max_concurrent_starts if config.sshkit_max_concurrent_starts sshkit.ssh_options = config.ssh_options diff --git a/lib/mrsk/configuration.rb b/lib/mrsk/configuration.rb index df389158..d6badbf3 100644 --- a/lib/mrsk/configuration.rb +++ b/lib/mrsk/configuration.rb @@ -153,7 +153,7 @@ class Mrsk::Configuration end def ssh_options - { user: ssh_user, proxy: ssh_proxy, auth_methods: [ "publickey" ] }.compact + { user: ssh_user, proxy: ssh_proxy, auth_methods: [ "publickey" ], keepalive: true, keepalive_interval: 30 }.compact end @@ -161,6 +161,15 @@ class Mrsk::Configuration raw_config.sshkit["max_concurrent_starts"] if raw_config.sshkit.present? end + def sshkit_pool_idle_timeout + if raw_config.sshkit.present? + raw_config.sshkit["pool_idle_timeout"] || 900 + else + 900 + end + end + + def healthcheck { "path" => "/up", "port" => 3000, "max_attempts" => 7 }.merge(raw_config.healthcheck || {}) end diff --git a/test/configuration_test.rb b/test/configuration_test.rb index e635cd04..47eee321 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -226,7 +226,13 @@ class ConfigurationTest < ActiveSupport::TestCase test "sshkit max concurrent starts" do config = Mrsk::Configuration.new(@deploy.tap { |c| c.merge!(sshkit: { "max_concurrent_starts" => 50 }) }) - assert_equal 50, @config.sshkit_max_concurrent_starts + assert_equal 50, config.sshkit_max_concurrent_starts + end + + test "sshkit pool idle timeout" do + assert_equal 900, @config.sshkit_pool_idle_timeout + config = Mrsk::Configuration.new(@deploy.tap { |c| c.merge!(sshkit: { "pool_idle_timeout" => 600 }) }) + assert_equal 600, config.sshkit_pool_idle_timeout end test "volume_args" do @@ -271,7 +277,22 @@ class ConfigurationTest < ActiveSupport::TestCase end test "to_h" do - assert_equal({ :roles=>["web"], :hosts=>["1.1.1.1", "1.1.1.2"], :primary_host=>"1.1.1.1", :version=>"missing", :repository=>"dhh/app", :absolute_image=>"dhh/app:missing", :service_with_version=>"app-missing", :env_args=>["-e", "REDIS_URL=\"redis://x/y\""], :ssh_options=>{:user=>"root", :auth_methods=>["publickey"]}, :volume_args=>["--volume", "/local/path:/container/path"], :builder=>{}, :logging=>["--log-opt", "max-size=\"10m\""], :healthcheck=>{"path"=>"/up", "port"=>3000, "max_attempts" => 7 }}, @config.to_h) + expected_config = \ + { :roles=>["web"], + :hosts=>["1.1.1.1", "1.1.1.2"], + :primary_host=>"1.1.1.1", + :version=>"missing", + :repository=>"dhh/app", + :absolute_image=>"dhh/app:missing", + :service_with_version=>"app-missing", + :env_args=>["-e", "REDIS_URL=\"redis://x/y\""], + :ssh_options=>{ :user=>"root", :auth_methods=>["publickey"], keepalive: true, keepalive_interval: 30 }, + :volume_args=>["--volume", "/local/path:/container/path"], + :builder=>{}, + :logging=>["--log-opt", "max-size=\"10m\""], + :healthcheck=>{ "path"=>"/up", "port"=>3000, "max_attempts" => 7 }} + + assert_equal expected_config, @config.to_h end test "min version is lower" do diff --git a/test/integration/main_test.rb b/test/integration/main_test.rb index cc7f8bbb..521661bd 100644 --- a/test/integration/main_test.rb +++ b/test/integration/main_test.rb @@ -51,7 +51,7 @@ class MainTest < IntegrationTest assert_equal "app-#{version}", config[:service_with_version] assert_equal [], config[:env_args] assert_equal [], config[:volume_args] - assert_equal({ user: "root", auth_methods: [ "publickey" ] }, config[:ssh_options]) + assert_equal({ user: "root", auth_methods: [ "publickey" ], keepalive: true, keepalive_interval: 30 }, config[:ssh_options]) assert_equal({ "multiarch" => false, "args" => { "COMMIT_SHA" => version } }, config[:builder]) assert_equal [ "--log-opt", "max-size=\"10m\"" ], config[:logging] assert_equal({ "path" => "/up", "port" => 3000, "max_attempts" => 7, "cmd" => "wget -qO- http://localhost > /dev/null" }, config[:healthcheck])