Merge pull request #755 from basecamp/lock-less

Lock less

lib/kamal/cli/accessory.rb

@@ -1,7 +1,7 @@
 class Kamal::Cli::Accessory < Kamal::Cli::Base
   desc "boot [NAME]", "Boot new accessory service on host (use NAME=all to boot all accessories)"
   def boot(name, login: true)
-    mutating do
+    with_lock do
       if name == "all"
         KAMAL.accessory_names.each { |accessory_name| boot(accessory_name) }
       else
@@ -21,7 +21,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "upload [NAME]", "Upload accessory files to host", hide: true
   def upload(name)
-    mutating do
+    with_lock do
       with_accessory(name) do |accessory, hosts|
         on(hosts) do
           accessory.files.each do |(local, remote)|
@@ -38,7 +38,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "directories [NAME]", "Create accessory directories on host", hide: true
   def directories(name)
-    mutating do
+    with_lock do
       with_accessory(name) do |accessory, hosts|
         on(hosts) do
           accessory.directories.keys.each do |host_path|
@@ -51,7 +51,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "reboot [NAME]", "Reboot existing accessory on host (stop container, remove container, start new container; use NAME=all to boot all accessories)"
   def reboot(name)
-    mutating do
+    with_lock do
       if name == "all"
         KAMAL.accessory_names.each { |accessory_name| reboot(accessory_name) }
       else
@@ -70,7 +70,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "start [NAME]", "Start existing accessory container on host"
   def start(name)
-    mutating do
+    with_lock do
       with_accessory(name) do |accessory, hosts|
         on(hosts) do
           execute *KAMAL.auditor.record("Started #{name} accessory"), verbosity: :debug
@@ -82,7 +82,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "stop [NAME]", "Stop existing accessory container on host"
   def stop(name)
-    mutating do
+    with_lock do
       with_accessory(name) do |accessory, hosts|
         on(hosts) do
           execute *KAMAL.auditor.record("Stopped #{name} accessory"), verbosity: :debug
@@ -94,7 +94,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "restart [NAME]", "Restart existing accessory container on host"
   def restart(name)
-    mutating do
+    with_lock do
       with_accessory(name) do
         stop(name)
         start(name)
@@ -174,17 +174,12 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
   desc "remove [NAME]", "Remove accessory container, image and data directory from host (use NAME=all to remove all accessories)"
   option :confirmed, aliases: "-y", type: :boolean, default: false, desc: "Proceed without confirmation question"
   def remove(name)
-    mutating do
+    confirming "This will remove all containers, images and data directories for #{name}. Are you sure?" do
-      if name == "all"
+      with_lock do
-        KAMAL.accessory_names.each { |accessory_name| remove(accessory_name) }
+        if name == "all"
-      else
+          KAMAL.accessory_names.each { |accessory_name| remove_accessory(accessory_name) }
-        confirming "This will remove all containers, images and data directories for #{name}. Are you sure?" do
+        else
-          with_accessory(name) do
+          remove_accessory(name)
-            stop(name)
-            remove_container(name)
-            remove_image(name)
-            remove_service_directory(name)
-          end
         end
       end
     end
@@ -192,7 +187,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "remove_container [NAME]", "Remove accessory container from host", hide: true
   def remove_container(name)
-    mutating do
+    with_lock do
       with_accessory(name) do |accessory, hosts|
         on(hosts) do
           execute *KAMAL.auditor.record("Remove #{name} accessory container"), verbosity: :debug
@@ -204,7 +199,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "remove_image [NAME]", "Remove accessory image from host", hide: true
   def remove_image(name)
-    mutating do
+    with_lock do
       with_accessory(name) do |accessory, hosts|
         on(hosts) do
           execute *KAMAL.auditor.record("Removed #{name} accessory image"), verbosity: :debug
@@ -216,7 +211,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
 
   desc "remove_service_directory [NAME]", "Remove accessory directory used for uploaded files and data directories from host", hide: true
   def remove_service_directory(name)
-    mutating do
+    with_lock do
       with_accessory(name) do |accessory, hosts|
         on(hosts) do
           execute *accessory.remove_service_directory
@@ -250,4 +245,13 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base
         accessory.hosts
       end
     end
+
+    def remove_accessory(name)
+      with_accessory(name) do
+        stop(name)
+        remove_container(name)
+        remove_image(name)
+        remove_service_directory(name)
+      end
+    end
 end

