Merge pull request #921 from basecamp/remote-hybrid-builders-cleanup

Build and clean remote builders correctly

lib/kamal/cli/build.rb

@@ -32,10 +32,15 @@ def push
 
     run_locally do
       begin
-        execute *KAMAL.builder.buildx_inspect
+        execute *KAMAL.builder.inspect_builder
       rescue SSHKit::Command::Failed => e
-        if e.message =~ /(context not found|no builder|does not exist)/
+        if e.message =~ /(context not found|no builder|no compatible builder|does not exist)/
           warn "Missing compatible builder, so creating a new one first"
+          begin
+            cli.remove
+          rescue SSHKit::Command::Failed
+            raise unless e.message =~ /(context not found|no builder|does not exist)/
+          end
           cli.create
         else
           raise

lib/kamal/commands/base.rb

@@ -81,6 +81,10 @@ def git(*args, path: nil)
         [ :git, *([ "-C", path ] if path), *args.compact ]
       end
 
+      def grep(*args)
+        args.compact.unshift :grep
+      end
+
       def tags(**details)
         Kamal::Tags.from_config(config, **details)
       end

lib/kamal/commands/builder.rb

@@ -1,7 +1,7 @@
 require "active_support/core_ext/string/filters"
 
 class Kamal::Commands::Builder < Kamal::Commands::Base
-  delegate :create, :remove, :push, :clean, :pull, :info, :buildx_inspect, :validate_image, :first_mirror, to: :target
+  delegate :create, :remove, :push, :clean, :pull, :info, :inspect_builder, :validate_image, :first_mirror, to: :target
   delegate :local?, :remote?, to: "config.builder"
 
   include Clone

lib/kamal/commands/builder/base.rb

@@ -32,7 +32,7 @@ def info
       docker(:buildx, :ls)
   end
 
-  def buildx_inspect
+  def inspect_builder
     docker :buildx, :inspect, builder_name unless docker_driver?
   end
 
@@ -101,10 +101,6 @@ def builder_config
       config.builder
     end
 
-    def context_host(builder_name)
-      docker :context, :inspect, builder_name, "--format", ENDPOINT_DOCKER_HOST_INSPECT
-    end
-
     def platform_options(arches)
       argumentize "--platform", arches.map { |arch| "linux/#{arch}" }.join(",") if arches.any?
     end

lib/kamal/commands/builder/hybrid.rb

@@ -16,6 +16,6 @@ def create_local_buildx
     end
 
     def append_remote_buildx
-      docker :buildx, :create, *platform_options(remote_arches), "--append", "--name", builder_name, builder_name
+      docker :buildx, :create, *platform_options(remote_arches), "--append", "--name", builder_name, remote_context_name
     end
 end

lib/kamal/commands/builder/remote.rb

@@ -17,21 +17,44 @@ def info
       docker(:buildx, :ls)
   end
 
+  def inspect_builder
+    combine \
+      combine inspect_buildx, inspect_remote_context,
+      [ "(echo no compatible builder && exit 1)" ],
+      by: "||"
+  end
+
   private
     def builder_name
-      "kamal-remote-#{driver}-#{remote.gsub(/[^a-z0-9_-]/, "-")}"
+      "kamal-remote-#{remote.gsub(/[^a-z0-9_-]/, "-")}"
+    end
+
+    def remote_context_name
+      "#{builder_name}-context"
+    end
+
+    def inspect_buildx
+      pipe \
+        docker(:buildx, :inspect, builder_name),
+        grep("-q", "Endpoint:.*#{remote_context_name}")
+    end
+
+    def inspect_remote_context
+      pipe \
+        docker(:context, :inspect, remote_context_name, "--format", ENDPOINT_DOCKER_HOST_INSPECT),
+        grep("-xq", remote)
     end
 
     def create_remote_context
-      docker :context, :create, builder_name, "--description", "'#{builder_name} host'", "--docker", "'host=#{remote}'"
+      docker :context, :create, remote_context_name, "--description", "'#{builder_name} host'", "--docker", "'host=#{remote}'"
     end
 
     def remove_remote_context
-      docker :context, :rm, builder_name
+      docker :context, :rm, remote_context_name
     end
 
     def create_buildx
-      docker :buildx, :create, "--name", builder_name, builder_name
+      docker :buildx, :create, "--name", builder_name, remote_context_name
     end
 
     def remove_buildx

lib/kamal/commands/prune.rb

@@ -9,7 +9,7 @@ def dangling_images
   def tagged_images
     pipe \
       docker(:image, :ls, *service_filter, "--format", "'{{.ID}} {{.Repository}}:{{.Tag}}'"),
-      "grep -v -w \"#{active_image_list}\"",
+      grep("-v -w \"#{active_image_list}\""),
       "while read image tag; do docker rmi $tag; done"
   end
 

test/cli/build_test.rb

@@ -119,6 +119,9 @@ class CliBuildTest < CliTestCase
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
         .with(:docker, "--version", "&&", :docker, :buildx, "version")
 
+      SSHKit::Backend::Abstract.any_instance.expects(:execute)
+        .with(:docker, :buildx, :rm, "kamal-local-docker-container")
+
       SSHKit::Backend::Abstract.any_instance.expects(:execute)
         .with(:docker, :buildx, :create, "--name", "kamal-local-docker-container", "--driver=docker-container")
 
