From dc9a95db2cafeef93f176e9c51b8cc752aa9703a Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Fri, 17 Jan 2025 14:58:27 +0000 Subject: [PATCH] Check for docker locally before registry login We were checking before `kamal build push`, but not `kamal registry login`. Since `kamal registry login` is called first by a deploy we don't get the nice error message. --- lib/kamal/cli.rb | 1 + lib/kamal/cli/base.rb | 14 ++++++++++++++ lib/kamal/cli/build.rb | 16 +--------------- lib/kamal/cli/registry.rb | 2 ++ lib/kamal/commands/base.rb | 14 ++++++++++++++ lib/kamal/commands/builder.rb | 20 -------------------- test/cli/build_test.rb | 2 +- test/cli/registry_test.rb | 10 ++++++++++ 8 files changed, 43 insertions(+), 36 deletions(-) diff --git a/lib/kamal/cli.rb b/lib/kamal/cli.rb index dc35c403..769b4a17 100644 --- a/lib/kamal/cli.rb +++ b/lib/kamal/cli.rb @@ -2,6 +2,7 @@ module Kamal::Cli class BootError < StandardError; end class HookError < StandardError; end class LockError < StandardError; end + class DependencyError < StandardError; end end # SSHKit uses instance eval, so we need a global const for ergonomics diff --git a/lib/kamal/cli/base.rb b/lib/kamal/cli/base.rb index 74f75ffc..8dde4752 100644 --- a/lib/kamal/cli/base.rb +++ b/lib/kamal/cli/base.rb @@ -195,5 +195,19 @@ module Kamal::Cli ENV.clear ENV.update(current_env) end + + def ensure_docker_installed + run_locally do + begin + execute *KAMAL.builder.ensure_docker_installed + rescue SSHKit::Command::Failed => e + error = e.message =~ /command not found/ ? + "Docker is not installed locally" : + "Docker buildx plugin is not installed locally" + + raise DependencyError, error + end + end + end end end diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 53ecb0bb..8897e2ae 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -13,7 +13,7 @@ class Kamal::Cli::Build < Kamal::Cli::Base def push cli = self - verify_local_dependencies + ensure_docker_installed run_hook "pre-build" uncommitted_changes = Kamal::Git.uncommitted_changes @@ -109,20 +109,6 @@ class Kamal::Cli::Build < Kamal::Cli::Base end private - def verify_local_dependencies - run_locally do - begin - execute *KAMAL.builder.ensure_local_dependencies_installed - rescue SSHKit::Command::Failed => e - build_error = e.message =~ /command not found/ ? - "Docker is not installed locally" : - "Docker buildx plugin is not installed locally" - - raise BuildError, build_error - end - end - end - def connect_to_remote_host(remote_host) remote_uri = URI.parse(remote_host) if remote_uri.scheme == "ssh" diff --git a/lib/kamal/cli/registry.rb b/lib/kamal/cli/registry.rb index 9d5d9d93..2fbdba1d 100644 --- a/lib/kamal/cli/registry.rb +++ b/lib/kamal/cli/registry.rb @@ -3,6 +3,8 @@ class Kamal::Cli::Registry < Kamal::Cli::Base option :skip_local, aliases: "-L", type: :boolean, default: false, desc: "Skip local login" option :skip_remote, aliases: "-R", type: :boolean, default: false, desc: "Skip remote login" def login + ensure_docker_installed + run_locally { execute *KAMAL.registry.login } unless options[:skip_local] on(KAMAL.hosts) { execute *KAMAL.registry.login } unless options[:skip_remote] end diff --git a/lib/kamal/commands/base.rb b/lib/kamal/commands/base.rb index 6d3f71ec..535d17c0 100644 --- a/lib/kamal/commands/base.rb +++ b/lib/kamal/commands/base.rb @@ -34,6 +34,12 @@ module Kamal::Commands [ :rm, path ] end + def ensure_docker_installed + combine \ + ensure_local_docker_installed, + ensure_local_buildx_installed + end + private def combine(*commands, by: "&&") commands @@ -104,5 +110,13 @@ module Kamal::Commands " -i #{key}" end end + + def ensure_local_docker_installed + docker "--version" + end + + def ensure_local_buildx_installed + docker :buildx, "version" + end end end diff --git a/lib/kamal/commands/builder.rb b/lib/kamal/commands/builder.rb index cd2980fb..a426b0ef 100644 --- a/lib/kamal/commands/builder.rb +++ b/lib/kamal/commands/builder.rb @@ -33,24 +33,4 @@ class Kamal::Commands::Builder < Kamal::Commands::Base def hybrid @hybrid ||= Kamal::Commands::Builder::Hybrid.new(config) end - - - def ensure_local_dependencies_installed - if name.native? - ensure_local_docker_installed - else - combine \ - ensure_local_docker_installed, - ensure_local_buildx_installed - end - end - - private - def ensure_local_docker_installed - docker "--version" - end - - def ensure_local_buildx_installed - docker :buildx, "version" - end end diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index 88f00743..60dfe036 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -155,7 +155,7 @@ class CliBuildTest < CliTestCase .raises(SSHKit::Command::Failed.new("no buildx")) Kamal::Commands::Builder.any_instance.stubs(:native_and_local?).returns(false) - assert_raises(Kamal::Cli::Build::BuildError) { run_command("push") } + assert_raises(Kamal::Cli::DependencyError) { run_command("push") } end test "push pre-build hook failure" do diff --git a/test/cli/registry_test.rb b/test/cli/registry_test.rb index c5423fe7..e89a15e4 100644 --- a/test/cli/registry_test.rb +++ b/test/cli/registry_test.rb @@ -43,6 +43,16 @@ class CliRegistryTest < CliTestCase end end + test "login with no docker" do + stub_setup + SSHKit::Backend::Abstract.any_instance.stubs(:execute) + .with(:docker, "--version", "&&", :docker, :buildx, "version") + .raises(SSHKit::Command::Failed.new("command not found")) + + assert_raises(Kamal::Cli::DependencyError) { run_command("login") } + end + + private def run_command(*command) stdouted { Kamal::Cli::Registry.start([ *command, "-c", "test/fixtures/deploy_with_accessories.yml" ]) }