lib/kamal/cli/app.rb

@@ -1,33 +1,31 @@
 class Kamal::Cli::App < Kamal::Cli::Base
   desc "boot", "Boot app on servers (or reboot app if already running)"
   def boot
-    mutating do
+    with_lock do
-      hold_lock_on_error do
+      say "Get most recent version available as an image...", :magenta unless options[:version]
-        say "Get most recent version available as an image...", :magenta unless options[:version]
+      using_version(version_or_latest) do |version|
-        using_version(version_or_latest) do |version|
+        say "Start container with version #{version} using a #{KAMAL.config.readiness_delay}s readiness delay (or reboot if already running)...", :magenta
-          say "Start container with version #{version} using a #{KAMAL.config.readiness_delay}s readiness delay (or reboot if already running)...", :magenta
 
-          # Assets are prepared in a separate step to ensure they are on all hosts before booting
+        # Assets are prepared in a separate step to ensure they are on all hosts before booting
-          on(KAMAL.hosts) do
+        on(KAMAL.hosts) do
-            KAMAL.roles_on(host).each do |role|
+          KAMAL.roles_on(host).each do |role|
-              Kamal::Cli::App::PrepareAssets.new(host, role, self).run
+            Kamal::Cli::App::PrepareAssets.new(host, role, self).run
-            end
           end
+        end
 
-          #  Primary hosts and roles are returned first, so they can open the barrier
+        # Primary hosts and roles are returned first, so they can open the barrier
-          barrier = Kamal::Cli::Healthcheck::Barrier.new if KAMAL.roles.many?
+        barrier = Kamal::Cli::Healthcheck::Barrier.new if KAMAL.roles.many?
 
-          on(KAMAL.hosts, **KAMAL.boot_strategy) do |host|
+        on(KAMAL.hosts, **KAMAL.boot_strategy) do |host|
-            KAMAL.roles_on(host).each do |role|
+          KAMAL.roles_on(host).each do |role|
-              Kamal::Cli::App::Boot.new(host, role, self, version, barrier).run
+            Kamal::Cli::App::Boot.new(host, role, self, version, barrier).run
-            end
           end
+        end
 
-          #  Tag once the app booted on all hosts
+        # Tag once the app booted on all hosts
-          on(KAMAL.hosts) do |host|
+        on(KAMAL.hosts) do |host|
-            execute *KAMAL.auditor.record("Tagging #{KAMAL.config.absolute_image} as the latest image"), verbosity: :debug
+          execute *KAMAL.auditor.record("Tagging #{KAMAL.config.absolute_image} as the latest image"), verbosity: :debug
-            execute *KAMAL.app.tag_latest_image
+          execute *KAMAL.app.tag_latest_image
-          end
         end
       end
     end
@@ -35,7 +33,7 @@ class Kamal::Cli::App < Kamal::Cli::Base
 
   desc "start", "Start existing app container on servers"
   def start
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do |host|
         roles = KAMAL.roles_on(host)
 
@@ -49,7 +47,7 @@ class Kamal::Cli::App < Kamal::Cli::Base
 
   desc "stop", "Stop app container on servers"
   def stop
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do |host|
         roles = KAMAL.roles_on(host)
 
@@ -135,11 +133,9 @@ class Kamal::Cli::App < Kamal::Cli::Base
   desc "stale_containers", "Detect app stale containers"
   option :stop, aliases: "-s", type: :boolean, default: false, desc: "Stop the stale containers found"
   def stale_containers
-    mutating do
+    stop = options[:stop]
-      stop = options[:stop]
-
-      cli = self
 
+    with_lock_if_stopping do
       on(KAMAL.hosts) do |host|
         roles = KAMAL.roles_on(host)
 
@@ -208,7 +204,7 @@ class Kamal::Cli::App < Kamal::Cli::Base
 
   desc "remove", "Remove app containers and images from servers"
   def remove
-    mutating do
+    with_lock do
       stop
       remove_containers
       remove_images
@@ -217,7 +213,7 @@ class Kamal::Cli::App < Kamal::Cli::Base
 
   desc "remove_container [VERSION]", "Remove app container with given version from servers", hide: true
   def remove_container(version)
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do |host|
         roles = KAMAL.roles_on(host)
 