@@ -210,16 +213,25 @@ class CliBuildTest < CliTestCase
   test "create remote" do
     run_command("create", fixture: :with_remote_builder).tap do |output|
       assert_match "Running /usr/bin/env true on 1.1.1.5", output
-      assert_match "docker context create kamal-remote-docker-container-ssh---app-1-1-1-5 --description 'kamal-remote-docker-container-ssh---app-1-1-1-5 host' --docker 'host=ssh://app@1.1.1.5'", output
-      assert_match "docker buildx create --name kamal-remote-docker-container-ssh---app-1-1-1-5 kamal-remote-docker-container-ssh---app-1-1-1-5", output
+      assert_match "docker context create kamal-remote-ssh---app-1-1-1-5-context --description 'kamal-remote-ssh---app-1-1-1-5 host' --docker 'host=ssh://app@1.1.1.5'", output
+      assert_match "docker buildx create --name kamal-remote-ssh---app-1-1-1-5 kamal-remote-ssh---app-1-1-1-5-context", output
     end
   end
 
   test "create remote with custom ports" do
     run_command("create", fixture: :with_remote_builder_and_custom_ports).tap do |output|
       assert_match "Running /usr/bin/env true on 1.1.1.5", output
-      assert_match "docker context create kamal-remote-docker-container-ssh---app-1-1-1-5-2122 --description 'kamal-remote-docker-container-ssh---app-1-1-1-5-2122 host' --docker 'host=ssh://app@1.1.1.5:2122'", output
-      assert_match "docker buildx create --name kamal-remote-docker-container-ssh---app-1-1-1-5-2122 kamal-remote-docker-container-ssh---app-1-1-1-5-2122", output
+      assert_match "docker context create kamal-remote-ssh---app-1-1-1-5-2122-context --description 'kamal-remote-ssh---app-1-1-1-5-2122 host' --docker 'host=ssh://app@1.1.1.5:2122'", output
+      assert_match "docker buildx create --name kamal-remote-ssh---app-1-1-1-5-2122 kamal-remote-ssh---app-1-1-1-5-2122-context", output
+    end
+  end
+
+  test "create hybrid" do
+    run_command("create", fixture: :with_hybrid_builder).tap do |output|
+      assert_match "Running /usr/bin/env true on 1.1.1.5", output
+      assert_match "docker buildx create --platform linux/#{Kamal::Utils.docker_arch} --name kamal-hybrid-docker-container-ssh---app-1-1-1-5 --driver=docker-container", output
+      assert_match "docker context create kamal-hybrid-docker-container-ssh---app-1-1-1-5-context --description 'kamal-hybrid-docker-container-ssh---app-1-1-1-5 host' --docker 'host=ssh://app@1.1.1.5'", output
+      assert_match "docker buildx create --platform linux/#{Kamal::Utils.docker_arch == "amd64" ? "arm64" : "amd64"} --append --name kamal-hybrid-docker-container-ssh---app-1-1-1-5 kamal-hybrid-docker-container-ssh---app-1-1-1-5-context", output
     end
   end
 

test/commands/builder_test.rb

@@ -41,7 +41,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase
     builder = new_builder_command(builder: { "arch" => [ "#{remote_arch}" ], "remote" => "ssh://app@host", "cache" => { "type" => "gha" } })
     assert_equal "remote", builder.name
     assert_equal \
-      "docker buildx build --push --platform linux/#{remote_arch} --builder kamal-remote-docker-container-ssh---app-host -t dhh/app:123 -t dhh/app:latest --cache-to type=gha --cache-from type=gha --label service=\"app\" --file Dockerfile .",
+      "docker buildx build --push --platform linux/#{remote_arch} --builder kamal-remote-ssh---app-host -t dhh/app:123 -t dhh/app:latest --cache-to type=gha --cache-from type=gha --label service=\"app\" --file Dockerfile .",
       builder.push.join(" ")
   end
 

test/fixtures/deploy_with_hybrid_builder.yml

@@ -0,0 +1,42 @@
+service: app
+image: dhh/app
+servers:
+  web:
+    - "1.1.1.1"
+    - "1.1.1.2"
+  workers:
+    - "1.1.1.3"
+    - "1.1.1.4"
+registry:
+  username: user
+  password: pw
+
+accessories:
+  mysql:
+    image: mysql:5.7
+    host: 1.1.1.3
+    port: 3306
+    env:
+      clear:
+        MYSQL_ROOT_HOST: '%'
+      secret:
+        - MYSQL_ROOT_PASSWORD
+    files:
+      - test/fixtures/files/my.cnf:/etc/mysql/my.cnf
+    directories:
+      - data:/var/lib/mysql
+  redis:
+    image: redis:latest
+    roles:
+      - web
+    port: 6379
+    directories:
+      - data:/data
+
+readiness_delay: 0
+
+builder:
+  arch:
+    - arm64
+    - amd64
+  remote: ssh://app@1.1.1.5

test/fixtures/deploy_with_remote_builder.yml

@@ -32,8 +32,6 @@ accessories:
     port: 6379
     directories:
       - data:/data
-builder:
-  arch: amd64
 
 readiness_delay: 0
 

test/fixtures/deploy_with_remote_builder_and_custom_ports.yml

@@ -32,8 +32,6 @@ accessories:
     port: 6379
     directories:
       - data:/data
-builder:
-  arch: amd64
 
 readiness_delay: 0