Move docker login into build command

We only need to run the docker login commands for pushing and pulling
images.

So let's move the logins into those commands. This ensures we are logged
in when calling `kamal build` commands directly.

Fixes: https://github.com/basecamp/kamal/issues/919

lib/kamal/cli/build.rb

@@ -15,6 +15,8 @@ class Kamal::Cli::Build < Kamal::Cli::Base
     cli = self
 
     ensure_docker_installed
+    login_to_registry_locally
+
     run_hook "pre-build"
 
     uncommitted_changes = Kamal::Git.uncommitted_changes
@@ -61,6 +63,8 @@ class Kamal::Cli::Build < Kamal::Cli::Base
 
   desc "pull", "Pull app image from registry onto servers"
   def pull
+    login_to_registry_remotely
+
     if (first_hosts = mirror_hosts).any?
       #  Pull on a single host per mirror first to seed them
       say "Pulling image on #{first_hosts.join(", ")} to seed the #{"mirror".pluralize(first_hosts.count)}...", :magenta
@@ -181,4 +185,16 @@ class Kamal::Cli::Build < Kamal::Cli::Base
         execute *KAMAL.builder.validate_image
       end
     end
+
+    def login_to_registry_locally
+      run_locally do
+        execute *KAMAL.registry.login
+      end
+    end
+
+    def login_to_registry_remotely
+      on(KAMAL.hosts) do
+        execute *KAMAL.registry.login
+      end
+    end
 end

lib/kamal/cli/main.rb

@@ -20,9 +20,6 @@ class Kamal::Cli::Main < Kamal::Cli::Base
     runtime = print_runtime do
       invoke_options = deploy_options
 
-      say "Log into image registry...", :magenta
-      invoke "kamal:cli:registry:login", [], invoke_options.merge(skip_local: options[:skip_push])
-
       if options[:skip_push]
         say "Pull app image...", :magenta
         invoke "kamal:cli:build:pull", [], invoke_options
@@ -52,7 +49,7 @@ class Kamal::Cli::Main < Kamal::Cli::Base
     run_hook "post-deploy", secrets: true, runtime: runtime.round.to_s
   end
 
-  desc "redeploy", "Deploy app to servers without bootstrapping servers, starting kamal-proxy, pruning, and registry login"
+  desc "redeploy", "Deploy app to servers without bootstrapping servers, starting kamal-proxy and pruning"
   option :skip_push, aliases: "-P", type: :boolean, default: false, desc: "Skip image build and push"
   def redeploy
     runtime = print_runtime do

test/cli/build_test.rb

@@ -57,6 +57,7 @@ class CliBuildTest < CliTestCase
       stub_setup
 
       SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "--version", "&&", :docker, :buildx, "version")
+      SSHKit::Backend::Abstract.any_instance.stubs(:execute).with { |*args| args[0..1] == [ :docker, :login ] }
 
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
         .with(:git, "-C", "#{Dir.tmpdir}/kamal-clones/app-#{pwd_sha}", :clone, Dir.pwd, "--recurse-submodules")
@@ -103,6 +104,7 @@ class CliBuildTest < CliTestCase
       stub_setup
 
       SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:docker, "--version", "&&", :docker, :buildx, "version")
+      SSHKit::Backend::Abstract.any_instance.stubs(:execute).with { |*args| args[0..1] == [ :docker, :login ] }
 
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
         .with(:git, "-C", "#{Dir.tmpdir}/kamal-clones/app-#{pwd_sha}", :clone, Dir.pwd, "--recurse-submodules")
@@ -139,6 +141,9 @@ class CliBuildTest < CliTestCase
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
         .with(:docker, "--version", "&&", :docker, :buildx, "version")
 
+      SSHKit::Backend::Abstract.any_instance.stubs(:execute)
+        .with { |*args| args[0..1] == [ :docker, :login ] }
+
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
         .with(:docker, :buildx, :rm, "kamal-local-docker-container")
 

test/cli/main_test.rb

@@ -21,7 +21,6 @@ class CliMainTest < CliTestCase
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:server:bootstrap", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:accessory:boot", [ "all" ], invoke_options)
     # deploy
-    Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: true))
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:pull", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:proxy:boot", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true))
@@ -32,7 +31,6 @@ class CliMainTest < CliTestCase
       assert_match /Ensure Docker is installed.../, output
       # deploy
       assert_match /Acquiring the deploy lock/, output
-      assert_match /Log into image registry/, output
       assert_match /Pull app image/, output
       assert_match /Ensure kamal-proxy is running/, output
       assert_match /Detect stale containers/, output
@@ -45,8 +43,7 @@ class CliMainTest < CliTestCase
     with_test_secrets("secrets" => "DB_PASSWORD=secret") do
       invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, "verbose" => true }
 
-      Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false))
-      Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options)
+        Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options)
       Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:proxy:boot", [], invoke_options)
       Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true))
       Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:boot", [], invoke_options)
@@ -56,8 +53,7 @@ class CliMainTest < CliTestCase
 
       run_command("deploy", "--verbose").tap do |output|
         assert_hook_ran "pre-connect", output
-        assert_match /Log into image registry/, output
-        assert_match /Build and push app image/, output
+          assert_match /Build and push app image/, output
         assert_hook_ran "pre-deploy", output
         assert_match /Ensure kamal-proxy is running/, output
         assert_match /Detect stale containers/, output
@@ -70,7 +66,6 @@ class CliMainTest < CliTestCase
   test "deploy with skip_push" do
     invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false }
 
-    Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: true))
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:pull", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:proxy:boot", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true))
@@ -79,7 +74,6 @@ class CliMainTest < CliTestCase
 
     run_command("deploy", "--skip_push").tap do |output|
       assert_match /Acquiring the deploy lock/, output
-      assert_match /Log into image registry/, output
       assert_match /Pull app image/, output
       assert_match /Ensure kamal-proxy is running/, output
       assert_match /Detect stale containers/, output
@@ -153,11 +147,11 @@ class CliMainTest < CliTestCase
     end
   end
 
-  test "deploy errors during outside section leave remove lock" do
-    invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false, :skip_local => false }
+  test "deploy errors during outside section leave remote lock" do
+    invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => false }
 
     Kamal::Cli::Main.any_instance.expects(:invoke)
-      .with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false))
+      .with("kamal:cli:build:deliver", [], invoke_options)
       .raises(RuntimeError)
 
     assert_not KAMAL.holding_lock?
@@ -170,7 +164,6 @@ class CliMainTest < CliTestCase
   test "deploy with skipped hooks" do
     invoke_options = { "config_file" => "test/fixtures/deploy_simple.yml", "version" => "999", "skip_hooks" => true }
 
-    Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false))
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:proxy:boot", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true))
@@ -185,7 +178,6 @@ class CliMainTest < CliTestCase
   test "deploy with missing secrets" do
     invoke_options = { "config_file" => "test/fixtures/deploy_with_secrets.yml", "version" => "999", "skip_hooks" => false }
 
-    Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:registry:login", [], invoke_options.merge(skip_local: false))
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:build:deliver", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:proxy:boot", [], invoke_options)
     Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:app:stale_containers", [], invoke_options.merge(stop: true))