@@ -231,7 +227,7 @@ class Kamal::Cli::App < Kamal::Cli::Base
 
   desc "remove_containers", "Remove all app containers from servers", hide: true
   def remove_containers
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do |host|
         roles = KAMAL.roles_on(host)
 
@@ -245,7 +241,7 @@ class Kamal::Cli::App < Kamal::Cli::Base
 
   desc "remove_images", "Remove all app images from servers", hide: true
   def remove_images
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do
         execute *KAMAL.auditor.record("Removed all app images"), verbosity: :debug
         execute *KAMAL.app.remove_images
@@ -288,4 +284,12 @@ class Kamal::Cli::App < Kamal::Cli::Base
     def version_or_latest
       options[:version] || KAMAL.config.latest_tag
     end
+
+    def with_lock_if_stopping
+      if options[:stop]
+        with_lock { yield }
+      else
+        yield
+      end
+    end
 end

lib/kamal/cli/base.rb

@@ -79,28 +79,27 @@ module Kamal::Cli
         puts "  Finished all in #{sprintf("%.1f seconds", runtime)}"
       end
 
-      def mutating
+      def with_lock
-        return yield if KAMAL.holding_lock?
+        if KAMAL.holding_lock?
-
-        run_hook "pre-connect"
-
-        ensure_run_and_locks_directory
-
-        acquire_lock
-
-        begin
           yield
-        rescue
+        else
-          if KAMAL.hold_lock_on_error?
+          ensure_run_and_locks_directory
-            error "  \e[31mDeploy lock was not released\e[0m"
+
-          else
+          acquire_lock
-            release_lock
+
+          begin
+            yield
+          rescue
+            begin
+              release_lock
+            rescue => e
+              say "Error releasing the deploy lock: #{e.message}", :red
+            end
+            raise
           end
 
-          raise
+          release_lock
         end
-
-        release_lock
       end
 
       def confirming(question)
@@ -141,16 +140,6 @@ module Kamal::Cli
         end
       end
 
-      def hold_lock_on_error
-        if KAMAL.hold_lock_on_error?
-          yield
-        else
-          KAMAL.hold_lock_on_error = true
-          yield
-          KAMAL.hold_lock_on_error = false
-        end
-      end
-
       def run_hook(hook, **extra_details)
         if !options[:skip_hooks] && KAMAL.hook.hook_exists?(hook)
           details = { hosts: KAMAL.hosts.join(","), command: command, subcommand: subcommand }
@@ -164,6 +153,15 @@ module Kamal::Cli
         end
       end
 
+      def on(*args, &block)
+        if !KAMAL.connected?
+          run_hook "pre-connect"
+          KAMAL.connected = true
+        end
+
+        super
+      end
+
       def command
         @kamal_command ||= begin
           invocation_class, invocation_commands = *first_invocation

lib/kamal/cli/build.rb

@@ -5,52 +5,48 @@ class Kamal::Cli::Build < Kamal::Cli::Base
 
   desc "deliver", "Build app and push app image to registry then pull image on servers"
   def deliver
-    mutating do
+    push
-      push
+    pull
-      pull
-    end
   end
 
   desc "push", "Build and push app image to registry"
   def push
-    mutating do
+    cli = self
-      cli = self
 
-      verify_local_dependencies
+    verify_local_dependencies
-      run_hook "pre-build"
+    run_hook "pre-build"
 
-      uncommitted_changes = Kamal::Git.uncommitted_changes
+    uncommitted_changes = Kamal::Git.uncommitted_changes
 
-      if KAMAL.config.builder.git_clone?
+    if KAMAL.config.builder.git_clone?
-        if uncommitted_changes.present?
+      if uncommitted_changes.present?
-          say "Building from a local git clone, so ignoring these uncommitted changes:\n #{uncommitted_changes}", :yellow
+        say "Building from a local git clone, so ignoring these uncommitted changes:\n #{uncommitted_changes}", :yellow
-        end
-
-        prepare_clone
-      elsif uncommitted_changes.present?
-        say "Building with uncommitted changes:\n #{uncommitted_changes}", :yellow
       end
 
-      # Get the command here to ensure the Dir.chdir doesn't interfere with it
+      prepare_clone
-      push = KAMAL.builder.push
+    elsif uncommitted_changes.present?
+      say "Building with uncommitted changes:\n #{uncommitted_changes}", :yellow
+    end
 
