From 24e4347c458694997d4481545f146d2039654ab8 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 20 Jan 2025 15:57:48 -0500 Subject: [PATCH 01/16] feat: Introduce a `build push --output` option which controls where the build result is exported. The default value is "registry" to reflect the current behavior of `build push`. Any value provided to this option will be passed to the `buildx build` command as a `--output=type=` flag. For example, the following command will push to the local docker image store: kamal build push --output=docker squash --- lib/kamal/cli/build.rb | 7 ++++--- lib/kamal/commands/builder/base.rb | 4 ++-- test/cli/build_test.rb | 31 +++++++++++++++++++++++++---- test/commands/builder_test.rb | 32 +++++++++++++++--------------- 4 files changed, 49 insertions(+), 25 deletions(-) diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 8897e2ae..f22922f9 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -5,11 +5,12 @@ class Kamal::Cli::Build < Kamal::Cli::Base desc "deliver", "Build app and push app image to registry then pull image on servers" def deliver - push - pull + invoke :push + invoke :pull end desc "push", "Build and push app image to registry" + option :output, type: :string, default: "registry", banner: "export_type", desc: "Exported type for the build result, and may be any exported type supported by 'buildx --output'." def push cli = self @@ -49,7 +50,7 @@ class Kamal::Cli::Build < Kamal::Cli::Base end # Get the command here to ensure the Dir.chdir doesn't interfere with it - push = KAMAL.builder.push + push = KAMAL.builder.push(cli.options[:output]) KAMAL.with_verbosity(:debug) do Dir.chdir(KAMAL.config.builder.build_directory) { execute *push } diff --git a/lib/kamal/commands/builder/base.rb b/lib/kamal/commands/builder/base.rb index dea04a3a..213cb6fe 100644 --- a/lib/kamal/commands/builder/base.rb +++ b/lib/kamal/commands/builder/base.rb @@ -13,9 +13,9 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base docker :image, :rm, "--force", config.absolute_image end - def push + def push(export_action = "registry") docker :buildx, :build, - "--push", + "--output=type=#{export_action}", *platform_options(arches), *([ "--builder", builder_name ] unless docker_driver?), *build_options, diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index 3cc3e2f6..e25ed0cc 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -26,7 +26,30 @@ class CliBuildTest < CliTestCase assert_match /Cloning repo into build directory/, output assert_match /git -C #{Dir.tmpdir}\/kamal-clones\/app-#{pwd_sha} clone #{Dir.pwd}/, output assert_match /docker --version && docker buildx version/, output - assert_match /docker buildx build --push --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile \. as .*@localhost/, output + assert_match /docker buildx build --output=type=registry --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile \. as .*@localhost/, output + end + end + end + + test "push --output=docker" do + with_build_directory do |build_directory| + Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) + hook_variables = { version: 999, service_version: "app@999", hosts: "1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", command: "build", subcommand: "push" } + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :"rev-parse", :HEAD) + .returns(Kamal::Git.revision) + + SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) + .with(:git, "-C", anything, :status, "--porcelain") + .returns("") + + run_command("push", "--output=docker", "--verbose").tap do |output| + assert_hook_ran "pre-build", output, **hook_variables + assert_match /Cloning repo into build directory/, output + assert_match /git -C #{Dir.tmpdir}\/kamal-clones\/app-#{pwd_sha} clone #{Dir.pwd}/, output + assert_match /docker --version && docker buildx version/, output + assert_match /docker buildx build --output=type=docker --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile \. as .*@localhost/, output end end end @@ -49,7 +72,7 @@ class CliBuildTest < CliTestCase SSHKit::Backend::Abstract.any_instance.expects(:execute).with(:git, "-C", build_directory, :submodule, :update, "--init") SSHKit::Backend::Abstract.any_instance.expects(:execute) - .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".") + .with(:docker, :buildx, :build, "--output=type=registry", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".") SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) .with(:git, "-C", anything, :"rev-parse", :HEAD) @@ -74,7 +97,7 @@ class CliBuildTest < CliTestCase assert_no_match /Cloning repo into build directory/, output assert_hook_ran "pre-build", output, **hook_variables assert_match /docker --version && docker buildx version/, output - assert_match /docker buildx build --push --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile . as .*@localhost/, output + assert_match /docker buildx build --output=type=registry --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile . as .*@localhost/, output end end @@ -140,7 +163,7 @@ class CliBuildTest < CliTestCase .returns("") SSHKit::Backend::Abstract.any_instance.expects(:execute) - .with(:docker, :buildx, :build, "--push", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-t", "dhh/app:999", "-t", "dhh/app:latest", "--label", "service=\"app\"", "--file", "Dockerfile", ".") + .with(:docker, :buildx, :build, "--output=type=registry", "--platform", "linux/amd64", "--builder", "kamal-local-docker-container", "-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 diff --git a/test/commands/builder_test.rb b/test/commands/builder_test.rb index c5e23ea8..33c26afa 100644 --- a/test/commands/builder_test.rb +++ b/test/commands/builder_test.rb @@ -9,7 +9,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "cache" => { "type" => "gha" } }) assert_equal "local", builder.name assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --cache-to type=gha --cache-from type=gha --label service=\"app\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -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 @@ -17,7 +17,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "arch" => [ "amd64" ] }) assert_equal "local", builder.name assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile .", builder.push.join(" ") end @@ -25,7 +25,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "cache" => { "type" => "gha" } }) assert_equal "local", builder.name assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --cache-to type=gha --cache-from type=gha --label service=\"app\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -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 @@ -33,7 +33,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "arch" => [ "amd64", "arm64" ], "remote" => "ssh://app@127.0.0.1", "cache" => { "type" => "gha" } }) assert_equal "hybrid", builder.name assert_equal \ - "docker buildx build --push --platform linux/amd64,linux/arm64 --builder kamal-hybrid-docker-container-ssh---app-127-0-0-1 -t dhh/app:123 -t dhh/app:latest --cache-to type=gha --cache-from type=gha --label service=\"app\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/amd64,linux/arm64 --builder kamal-hybrid-docker-container-ssh---app-127-0-0-1 -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 @@ -41,7 +41,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "arch" => [ "amd64", "arm64" ], "remote" => "ssh://app@127.0.0.1", "cache" => { "type" => "gha" }, "local" => false }) assert_equal "remote", builder.name assert_equal \ - "docker buildx build --push --platform linux/amd64,linux/arm64 --builder kamal-remote-ssh---app-127-0-0-1 -t dhh/app:123 -t dhh/app:latest --cache-to type=gha --cache-from type=gha --label service=\"app\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/amd64,linux/arm64 --builder kamal-remote-ssh---app-127-0-0-1 -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 @@ -49,7 +49,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-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 --output=type=registry --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 @@ -57,7 +57,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "arch" => [ "#{local_arch}" ], "remote" => "ssh://app@host", "cache" => { "type" => "gha" } }) assert_equal "local", builder.name assert_equal \ - "docker buildx build --push --platform linux/#{local_arch} --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --cache-to type=gha --cache-from type=gha --label service=\"app\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/#{local_arch} --builder kamal-local-docker-container -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 @@ -65,7 +65,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "arch" => [ "#{local_arch}" ], "driver" => "cloud docker-org-name/builder-name" }) assert_equal "cloud", builder.name assert_equal \ - "docker buildx build --push --platform linux/#{local_arch} --builder cloud-docker-org-name-builder-name -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/#{local_arch} --builder cloud-docker-org-name-builder-name -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile .", builder.push.join(" ") end @@ -112,14 +112,14 @@ class CommandsBuilderTest < ActiveSupport::TestCase test "build context" do builder = new_builder_command(builder: { "context" => ".." }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile ..", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile ..", builder.push.join(" ") end test "push with build args" do builder = new_builder_command(builder: { "args" => { "a" => 1, "b" => 2 } }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --build-arg a=\"1\" --build-arg b=\"2\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --build-arg a=\"1\" --build-arg b=\"2\" --file Dockerfile .", builder.push.join(" ") end @@ -128,7 +128,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase FileUtils.touch("Dockerfile") builder = new_builder_command(builder: { "secrets" => [ "a", "b" ] }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --secret id=\"a\" --secret id=\"b\" --file Dockerfile .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --secret id=\"a\" --secret id=\"b\" --file Dockerfile .", builder.push.join(" ") end end @@ -148,35 +148,35 @@ class CommandsBuilderTest < ActiveSupport::TestCase test "context build" do builder = new_builder_command(builder: { "context" => "./foo" }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile ./foo", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile ./foo", builder.push.join(" ") end test "push with provenance" do builder = new_builder_command(builder: { "provenance" => "mode=max" }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --provenance mode=max .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --provenance mode=max .", builder.push.join(" ") end test "push with provenance false" do builder = new_builder_command(builder: { "provenance" => false }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --provenance false .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --provenance false .", builder.push.join(" ") end test "push with sbom" do builder = new_builder_command(builder: { "sbom" => true }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --sbom true .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --sbom true .", builder.push.join(" ") end test "push with sbom false" do builder = new_builder_command(builder: { "sbom" => false }) assert_equal \ - "docker buildx build --push --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --sbom false .", + "docker buildx build --output=type=registry --platform linux/amd64 --builder kamal-local-docker-container -t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --sbom false .", builder.push.join(" ") end From 2127f1708afacdefe8fa2fba9ddcf4296b241f6e Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 20 Jan 2025 16:12:26 -0500 Subject: [PATCH 02/16] feat: Introduce a `build dev` command which will build a "dirty" image using the working directory. This command is different from `build push` in two important ways: - the image tags will have a suffix of `-dirty` - the export action is "docker", pushing to the local docker image store The command also supports the `--output` option just added to `build push` to override that default. This command is intended to allow developers to quickly iterate on a docker image built from their local working directory while avoiding any confusion with a pristine image built from a git clone, and keeping those images on the local dev system by default. --- lib/kamal/cli/build.rb | 22 ++++++++++++++++++++++ lib/kamal/commands/builder.rb | 2 +- lib/kamal/commands/builder/base.rb | 15 +++++++++++---- test/cli/build_test.rb | 24 ++++++++++++++++++++++++ test/commands/builder_test.rb | 10 +++++----- 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index f22922f9..80cc6637 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -109,6 +109,28 @@ class Kamal::Cli::Build < Kamal::Cli::Base end end + desc "dev", "Build using the working directory, tag it as dirty, and push to local image store." + option :output, type: :string, default: "docker", banner: "export_type", desc: "Exported type for the build result, and may be any exported type supported by 'buildx --output'." + def dev + cli = self + + ensure_docker_installed + + uncommitted_changes = Kamal::Git.uncommitted_changes + if uncommitted_changes.present? + say "WARNING: building with uncommitted changes:\n #{uncommitted_changes}", :yellow + end + + with_env(KAMAL.config.builder.secrets) do + run_locally do + build = KAMAL.builder.push(cli.options[:output], tag_as_dirty: true) + KAMAL.with_verbosity(:debug) do + execute(*build) + end + end + end + end + private def connect_to_remote_host(remote_host) remote_uri = URI.parse(remote_host) diff --git a/lib/kamal/commands/builder.rb b/lib/kamal/commands/builder.rb index 687eb35e..ddc7be7c 100644 --- a/lib/kamal/commands/builder.rb +++ b/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, :inspect_builder, :validate_image, :first_mirror, to: :target + delegate :create, :remove, :dev, :push, :clean, :pull, :info, :inspect_builder, :validate_image, :first_mirror, to: :target delegate :local?, :remote?, :cloud?, to: "config.builder" include Clone diff --git a/lib/kamal/commands/builder/base.rb b/lib/kamal/commands/builder/base.rb index 213cb6fe..0bd613cb 100644 --- a/lib/kamal/commands/builder/base.rb +++ b/lib/kamal/commands/builder/base.rb @@ -13,11 +13,12 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base docker :image, :rm, "--force", config.absolute_image end - def push(export_action = "registry") + def push(export_action = "registry", tag_as_dirty: false) docker :buildx, :build, "--output=type=#{export_action}", *platform_options(arches), *([ "--builder", builder_name ] unless docker_driver?), + *build_tag_options(tag_as_dirty: tag_as_dirty), *build_options, build_context end @@ -37,7 +38,7 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base end def build_options - [ *build_tags, *build_cache, *build_labels, *build_args, *build_secrets, *build_dockerfile, *build_target, *build_ssh, *builder_provenance, *builder_sbom ] + [ *build_cache, *build_labels, *build_args, *build_secrets, *build_dockerfile, *build_target, *build_ssh, *builder_provenance, *builder_sbom ] end def build_context @@ -58,8 +59,14 @@ class Kamal::Commands::Builder::Base < Kamal::Commands::Base end private - def build_tags - [ "-t", config.absolute_image, "-t", config.latest_image ] + def build_tag_names(tag_as_dirty: false) + tag_names = [ config.absolute_image, config.latest_image ] + tag_names.map! { |t| "#{t}-dirty" } if tag_as_dirty + tag_names + end + + def build_tag_options(tag_as_dirty: false) + build_tag_names(tag_as_dirty: tag_as_dirty).flat_map { |name| [ "-t", name ] } end def build_cache diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index e25ed0cc..f93f26ec 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -298,6 +298,30 @@ class CliBuildTest < CliTestCase end end + test "dev" do + with_build_directory do |build_directory| + Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) + + run_command("dev", "--verbose").tap do |output| + assert_no_match(/Cloning repo into build directory/, output) + assert_match(/docker --version && docker buildx version/, output) + assert_match(/docker buildx build --output=type=docker --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999-dirty -t dhh\/app:latest-dirty --label service="app" --file Dockerfile \. as .*@localhost/, output) + end + end + end + + test "dev --output=local" do + with_build_directory do |build_directory| + Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) + + run_command("dev", "--output=local", "--verbose").tap do |output| + assert_no_match(/Cloning repo into build directory/, output) + assert_match(/docker --version && docker buildx version/, output) + assert_match(/docker buildx build --output=type=local --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999-dirty -t dhh\/app:latest-dirty --label service="app" --file Dockerfile \. as .*@localhost/, output) + end + end + end + private def run_command(*command, fixture: :with_accessories) stdouted { Kamal::Cli::Build.start([ *command, "-c", "test/fixtures/deploy_#{fixture}.yml" ]) } diff --git a/test/commands/builder_test.rb b/test/commands/builder_test.rb index 33c26afa..8ac8382e 100644 --- a/test/commands/builder_test.rb +++ b/test/commands/builder_test.rb @@ -72,7 +72,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase test "build args" do builder = new_builder_command(builder: { "args" => { "a" => 1, "b" => 2 } }) assert_equal \ - "-t dhh/app:123 -t dhh/app:latest --label service=\"app\" --build-arg a=\"1\" --build-arg b=\"2\" --file Dockerfile", + "--label service=\"app\" --build-arg a=\"1\" --build-arg b=\"2\" --file Dockerfile", builder.target.build_options.join(" ") end @@ -81,7 +81,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase FileUtils.touch("Dockerfile") builder = new_builder_command(builder: { "secrets" => [ "token_a", "token_b" ] }) assert_equal \ - "-t dhh/app:123 -t dhh/app:latest --label service=\"app\" --secret id=\"token_a\" --secret id=\"token_b\" --file Dockerfile", + "--label service=\"app\" --secret id=\"token_a\" --secret id=\"token_b\" --file Dockerfile", builder.target.build_options.join(" ") end end @@ -90,7 +90,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase Pathname.any_instance.expects(:exist?).returns(true).once builder = new_builder_command(builder: { "dockerfile" => "Dockerfile.xyz" }) assert_equal \ - "-t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile.xyz", + "--label service=\"app\" --file Dockerfile.xyz", builder.target.build_options.join(" ") end @@ -105,7 +105,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase test "build target" do builder = new_builder_command(builder: { "target" => "prod" }) assert_equal \ - "-t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --target prod", + "--label service=\"app\" --file Dockerfile --target prod", builder.target.build_options.join(" ") end @@ -137,7 +137,7 @@ class CommandsBuilderTest < ActiveSupport::TestCase builder = new_builder_command(builder: { "ssh" => "default=$SSH_AUTH_SOCK" }) assert_equal \ - "-t dhh/app:123 -t dhh/app:latest --label service=\"app\" --file Dockerfile --ssh default=$SSH_AUTH_SOCK", + "--label service=\"app\" --file Dockerfile --ssh default=$SSH_AUTH_SOCK", builder.target.build_options.join(" ") end From 3c01dc75fdb622adf0763b7f00ad79a9d8bec43a Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Mon, 20 Jan 2025 18:53:48 -0500 Subject: [PATCH 03/16] ci: use `fail-fast: false` instead of `continue-on-error: true` This will give us proper red/green signal on the test suite while still running the entire matrix. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dcd1b0ec..8da7d39c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,7 @@ jobs: run: bundle exec rubocop --parallel tests: strategy: + fail-fast: false matrix: ruby-version: - "3.1" @@ -37,7 +38,6 @@ jobs: gemfile: gemfiles/rails_edge.gemfile name: ${{ format('Tests (Ruby {0})', matrix.ruby-version) }} runs-on: ubuntu-latest - continue-on-error: true env: BUNDLE_GEMFILE: ${{ github.workspace }}/${{ matrix.gemfile }} steps: From 214d4fd321238a36baafcebd4028c1bd6366c7b4 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Sat, 1 Feb 2025 22:15:06 -0500 Subject: [PATCH 04/16] The `build dev` command warns about untracked and uncommitted files so there's a complete picture of what is being packaged that's not in git. --- lib/kamal/cli/build.rb | 19 ++++++++++++++++--- lib/kamal/docker.rb | 30 ++++++++++++++++++++++++++++++ lib/kamal/git.rb | 10 ++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 lib/kamal/docker.rb diff --git a/lib/kamal/cli/build.rb b/lib/kamal/cli/build.rb index 80cc6637..0187e686 100644 --- a/lib/kamal/cli/build.rb +++ b/lib/kamal/cli/build.rb @@ -116,9 +116,22 @@ class Kamal::Cli::Build < Kamal::Cli::Base ensure_docker_installed - uncommitted_changes = Kamal::Git.uncommitted_changes - if uncommitted_changes.present? - say "WARNING: building with uncommitted changes:\n #{uncommitted_changes}", :yellow + docker_included_files = Set.new(Kamal::Docker.included_files) + git_uncommitted_files = Set.new(Kamal::Git.uncommitted_files) + git_untracked_files = Set.new(Kamal::Git.untracked_files) + + docker_uncommitted_files = docker_included_files & git_uncommitted_files + if docker_uncommitted_files.any? + say "WARNING: Files with uncommitted changes will be present in the dev container:", :yellow + docker_uncommitted_files.sort.each { |f| say " #{f}", :yellow } + say + end + + docker_untracked_files = docker_included_files & git_untracked_files + if docker_untracked_files.any? + say "WARNING: Untracked files will be present in the dev container:", :yellow + docker_untracked_files.sort.each { |f| say " #{f}", :yellow } + say end with_env(KAMAL.config.builder.secrets) do diff --git a/lib/kamal/docker.rb b/lib/kamal/docker.rb new file mode 100644 index 00000000..6ae7b7f3 --- /dev/null +++ b/lib/kamal/docker.rb @@ -0,0 +1,30 @@ +require "tempfile" +require "open3" + +module Kamal::Docker + extend self + BUILD_CHECK_TAG = "kamal-local-build-check" + + def included_files + Tempfile.create do |dockerfile| + dockerfile.write(<<~DOCKERFILE) + FROM busybox + COPY . app + WORKDIR app + CMD find . -type f | sed "s|^\./||" + DOCKERFILE + dockerfile.close + + cmd = "docker buildx build -t=#{BUILD_CHECK_TAG} -f=#{dockerfile.path} ." + system(cmd) || raise("failed to build check image") + end + + cmd = "docker run --rm #{BUILD_CHECK_TAG}" + out, err, status = Open3.capture3(cmd) + unless status + raise "failed to run check image:\n#{err}" + end + + out.lines.map(&:strip) + end +end diff --git a/lib/kamal/git.rb b/lib/kamal/git.rb index a286f735..e91983bb 100644 --- a/lib/kamal/git.rb +++ b/lib/kamal/git.rb @@ -24,4 +24,14 @@ module Kamal::Git def root `git rev-parse --show-toplevel`.strip end + + # returns an array of relative path names of files with uncommitted changes + def uncommitted_files + `git ls-files --modified`.lines.map(&:strip) + end + + # returns an array of relative path names of untracked files, including gitignored files + def untracked_files + `git ls-files --others`.lines.map(&:strip) + end end From d710b5a22bf40da1952dfb36a6d775096645964d Mon Sep 17 00:00:00 2001 From: Neil Johari Date: Sun, 2 Feb 2025 23:43:51 -0800 Subject: [PATCH 05/16] Allow ommitting AWS account while fetching secrets --- .../secrets/adapters/aws_secrets_manager.rb | 19 +++++--- .../aws_secrets_manager_adapter_test.rb | 43 ++++++++++++++++--- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index 48add1ac..bd81c754 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -1,15 +1,18 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Base + def requires_account? + false + end + private def login(_account) nil end - def fetch_secrets(secrets, from:, account:, session:) + def fetch_secrets(secrets, from:, account: nil, session:) {}.tap do |results| get_from_secrets_manager(prefixed_secrets(secrets, from: from), account: account).each do |secret| secret_name = secret["Name"] secret_string = JSON.parse(secret["SecretString"]) - secret_string.each do |key, value| results["#{secret_name}/#{key}"] = value end @@ -19,8 +22,14 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Ba end end - def get_from_secrets_manager(secrets, account:) - `aws secretsmanager batch-get-secret-value --secret-id-list #{secrets.map(&:shellescape).join(" ")} --profile #{account.shellescape}`.tap do |secrets| + def get_from_secrets_manager(secrets, account: nil) + profile_opt = account ? "--profile #{account.shellescape}" : "" + + args = [ "aws", "secretsmanager", "batch-get-secret-value", "--secret-id-list" ] + secrets.map(&:shellescape) + args += [ "--profile", account.shellescape ] if account + cmd = args.join(" ") + + `#{cmd}`.tap do |secrets| raise RuntimeError, "Could not read #{secrets} from AWS Secrets Manager" unless $?.success? secrets = JSON.parse(secrets) @@ -39,4 +48,4 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Ba `aws --version 2> /dev/null` $?.success? end -end +end \ No newline at end of file diff --git a/test/secrets/aws_secrets_manager_adapter_test.rb b/test/secrets/aws_secrets_manager_adapter_test.rb index 7616342d..00f3de08 100644 --- a/test/secrets/aws_secrets_manager_adapter_test.rb +++ b/test/secrets/aws_secrets_manager_adapter_test.rb @@ -156,14 +156,45 @@ class AwsSecretsManagerAdapterTest < SecretAdapterTestCase assert_equal "AWS CLI is not installed", error.message end + test "fetch without account option omits --profile" do + stub_ticks.with("aws --version 2> /dev/null") + stub_ticks + .with("aws secretsmanager batch-get-secret-value --secret-id-list secret/KEY1 secret/KEY2") + .returns(<<~JSON) + { + "SecretValues": [ + { + "ARN": "arn:aws:secretsmanager:us-east-1:aaaaaaaaaaaa:secret:secret", + "Name": "secret", + "VersionId": "vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv", + "SecretString": "{\\"KEY1\\":\\"VALUE1\\", \\"KEY2\\":\\"VALUE2\\"}", + "VersionStages": [ + "AWSCURRENT" + ], + "CreatedDate": "2024-01-01T00:00:00.000000" + } + ], + "Errors": [] + } + JSON + + json = JSON.parse(shellunescape(run_command("fetch", "--from", "secret", "KEY1", "KEY2", account: nil))) + + expected_json = { + "secret/KEY1"=>"VALUE1", + "secret/KEY2"=>"VALUE2" + } + assert_equal expected_json, json + end + private - def run_command(*command) + def run_command(*command, account: "default") stdouted do - Kamal::Cli::Secrets.start \ - [ *command, - "-c", "test/fixtures/deploy_with_accessories.yml", - "--adapter", "aws_secrets_manager", - "--account", "default" ] + args = [ *command, + "-c", "test/fixtures/deploy_with_accessories.yml", + "--adapter", "aws_secrets_manager" ] + args += [ "--account", account ] if account + Kamal::Cli::Secrets.start(args) end end end From c7d1711e3099b956f704091290d275eedc296ba9 Mon Sep 17 00:00:00 2001 From: Neil Johari Date: Sun, 2 Feb 2025 23:46:09 -0800 Subject: [PATCH 06/16] Remove unnecessary var --- lib/kamal/secrets/adapters/aws_secrets_manager.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index bd81c754..82c46d91 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -23,8 +23,6 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Ba end def get_from_secrets_manager(secrets, account: nil) - profile_opt = account ? "--profile #{account.shellescape}" : "" - args = [ "aws", "secretsmanager", "batch-get-secret-value", "--secret-id-list" ] + secrets.map(&:shellescape) args += [ "--profile", account.shellescape ] if account cmd = args.join(" ") From ff3538f81ddc6c4850e9749a4e50249035f5f073 Mon Sep 17 00:00:00 2001 From: Neil Johari Date: Sun, 2 Feb 2025 23:54:53 -0800 Subject: [PATCH 07/16] Undo accidental line deletion --- lib/kamal/secrets/adapters/aws_secrets_manager.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index 82c46d91..fb3f887f 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -13,6 +13,7 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Ba get_from_secrets_manager(prefixed_secrets(secrets, from: from), account: account).each do |secret| secret_name = secret["Name"] secret_string = JSON.parse(secret["SecretString"]) + secret_string.each do |key, value| results["#{secret_name}/#{key}"] = value end From cd73cea850363afc6b70914886f602f6c32a9017 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 3 Feb 2025 14:14:39 +0000 Subject: [PATCH 08/16] Add pre and post app boot hooks Add two new hooks pre-app-boot and post-app-boot. They are analagous to the pre/post proxy reboot hooks. If the boot strategy deploys in groups, then the hooks are called once per group of hosts and `KAMAL_HOSTS` contains a comma delimited list of the hosts in that group. If all hosts are deployed to at once, then they are called once with `KAMAL_HOSTS` containing all the hosts. It is possible to have pauses between groups of hosts in the boot config, where this is the case the pause happens after the post-app-boot hook is called. --- lib/kamal/cli/app.rb | 18 +++++-- .../sample_hooks/post-app-boot.sample | 3 ++ .../sample_hooks/pre-app-boot.sample | 3 ++ lib/kamal/commander.rb | 8 --- test/cli/app_test.rb | 15 ++++-- test/cli/build_test.rb | 9 ++-- test/cli/cli_test_case.rb | 5 +- test/cli/main_test.rb | 20 +++---- test/commander_test.rb | 22 -------- test/configuration/boot_test.rb | 54 +++++++++++++++++++ ...ploy_with_low_percentage_boot_strategy.yml | 19 ------- .../deploy_with_percentage_boot_strategy.yml | 19 ------- test/integration/app_test.rb | 4 +- .../deployer/app/.kamal/hooks/post-app-boot | 3 ++ .../deployer/app/.kamal/hooks/pre-app-boot | 3 ++ test/integration/main_test.rb | 6 +-- test/test_helper.rb | 4 ++ 17 files changed, 116 insertions(+), 99 deletions(-) create mode 100755 lib/kamal/cli/templates/sample_hooks/post-app-boot.sample create mode 100755 lib/kamal/cli/templates/sample_hooks/pre-app-boot.sample create mode 100644 test/configuration/boot_test.rb delete mode 100644 test/fixtures/deploy_with_low_percentage_boot_strategy.yml delete mode 100644 test/fixtures/deploy_with_percentage_boot_strategy.yml create mode 100755 test/integration/docker/deployer/app/.kamal/hooks/post-app-boot create mode 100755 test/integration/docker/deployer/app/.kamal/hooks/pre-app-boot diff --git a/lib/kamal/cli/app.rb b/lib/kamal/cli/app.rb index fb665af9..553ccbb2 100644 --- a/lib/kamal/cli/app.rb +++ b/lib/kamal/cli/app.rb @@ -16,10 +16,18 @@ class Kamal::Cli::App < Kamal::Cli::Base # Primary hosts and roles are returned first, so they can open the barrier barrier = Kamal::Cli::Healthcheck::Barrier.new - on(KAMAL.hosts, **KAMAL.boot_strategy) do |host| - KAMAL.roles_on(host).each do |role| - Kamal::Cli::App::Boot.new(host, role, self, version, barrier).run + host_boot_groups.each do |hosts| + host_list = Array(hosts).join(",") + run_hook "pre-app-boot", hosts: host_list + + on(hosts) do |host| + KAMAL.roles_on(host).each do |role| + Kamal::Cli::App::Boot.new(host, role, self, version, barrier).run + end end + + run_hook "post-app-boot", hosts: host_list + sleep KAMAL.config.boot.wait if KAMAL.config.boot.wait end # Tag once the app booted on all hosts @@ -340,4 +348,8 @@ class Kamal::Cli::App < Kamal::Cli::Base yield end end + + def host_boot_groups + KAMAL.config.boot.limit ? KAMAL.hosts.each_slice(KAMAL.config.boot.limit).to_a : [ KAMAL.hosts ] + end end diff --git a/lib/kamal/cli/templates/sample_hooks/post-app-boot.sample b/lib/kamal/cli/templates/sample_hooks/post-app-boot.sample new file mode 100755 index 00000000..70f9c4bc --- /dev/null +++ b/lib/kamal/cli/templates/sample_hooks/post-app-boot.sample @@ -0,0 +1,3 @@ +#!/bin/sh + +echo "Booted app version $KAMAL_VERSION on $KAMAL_HOSTS..." diff --git a/lib/kamal/cli/templates/sample_hooks/pre-app-boot.sample b/lib/kamal/cli/templates/sample_hooks/pre-app-boot.sample new file mode 100755 index 00000000..45f73550 --- /dev/null +++ b/lib/kamal/cli/templates/sample_hooks/pre-app-boot.sample @@ -0,0 +1,3 @@ +#!/bin/sh + +echo "Booting app version $KAMAL_VERSION on $KAMAL_HOSTS..." diff --git a/lib/kamal/commander.rb b/lib/kamal/commander.rb index 6a461276..97a82ce6 100644 --- a/lib/kamal/commander.rb +++ b/lib/kamal/commander.rb @@ -136,14 +136,6 @@ class Kamal::Commander SSHKit.config.output_verbosity = old_level end - def boot_strategy - if config.boot.limit.present? - { in: :groups, limit: config.boot.limit, wait: config.boot.wait } - else - {} - end - end - def holding_lock? self.holding_lock end diff --git a/test/cli/app_test.rb b/test/cli/app_test.rb index fa5049ba..19190032 100644 --- a/test/cli/app_test.rb +++ b/test/cli/app_test.rb @@ -37,13 +37,20 @@ 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(2) # ensure locks dir, acquire & release lock - Kamal::Cli::App.any_instance.stubs(:on).with([ "1.1.1.1" ]) # tag container + Kamal::Cli::App.any_instance.stubs(:on).with("1.1.1.1").twice + Kamal::Cli::App.any_instance.stubs(:on).with([ "1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4" ]).times(3) # Strategy is used when booting the containers - Kamal::Cli::App.any_instance.expects(:on).with([ "1.1.1.1" ], in: :groups, limit: 3, wait: 2).with_block_given + Kamal::Cli::App.any_instance.expects(:on).with([ "1.1.1.1", "1.1.1.2", "1.1.1.3" ]).with_block_given + Kamal::Cli::App.any_instance.expects(:on).with([ "1.1.1.4" ]).with_block_given + Object.any_instance.expects(:sleep).with(2).twice - run_command("boot", config: :with_boot_strategy) + Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) + + run_command("boot", config: :with_boot_strategy, host: nil).tap do |output| + assert_hook_ran "pre-app-boot", output, count: 2 + assert_hook_ran "post-app-boot", output, count: 2 + end end test "boot errors don't leave lock in place" do diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index f93f26ec..76db2924 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -11,7 +11,6 @@ class CliBuildTest < CliTestCase test "push" do with_build_directory do |build_directory| Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) - hook_variables = { version: 999, service_version: "app@999", hosts: "1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", command: "build", subcommand: "push" } SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) .with(:git, "-C", anything, :"rev-parse", :HEAD) @@ -22,7 +21,7 @@ class CliBuildTest < CliTestCase .returns("") run_command("push", "--verbose").tap do |output| - assert_hook_ran "pre-build", output, **hook_variables + assert_hook_ran "pre-build", output assert_match /Cloning repo into build directory/, output assert_match /git -C #{Dir.tmpdir}\/kamal-clones\/app-#{pwd_sha} clone #{Dir.pwd}/, output assert_match /docker --version && docker buildx version/, output @@ -34,7 +33,6 @@ class CliBuildTest < CliTestCase test "push --output=docker" do with_build_directory do |build_directory| Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) - hook_variables = { version: 999, service_version: "app@999", hosts: "1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", command: "build", subcommand: "push" } SSHKit::Backend::Abstract.any_instance.expects(:capture_with_info) .with(:git, "-C", anything, :"rev-parse", :HEAD) @@ -45,7 +43,7 @@ class CliBuildTest < CliTestCase .returns("") run_command("push", "--output=docker", "--verbose").tap do |output| - assert_hook_ran "pre-build", output, **hook_variables + assert_hook_ran "pre-build", output assert_match /Cloning repo into build directory/, output assert_match /git -C #{Dir.tmpdir}\/kamal-clones\/app-#{pwd_sha} clone #{Dir.pwd}/, output assert_match /docker --version && docker buildx version/, output @@ -91,11 +89,10 @@ class CliBuildTest < CliTestCase test "push without clone" do Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) - hook_variables = { version: 999, service_version: "app@999", hosts: "1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", command: "build", subcommand: "push" } run_command("push", "--verbose", fixture: :without_clone).tap do |output| assert_no_match /Cloning repo into build directory/, output - assert_hook_ran "pre-build", output, **hook_variables + assert_hook_ran "pre-build", output assert_match /docker --version && docker buildx version/, output assert_match /docker buildx build --output=type=registry --platform linux\/amd64 --builder kamal-local-docker-container -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile . as .*@localhost/, output end diff --git a/test/cli/cli_test_case.rb b/test/cli/cli_test_case.rb index d4b57923..b7ca9eca 100644 --- a/test/cli/cli_test_case.rb +++ b/test/cli/cli_test_case.rb @@ -40,8 +40,9 @@ class CliTestCase < ActiveSupport::TestCase .with(:docker, :buildx, :inspect, "kamal-local-docker-container") end - def assert_hook_ran(hook, output, version:, service_version:, hosts:, command:, subcommand: nil, runtime: false, secrets: false) - assert_match %r{usr/bin/env\s\.kamal/hooks/#{hook}}, output + def assert_hook_ran(hook, output, count: 1) + regexp = ([ "/usr/bin/env .kamal/hooks/#{hook}" ] * count).join(".*") + assert_match /#{regexp}/m, output end def with_argv(*argv) diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index e901c3ba..becace43 100644 --- a/test/cli/main_test.rb +++ b/test/cli/main_test.rb @@ -53,17 +53,16 @@ class CliMainTest < CliTestCase Kamal::Cli::Main.any_instance.expects(:invoke).with("kamal:cli:prune:all", [], invoke_options) Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) - hook_variables = { version: 999, service_version: "app@999", hosts: "1.1.1.1,1.1.1.2", command: "deploy" } run_command("deploy", "--verbose").tap do |output| - assert_hook_ran "pre-connect", output, **hook_variables + assert_hook_ran "pre-connect", output assert_match /Log into image registry/, output assert_match /Build and push app image/, output - assert_hook_ran "pre-deploy", output, **hook_variables, secrets: true + assert_hook_ran "pre-deploy", output assert_match /Ensure kamal-proxy is running/, output assert_match /Detect stale containers/, output assert_match /Prune old containers and images/, output - assert_hook_ran "post-deploy", output, **hook_variables, runtime: true, secrets: true + assert_hook_ran "post-deploy", output end end end @@ -205,14 +204,12 @@ class CliMainTest < CliTestCase Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) - hook_variables = { version: 999, service_version: "app@999", hosts: "1.1.1.1,1.1.1.2", command: "redeploy" } - run_command("redeploy", "--verbose").tap do |output| - assert_hook_ran "pre-connect", output, **hook_variables + assert_hook_ran "pre-connect", output assert_match /Build and push app image/, output - assert_hook_ran "pre-deploy", output, **hook_variables + assert_hook_ran "pre-deploy", output assert_match /Running the pre-deploy hook.../, output - assert_hook_ran "post-deploy", output, **hook_variables, runtime: true + assert_hook_ran "post-deploy", output end end @@ -258,14 +255,13 @@ class CliMainTest < CliTestCase .returns("running").at_least_once # health check Kamal::Commands::Hook.any_instance.stubs(:hook_exists?).returns(true) - hook_variables = { version: 123, service_version: "app@123", hosts: "1.1.1.1,1.1.1.2,1.1.1.3,1.1.1.4", command: "rollback" } run_command("rollback", "--verbose", "123", config_file: "deploy_with_accessories").tap do |output| - assert_hook_ran "pre-deploy", output, **hook_variables + assert_hook_ran "pre-deploy", output assert_match "docker tag dhh/app:123 dhh/app:latest", output assert_match "docker run --detach --restart unless-stopped --name app-web-123", output assert_match "docker container ls --all --filter name=^app-web-version-to-rollback$ --quiet | xargs docker stop", output, "Should stop the container that was previously running" - assert_hook_ran "post-deploy", output, **hook_variables, runtime: true + assert_hook_ran "post-deploy", output end end diff --git a/test/commander_test.rb b/test/commander_test.rb index 4f0a829e..4a3aa2e2 100644 --- a/test/commander_test.rb +++ b/test/commander_test.rb @@ -104,28 +104,6 @@ class CommanderTest < ActiveSupport::TestCase assert_equal [ "web", "workers" ], @kamal.roles_on("1.1.1.1").map(&:name) end - test "default group strategy" do - assert_empty @kamal.boot_strategy - end - - test "specific limit group strategy" do - configure_with(:deploy_with_boot_strategy) - - assert_equal({ in: :groups, limit: 3, wait: 2 }, @kamal.boot_strategy) - end - - test "percentage-based group strategy" do - configure_with(:deploy_with_percentage_boot_strategy) - - assert_equal({ in: :groups, limit: 1, wait: 2 }, @kamal.boot_strategy) - end - - test "percentage-based group strategy limit is at least 1" do - configure_with(:deploy_with_low_percentage_boot_strategy) - - assert_equal({ in: :groups, limit: 1, wait: 2 }, @kamal.boot_strategy) - end - test "try to match the primary role from a list of specific roles" do configure_with(:deploy_primary_web_role_override) diff --git a/test/configuration/boot_test.rb b/test/configuration/boot_test.rb new file mode 100644 index 00000000..a01e5b06 --- /dev/null +++ b/test/configuration/boot_test.rb @@ -0,0 +1,54 @@ +require "test_helper" + +class ConfigurationBootTest < ActiveSupport::TestCase + test "no group strategy" do + deploy = { + service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" }, builder: { "arch" => "amd64" }, + servers: { "web" => [ "1.1.1.1", "1.1.1.2" ], "workers" => [ "1.1.1.3", "1.1.1.4" ] } + } + + config = Kamal::Configuration.new(deploy) + + assert_nil config.boot.limit + assert_nil config.boot.wait + end + + test "specific limit group strategy" do + deploy = { + service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" }, builder: { "arch" => "amd64" }, + servers: { "web" => [ "1.1.1.1", "1.1.1.2" ], "workers" => [ "1.1.1.3", "1.1.1.4" ] }, + boot: { "limit" => 3, "wait" => 2 } + } + + config = Kamal::Configuration.new(deploy) + + assert_equal 3, config.boot.limit + assert_equal 2, config.boot.wait + end + + test "percentage-based group strategy" do + deploy = { + service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" }, builder: { "arch" => "amd64" }, + servers: { "web" => [ "1.1.1.1", "1.1.1.2" ], "workers" => [ "1.1.1.3", "1.1.1.4" ] }, + boot: { "limit" => "50%", "wait" => 2 } + } + + config = Kamal::Configuration.new(deploy) + + assert_equal 2, config.boot.limit + assert_equal 2, config.boot.wait + end + + test "percentage-based group strategy limit is at least 1" do + deploy = { + service: "app", image: "dhh/app", registry: { "username" => "dhh", "password" => "secret" }, builder: { "arch" => "amd64" }, + servers: { "web" => [ "1.1.1.1", "1.1.1.2" ], "workers" => [ "1.1.1.3", "1.1.1.4" ] }, + boot: { "limit" => "1%", "wait" => 2 } + } + + config = Kamal::Configuration.new(deploy) + + assert_equal 1, config.boot.limit + assert_equal 2, config.boot.wait + end +end diff --git a/test/fixtures/deploy_with_low_percentage_boot_strategy.yml b/test/fixtures/deploy_with_low_percentage_boot_strategy.yml deleted file mode 100644 index 8698b89f..00000000 --- a/test/fixtures/deploy_with_low_percentage_boot_strategy.yml +++ /dev/null @@ -1,19 +0,0 @@ -service: app -image: dhh/app -servers: - web: - - "1.1.1.1" - - "1.1.1.2" - workers: - - "1.1.1.3" - - "1.1.1.4" -builder: - arch: amd64 - -registry: - username: user - password: pw - -boot: - limit: 1% - wait: 2 diff --git a/test/fixtures/deploy_with_percentage_boot_strategy.yml b/test/fixtures/deploy_with_percentage_boot_strategy.yml deleted file mode 100644 index 8698b89f..00000000 --- a/test/fixtures/deploy_with_percentage_boot_strategy.yml +++ /dev/null @@ -1,19 +0,0 @@ -service: app -image: dhh/app -servers: - web: - - "1.1.1.1" - - "1.1.1.2" - workers: - - "1.1.1.3" - - "1.1.1.4" -builder: - arch: amd64 - -registry: - username: user - password: pw - -boot: - limit: 1% - wait: 2 diff --git a/test/integration/app_test.rb b/test/integration/app_test.rb index 039ef012..f128054b 100644 --- a/test/integration/app_test.rb +++ b/test/integration/app_test.rb @@ -15,7 +15,9 @@ class AppTest < IntegrationTest # kamal app start does not wait wait_for_app_to_be_up - kamal :app, :boot + output = kamal :app, :boot, "--verbose", capture: true + assert_match "Booting app on vm1,vm2...", output + assert_match "Booted app on vm1,vm2...", output wait_for_app_to_be_up diff --git a/test/integration/docker/deployer/app/.kamal/hooks/post-app-boot b/test/integration/docker/deployer/app/.kamal/hooks/post-app-boot new file mode 100755 index 00000000..c81b04da --- /dev/null +++ b/test/integration/docker/deployer/app/.kamal/hooks/post-app-boot @@ -0,0 +1,3 @@ +#!/bin/sh +echo "Booted app on ${KAMAL_HOSTS}..." +mkdir -p /tmp/${TEST_ID} && touch /tmp/${TEST_ID}/post-app-boot diff --git a/test/integration/docker/deployer/app/.kamal/hooks/pre-app-boot b/test/integration/docker/deployer/app/.kamal/hooks/pre-app-boot new file mode 100755 index 00000000..0328f18a --- /dev/null +++ b/test/integration/docker/deployer/app/.kamal/hooks/pre-app-boot @@ -0,0 +1,3 @@ +#!/bin/sh +echo "Booting app on ${KAMAL_HOSTS}..." +mkdir -p /tmp/${TEST_ID} && touch /tmp/${TEST_ID}/pre-app-boot diff --git a/test/integration/main_test.rb b/test/integration/main_test.rb index a48051fe..04040c38 100644 --- a/test/integration/main_test.rb +++ b/test/integration/main_test.rb @@ -8,19 +8,19 @@ class MainTest < IntegrationTest kamal :deploy assert_app_is_up version: first_version - assert_hooks_ran "pre-connect", "pre-build", "pre-deploy", "post-deploy" + assert_hooks_ran "pre-connect", "pre-build", "pre-deploy", "pre-app-boot", "post-app-boot", "post-deploy" assert_envs version: first_version second_version = update_app_rev kamal :redeploy assert_app_is_up version: second_version - assert_hooks_ran "pre-connect", "pre-build", "pre-deploy", "post-deploy" + assert_hooks_ran "pre-connect", "pre-build", "pre-deploy", "pre-app-boot", "post-app-boot", "post-deploy" assert_accumulated_assets first_version, second_version kamal :rollback, first_version - assert_hooks_ran "pre-connect", "pre-deploy", "post-deploy" + assert_hooks_ran "pre-connect", "pre-deploy", "pre-app-boot", "post-app-boot", "post-deploy" assert_app_is_up version: first_version details = kamal :details, capture: true diff --git a/test/test_helper.rb b/test/test_helper.rb index f5811872..e77a0a8a 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -36,6 +36,10 @@ class ActiveSupport::TestCase extend Rails::LineFiltering private + setup do + SSHKit::Backend::Netssh.pool.close_connections + end + def stdouted capture(:stdout) { yield }.strip end From 04a96aa5bec3e0a502025fde7a1c4ba7cc991843 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Mon, 3 Feb 2025 16:44:01 +0000 Subject: [PATCH 09/16] Allow accessory roles with no hosts Only raise an exception if the role is not found, not it it has no hosts. --- lib/kamal/configuration/accessory.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/kamal/configuration/accessory.rb b/lib/kamal/configuration/accessory.rb index ccb845fd..aed60492 100644 --- a/lib/kamal/configuration/accessory.rb +++ b/lib/kamal/configuration/accessory.rb @@ -201,7 +201,11 @@ class Kamal::Configuration::Accessory def hosts_from_roles if accessory_config.key?("roles") accessory_config["roles"].flat_map do |role| - config.role(role)&.hosts || raise(Kamal::ConfigurationError, "Unknown role in accessories config: '#{role}'") + unless (role = config.role(role)) + raise Kamal::ConfigurationError, "Unknown role in accessories config: '#{role}'" unless config.role(role) + end + + role.hosts end end end From e69611efb65bf2a4c455920e84b25f9f264348e6 Mon Sep 17 00:00:00 2001 From: Neil Johari Date: Mon, 3 Feb 2025 08:56:06 -0800 Subject: [PATCH 10/16] Add final newline --- lib/kamal/secrets/adapters/aws_secrets_manager.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index fb3f887f..1ed5089b 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -47,4 +47,5 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Ba `aws --version 2> /dev/null` $?.success? end -end \ No newline at end of file +end + From 07d05ad58ad76bb855285d9650622b026db79e02 Mon Sep 17 00:00:00 2001 From: Neil Johari Date: Mon, 3 Feb 2025 09:44:39 -0800 Subject: [PATCH 11/16] Run rubocop auto correct --- lib/kamal/secrets/adapters/aws_secrets_manager.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/kamal/secrets/adapters/aws_secrets_manager.rb b/lib/kamal/secrets/adapters/aws_secrets_manager.rb index 1ed5089b..d37b4246 100644 --- a/lib/kamal/secrets/adapters/aws_secrets_manager.rb +++ b/lib/kamal/secrets/adapters/aws_secrets_manager.rb @@ -48,4 +48,3 @@ class Kamal::Secrets::Adapters::AwsSecretsManager < Kamal::Secrets::Adapters::Ba $?.success? end end - From 8775d202bd82ab3e9b2829e763e8964d1cf572e3 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Tue, 4 Feb 2025 09:01:42 +0000 Subject: [PATCH 12/16] Prep for 2.5 - Reset KAMAL on alias command, rather than relying on checking "invoked_via_subcommand" - Validate the accessories roles when loading the configuration not later on when trying to access them --- lib/kamal/cli/alias/command.rb | 1 + lib/kamal/cli/base.rb | 2 +- lib/kamal/commander.rb | 27 ++++++++++++++++----------- lib/kamal/configuration/accessory.rb | 16 +++++++++------- 4 files changed, 27 insertions(+), 19 deletions(-) diff --git a/lib/kamal/cli/alias/command.rb b/lib/kamal/cli/alias/command.rb index 4bb70c5a..49c5a57f 100644 --- a/lib/kamal/cli/alias/command.rb +++ b/lib/kamal/cli/alias/command.rb @@ -1,6 +1,7 @@ class Kamal::Cli::Alias::Command < Thor::DynamicCommand def run(instance, args = []) if (_alias = KAMAL.config.aliases[name]) + KAMAL.reset Kamal::Cli::Main.start(Shellwords.split(_alias.command) + ARGV[1..-1]) else super diff --git a/lib/kamal/cli/base.rb b/lib/kamal/cli/base.rb index 8dde4752..0a6dd6e7 100644 --- a/lib/kamal/cli/base.rb +++ b/lib/kamal/cli/base.rb @@ -31,7 +31,7 @@ module Kamal::Cli super end - initialize_commander unless config[:invoked_via_subcommand] + initialize_commander unless KAMAL.configured? end private diff --git a/lib/kamal/commander.rb b/lib/kamal/commander.rb index 97a82ce6..b0e8f0ba 100644 --- a/lib/kamal/commander.rb +++ b/lib/kamal/commander.rb @@ -4,13 +4,20 @@ require "active_support/core_ext/object/blank" class Kamal::Commander attr_accessor :verbosity, :holding_lock, :connected + attr_reader :specific_roles, :specific_hosts delegate :hosts, :roles, :primary_host, :primary_role, :roles_on, :proxy_hosts, :accessory_hosts, to: :specifics def initialize + reset + end + + def reset self.verbosity = :info self.holding_lock = false self.connected = false - @specifics = nil + @specifics = @specific_roles = @specific_hosts = nil + @config = @config_kwargs = nil + @commands = {} end def config @@ -28,8 +35,6 @@ class Kamal::Commander @config || @config_kwargs end - attr_reader :specific_roles, :specific_hosts - def specific_primary! @specifics = nil if specific_roles.present? @@ -89,35 +94,35 @@ class Kamal::Commander end def builder - @builder ||= Kamal::Commands::Builder.new(config) + @commands[:builder] ||= Kamal::Commands::Builder.new(config) end def docker - @docker ||= Kamal::Commands::Docker.new(config) + @commands[:docker] ||= Kamal::Commands::Docker.new(config) end def hook - @hook ||= Kamal::Commands::Hook.new(config) + @commands[:hook] ||= Kamal::Commands::Hook.new(config) end def lock - @lock ||= Kamal::Commands::Lock.new(config) + @commands[:lock] ||= Kamal::Commands::Lock.new(config) end def proxy - @proxy ||= Kamal::Commands::Proxy.new(config) + @commands[:proxy] ||= Kamal::Commands::Proxy.new(config) end def prune - @prune ||= Kamal::Commands::Prune.new(config) + @commands[:prune] ||= Kamal::Commands::Prune.new(config) end def registry - @registry ||= Kamal::Commands::Registry.new(config) + @commands[:registry] ||= Kamal::Commands::Registry.new(config) end def server - @server ||= Kamal::Commands::Server.new(config) + @commands[:server] ||= Kamal::Commands::Server.new(config) end def alias(name) diff --git a/lib/kamal/configuration/accessory.rb b/lib/kamal/configuration/accessory.rb index aed60492..62979627 100644 --- a/lib/kamal/configuration/accessory.rb +++ b/lib/kamal/configuration/accessory.rb @@ -16,6 +16,8 @@ class Kamal::Configuration::Accessory context: "accessories/#{name}", with: Kamal::Configuration::Validator::Accessory + ensure_valid_roles + @env = initialize_env @proxy = initialize_proxy if running_proxy? @registry = initialize_registry if accessory_config["registry"].present? @@ -200,17 +202,17 @@ class Kamal::Configuration::Accessory def hosts_from_roles if accessory_config.key?("roles") - accessory_config["roles"].flat_map do |role| - unless (role = config.role(role)) - raise Kamal::ConfigurationError, "Unknown role in accessories config: '#{role}'" unless config.role(role) - end - - role.hosts - end + accessory_config["roles"].flat_map { |role| config.role(role)&.hosts } end end def network accessory_config["network"] || DEFAULT_NETWORK end + + def ensure_valid_roles + if accessory_config["roles"] && (missing_roles = accessory_config["roles"] - config.roles.map(&:name)).any? + raise Kamal::ConfigurationError, "accessories/#{name}: unknown roles #{missing_roles.join(", ")}" + end + end end From 9af2425fbd08691093a19bf6f7789837214747c4 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Tue, 4 Feb 2025 11:05:25 +0000 Subject: [PATCH 13/16] Doc changes for 2.5 Sync up the accessory.yml file with the latest changes in the kamal-site repo. --- lib/kamal/configuration/docs/accessory.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/kamal/configuration/docs/accessory.yml b/lib/kamal/configuration/docs/accessory.yml index 571d7217..1852681e 100644 --- a/lib/kamal/configuration/docs/accessory.yml +++ b/lib/kamal/configuration/docs/accessory.yml @@ -118,5 +118,6 @@ accessories: # Proxy # + # You can run your accessory behind the Kamal proxy. See kamal docs proxy for more information proxy: ... From 45197e46f604f142bb2dbdf3f5df6bdf43032ace Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Tue, 4 Feb 2025 11:19:44 +0000 Subject: [PATCH 14/16] Bump version for 2.5.0 --- Gemfile.lock | 2 +- lib/kamal/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 67963d62..e57100b2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - kamal (2.4.0) + kamal (2.5.0) activesupport (>= 7.0) base64 (~> 0.2) bcrypt_pbkdf (~> 1.0) diff --git a/lib/kamal/version.rb b/lib/kamal/version.rb index 9f3a9126..dc4a08cc 100644 --- a/lib/kamal/version.rb +++ b/lib/kamal/version.rb @@ -1,3 +1,3 @@ module Kamal - VERSION = "2.4.0" + VERSION = "2.5.0" end From fdf7e6927a117799859a671ad5031a10fa836e81 Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Thu, 6 Feb 2025 11:11:51 +0000 Subject: [PATCH 15/16] Only check for docker when logging in locally If we are skipping a local registry login, we don't need docker installed locally. Fixes: https://github.com/basecamp/kamal/issues/1400 --- lib/kamal/cli/registry.rb | 2 +- test/cli/registry_test.rb | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/kamal/cli/registry.rb b/lib/kamal/cli/registry.rb index 2fbdba1d..289f9e4b 100644 --- a/lib/kamal/cli/registry.rb +++ b/lib/kamal/cli/registry.rb @@ -3,7 +3,7 @@ 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 + ensure_docker_installed unless options[:skip_local] run_locally { execute *KAMAL.registry.login } unless options[:skip_local] on(KAMAL.hosts) { execute *KAMAL.registry.login } unless options[:skip_remote] diff --git a/test/cli/registry_test.rb b/test/cli/registry_test.rb index e89a15e4..f3617985 100644 --- a/test/cli/registry_test.rb +++ b/test/cli/registry_test.rb @@ -52,6 +52,18 @@ class CliRegistryTest < CliTestCase assert_raises(Kamal::Cli::DependencyError) { run_command("login") } end + test "allow remote 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")) + + SSHKit::Backend::Abstract.any_instance.stubs(:execute) + .with { |*args| args[0..1] == [ :docker, :login ] } + + assert_nothing_raised { run_command("login", "--skip-local") } + end + private def run_command(*command) From 28be0300b9705aa52edbacf3b484956ac222e85a Mon Sep 17 00:00:00 2001 From: Donal McBreen Date: Thu, 6 Feb 2025 13:54:46 +0000 Subject: [PATCH 16/16] Bump version for 2.5.1 --- Gemfile.lock | 2 +- lib/kamal/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index e57100b2..56ba684b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - kamal (2.5.0) + kamal (2.5.1) activesupport (>= 7.0) base64 (~> 0.2) bcrypt_pbkdf (~> 1.0) diff --git a/lib/kamal/version.rb b/lib/kamal/version.rb index dc4a08cc..eefac514 100644 --- a/lib/kamal/version.rb +++ b/lib/kamal/version.rb @@ -1,3 +1,3 @@ module Kamal - VERSION = "2.5.0" + VERSION = "2.5.1" end