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 deployment barrier. Until one of the
primary role containers passes its healthcheck, we'll keep the barrier
up and avoid stopping the containers on the non-primary roles.

It the primary role container fails its healthcheck, we'll close the
barrier and shut down the new containers on the waiting 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.

Author: djmb

lib/kamal/cli/app.rb

@@ -14,9 +14,12 @@ def boot
             end
           end
 
+          barrier = Kamal::Cli::Healthcheck::Barrier.new if KAMAL.roles.many?
+
           on(KAMAL.hosts, **KAMAL.boot_strategy) do |host|
+            # Ensure primary role is 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, self, version, barrier).run
             end
           end
 
@@ -282,4 +285,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, :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, sshkit, version, barrier)
     @host = host
     @role = role
     @version = version
+    @barrier = 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_barrier
+    rescue => e
+      close_barrier if barrier_role?
+      execute *app.stop(version: version), raise_on_non_zero_exit: false
+
+      raise
     end
 
     def stop_old_version(version)
@@ -64,4 +62,45 @@ def stop_old_version(version)
 
       execute *app.clean_up_assets if assets?
     end
+
+    def reach_barrier
+      if barrier
+        if barrier_role?
+          if barrier.open
+            info "Opened barrier (#{host})"
+          end
+        else
+          wait_for_barrier
+        end
+      end
+    end
+
+    def wait_for_barrier
+      info "Waiting at web barrier (#{host})..."
+      barrier.wait
+      info "Barrier opened (#{host})"
+    rescue Kamal::Cli::Healthcheck::Error
+      info "Barrier closed, shutting down new container... (#{host})"
+      raise
+    end
+
+    def close_barrier
+      barrier&.close
+    end
+
+    def barrier_role?
+      role == KAMAL.primary_role
+    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,31 @@
+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)
+      true
+    rescue Concurrent::MultipleAssignmentError
+      false
+    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

@@ -150,6 +150,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