-      run_locally do
+    # Get the command here to ensure the Dir.chdir doesn't interfere with it
-        begin
+    push = KAMAL.builder.push
-          KAMAL.with_verbosity(:debug) do
-            Dir.chdir(KAMAL.config.builder.build_directory) { execute *push }
-          end
-        rescue SSHKit::Command::Failed => e
-          if e.message =~ /(no builder)|(no such file or directory)/
-            warn "Missing compatible builder, so creating a new one first"
 
-            if cli.create
+    run_locally do
-              KAMAL.with_verbosity(:debug) do
+      begin
-                Dir.chdir(KAMAL.config.builder.build_directory) { execute *push }
+        KAMAL.with_verbosity(:debug) do
-              end
+          Dir.chdir(KAMAL.config.builder.build_directory) { execute *push }
+        end
+      rescue SSHKit::Command::Failed => e
+        if e.message =~ /(no builder)|(no such file or directory)/
+          warn "Missing compatible builder, so creating a new one first"
+
+          if cli.create
+            KAMAL.with_verbosity(:debug) do
+              Dir.chdir(KAMAL.config.builder.build_directory) { execute *push }
             end
-          else
-            raise
           end
+        else
+          raise
         end
       end
     end
@@ -58,34 +54,30 @@ class Kamal::Cli::Build < Kamal::Cli::Base
 
   desc "pull", "Pull app image from registry onto servers"
   def pull
-    mutating do
+    on(KAMAL.hosts) do
-      on(KAMAL.hosts) do
+      execute *KAMAL.auditor.record("Pulled image with version #{KAMAL.config.version}"), verbosity: :debug
-        execute *KAMAL.auditor.record("Pulled image with version #{KAMAL.config.version}"), verbosity: :debug
+      execute *KAMAL.builder.clean, raise_on_non_zero_exit: false
-        execute *KAMAL.builder.clean, raise_on_non_zero_exit: false
+      execute *KAMAL.builder.pull
-        execute *KAMAL.builder.pull
+      execute *KAMAL.builder.validate_image
-        execute *KAMAL.builder.validate_image
-      end
     end
   end
 
   desc "create", "Create a build setup"
   def create
-    mutating do
+    if (remote_host = KAMAL.config.builder.remote_host)
-      if (remote_host = KAMAL.config.builder.remote_host)
+      connect_to_remote_host(remote_host)
-        connect_to_remote_host(remote_host)
+    end
-      end
 
-      run_locally do
+    run_locally do
-        begin
+      begin
-          debug "Using builder: #{KAMAL.builder.name}"
+        debug "Using builder: #{KAMAL.builder.name}"
-          execute *KAMAL.builder.create
+        execute *KAMAL.builder.create
-        rescue SSHKit::Command::Failed => e
+      rescue SSHKit::Command::Failed => e
-          if e.message =~ /stderr=(.*)/
+        if e.message =~ /stderr=(.*)/
-            error "Couldn't create remote builder: #{$1}"
+          error "Couldn't create remote builder: #{$1}"
-            false
+          false
-          else
+        else
-            raise
+          raise
-          end
         end
       end
     end
@@ -93,11 +85,9 @@ class Kamal::Cli::Build < Kamal::Cli::Base
 
   desc "remove", "Remove build setup"
   def remove
-    mutating do
+    run_locally do
-      run_locally do
+      debug "Using builder: #{KAMAL.builder.name}"
-        debug "Using builder: #{KAMAL.builder.name}"
+      execute *KAMAL.builder.remove
-        execute *KAMAL.builder.remove
-      end
     end
   end
 

lib/kamal/cli/env.rb

@@ -3,7 +3,7 @@ require "tempfile"
 class Kamal::Cli::Env < Kamal::Cli::Base
   desc "push", "Push the env file to the remote hosts"
   def push
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do
         execute *KAMAL.auditor.record("Pushed env files"), verbosity: :debug
 
@@ -30,7 +30,7 @@ class Kamal::Cli::Env < Kamal::Cli::Base
 
   desc "delete", "Delete the env file from the remote hosts"
   def delete
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do
         execute *KAMAL.auditor.record("Deleted env files"), verbosity: :debug
 

lib/kamal/cli/main.rb

