Remove the healthcheck step

To speed up deployments, we'll remove the healthcheck step.

This adds some risk to deployments for non-web roles - if they don't
have a Docker healthcheck configured then the only check we do is if
the container is running.

If there is a bad image we might see the container running before it
exits and deploy it. Previously the healthcheck step would have avoided
this by ensuring a web container could boot and serve traffic first.

To mitigate this, we'll add a web barrier. Non web containers will wait
before shutting down the old containers until at least one web
container has passed its healthcheck.

It the web container fails its healthcheck, we'll close the barrier and
shut down the new containers on the non-web roles.

We also have a new integration test to check we correctly handle a
a broken image. This highlighted that SSHKit's default runner will
stop at the first error it encounters. We'll now have a custom runner
that waits for all threads to finish allowing them to clean up.

Finally, we only tag an image as the latest after we have successfully
started the container and passed the web barrier, if applicable.

That means that if we have a deployment that completes on some hosts
but not others we can run `kamal app version --quiet` to see which
version is running on each host.

Author: djmb

lib/kamal/cli/app.rb

@@ -9,14 +9,19 @@ def boot
 
           # Assets are prepared in a separate step to ensure they are on all hosts before booting
           on(KAMAL.hosts) do
+            execute *KAMAL.auditor.record("Tagging #{KAMAL.config.absolute_image} as the latest image"), verbosity: :debug
+
             KAMAL.roles_on(host).each do |role|
               Kamal::Cli::App::PrepareAssets.new(host, role, self).run
             end
           end
 
+          web_barrier = Kamal::Cli::Healthcheck::Barrier.new if web_and_non_web_roles?
+
           on(KAMAL.hosts, **KAMAL.boot_strategy) do |host|
+            # Ensure web roles are booted first to allow the web barrier to be opened
             KAMAL.roles_on(host).each do |role|
-              Kamal::Cli::App::Boot.new(host, role, version, self).run
+              Kamal::Cli::App::Boot.new(host, role, version, web_barrier, self).run
             end
           end
 
@@ -282,4 +287,8 @@ def current_running_version(host: KAMAL.primary_host)
     def version_or_latest
       options[:version] || KAMAL.config.latest_tag
     end
+
+    def web_and_non_web_roles?
+      KAMAL.roles.any?(&:running_traefik?) && !KAMAL.roles.all?(&:running_traefik?)
+    end
 end

lib/kamal/cli/app/boot.rb

@@ -1,12 +1,13 @@
 class Kamal::Cli::App::Boot
-  attr_reader :host, :role, :version, :sshkit
+  attr_reader :host, :role, :version, :web_barrier, :sshkit
   delegate :execute, :capture_with_info, :info, to: :sshkit
-  delegate :uses_cord?, :assets?, to: :role
+  delegate :uses_cord?, :assets?, :running_traefik?, to: :role
 
-  def initialize(host, role, version, sshkit)
+  def initialize(host, role, version, web_barrier, sshkit)
     @host = host
     @role = role
     @version = version
+    @web_barrier = web_barrier
     @sshkit = sshkit
   end
 
@@ -21,18 +22,6 @@ def run
   end
 
   private
-    def app
-      @app ||= KAMAL.app(role: role)
-    end
-
-    def auditor
-      @auditor = KAMAL.auditor(role: role)
-    end
-
-    def audit(message)
-      execute *auditor.record(message), verbosity: :debug
-    end
-
     def old_version_renamed_if_clashing
       if capture_with_info(*app.container_id_for_version(version), raise_on_non_zero_exit: false).present?
         renamed_version = "#{version}_replaced_#{SecureRandom.hex(8)}"
@@ -46,9 +35,18 @@ def old_version_renamed_if_clashing
 
     def start_new_version
       audit "Booted app version #{version}"
+
       execute *app.tie_cord(role.cord_host_file) if uses_cord?
       execute *app.run(hostname: "#{host}-#{SecureRandom.hex(6)}")
+
       Kamal::Cli::Healthcheck::Poller.wait_for_healthy(pause_after_ready: true) { capture_with_info(*app.status(version: version)) }
+
+      reach_web_barrier
+    rescue => e
+      close_web_barrier if running_traefik?
+      execute *app.stop(version: version), raise_on_non_zero_exit: false
+
+      raise
     end
 
     def stop_old_version(version)
@@ -64,4 +62,41 @@ def stop_old_version(version)
 
       execute *app.clean_up_assets if assets?
     end
+
+    def reach_web_barrier
+      if web_barrier
+        if running_traefik?
+          web_barrier.open
+        else
+          wait_for_web_barrier
+        end
+      end
+    end
+
+    def wait_for_web_barrier
+      info "Waiting at web barrier (#{host})..."
+      web_barrier.wait
+      info "Barrier opened (#{host})"
+    rescue Kamal::Cli::Healthcheck::Error
+      info "Barrier closed, shutting down new container... (#{host})"
+      raise
+    end
+
+    def close_web_barrier
+      if web_barrier
+        web_barrier.close
+      end
+    end
+
+    def app
+      @app ||= KAMAL.app(role: role)
+    end
+
+    def auditor
+      @auditor = KAMAL.auditor(role: role)
+    end
+
+    def audit(message)
+      execute *auditor.record(message), verbosity: :debug
+    end
 end

