Simplified deploy/drain timeouts

Remove `stop_wait_time` and `readiness_timeout` from the root config
and remove `deploy_timeout` and `drain_timeout` from the proxy config.

Instead we'll just have `deploy_timeout` and `drain_timeout` in the
root config.

For roles that run the proxy, they are passed to the kamal-proxy deploy
command. Once that returns we can assume the container is ready to
shut down.

For other roles, we'll use the `deploy_timeout` when polling the
container to see if it is ready and the `drain_timeout` when stopping
the container.

lib/kamal/cli/healthcheck/poller.rb

@@ -3,7 +3,7 @@ module Kamal::Cli::Healthcheck::Poller
 
   def wait_for_healthy(role, &block)
     attempt = 1
-    timeout_at = Time.now + KAMAL.config.readiness_timeout
+    timeout_at = Time.now + KAMAL.config.deploy_timeout
     readiness_delay = KAMAL.config.readiness_delay
 
     begin
@@ -19,7 +19,7 @@ module Kamal::Cli::Healthcheck::Poller
       end
 
       unless %w[ running healthy ].include?(status)
-        raise Kamal::Cli::Healthcheck::Error, "container not ready after #{KAMAL.config.readiness_timeout} seconds (#{status})"
+        raise Kamal::Cli::Healthcheck::Error, "container not ready after #{KAMAL.config.deploy_timeout} seconds (#{status})"
       end
     rescue Kamal::Cli::Healthcheck::Error => e
       time_left = timeout_at - Time.now

lib/kamal/commands/app.rb

@@ -43,7 +43,7 @@ class Kamal::Commands::App < Kamal::Commands::Base
   def stop(version: nil)
     pipe \
       version ? container_id_for_version(version) : current_running_container_id,
-      xargs(config.stop_wait_time ? docker(:stop, "-t", config.stop_wait_time) : docker(:stop))
+      xargs(docker(:stop, *role.stop_args))
   end
 
   def info

lib/kamal/configuration.rb

@@ -6,7 +6,7 @@ require "erb"
 require "net/ssh/proxy/jump"
 
 class Kamal::Configuration
-  delegate :service, :image, :labels, :stop_wait_time, :hooks_path, to: :raw_config, allow_nil: true
+  delegate :service, :image, :labels, :hooks_path, to: :raw_config, allow_nil: true
   delegate :argumentize, :optionize, to: Kamal::Utils
 
   attr_reader :destination, :raw_config, :secrets
@@ -189,8 +189,12 @@ class Kamal::Configuration
     raw_config.readiness_delay || 7
   end
 
-  def readiness_timeout
-    raw_config.readiness_timeout || 30
+  def deploy_timeout
+    raw_config.deploy_timeout || 30
+  end
+
+  def drain_timeout
+    raw_config.drain_timeout || 30
   end
 
 

lib/kamal/configuration/docs/configuration.yml

@@ -93,11 +93,6 @@ primary_role: workers
 # Whether roles with no servers are allowed. Defaults to `false`.
 allow_empty_roles: false
 
-# Stop wait time
-#
-# How long we wait for a container to stop before killing it, defaults to 30 seconds
-stop_wait_time: 60
-
 # Retain containers
 #
 # How many old containers and images we retain, defaults to 5
@@ -114,11 +109,15 @@ minimum_version: 1.3.0
 # This only applies to containers that do not run a proxy or specify a healthcheck
 readiness_delay: 4
 
-# Readiness timeout
+# Deploy timeout
 #
 # How long to wait for a container to become ready, default 30
-# This only applies to containers that do not run a proxy
-readiness_timeout: 4
+deploy_timeout: 10
+
+# Drain timeout
+#
+# How long to wait for a containers to drain, default 30
+drain_timeout: 10
 
 # Run directory
 #

lib/kamal/configuration/docs/proxy.yml

@@ -38,11 +38,6 @@ proxy:
   # Defaults to false
   ssl: true
 
-  # Deploy timeout
-  #
-  # How long to wait for the app to boot when deploying, defaults to 30 seconds
-  deploy_timeout: 10s
-
   # Response timeout
   #
   # How long to wait for requests to complete before timing out, defaults to 30 seconds

lib/kamal/configuration/proxy.rb

@@ -39,12 +39,12 @@ class Kamal::Configuration::Proxy
     {
       host: proxy_config["host"],
       tls: proxy_config["ssl"],
-      "deploy-timeout": proxy_config["deploy_timeout"],
-      "drain-timeout": proxy_config["drain_timeout"],
-      "health-check-interval": proxy_config.dig("healthcheck", "interval"),
-      "health-check-timeout": proxy_config.dig("healthcheck", "timeout"),
+      "deploy-timeout": seconds_duration(config.deploy_timeout),
+      "drain-timeout": seconds_duration(config.drain_timeout),
+      "health-check-interval": seconds_duration(proxy_config.dig("healthcheck", "interval")),
+      "health-check-timeout": seconds_duration(proxy_config.dig("healthcheck", "timeout")),
       "health-check-path": proxy_config.dig("healthcheck", "path"),
-      "target-timeout": proxy_config["response_timeout"],
+      "target-timeout": seconds_duration(proxy_config["response_timeout"]),
       "buffer-requests": proxy_config.fetch("buffering", { "requests": true }).fetch("requests", true),
       "buffer-responses": proxy_config.fetch("buffering", { "responses": true }).fetch("responses", true),
       "buffer-memory": proxy_config.dig("buffering", "memory"),
@@ -68,4 +68,8 @@ class Kamal::Configuration::Proxy
 
   private
     attr_reader :config, :proxy_config
+
+    def seconds_duration(value)
+      value ? "#{value}s" : nil
+    end
 end

lib/kamal/configuration/role.rb

@@ -65,6 +65,12 @@ class Kamal::Configuration::Role
     @logging ||= config.logging.merge(specialized_logging)
   end
 
+  def stop_args
+    # When deploying with the proxy, kamal-proxy will drain request before returning so we don't need to wait.
+    timeout = running_proxy? ? nil : config.drain_timeout
+
+    [ *argumentize("-t", timeout) ]
+  end
 
   def env(host)
     @envs ||= {}