@@ -3,7 +3,7 @@ class Kamal::Cli::Main < Kamal::Cli::Base
   option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push"
   def setup
     print_runtime do
-      mutating do
+      with_lock do
         invoke_options = deploy_options
 
         say "Ensure Docker is installed...", :magenta
@@ -22,20 +22,20 @@ class Kamal::Cli::Main < Kamal::Cli::Base
   option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push"
   def deploy
     runtime = print_runtime do
-      mutating do
+      invoke_options = deploy_options
-        invoke_options = deploy_options
 
-        say "Log into image registry...", :magenta
+      say "Log into image registry...", :magenta
-        invoke "kamal:cli:registry:login", [], invoke_options
+      invoke "kamal:cli:registry:login", [], invoke_options
 
-        if options[:skip_push]
+      if options[:skip_push]
-          say "Pull app image...", :magenta
+        say "Pull app image...", :magenta
-          invoke "kamal:cli:build:pull", [], invoke_options
+        invoke "kamal:cli:build:pull", [], invoke_options
-        else
+      else
-          say "Build and push app image...", :magenta
+        say "Build and push app image...", :magenta
-          invoke "kamal:cli:build:deliver", [], invoke_options
+        invoke "kamal:cli:build:deliver", [], invoke_options
-        end
+      end
 
+      with_lock do
         run_hook "pre-deploy"
 
         say "Ensure Traefik is running...", :magenta
@@ -58,17 +58,17 @@ class Kamal::Cli::Main < Kamal::Cli::Base
   option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push"
   def redeploy
     runtime = print_runtime do
-      mutating do
+      invoke_options = deploy_options
-        invoke_options = deploy_options
 
-        if options[:skip_push]
+      if options[:skip_push]
-          say "Pull app image...", :magenta
+        say "Pull app image...", :magenta
-          invoke "kamal:cli:build:pull", [], invoke_options
+        invoke "kamal:cli:build:pull", [], invoke_options
-        else
+      else
-          say "Build and push app image...", :magenta
+        say "Build and push app image...", :magenta
-          invoke "kamal:cli:build:deliver", [], invoke_options
+        invoke "kamal:cli:build:deliver", [], invoke_options
-        end
+      end
 
+      with_lock do
         run_hook "pre-deploy"
 
         say "Detect stale containers...", :magenta
@@ -85,7 +85,7 @@ class Kamal::Cli::Main < Kamal::Cli::Base
   def rollback(version)
     rolled_back = false
     runtime = print_runtime do
-      mutating do
+      with_lock do
         invoke_options = deploy_options
 
         KAMAL.config.version = version
@@ -192,8 +192,8 @@ class Kamal::Cli::Main < Kamal::Cli::Base
   desc "remove", "Remove Traefik, app, accessories, and registry session from servers"
   option :confirmed, aliases: "-y", type: :boolean, default: false, desc: "Proceed without confirmation question"
   def remove
-    mutating do
+    confirming "This will remove all containers and images. Are you sure?" do
-      confirming "This will remove all containers and images. Are you sure?" do
+      with_lock do
         invoke "kamal:cli:traefik:remove", [], options.without(:confirmed)
         invoke "kamal:cli:app:remove", [], options.without(:confirmed)
         invoke "kamal:cli:accessory:remove", [ "all" ], options

lib/kamal/cli/prune.rb

@@ -1,7 +1,7 @@
 class Kamal::Cli::Prune < Kamal::Cli::Base
   desc "all", "Prune unused images and stopped containers"
   def all
-    mutating do
+    with_lock do
       containers
       images
     end
@@ -9,7 +9,7 @@ class Kamal::Cli::Prune < Kamal::Cli::Base
 
   desc "images", "Prune unused images"
   def images
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do
         execute *KAMAL.auditor.record("Pruned images"), verbosity: :debug
         execute *KAMAL.prune.dangling_images
@@ -24,7 +24,7 @@ class Kamal::Cli::Prune < Kamal::Cli::Base
     retain = options.fetch(:retain, KAMAL.config.retain_containers)
     raise "retain must be at least 1" if retain < 1
 
-    mutating do
+    with_lock do
       on(KAMAL.hosts) do
         execute *KAMAL.auditor.record("Pruned containers"), verbosity: :debug
         execute *KAMAL.prune.app_containers(retain: retain)

lib/kamal/cli/server.rb