lib/kamal/cli/healthcheck.rb

@@ -0,0 +1,29 @@
+class Kamal::Cli::Healthcheck::Barrier
+  def initialize
+    @ivar = Concurrent::IVar.new
+  end
+
+  def close
+    set(false)
+  end
+
+  def open
+    set(true)
+  end
+
+  def wait
+    unless opened?
+      raise Kamal::Cli::Healthcheck::Error.new("Halted at barrier")
+    end
+  end
+
+  private
+    def opened?
+      @ivar.value
+    end
+
+    def set(value)
+      @ivar.set(value)
+    rescue Concurrent::MultipleAssignmentError
+    end
+end

lib/kamal/cli/healthcheck/barrier.rb

@@ -0,0 +1,2 @@
+class Kamal::Cli::Healthcheck::Error < StandardError
+end

lib/kamal/cli/healthcheck/error.rb

@@ -3,7 +3,6 @@ module Kamal::Cli::Healthcheck::Poller
 
   TRAEFIK_UPDATE_DELAY = 5
 
-  class HealthcheckError < StandardError; end
 
   def wait_for_healthy(pause_after_ready: false, &block)
     attempt = 1
@@ -16,9 +15,9 @@ def wait_for_healthy(pause_after_ready: false, &block)
       when "running" # No health check configured
         sleep KAMAL.config.readiness_delay if pause_after_ready
       else
-        raise HealthcheckError, "container not ready (#{status})"
+        raise Kamal::Cli::Healthcheck::Error, "container not ready (#{status})"
       end
-    rescue HealthcheckError => e
+    rescue Kamal::Cli::Healthcheck::Error => e
       if attempt <= max_attempts
         info "#{e.message}, retrying in #{attempt}s (attempt #{attempt}/#{max_attempts})..."
         sleep attempt
@@ -41,9 +40,9 @@ def wait_for_unhealthy(pause_after_ready: false, &block)
       when "unhealthy"
         sleep TRAEFIK_UPDATE_DELAY if pause_after_ready
       else
-        raise HealthcheckError, "container not unhealthy (#{status})"
+        raise Kamal::Cli::Healthcheck::Error, "container not unhealthy (#{status})"
       end
-    rescue HealthcheckError => e
+    rescue Kamal::Cli::Healthcheck::Error => e
       if attempt <= max_attempts
         info "#{e.message}, retrying in #{attempt}s (attempt #{attempt}/#{max_attempts})..."
         sleep attempt

lib/kamal/cli/healthcheck/poller.rb

@@ -41,11 +41,6 @@ def deploy
         say "Ensure Traefik is running...", :magenta
         invoke "kamal:cli:traefik:boot", [], invoke_options
 
-        if KAMAL.config.role(KAMAL.config.primary_role).running_traefik?
-          say "Ensure app can pass healthcheck...", :magenta
-          invoke "kamal:cli:healthcheck:perform", [], invoke_options
-        end
-
         say "Detect stale containers...", :magenta
         invoke "kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true)
 
@@ -76,9 +71,6 @@ def redeploy
 
         run_hook "pre-deploy"
 
-        say "Ensure app can pass healthcheck...", :magenta
-        invoke "kamal:cli:healthcheck:perform", [], invoke_options
-
         say "Detect stale containers...", :magenta
         invoke "kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true)
 
@@ -223,9 +215,6 @@ def version
   desc "env", "Manage environment files"
   subcommand "env", Kamal::Cli::Env
 
-  desc "healthcheck", "Healthcheck application"
-  subcommand "healthcheck", Kamal::Cli::Healthcheck
-
   desc "lock", "Manage the deploy lock"
   subcommand "lock", Kamal::Cli::Lock
 

lib/kamal/cli/main.rb

@@ -61,9 +61,9 @@ def primary_role
   end
 
   def roles
-    (specific_roles || config.roles).select do |role|
-      ((specific_hosts || config.all_hosts) & role.hosts).any?
-    end
+    (specific_roles || config.roles) \
+      .select { |role| ((specific_hosts || config.all_hosts) & role.hosts).any? }
+      .sort_by { |role| role.running_traefik? ? 0 : 1 }
   end
 
   def hosts
@@ -178,6 +178,7 @@ def configure_sshkit_with(config)
         sshkit.max_concurrent_starts = config.sshkit.max_concurrent_starts
         sshkit.ssh_options = config.ssh.options
       end
+      SSHKit.config.default_runner = SSHKit::Runner::ParallelCompleteAll
       SSHKit.config.command_map[:docker] = "docker" # No need to use /usr/bin/env, just clogs up the logs
       SSHKit.config.output_verbosity = verbosity
     end

lib/kamal/commander.rb

@@ -188,7 +188,7 @@ def sshkit
 
 
   def healthcheck
-    { "path" => "/up", "port" => 3000, "max_attempts" => 7, "exposed_port" => 3999, "cord" => "/tmp/kamal-cord", "log_lines" => 50 }.merge(raw_config.healthcheck || {})
+    { "path" => "/up", "port" => 3000, "max_attempts" => 7, "cord" => "/tmp/kamal-cord", "log_lines" => 50 }.merge(raw_config.healthcheck || {})
   end
 
   def healthcheck_service