From b52e66814a11836191640d5c50e08f7d8cf19241 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Wed, 5 Jun 2024 11:52:45 +0100 Subject: [PATCH] Check that we have valid contexts before building Load the hosts from the contexts before trying to build. If there is no context, we'll create one. If there is one but the hosts don't match we'll re-create. Where we just have a local context, there won't be any hosts but we still inspect the builder to check that it exists. --- lib/kamal/cli/build.rb | 23 +++++----- lib/kamal/commands/builder.rb | 3 +- lib/kamal/commands/builder/base.rb | 13 ++++++ lib/kamal/commands/builder/multiarch.rb | 4 ++ .../commands/builder/multiarch/remote.rb | 10 +++++ lib/kamal/commands/builder/native/cached.rb | 11 ++++- lib/kamal/commands/builder/native/remote.rb | 8 ++++ test/cli/build_test.rb | 13 ++++-- test/cli/cli_test_case.rb | 3 ++ test/cli/main_test.rb | 8 ++++ test/commands/builder_test.rb | 42 +++++++++++++++++++ 11 files changed, 122 insertions(+), 16 deletions(-) diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 45b53eb7..54d4d6db 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -35,22 +35,25 @@ class Kamal::Cli::Build < Kamal::Cli::Base run_locally do begin - KAMAL.with_verbosity(:debug) do - Dir.chdir(KAMAL.config.builder.build_directory) { execute *push } + context_hosts = capture_with_info(*KAMAL.builder.context_hosts).split("\n") + + if context_hosts != KAMAL.builder.config_context_hosts + warn "Context hosts have changed, so re-creating builder, was: #{context_hosts.join(", ")}], now: #{KAMAL.builder.config_context_hosts.join(", ")}" + cli.remove + cli.create 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 + warn "Missing compatible builder, so creating a new one first" + if e.message =~ /(context not found|no builder)/ + cli.create else raise end end + + KAMAL.with_verbosity(:debug) do + Dir.chdir(KAMAL.config.builder.build_directory) { execute *push } + end end end diff --git a/lib/kamal/commands/builder.rb b/lib/kamal/commands/builder.rb index 2e5862a8..ad38fb4f 100644 --- a/lib/kamal/commands/builder.rb +++ b/lib/kamal/commands/builder.rb @@ -1,7 +1,8 @@ require "active_support/core_ext/string/filters" class Kamal::Commands::Builder < Kamal::Commands::Base - delegate :create, :remove, :push, :clean, :pull, :info, :validate_image, to: :target + delegate :create, :remove, :push, :clean, :pull, :info, :context_hosts, :config_context_hosts, :validate_image, + to: :target include Clone diff --git a/lib/kamal/commands/builder/base.rb b/lib/kamal/commands/builder/base.rb index d4d79e16..112b7090 100644 --- a/lib/kamal/commands/builder/base.rb +++ b/lib/kamal/commands/builder/base.rb @@ -2,6 +2,8 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base class BuilderError < StandardError; end + ENDPOINT_DOCKER_HOST_INSPECT = "'{{.Endpoints.docker.Host}}'" + delegate :argumentize, to: Kamal::Utils delegate :args, :secrets, :dockerfile, :target, :local_arch, :local_host, :remote_arch, :remote_host, :cache_from, :cache_to, :ssh, to: :builder_config @@ -30,6 +32,13 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base ) end + def context_hosts + :true + end + + def config_context_hosts + [] + end private def build_tags @@ -74,4 +83,8 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base def builder_config config.builder end + + def context_host(builder_name) + docker :context, :inspect, builder_name, "--format", ENDPOINT_DOCKER_HOST_INSPECT + end end diff --git a/lib/kamal/commands/builder/multiarch.rb b/lib/kamal/commands/builder/multiarch.rb index d232226c..ae1d423f 100644 --- a/lib/kamal/commands/builder/multiarch.rb +++ b/lib/kamal/commands/builder/multiarch.rb @@ -22,6 +22,10 @@ class Kamal::Commands::Builder::Multiarch < Kamal::Commands::Builder::Base build_context end + def context_hosts + docker :buildx, :inspect, builder_name, "> /dev/null" + end + private def builder_name "kamal-#{config.service}-multiarch" diff --git a/lib/kamal/commands/builder/multiarch/remote.rb b/lib/kamal/commands/builder/multiarch/remote.rb index 1f71979d..d60bee77 100644 --- a/lib/kamal/commands/builder/multiarch/remote.rb +++ b/lib/kamal/commands/builder/multiarch/remote.rb @@ -12,6 +12,16 @@ class Kamal::Commands::Builder::Multiarch::Remote < Kamal::Commands::Builder::Mu super end + def context_hosts + chain \ + context_host(builder_name_with_arch(local_arch)), + context_host(builder_name_with_arch(remote_arch)) + end + + def config_context_hosts + [ local_host, remote_host ].compact + end + private def builder_name super + "-remote" diff --git a/lib/kamal/commands/builder/native/cached.rb b/lib/kamal/commands/builder/native/cached.rb index f72d1192..8f65d5f3 100644 --- a/lib/kamal/commands/builder/native/cached.rb +++ b/lib/kamal/commands/builder/native/cached.rb @@ -1,6 +1,6 @@ class Kamal::Commands::Builder::Native::Cached < Kamal::Commands::Builder::Native def create - docker :buildx, :create, "--use", "--driver=docker-container" + docker :buildx, :create, "--name", builder_name, "--use", "--driver=docker-container" end def remove @@ -13,4 +13,13 @@ class Kamal::Commands::Builder::Native::Cached < Kamal::Commands::Builder::Nativ *build_options, build_context end + + def context_hosts + docker :buildx, :inspect, builder_name, "> /dev/null" + end + + private + def builder_name + "kamal-#{config.service}-native-cached" + end end diff --git a/lib/kamal/commands/builder/native/remote.rb b/lib/kamal/commands/builder/native/remote.rb index a14a776a..9d03b8db 100644 --- a/lib/kamal/commands/builder/native/remote.rb +++ b/lib/kamal/commands/builder/native/remote.rb @@ -26,6 +26,14 @@ class Kamal::Commands::Builder::Native::Remote < Kamal::Commands::Builder::Nativ build_context end + def context_hosts + context_host(builder_name_with_arch) + end + + def config_context_hosts + [ remote_host ] + end + private def builder_name diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index b0bbd83b..e7c7b333 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -21,6 +21,10 @@ class CliBuildTest < CliTestCase .with(:git, "-C", anything, :status, "--porcelain") .returns("") + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:docker, :buildx, :inspect, "kamal-app-multiarch", "> /dev/null") + .returns("") + run_command("push", "--verbose").tap do |output| assert_hook_ran "pre-build", output, **hook_variables assert_match /Cloning repo into build directory/, output @@ -121,11 +125,9 @@ class CliBuildTest < CliTestCase SSHKit::Backend::Abstract.any_instance.expects(:execute) .with(:docker, :buildx, :create, "--use", "--name", "kamal-app-multiarch") - SSHKit::Backend::Abstract.any_instance.expects(:execute) - .with { |*args| args[0..1] == [ :docker, :buildx ] } + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:docker, :buildx, :inspect, "kamal-app-multiarch", "> /dev/null") .raises(SSHKit::Command::Failed.new("no builder")) - .then - .returns(true) SSHKit::Backend::Abstract.any_instance.expects(:execute).with { |*args| args.first.start_with?("git") } @@ -137,6 +139,9 @@ class CliBuildTest < CliTestCase .with(:git, "-C", anything, :status, "--porcelain") .returns("") + SSHKit::Backend::Abstract.any_instance.expects(:execute) + .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64,linux/arm64", "--builder", "kamal-app-multiarch", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".") + run_command("push").tap do |output| assert_match /WARN Missing compatible builder, so creating a new one first/, output end diff --git a/test/cli/cli_test_case.rb b/test/cli/cli_test_case.rb index af1589ee..5db20c05 100644 --- a/test/cli/cli_test_case.rb +++ b/test/cli/cli_test_case.rb @@ -36,6 +36,9 @@ class CliTestCase < ActiveSupport::TestCase .with { |arg1, arg2| arg1 == :mkdir && arg2 == ".kamal/locks/app" } SSHKit::Backend::Abstract.any_instance.stubs(:execute) .with { |arg1, arg2| arg1 == :rm && arg2 == ".kamal/locks/app/details" } + SSHKit::Backend::Abstract.any_instance.stubs(:capture_with_info) + .with { |*args| args[0..2] == [ :docker, :buildx, :inspect ] } + .returns("") end def assert_hook_ran(hook, output, version:, service_version:, hosts:, command:, subcommand: nil, runtime: false) diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index 595556fe..4d74632f 100644 --- a/test/cli/main_test.rb +++ b/test/cli/main_test.rb @@ -116,6 +116,10 @@ class CliMainTest < CliTestCase .with(:git, "-C", anything, :status, "--porcelain") .returns("") + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:docker, :buildx, :inspect, "kamal-app-multiarch", "> /dev/null") + .returns("") + assert_raises(Kamal::Cli::LockError) do run_command("deploy") end @@ -145,6 +149,10 @@ class CliMainTest < CliTestCase .with(:git, "-C", anything, :status, "--porcelain") .returns("") + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:docker, :buildx, :inspect, "kamal-app-multiarch", "> /dev/null") + .returns("") + assert_raises(SSHKit::Runner::ExecuteError) do run_command("deploy") end diff --git a/test/commands/builder_test.rb b/test/commands/builder_test.rb index 064454a2..6bc9795d 100644 --- a/test/commands/builder_test.rb +++ b/test/commands/builder_test.rb @@ -158,6 +158,48 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder.push.join(" ") end + test "multiarch context hosts" do + command = new_builder_command + assert_equal "docker buildx inspect kamal-app-multiarch > /dev/null", command.context_hosts.join(" ") + assert_equal "", command.config_context_hosts.join(" ") + end + + test "native context hosts" do + command = new_builder_command(builder: { "multiarch" => false }) + assert_equal :true, command.context_hosts + assert_equal "", command.config_context_hosts.join(" ") + end + + test "native cached context hosts" do + command = new_builder_command(builder: { "multiarch" => false, "cache" => { "type" => "registry" } }) + assert_equal "docker buildx inspect kamal-app-native-cached > /dev/null", command.context_hosts.join(" ") + assert_equal "", command.config_context_hosts.join(" ") + end + + test "native remote context hosts" do + command = new_builder_command(builder: { "remote" => { "arch" => "amd64", "host" => "ssh://host" } }) + assert_equal "docker context inspect kamal-app-native-remote-amd64 --format '{{.Endpoints.docker.Host}}'", command.context_hosts.join(" ") + assert_equal [ "ssh://host" ], command.config_context_hosts + end + + test "multiarch remote context hosts" do + command = new_builder_command(builder: { + "remote" => { "arch" => "amd64", "host" => "ssh://host" }, + "local" => { "arch" => "arm64" } + }) + assert_equal "docker context inspect kamal-app-multiarch-remote-arm64 --format '{{.Endpoints.docker.Host}}' ; docker context inspect kamal-app-multiarch-remote-amd64 --format '{{.Endpoints.docker.Host}}'", command.context_hosts.join(" ") + assert_equal [ "ssh://host" ], command.config_context_hosts + end + + test "multiarch remote context hosts with local host" do + command = new_builder_command(builder: { + "remote" => { "arch" => "amd64", "host" => "ssh://host" }, + "local" => { "arch" => "arm64", "host" => "unix:///var/run/docker.sock" } + }) + assert_equal "docker context inspect kamal-app-multiarch-remote-arm64 --format '{{.Endpoints.docker.Host}}' ; docker context inspect kamal-app-multiarch-remote-amd64 --format '{{.Endpoints.docker.Host}}'", command.context_hosts.join(" ") + assert_equal [ "unix:///var/run/docker.sock", "ssh://host" ], command.config_context_hosts + end + private def new_builder_command(additional_config = {}) Kamal::Commands::Builder.new(Kamal::Configuration.new(@config.merge(additional_config), version: "123"))