@@ -23,25 +23,27 @@ class Kamal::Cli::Server < Kamal::Cli::Base
 
   desc "bootstrap", "Set up Docker to run Kamal apps"
   def bootstrap
-    missing = []
+    with_lock do
+      missing = []
 
-    on(KAMAL.hosts | KAMAL.accessory_hosts) do |host|
+      on(KAMAL.hosts | KAMAL.accessory_hosts) do |host|
-      unless execute(*KAMAL.docker.installed?, raise_on_non_zero_exit: false)
+        unless execute(*KAMAL.docker.installed?, raise_on_non_zero_exit: false)
-        if execute(*KAMAL.docker.superuser?, raise_on_non_zero_exit: false)
+          if execute(*KAMAL.docker.superuser?, raise_on_non_zero_exit: false)
-          info "Missing Docker on #{host}. Installing…"
+            info "Missing Docker on #{host}. Installing…"
-          execute *KAMAL.docker.install
+            execute *KAMAL.docker.install
-        else
+          else
-          missing << host
+            missing << host
+          end
         end
+
+        execute(*KAMAL.server.ensure_run_directory)
       end
 
-      execute(*KAMAL.server.ensure_run_directory)
+      if missing.any?
-    end
+        raise "Docker is not installed on #{missing.join(", ")} and can't be automatically installed without having root access and either `wget` or `curl`. Install Docker manually: https://docs.docker.com/engine/install/"
+      end
 
-    if missing.any?
+      run_hook "docker-setup"
-      raise "Docker is not installed on #{missing.join(", ")} and can't be automatically installed without having root access and either `wget` or `curl`. Install Docker manually: https://docs.docker.com/engine/install/"
     end
-
-    run_hook "docker-setup"
   end
 end

lib/kamal/cli/traefik.rb

@@ -1,7 +1,7 @@
 class Kamal::Cli::Traefik < Kamal::Cli::Base
   desc "boot", "Boot Traefik on servers"
   def boot
-    mutating do
+    with_lock do
       on(KAMAL.traefik_hosts) do
         execute *KAMAL.registry.login
         execute *KAMAL.traefik.start_or_run
@@ -14,7 +14,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base
   option :confirmed, aliases: "-y", type: :boolean, default: false, desc: "Proceed without confirmation question"
   def reboot
     confirming "This will cause a brief outage on each host. Are you sure?" do
-      mutating do
+      with_lock do
         host_groups = options[:rolling] ? KAMAL.traefik_hosts : [ KAMAL.traefik_hosts ]
         host_groups.each do |hosts|
           host_list = Array(hosts).join(",")
@@ -34,7 +34,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base
 
   desc "start", "Start existing Traefik container on servers"
   def start
-    mutating do
+    with_lock do
       on(KAMAL.traefik_hosts) do
         execute *KAMAL.auditor.record("Started traefik"), verbosity: :debug
         execute *KAMAL.traefik.start
@@ -44,7 +44,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base
 
   desc "stop", "Stop existing Traefik container on servers"
   def stop
-    mutating do
+    with_lock do
       on(KAMAL.traefik_hosts) do
         execute *KAMAL.auditor.record("Stopped traefik"), verbosity: :debug
         execute *KAMAL.traefik.stop, raise_on_non_zero_exit: false
@@ -54,7 +54,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base
 
   desc "restart", "Restart existing Traefik container on servers"
   def restart
-    mutating do
+    with_lock do
       stop
       start
     end
@@ -91,7 +91,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base
 
   desc "remove", "Remove Traefik container and image from servers"
   def remove
-    mutating do
+    with_lock do
       stop
       remove_container
       remove_image
@@ -100,7 +100,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base
 
   desc "remove_container", "Remove Traefik container from servers", hide: true
   def remove_container
-    mutating do
+    with_lock do
       on(KAMAL.traefik_hosts) do
         execute *KAMAL.auditor.record("Removed traefik container"), verbosity: :debug
         execute *KAMAL.traefik.remove_container
@@ -110,7 +110,7 @@ class Kamal::Cli::Traefik < Kamal::Cli::Base
 
   desc "remove_image", "Remove Traefik image from servers", hide: true
   def remove_image
-    mutating do
+    with_lock do
       on(KAMAL.traefik_hosts) do
         execute *KAMAL.auditor.record("Removed traefik image"), verbosity: :debug
         execute *KAMAL.traefik.remove_image

