Improve port forwarding setup

- Include keepalive options when forwarding port for remote builder
- Log port forwarding errors.

Author: djmb

lib/kamal/cli/build.rb

@@ -70,7 +70,7 @@ def push
   def pull
     login_to_registry_remotely unless KAMAL.registry.local?
 
-    forward_local_registry_port(KAMAL.hosts) do
+    forward_local_registry_port(KAMAL.hosts, **KAMAL.config.ssh.options) do
       if (first_hosts = mirror_hosts).any?
         #  Pull on a single host per mirror first to seed them
         say "Pulling image on #{first_hosts.join(", ")} to seed the #{"mirror".pluralize(first_hosts.count)}...", :magenta
@@ -213,17 +213,27 @@ def login_to_registry_remotely
     def forward_local_registry_port_for_remote_builder(&block)
       if KAMAL.builder.remote?
         remote_uri = URI(KAMAL.config.builder.remote)
-        forward_local_registry_port([ remote_uri.host ], user: remote_uri.user, proxy: nil, ssh_port: remote_uri.port, &block)
+        forward_local_registry_port([ remote_uri.host ], **remote_builder_ssh_options(remote_uri), &block)
       else
         yield
       end
     end
 
-    def forward_local_registry_port(hosts, user: KAMAL.config.ssh.user, proxy: KAMAL.config.ssh.proxy, ssh_port: nil, &block)
+    def forward_local_registry_port(hosts, **ssh_options, &block)
       if KAMAL.config.registry.local?
-        PortForwarding.new(hosts, KAMAL.config.registry.local_port, user: user, proxy: proxy, ssh_port: ssh_port).forward(&block)
+        say "Setting up local registry port forwarding to #{hosts.join(', ')}..."
+        PortForwarding.new(hosts, KAMAL.config.registry.local_port, **ssh_options).forward(&block)
       else
         yield
       end
     end
+
+    def remote_builder_ssh_options(remote_uri)
+      { user: remote_uri.user,
+        port: remote_uri.port,
+        keepalive: KAMAL.config.ssh.options[:keepalive],
+        keepalive_interval: KAMAL.config.ssh.options[:keepalive_interval],
+        logger: KAMAL.config.ssh.options[:logger]
+      }.compact
+    end
 end

lib/kamal/cli/build/port_forwarding.rb

@@ -1,14 +1,12 @@
 require "concurrent/atomic/count_down_latch"
 
 class Kamal::Cli::Build::PortForwarding
-  attr_reader :hosts, :port, :user, :proxy, :ssh_port
+  attr_reader :hosts, :port, :ssh_options
 
-  def initialize(hosts, port, user: nil, proxy: nil, ssh_port: nil)
+  def initialize(hosts, port, **ssh_options)
     @hosts = hosts
     @port = port
-    @user = user
-    @proxy = proxy
-    @ssh_port = ssh_port
+    @ssh_options = ssh_options
   end
 
   def forward
@@ -21,38 +19,48 @@ def forward
   end
 
   private
+    def stop
+      @done = true
+      @threads.to_a.each(&:join)
+    end
 
-  def stop
-    @done = true
-    @threads.to_a.each(&:join)
-  end
+    def forward_ports
+      ready = Concurrent::CountDownLatch.new(hosts.size)
 
-  def forward_ports
-    ready = Concurrent::CountDownLatch.new(hosts.size)
-
-    @threads = hosts.map do |host|
-      Thread.new do
-        Net::SSH.start(host, user, **{ port: ssh_port, proxy: proxy }.compact) do |ssh|
-          ssh.forward.remote(port, "localhost", port, "127.0.0.1") do |remote_port, bind_address|
-            if remote_port == :error
-              raise "Failed to establish port forward on #{host}"
-            else
-              ready.count_down
-            end
-          end
+      @threads = hosts.map do |host|
+        Thread.new do
+          begin
+            Net::SSH.start(host, ssh_options[:user], **ssh_options.except(:user)) do |ssh|
+              ssh.forward.remote(port, "localhost", port, "127.0.0.1") do |remote_port, bind_address|
+                if remote_port == :error
+                  raise "Failed to establish port forward on #{host}"
+                else
+                  ready.count_down
+                end
+              end
 
-          ssh.loop(0.1) do
-            if @done
-              ssh.forward.cancel_remote(port, "127.0.0.1")
-              break
-            else
-              true
+              ssh.loop(0.1) do
+                if @done
+                  ssh.forward.cancel_remote(port, "127.0.0.1")
+                  break
+                else
+                  true
+                end
+              end
             end
+          rescue Exception => e
+            error "Error setting up port forwarding to #{host}: #{e.class}: #{e.message}"
+            error e.backtrace.join("\n")
+
+            raise
           end
         end
       end
+
+      raise "Timed out waiting for port forwarding to be established" unless ready.wait(30)
     end
 
-    raise "Timed out waiting for port forwarding to be established" unless ready.wait(30)
-  end
+    def error(message)
+      SSHKit.config.output.error(message)
+    end
 end

test/cli/build_test.rb

@@ -428,8 +428,14 @@ class CliBuildTest < CliTestCase
     port_forwarding_mock = mock("port_forwarding")
     port_forwarding_mock.expects(:forward).yields
     Kamal::Cli::Build::PortForwarding.expects(:new)
-      .with([ "1.1.1.1", "1.1.1.2" ], 5000, user: "root", proxy: nil, ssh_port: nil)
-      .returns(port_forwarding_mock)
+      .with([ "1.1.1.1", "1.1.1.2" ], 5000, has_entries(
+        user: "root",
+        port: 22,
+        logger: instance_of(Logger),
+        keepalive: true,
+        keepalive_interval: 30
+      )
+    ).returns(port_forwarding_mock)
 
     run_command("pull", fixture: :with_local_registry).tap do |output|
       assert_match /docker pull localhost:5000\/dhh\/app:999/, output