From 6c51e596ae8c6674d4a8d53a168cc55dbffd459e Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 16 Sep 2024 12:40:07 +0100 Subject: [PATCH] Put locks directories in .kamal so they leave no trace when deleted --- lib/kamal/cli/accessory.rb | 8 ++++---- lib/kamal/cli/app.rb | 2 +- lib/kamal/cli/base.rb | 10 +++------- lib/kamal/cli/lock.rb | 2 +- lib/kamal/cli/proxy.rb | 2 +- lib/kamal/commands/accessory.rb | 2 +- lib/kamal/commands/lock.rb | 8 ++------ lib/kamal/commands/server.rb | 4 ++-- test/cli/accessory_test.rb | 10 +++++----- test/cli/app_test.rb | 2 +- test/cli/cli_test_case.rb | 6 +++--- test/cli/lock_test.rb | 2 +- test/cli/main_test.rb | 14 ++++---------- test/cli/server_test.rb | 6 +++--- test/commands/lock_test.rb | 6 +++--- test/commands/server_test.rb | 4 ++-- 16 files changed, 37 insertions(+), 51 deletions(-) diff --git a/lib/kamal/cli/accessory.rb b/lib/kamal/cli/accessory.rb index e73c6f39..bc84fe52 100644 --- a/lib/kamal/cli/accessory.rb +++ b/lib/kamal/cli/accessory.rb @@ -207,12 +207,12 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base end end - desc "remove_app_directory [NAME]", "Remove accessory directory used for uploaded files and data directories from host", hide: true - def remove_app_directory(name) + desc "remove_service_directory [NAME]", "Remove accessory directory used for uploaded files and data directories from host", hide: true + def remove_service_directory(name) with_lock do with_accessory(name) do |accessory, hosts| on(hosts) do - execute *accessory.remove_app_directory + execute *accessory.remove_service_directory end end end @@ -248,7 +248,7 @@ class Kamal::Cli::Accessory < Kamal::Cli::Base stop(name) remove_container(name) remove_image(name) - remove_app_directory(name) + remove_service_directory(name) end def prepare(name) diff --git a/lib/kamal/cli/app.rb b/lib/kamal/cli/app.rb index 195b5ec7..2a02ed65 100644 --- a/lib/kamal/cli/app.rb +++ b/lib/kamal/cli/app.rb @@ -281,7 +281,7 @@ class Kamal::Cli::App < Kamal::Cli::Base roles.each do |role| execute *KAMAL.auditor.record("Removed #{KAMAL.config.app_directory} on all servers", role: role), verbosity: :debug - execute *KAMAL.server.remove_app_directory + execute *KAMAL.server.remove_app_directory, raise_on_non_zero_exit: false end end end diff --git a/lib/kamal/cli/base.rb b/lib/kamal/cli/base.rb index c5a3e376..b3af23cc 100644 --- a/lib/kamal/cli/base.rb +++ b/lib/kamal/cli/base.rb @@ -101,7 +101,7 @@ module Kamal::Cli end def acquire_lock - ensure_app_and_locks_directory + ensure_run_directory raise_if_locked do say "Acquiring the deploy lock...", :magenta @@ -174,13 +174,9 @@ module Kamal::Cli instance_variable_get("@_invocations").first end - def ensure_app_and_locks_directory + def ensure_run_directory on(KAMAL.hosts) do - execute(*KAMAL.server.ensure_app_directory) - end - - on(KAMAL.primary_host) do - execute(*KAMAL.lock.ensure_locks_directory) + execute(*KAMAL.server.ensure_run_directory) end end end diff --git a/lib/kamal/cli/lock.rb b/lib/kamal/cli/lock.rb index 08afb1f4..ceab1f27 100644 --- a/lib/kamal/cli/lock.rb +++ b/lib/kamal/cli/lock.rb @@ -12,7 +12,7 @@ class Kamal::Cli::Lock < Kamal::Cli::Base option :message, aliases: "-m", type: :string, desc: "A lock message", required: true def acquire message = options[:message] - ensure_app_and_locks_directory + ensure_run_directory raise_if_locked do on(KAMAL.primary_host) do diff --git a/lib/kamal/cli/proxy.rb b/lib/kamal/cli/proxy.rb index b3c47a49..3ac02c76 100644 --- a/lib/kamal/cli/proxy.rb +++ b/lib/kamal/cli/proxy.rb @@ -192,7 +192,7 @@ class Kamal::Cli::Proxy < Kamal::Cli::Base with_lock do on(KAMAL.proxy_hosts) do execute *KAMAL.auditor.record("Removed #{KAMAL.config.proxy_directory}"), verbosity: :debug - execute *KAMAL.proxy.remove_host_directory + execute *KAMAL.proxy.remove_host_directory, raise_on_non_zero_exit: false end end end diff --git a/lib/kamal/commands/accessory.rb b/lib/kamal/commands/accessory.rb index 22220d3f..787f7d43 100644 --- a/lib/kamal/commands/accessory.rb +++ b/lib/kamal/commands/accessory.rb @@ -90,7 +90,7 @@ class Kamal::Commands::Accessory < Kamal::Commands::Base end end - def remove_app_directory + def remove_service_directory [ :rm, "-rf", service_name ] end diff --git a/lib/kamal/commands/lock.rb b/lib/kamal/commands/lock.rb index 395b6f3a..aafaec70 100644 --- a/lib/kamal/commands/lock.rb +++ b/lib/kamal/commands/lock.rb @@ -44,14 +44,10 @@ class Kamal::Commands::Lock < Kamal::Commands::Base "/dev/null" end - def locks_dir - File.join(config.run_directory, "locks") - end - def lock_dir - dir_name = [ config.service, config.destination ].compact.join("-") + dir_name = [ "lock", config.service, config.destination ].compact.join("-") - File.join(locks_dir, dir_name) + File.join(config.run_directory, dir_name) end def lock_details_file diff --git a/lib/kamal/commands/server.rb b/lib/kamal/commands/server.rb index e8bf97e3..305903f6 100644 --- a/lib/kamal/commands/server.rb +++ b/lib/kamal/commands/server.rb @@ -1,6 +1,6 @@ class Kamal::Commands::Server < Kamal::Commands::Base - def ensure_app_directory - make_directory config.app_directory + def ensure_run_directory + make_directory config.run_directory end def remove_app_directory diff --git a/test/cli/accessory_test.rb b/test/cli/accessory_test.rb index 2dbff62d..5bb8762a 100644 --- a/test/cli/accessory_test.rb +++ b/test/cli/accessory_test.rb @@ -166,7 +166,7 @@ class CliAccessoryTest < CliTestCase Kamal::Cli::Accessory.any_instance.expects(:stop).with("mysql") Kamal::Cli::Accessory.any_instance.expects(:remove_container).with("mysql") Kamal::Cli::Accessory.any_instance.expects(:remove_image).with("mysql") - Kamal::Cli::Accessory.any_instance.expects(:remove_app_directory).with("mysql") + Kamal::Cli::Accessory.any_instance.expects(:remove_service_directory).with("mysql") run_command("remove", "mysql", "-y") end @@ -175,11 +175,11 @@ class CliAccessoryTest < CliTestCase Kamal::Cli::Accessory.any_instance.expects(:stop).with("mysql") Kamal::Cli::Accessory.any_instance.expects(:remove_container).with("mysql") Kamal::Cli::Accessory.any_instance.expects(:remove_image).with("mysql") - Kamal::Cli::Accessory.any_instance.expects(:remove_app_directory).with("mysql") + Kamal::Cli::Accessory.any_instance.expects(:remove_service_directory).with("mysql") Kamal::Cli::Accessory.any_instance.expects(:stop).with("redis") Kamal::Cli::Accessory.any_instance.expects(:remove_container).with("redis") Kamal::Cli::Accessory.any_instance.expects(:remove_image).with("redis") - Kamal::Cli::Accessory.any_instance.expects(:remove_app_directory).with("redis") + Kamal::Cli::Accessory.any_instance.expects(:remove_service_directory).with("redis") run_command("remove", "all", "-y") end @@ -192,8 +192,8 @@ class CliAccessoryTest < CliTestCase assert_match "docker image rm --force mysql", run_command("remove_image", "mysql") end - test "remove_app_directory" do - assert_match "rm -rf app-mysql", run_command("remove_app_directory", "mysql") + test "remove_service_directory" do + assert_match "rm -rf app-mysql", run_command("remove_service_directory", "mysql") end test "hosts param respected" do diff --git a/test/cli/app_test.rb b/test/cli/app_test.rb index 860078cf..f3bd9fe5 100644 --- a/test/cli/app_test.rb +++ b/test/cli/app_test.rb @@ -37,7 +37,7 @@ class CliAppTest < CliTestCase end test "boot uses group strategy when specified" do - Kamal::Cli::App.any_instance.stubs(:on).with("1.1.1.1").times(3) # ensure locks dir, acquire & release lock + Kamal::Cli::App.any_instance.stubs(:on).with("1.1.1.1").times(2) # ensure locks dir, acquire & release lock Kamal::Cli::App.any_instance.stubs(:on).with([ "1.1.1.1" ]) # tag container # Strategy is used when booting the containers diff --git a/test/cli/cli_test_case.rb b/test/cli/cli_test_case.rb index 2ffebd9c..5a2bb76f 100644 --- a/test/cli/cli_test_case.rb +++ b/test/cli/cli_test_case.rb @@ -31,11 +31,11 @@ class CliTestCase < ActiveSupport::TestCase SSHKit::Backend::Abstract.any_instance.stubs(:execute) .with { |*args| args == [ :mkdir, "-p", ".kamal/apps/app" ] } SSHKit::Backend::Abstract.any_instance.stubs(:execute) - .with { |arg1, arg2, arg3| arg1 == :mkdir && arg2 == "-p" && arg3 == ".kamal/locks" } + .with { |arg1, arg2, arg3| arg1 == :mkdir && arg2 == "-p" && arg3 == ".kamal/lock-app" } SSHKit::Backend::Abstract.any_instance.stubs(:execute) - .with { |arg1, arg2| arg1 == :mkdir && arg2 == ".kamal/locks/app" } + .with { |arg1, arg2| arg1 == :mkdir && arg2 == ".kamal/lock-app" } SSHKit::Backend::Abstract.any_instance.stubs(:execute) - .with { |arg1, arg2| arg1 == :rm && arg2 == ".kamal/locks/app/details" } + .with { |arg1, arg2| arg1 == :rm && arg2 == ".kamal/lock-app/details" } SSHKit::Backend::Abstract.any_instance.stubs(:execute) .with(:docker, :buildx, :inspect, "kamal-local-docker-container") end diff --git a/test/cli/lock_test.rb b/test/cli/lock_test.rb index f6874d49..66482153 100644 --- a/test/cli/lock_test.rb +++ b/test/cli/lock_test.rb @@ -3,7 +3,7 @@ require_relative "cli_test_case" class CliLockTest < CliTestCase test "status" do run_command("status").tap do |output| - assert_match "Running /usr/bin/env stat .kamal/locks/app > /dev/null && cat .kamal/locks/app/details | base64 -d on 1.1.1.1", output + assert_match "Running /usr/bin/env stat .kamal/lock-app > /dev/null && cat .kamal/lock-app/details | base64 -d on 1.1.1.1", output end end diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index 7f878e5b..607fee21 100644 --- a/test/cli/main_test.rb +++ b/test/cli/main_test.rb @@ -100,14 +100,11 @@ class CliMainTest < CliTestCase .with { |*args| args == [ :mkdir, "-p", ".kamal/apps/app" ] } SSHKit::Backend::Abstract.any_instance.stubs(:execute) - .with { |*args| args == [ :mkdir, "-p", ".kamal/locks" ] } - - SSHKit::Backend::Abstract.any_instance.stubs(:execute) - .with { |*arg| arg[0..1] == [ :mkdir, ".kamal/locks/app" ] } - .raises(RuntimeError, "mkdir: cannot create directory ‘kamal/locks/app’: File exists") + .with { |*arg| arg[0..1] == [ :mkdir, ".kamal/lock-app" ] } + .raises(RuntimeError, "mkdir: cannot create directory ‘kamal/lock-app’: File exists") SSHKit::Backend::Abstract.any_instance.expects(:capture_with_debug) - .with(:stat, ".kamal/locks/app", ">", "/dev/null", "&&", :cat, ".kamal/locks/app/details", "|", :base64, "-d") + .with(:stat, ".kamal/lock-app", ">", "/dev/null", "&&", :cat, ".kamal/lock-app/details", "|", :base64, "-d") SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) .with(:git, "-C", anything, :"rev-parse", :HEAD) @@ -137,10 +134,7 @@ class CliMainTest < CliTestCase .with { |*args| args == [ :mkdir, "-p", ".kamal/apps/app" ] } SSHKit::Backend::Abstract.any_instance.stubs(:execute) - .with { |*args| args == [ :mkdir, "-p", ".kamal/locks" ] } - - SSHKit::Backend::Abstract.any_instance.stubs(:execute) - .with { |*arg| arg[0..1] == [ :mkdir, ".kamal/locks/app" ] } + .with { |*arg| arg[0..1] == [ :mkdir, ".kamal/lock-app" ] } .raises(SocketError, "getaddrinfo: nodename nor servname provided, or not known") SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) diff --git a/test/cli/server_test.rb b/test/cli/server_test.rb index 0436a3ee..110e217d 100644 --- a/test/cli/server_test.rb +++ b/test/cli/server_test.rb @@ -32,7 +32,7 @@ class CliServerTest < CliTestCase 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/apps/app").returns("").at_least_once + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:mkdir, "-p", ".kamal").returns("").at_least_once assert_equal "Acquiring the deploy lock...\nReleasing the deploy lock...", run_command("bootstrap") end @@ -41,7 +41,7 @@ class CliServerTest < CliTestCase 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/apps/app").returns("").at_least_once + SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:mkdir, "-p", ".kamal").returns("").at_least_once assert_raise RuntimeError, "Docker is not installed on 1.1.1.1, 1.1.1.3, 1.1.1.4, 1.1.1.2 and can't be automatically installed without having root access and the `curl` command available. Install Docker manually: https://docs.docker.com/engine/install/" do run_command("bootstrap") @@ -53,7 +53,7 @@ class CliServerTest < CliTestCase 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/apps/app").returns("").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 diff --git a/test/commands/lock_test.rb b/test/commands/lock_test.rb index a8757251..0ad0ba46 100644 --- a/test/commands/lock_test.rb +++ b/test/commands/lock_test.rb @@ -10,19 +10,19 @@ class CommandsLockTest < ActiveSupport::TestCase test "status" do assert_equal \ - "stat .kamal/locks/app-production > /dev/null && cat .kamal/locks/app-production/details | base64 -d", + "stat .kamal/lock-app-production > /dev/null && cat .kamal/lock-app-production/details | base64 -d", new_command.status.join(" ") end test "acquire" do assert_match \ - %r{mkdir \.kamal/locks/app-production && echo ".*" > \.kamal/locks/app-production/details}m, + %r{mkdir \.kamal/lock-app-production && echo ".*" > \.kamal/lock-app-production/details}m, new_command.acquire("Hello", "123").join(" ") end test "release" do assert_match \ - "rm .kamal/locks/app-production/details && rm -r .kamal/locks/app-production", + "rm .kamal/lock-app-production/details && rm -r .kamal/lock-app-production", new_command.release.join(" ") end diff --git a/test/commands/server_test.rb b/test/commands/server_test.rb index 5db2ac59..648821b4 100644 --- a/test/commands/server_test.rb +++ b/test/commands/server_test.rb @@ -8,8 +8,8 @@ class CommandsServerTest < ActiveSupport::TestCase } end - test "ensure service directory" do - assert_equal "mkdir -p .kamal/apps/app", new_command.ensure_app_directory.join(" ") + test "ensure run directory" do + assert_equal "mkdir -p .kamal", new_command.ensure_run_directory.join(" ") end private