lib/kamal/commander.rb

@@ -2,13 +2,13 @@ require "active_support/core_ext/enumerable"
 require "active_support/core_ext/module/delegation"
 
 class Kamal::Commander
-  attr_accessor :verbosity, :holding_lock, :hold_lock_on_error
+  attr_accessor :verbosity, :holding_lock, :connected
   delegate :hosts, :roles, :primary_host, :primary_role, :roles_on, :traefik_hosts, :accessory_hosts, to: :specifics
 
   def initialize
     self.verbosity = :info
     self.holding_lock = false
-    self.hold_lock_on_error = false
+    self.connected = false
     @specifics = nil
   end
 
@@ -138,8 +138,8 @@ class Kamal::Commander
     self.holding_lock
   end
 
-  def hold_lock_on_error?
+  def connected?
-    self.hold_lock_on_error
+    self.connected
   end
 
   private

test/cli/app_test.rb

@@ -54,14 +54,14 @@ class CliAppTest < CliTestCase
     run_command("boot", config: :with_boot_strategy)
   end
 
-  test "boot errors leave lock in place" do
+  test "boot errors don't leave lock in place" do
     Kamal::Cli::App.any_instance.expects(:using_version).raises(RuntimeError)
 
     assert_not KAMAL.holding_lock?
     assert_raises(RuntimeError) do
       stderred { run_command("boot") }
     end
-    assert KAMAL.holding_lock?
+    assert_not KAMAL.holding_lock?
   end
 
   test "boot with assets" do

test/cli/main_test.rb

@@ -92,6 +92,9 @@ class CliMainTest < CliTestCase
   test "deploy when locked" do
     Thread.report_on_exception = false
 
+    SSHKit::Backend::Abstract.any_instance.stubs(:execute)
+    Dir.stubs(:chdir)
+
     SSHKit::Backend::Abstract.any_instance.stubs(:execute)
       .with { |*args| args == [ :mkdir, "-p", ".kamal" ] }
 
@@ -113,6 +116,9 @@ class CliMainTest < CliTestCase
   test "deploy error when locking" do
     Thread.report_on_exception = false
 
+    SSHKit::Backend::Abstract.any_instance.stubs(:execute)
+    Dir.stubs(:chdir)
+
     SSHKit::Backend::Abstract.any_instance.stubs(:execute)
       .with { |*args| args == [ :mkdir, "-p", ".kamal" ] }
 

test/cli/server_test.rb

@@ -16,13 +16,15 @@ class CliServerTest < CliTestCase
   end
 
   test "bootstrap already installed" do
+    stub_setup
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "-v", raise_on_non_zero_exit: false).returns(true).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:mkdir, "-p", ".kamal").returns("").at_least_once
 
-    assert_equal "", run_command("bootstrap")
+    assert_equal "Acquiring the deploy lock...\nReleasing the deploy lock...", run_command("bootstrap")
   end
 
   test "bootstrap install as non-root user" do
+    stub_setup
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "-v", raise_on_non_zero_exit: false).returns(false).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with('[ "${EUID:-$(id -u)}" -eq 0 ] || command -v sudo >/dev/null || command -v su >/dev/null', raise_on_non_zero_exit: false).returns(false).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:mkdir, "-p", ".kamal").returns("").at_least_once
@@ -33,11 +35,13 @@ class CliServerTest < CliTestCase
   end
 
   test "bootstrap install as root user" do
+    stub_setup
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "-v", raise_on_non_zero_exit: false).returns(false).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with('[ "${EUID:-$(id -u)}" -eq 0 ] || command -v sudo >/dev/null || command -v su >/dev/null', raise_on_non_zero_exit: false).returns(true).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:sh, "-c", "'curl -fsSL https://get.docker.com || wget -O - https://get.docker.com || echo \"exit 1\"'", "|", :sh).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:mkdir, "-p", ".kamal").returns("").at_least_once
     Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true)
+    SSHKit::Backend::Abstract.any_instance.expects(:execute).with(".kamal/hooks/pre-connect", anything).at_least_once
     SSHKit::Backend::Abstract.any_instance.expects(:execute).with(".kamal/hooks/docker-setup", anything).at_least_once
 
     run_command("bootstrap").tap do |output|