From 6a7c90cf4d5f7b3532390c4ceec3afd09cce4de6 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 3 Apr 2024 12:38:10 +0100 Subject: [PATCH 1/9] Only stopping containers locks --- lib/kamal/cli/app.rb | 6 ++---- lib/kamal/cli/base.rb | 4 ++-- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/kamal/cli/app.rb b/lib/kamal/cli/app.rb index 523eb487..d28de3c4 100644 --- a/lib/kamal/cli/app.rb +++ b/lib/kamal/cli/app.rb @@ -135,11 +135,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] - - cli = self + stop = options[:stop] + mutating(mutates: stop) do on(KAMAL.hosts) do |host| roles = KAMAL.roles_on(host) diff --git a/lib/kamal/cli/base.rb b/lib/kamal/cli/base.rb index 206f63a4..1cee5484 100644 --- a/lib/kamal/cli/base.rb +++ b/lib/kamal/cli/base.rb @@ -79,8 +79,8 @@ module Kamal::Cli puts " Finished all in #{sprintf("%.1f seconds", runtime)}" end - def mutating - return yield if KAMAL.holding_lock? + def mutating(mutates: true) + return yield if KAMAL.holding_lock? || !mutates run_hook "pre-connect" From d2a719998af73993228a8852d508f513429e2d94 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 3 Apr 2024 12:39:40 +0100 Subject: [PATCH 2/9] Building doesn't need a deploy lock --- lib/kamal/cli/build.rb | 116 +++++++++++++++++++---------------------- 1 file changed, 54 insertions(+), 62 deletions(-) diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 1a50f8ff..0e0a2640 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -5,87 +5,81 @@ 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 - pull - end + push + pull end desc "push", "Build and push app image to registry" def push - mutating do - cli = self + cli = self - verify_local_dependencies - run_hook "pre-build" + verify_local_dependencies + run_hook "pre-build" - uncommitted_changes = Kamal::Git.uncommitted_changes + uncommitted_changes = Kamal::Git.uncommitted_changes - if KAMAL.config.builder.git_clone? - if uncommitted_changes.present? - 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 + if KAMAL.config.builder.git_clone? + if uncommitted_changes.present? + say "Building from a local git clone, so ignoring these uncommitted changes:\n #{uncommitted_changes}", :yellow end - # Get the command here to ensure the Dir.chdir doesn't interfere with it - push = KAMAL.builder.push + prepare_clone + elsif uncommitted_changes.present? + say "Building with uncommitted changes:\n #{uncommitted_changes}", :yellow + end - run_locally do - begin - 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" + # Get the command here to ensure the Dir.chdir doesn't interfere with it + push = KAMAL.builder.push - if cli.create - KAMAL.with_verbosity(:debug) do - Dir.chdir(KAMAL.config.builder.build_directory) { execute *push } - end - end - else - raise - end + run_locally do + begin + 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 + KAMAL.with_verbosity(:debug) do + Dir.chdir(KAMAL.config.builder.build_directory) { execute *push } + end + end + else + raise + end + else + raise end end end desc "pull", "Pull app image from registry onto servers" def pull - mutating do - on(KAMAL.hosts) do - 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.pull - execute *KAMAL.builder.validate_image - end + on(KAMAL.hosts) do + 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.pull + execute *KAMAL.builder.validate_image end end desc "create", "Create a build setup" def create - mutating do - if (remote_host = KAMAL.config.builder.remote_host) - connect_to_remote_host(remote_host) - end + if (remote_host = KAMAL.config.builder.remote_host) + connect_to_remote_host(remote_host) + end - run_locally do - begin - debug "Using builder: #{KAMAL.builder.name}" - execute *KAMAL.builder.create - rescue SSHKit::Command::Failed => e - if e.message =~ /stderr=(.*)/ - error "Couldn't create remote builder: #{$1}" - false - else - raise - end + run_locally do + begin + debug "Using builder: #{KAMAL.builder.name}" + execute *KAMAL.builder.create + rescue SSHKit::Command::Failed => e + if e.message =~ /stderr=(.*)/ + error "Couldn't create remote builder: #{$1}" + false + else + raise end end end @@ -93,11 +87,9 @@ class Kamal::Cli::Build < Kamal::Cli::Base desc "remove", "Remove build setup" def remove - mutating do - run_locally do - debug "Using builder: #{KAMAL.builder.name}" - execute *KAMAL.builder.remove - end + run_locally do + debug "Using builder: #{KAMAL.builder.name}" + execute *KAMAL.builder.remove end end From 64f5955444e7acf9b31b89444228576bced140b9 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 3 Apr 2024 12:45:11 +0100 Subject: [PATCH 3/9] Don't hold lock on error --- lib/kamal/cli/app.rb | 38 ++++++++++++++++++-------------------- lib/kamal/cli/base.rb | 17 +++-------------- lib/kamal/commander.rb | 7 +------ test/cli/app_test.rb | 4 ++-- 4 files changed, 24 insertions(+), 42 deletions(-) diff --git a/lib/kamal/cli/app.rb b/lib/kamal/cli/app.rb index d28de3c4..ac2b9e40 100644 --- a/lib/kamal/cli/app.rb +++ b/lib/kamal/cli/app.rb @@ -2,32 +2,30 @@ class Kamal::Cli::App < Kamal::Cli::Base desc "boot", "Boot app on servers (or reboot app if already running)" def boot mutating do - hold_lock_on_error do - say "Get most recent version available as an image...", :magenta unless options[: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 "Get most recent version available as an image...", :magenta unless options[: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 - # Assets are prepared in a separate step to ensure they are on all hosts before booting - on(KAMAL.hosts) do - KAMAL.roles_on(host).each do |role| - Kamal::Cli::App::PrepareAssets.new(host, role, self).run - end + # Assets are prepared in a separate step to ensure they are on all hosts before booting + on(KAMAL.hosts) do + KAMAL.roles_on(host).each do |role| + Kamal::Cli::App::PrepareAssets.new(host, role, self).run end + end - #  Primary hosts and roles are returned first, so they can open the barrier - barrier = Kamal::Cli::Healthcheck::Barrier.new if KAMAL.roles.many? + # Primary hosts and roles are returned first, so they can open the barrier + barrier = Kamal::Cli::Healthcheck::Barrier.new if KAMAL.roles.many? - on(KAMAL.hosts, **KAMAL.boot_strategy) do |host| - KAMAL.roles_on(host).each do |role| - Kamal::Cli::App::Boot.new(host, role, self, version, barrier).run - end + on(KAMAL.hosts, **KAMAL.boot_strategy) do |host| + KAMAL.roles_on(host).each do |role| + Kamal::Cli::App::Boot.new(host, role, self, version, barrier).run end + end - #  Tag once the app booted on all hosts - on(KAMAL.hosts) do |host| - execute *KAMAL.auditor.record("Tagging #{KAMAL.config.absolute_image} as the latest image"), verbosity: :debug - execute *KAMAL.app.tag_latest_image - end + # Tag once the app booted on all hosts + on(KAMAL.hosts) do |host| + execute *KAMAL.auditor.record("Tagging #{KAMAL.config.absolute_image} as the latest image"), verbosity: :debug + execute *KAMAL.app.tag_latest_image end end end diff --git a/lib/kamal/cli/base.rb b/lib/kamal/cli/base.rb index 1cee5484..d363a8ee 100644 --- a/lib/kamal/cli/base.rb +++ b/lib/kamal/cli/base.rb @@ -91,12 +91,11 @@ module Kamal::Cli begin yield rescue - if KAMAL.hold_lock_on_error? - error " \e[31mDeploy lock was not released\e[0m" - else + begin release_lock + rescue => e + say "Error releasing the deploy lock: #{e.message}", :red end - raise end @@ -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 } diff --git a/lib/kamal/commander.rb b/lib/kamal/commander.rb index e7c5d21f..9937cb69 100644 --- a/lib/kamal/commander.rb +++ b/lib/kamal/commander.rb @@ -2,13 +2,12 @@ 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 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 @specifics = nil end @@ -138,10 +137,6 @@ class Kamal::Commander self.holding_lock end - def hold_lock_on_error? - self.hold_lock_on_error - end - private # Lazy setup of SSHKit def configure_sshkit_with(config) diff --git a/test/cli/app_test.rb b/test/cli/app_test.rb index 7a2a266e..643f8d85 100644 --- a/test/cli/app_test.rb +++ b/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 From b12654ccd0da55ed6d74b6434e61048cdb33a185 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 20 May 2024 13:01:22 +0100 Subject: [PATCH 4/9] Don't lock until confirmed --- lib/kamal/cli/main.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/kamal/cli/main.rb b/lib/kamal/cli/main.rb index f17fd6cd..90e63aba 100644 --- a/lib/kamal/cli/main.rb +++ b/lib/kamal/cli/main.rb @@ -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 + mutating do invoke "kamal:cli:traefik:remove", [], options.without(:confirmed) invoke "kamal:cli:app:remove", [], options.without(:confirmed) invoke "kamal:cli:accessory:remove", [ "all" ], options From 96ef0fbc4d047c6c5ec27d3bea10b092732b0527 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 20 May 2024 14:12:14 +0100 Subject: [PATCH 5/9] Fix merge error --- lib/kamal/cli/build.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 0e0a2640..0af76b8c 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -48,8 +48,6 @@ class Kamal::Cli::Build < Kamal::Cli::Base else raise end - else - raise end end end From 83d0078525096af35353e9feb80cc61cf08c13e0 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 20 May 2024 14:12:26 +0100 Subject: [PATCH 6/9] Confirm outside mutating --- lib/kamal/cli/accessory.rb | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/lib/kamal/cli/accessory.rb b/lib/kamal/cli/accessory.rb index 4c695a28..8a4f6143 100644 --- a/lib/kamal/cli/accessory.rb +++ b/lib/kamal/cli/accessory.rb @@ -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 - if name == "all" - KAMAL.accessory_names.each { |accessory_name| remove(accessory_name) } - else - confirming "This will remove all containers, images and data directories for #{name}. Are you sure?" do - with_accessory(name) do - stop(name) - remove_container(name) - remove_image(name) - remove_service_directory(name) - end + confirming "This will remove all containers, images and data directories for #{name}. Are you sure?" do + mutating do + if name == "all" + KAMAL.accessory_names.each { |accessory_name| remove_accessory(accessory_name) } + else + remove_accessory(name) end end end @@ -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 From 0e73f027436059ee92c0d30223d5f5da3e7dbd76 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 20 May 2024 15:03:38 +0100 Subject: [PATCH 7/9] Split lock and connection setup Allow run the pre-connect hook before the first SSH command is executed, but only run the locking in `with_lock` blocks. --- lib/kamal/cli/accessory.rb | 22 ++++++++--------- lib/kamal/cli/app.rb | 24 ++++++++++++------- lib/kamal/cli/base.rb | 47 ++++++++++++++++++++++--------------- lib/kamal/cli/env.rb | 4 ++-- lib/kamal/cli/main.rb | 48 +++++++++++++++++++------------------- lib/kamal/cli/prune.rb | 6 ++--- lib/kamal/cli/server.rb | 30 +++++++++++++----------- lib/kamal/cli/traefik.rb | 16 ++++++------- lib/kamal/commander.rb | 7 +++++- test/cli/main_test.rb | 6 +++++ test/cli/server_test.rb | 6 ++++- 11 files changed, 125 insertions(+), 91 deletions(-) diff --git a/lib/kamal/cli/accessory.rb b/lib/kamal/cli/accessory.rb index 8a4f6143..e1d31b77 100644 --- a/lib/kamal/cli/accessory.rb +++ b/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) @@ -175,7 +175,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base option :confirmed, aliases: "-y", type: :boolean, default: false, desc: "Proceed without confirmation question" def remove(name) confirming "This will remove all containers, images and data directories for #{name}. Are you sure?" do - mutating do + with_lock do if name == "all" KAMAL.accessory_names.each { |accessory_name| remove_accessory(accessory_name) } else @@ -187,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 @@ -199,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 @@ -211,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 diff --git a/lib/kamal/cli/app.rb b/lib/kamal/cli/app.rb index ac2b9e40..ee8ab901 100644 --- a/lib/kamal/cli/app.rb +++ b/lib/kamal/cli/app.rb @@ -1,7 +1,7 @@ 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 say "Get most recent version available as an image...", :magenta unless options[: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 @@ -33,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) @@ -47,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,7 +135,7 @@ class Kamal::Cli::App < Kamal::Cli::Base def stale_containers stop = options[:stop] - mutating(mutates: stop) do + with_lock_if_stopping do on(KAMAL.hosts) do |host| roles = KAMAL.roles_on(host) @@ -204,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 @@ -213,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) @@ -227,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) @@ -241,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 @@ -284,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 diff --git a/lib/kamal/cli/base.rb b/lib/kamal/cli/base.rb index d363a8ee..e648281f 100644 --- a/lib/kamal/cli/base.rb +++ b/lib/kamal/cli/base.rb @@ -79,27 +79,27 @@ module Kamal::Cli puts " Finished all in #{sprintf("%.1f seconds", runtime)}" end - def mutating(mutates: true) - return yield if KAMAL.holding_lock? || !mutates - - run_hook "pre-connect" - - ensure_run_and_locks_directory - - acquire_lock - - begin + def with_lock + if KAMAL.holding_lock? yield - rescue - begin - release_lock - rescue => e - say "Error releasing the deploy lock: #{e.message}", :red - end - raise - end + else + ensure_run_and_locks_directory - release_lock + acquire_lock + + begin + yield + rescue + begin + release_lock + rescue => e + say "Error releasing the deploy lock: #{e.message}", :red + end + raise + end + + release_lock + end end def confirming(question) @@ -153,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 diff --git a/lib/kamal/cli/env.rb b/lib/kamal/cli/env.rb index 56ba505a..f12174a7 100644 --- a/lib/kamal/cli/env.rb +++ b/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 diff --git a/lib/kamal/cli/main.rb b/lib/kamal/cli/main.rb index 90e63aba..2816ab08 100644 --- a/lib/kamal/cli/main.rb +++ b/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,22 +22,22 @@ 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 - invoke "kamal:cli:registry:login", [], invoke_options + say "Log into image registry...", :magenta + invoke "kamal:cli:registry:login", [], invoke_options - if options[:skip_push] - say "Pull app image...", :magenta - invoke "kamal:cli:build:pull", [], invoke_options - else - say "Build and push app image...", :magenta - invoke "kamal:cli:build:deliver", [], invoke_options - end + if options[:skip_push] + say "Pull app image...", :magenta + invoke "kamal:cli:build:pull", [], invoke_options + else + say "Build and push app image...", :magenta + invoke "kamal:cli:build:deliver", [], invoke_options + end - run_hook "pre-deploy" + run_hook "pre-deploy" + with_lock do say "Ensure Traefik is running...", :magenta invoke "kamal:cli:traefik:boot", [], invoke_options @@ -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] - say "Pull app image...", :magenta - invoke "kamal:cli:build:pull", [], invoke_options - else - say "Build and push app image...", :magenta - invoke "kamal:cli:build:deliver", [], invoke_options - end + if options[:skip_push] + say "Pull app image...", :magenta + invoke "kamal:cli:build:pull", [], invoke_options + else + say "Build and push app image...", :magenta + invoke "kamal:cli:build:deliver", [], invoke_options + 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 @@ -193,7 +193,7 @@ class Kamal::Cli::Main < Kamal::Cli::Base option :confirmed, aliases: "-y", type: :boolean, default: false, desc: "Proceed without confirmation question" def remove confirming "This will remove all containers and images. Are you sure?" do - mutating 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 diff --git a/lib/kamal/cli/prune.rb b/lib/kamal/cli/prune.rb index 498e4ec4..7635e97d 100644 --- a/lib/kamal/cli/prune.rb +++ b/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) diff --git a/lib/kamal/cli/server.rb b/lib/kamal/cli/server.rb index fe0ae62f..b545050f 100644 --- a/lib/kamal/cli/server.rb +++ b/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| - unless execute(*KAMAL.docker.installed?, raise_on_non_zero_exit: false) - if execute(*KAMAL.docker.superuser?, raise_on_non_zero_exit: false) - info "Missing Docker on #{host}. Installing…" - execute *KAMAL.docker.install - else - missing << host + on(KAMAL.hosts | KAMAL.accessory_hosts) do |host| + unless execute(*KAMAL.docker.installed?, raise_on_non_zero_exit: false) + if execute(*KAMAL.docker.superuser?, raise_on_non_zero_exit: false) + info "Missing Docker on #{host}. Installing…" + execute *KAMAL.docker.install + else + missing << host + end end + + execute(*KAMAL.server.ensure_run_directory) end - execute(*KAMAL.server.ensure_run_directory) - end + if missing.any? + 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? - 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/" + run_hook "docker-setup" end - - run_hook "docker-setup" end end diff --git a/lib/kamal/cli/traefik.rb b/lib/kamal/cli/traefik.rb index c13e2d80..d192ee1a 100644 --- a/lib/kamal/cli/traefik.rb +++ b/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 diff --git a/lib/kamal/commander.rb b/lib/kamal/commander.rb index 9937cb69..c28fda82 100644 --- a/lib/kamal/commander.rb +++ b/lib/kamal/commander.rb @@ -2,12 +2,13 @@ require "active_support/core_ext/enumerable" require "active_support/core_ext/module/delegation" class Kamal::Commander - attr_accessor :verbosity, :holding_lock + 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.connected = false @specifics = nil end @@ -137,6 +138,10 @@ class Kamal::Commander self.holding_lock end + def connected? + self.connected + end + private # Lazy setup of SSHKit def configure_sshkit_with(config) diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index 358a7394..f7e479d7 100644 --- a/test/cli/main_test.rb +++ b/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" ] } diff --git a/test/cli/server_test.rb b/test/cli/server_test.rb index c3dbb0bb..5d9fec4d 100644 --- a/test/cli/server_test.rb +++ b/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| From 5ff1203c80a3c7c51c4650a5a9131bf13eb15fd1 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 20 May 2024 15:17:27 +0100 Subject: [PATCH 8/9] Always lock before pre-deploy hook --- lib/kamal/cli/main.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/kamal/cli/main.rb b/lib/kamal/cli/main.rb index 2816ab08..c8641848 100644 --- a/lib/kamal/cli/main.rb +++ b/lib/kamal/cli/main.rb @@ -35,9 +35,9 @@ class Kamal::Cli::Main < Kamal::Cli::Base invoke "kamal:cli:build:deliver", [], invoke_options end - run_hook "pre-deploy" - with_lock do + run_hook "pre-deploy" + say "Ensure Traefik is running...", :magenta invoke "kamal:cli:traefik:boot", [], invoke_options From 187861fa60b4a027dcc6d3fe3e485bb8a678c2db Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Tue, 21 May 2024 12:20:19 +0100 Subject: [PATCH 9/9] Space not tab --- lib/kamal/cli/app.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/kamal/cli/app.rb b/lib/kamal/cli/app.rb index ee8ab901..809607a9 100644 --- a/lib/kamal/cli/app.rb +++ b/lib/kamal/cli/app.rb @@ -13,7 +13,7 @@ class Kamal::Cli::App < Kamal::Cli::Base 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? on(KAMAL.hosts, **KAMAL.boot_strategy) do |host|