From bfb70b211848e55db6b9a3603f28438a1ed28b50 Mon Sep 17 00:00:00 2001 From: Jberczel Date: Mon, 1 May 2023 15:26:50 -0400 Subject: [PATCH 01/11] Add local dependencies check Add checks for: * Docker installed locally * Docker buildx plugin installed locally * Dockerfile exists If checks fail, it will halt deployment and provide more specific error messages. Also adds a cli subcommand: `mrsk build dependencies` Fixes: #109 and #237 --- lib/mrsk/cli/build.rb | 26 +++++++++++++++++++++++--- lib/mrsk/commands/builder.rb | 24 ++++++++++++++++++++++++ lib/mrsk/commands/builder/base.rb | 10 +++++++++- test/cli/build_test.rb | 18 ++++++++++++++++++ test/commands/builder_test.rb | 9 +++++++++ 5 files changed, 83 insertions(+), 4 deletions(-) diff --git a/lib/mrsk/cli/build.rb b/lib/mrsk/cli/build.rb index 289f0bcb0..695b0df21 100644 --- a/lib/mrsk/cli/build.rb +++ b/lib/mrsk/cli/build.rb @@ -1,4 +1,7 @@ class Mrsk::Cli::Build < Mrsk::Cli::Base + + class BuildError < StandardError; end + desc "deliver", "Build app and push app image to registry then pull image on servers" def deliver with_lock do @@ -10,16 +13,18 @@ def deliver desc "push", "Build and push app image to registry" def push with_lock do - cli = self + cli_build = self run_locally do begin - MRSK.with_verbosity(:debug) { execute *MRSK.builder.push } + if cli_build.dependencies + MRSK.with_verbosity(:debug) { execute *MRSK.builder.push } + end rescue SSHKit::Command::Failed => e if e.message =~ /(no builder)|(no such file or directory)/ error "Missing compatible builder, so creating a new one first" - if cli.create + if cli_build.create MRSK.with_verbosity(:debug) { execute *MRSK.builder.push } end else @@ -77,4 +82,19 @@ def details puts capture(*MRSK.builder.info) end end + + desc "dependencies", "Check local dependencies" + def dependencies + run_locally do + begin + execute *MRSK.builder.dependencies + rescue SSHKit::Command::Failed => e + build_error = e.message =~ /command not found/ ? + "Docker is not installed locally" : + "Docker buildx plugin is not installed locally" + raise BuildError, build_error + end + end + true + end end diff --git a/lib/mrsk/commands/builder.rb b/lib/mrsk/commands/builder.rb index 85dbce0a7..5c3f18e41 100644 --- a/lib/mrsk/commands/builder.rb +++ b/lib/mrsk/commands/builder.rb @@ -33,4 +33,28 @@ def multiarch def multiarch_remote @multiarch_remote ||= Mrsk::Commands::Builder::Multiarch::Remote.new(config) end + + def native_and_local? + name == 'native' + end + + def dependencies + if native_and_local? + docker_version + else + combine \ + docker_version, + docker_buildx_version + end + end + + private + + def docker_version + docker "--version" + end + + def docker_buildx_version + docker :buildx, "version" + end end diff --git a/lib/mrsk/commands/builder/base.rb b/lib/mrsk/commands/builder/base.rb index 15c6b1889..460cf1419 100644 --- a/lib/mrsk/commands/builder/base.rb +++ b/lib/mrsk/commands/builder/base.rb @@ -1,6 +1,9 @@ + class Mrsk::Commands::Builder::Base < Mrsk::Commands::Base delegate :argumentize, to: Mrsk::Utils + class BuilderError < StandardError; end + def clean docker :image, :rm, "--force", config.absolute_image end @@ -17,6 +20,7 @@ def build_context context end + private def build_tags [ "-t", config.absolute_image, "-t", config.latest_image ] @@ -35,7 +39,11 @@ def build_secrets end def build_dockerfile - argumentize "--file", dockerfile + if Pathname.new(File.expand_path(dockerfile)).exist? + argumentize "--file", dockerfile + else + raise BuilderError, "Missing Dockerfile" + end end def args diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index a610d8496..fa5c958d4 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -9,6 +9,7 @@ class CliBuildTest < CliTestCase end test "push" do + Mrsk::Cli::Build.any_instance.stubs(:dependencies).returns(true) run_command("push").tap do |output| assert_match /docker buildx build --push --platform linux\/amd64,linux\/arm64 --builder mrsk-app-multiarch -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile \. as .*@localhost/, output end @@ -16,6 +17,7 @@ class CliBuildTest < CliTestCase test "push without builder" do stub_locking + Mrsk::Cli::Build.any_instance.stubs(:dependencies).returns(true) SSHKit::Backend::Abstract.any_instance.stubs(:execute) .with { |arg| arg == :docker } .raises(SSHKit::Command::Failed.new("no builder")) @@ -68,6 +70,22 @@ class CliBuildTest < CliTestCase end end + test "dependencies" do + Mrsk::Commands::Builder.any_instance.stubs(:native_and_local?).returns(false) + run_command("dependencies").tap do |output| + assert_match /docker --version && docker buildx version/, output + end + end + + test "dependencies with no buildx plugin" do + SSHKit::Backend::Abstract.any_instance.stubs(:execute) + .with(:docker, "--version", "&&", :docker, :buildx, "version") + .raises(SSHKit::Command::Failed.new("no buildx")) + + Mrsk::Commands::Builder.any_instance.stubs(:native_and_local?).returns(false) + assert_raises(Mrsk::Cli::Build::BuildError) { run_command("dependencies") } + end + private def run_command(*command) stdouted { Mrsk::Cli::Build.start([*command, "-c", "test/fixtures/deploy_with_accessories.yml"]) } diff --git a/test/commands/builder_test.rb b/test/commands/builder_test.rb index 1ecea021a..e5f83f1c6 100644 --- a/test/commands/builder_test.rb +++ b/test/commands/builder_test.rb @@ -52,12 +52,21 @@ class CommandsBuilderTest < ActiveSupport::TestCase end test "build dockerfile" do + 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", builder.target.build_options.join(" ") end + test "missing dockerfile" do + Pathname.any_instance.expects(:exist?).returns(false).once + builder = new_builder_command(builder: { "dockerfile" => "Dockerfile.xyz" }) + assert_raises(Mrsk::Commands::Builder::Base::BuilderError) do + builder.target.build_options.join(" ") + end + end + test "build context" do builder = new_builder_command(builder: { "context" => ".." }) assert_equal \ From dc3be30b1606cecbf82c02ce0170352a8b198d8d Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 10:29:49 +0200 Subject: [PATCH 02/11] Style --- lib/mrsk/cli/build.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/mrsk/cli/build.rb b/lib/mrsk/cli/build.rb index 695b0df21..cd2f06028 100644 --- a/lib/mrsk/cli/build.rb +++ b/lib/mrsk/cli/build.rb @@ -1,5 +1,4 @@ class Mrsk::Cli::Build < Mrsk::Cli::Base - class BuildError < StandardError; end desc "deliver", "Build app and push app image to registry then pull image on servers" From cbda851436ee0e876e73cbf3ca7a9d68148c81d1 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 13:34:56 +0200 Subject: [PATCH 03/11] Style --- lib/mrsk/commands/builder.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/mrsk/commands/builder.rb b/lib/mrsk/commands/builder.rb index 5c3f18e41..c8fb1c59f 100644 --- a/lib/mrsk/commands/builder.rb +++ b/lib/mrsk/commands/builder.rb @@ -42,14 +42,13 @@ def dependencies if native_and_local? docker_version else - combine \ - docker_version, - docker_buildx_version + combine \ + docker_version, + docker_buildx_version end end private - def docker_version docker "--version" end From f719540e0cf0c84d1c0b732a070a3dc0f8fc06ca Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 13:35:05 +0200 Subject: [PATCH 04/11] Style --- lib/mrsk/cli/build.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/mrsk/cli/build.rb b/lib/mrsk/cli/build.rb index cd2f06028..0c2bcfde7 100644 --- a/lib/mrsk/cli/build.rb +++ b/lib/mrsk/cli/build.rb @@ -12,18 +12,18 @@ def deliver desc "push", "Build and push app image to registry" def push with_lock do - cli_build = self + cli = self run_locally do begin - if cli_build.dependencies + if cli.dependencies MRSK.with_verbosity(:debug) { execute *MRSK.builder.push } end rescue SSHKit::Command::Failed => e if e.message =~ /(no builder)|(no such file or directory)/ error "Missing compatible builder, so creating a new one first" - if cli_build.create + if cli.create MRSK.with_verbosity(:debug) { execute *MRSK.builder.push } end else @@ -94,6 +94,7 @@ def dependencies raise BuildError, build_error end end + true end end From ae2effb80cce93b0079e6f71a3ff4056b06ea6da Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 14:04:23 +0200 Subject: [PATCH 05/11] Improve clarity and intent --- lib/mrsk/cli/build.rb | 10 ++++++---- lib/mrsk/commands/builder.rb | 19 ++++++++----------- test/cli/build_test.rb | 13 +++++++------ test/configuration_test.rb | 2 +- 4 files changed, 22 insertions(+), 22 deletions(-) diff --git a/lib/mrsk/cli/build.rb b/lib/mrsk/cli/build.rb index 0c2bcfde7..75558812c 100644 --- a/lib/mrsk/cli/build.rb +++ b/lib/mrsk/cli/build.rb @@ -16,7 +16,7 @@ def push run_locally do begin - if cli.dependencies + if cli.verify_dependencies MRSK.with_verbosity(:debug) { execute *MRSK.builder.push } end rescue SSHKit::Command::Failed => e @@ -82,15 +82,17 @@ def details end end - desc "dependencies", "Check local dependencies" - def dependencies + + desc "", "" # Really a private method, but needed to be invoked from #push + def verify_dependencies run_locally do begin - execute *MRSK.builder.dependencies + execute *MRSK.builder.ensure_dependencies_installed rescue SSHKit::Command::Failed => e build_error = e.message =~ /command not found/ ? "Docker is not installed locally" : "Docker buildx plugin is not installed locally" + raise BuildError, build_error end end diff --git a/lib/mrsk/commands/builder.rb b/lib/mrsk/commands/builder.rb index c8fb1c59f..f7710b37c 100644 --- a/lib/mrsk/commands/builder.rb +++ b/lib/mrsk/commands/builder.rb @@ -2,7 +2,7 @@ class Mrsk::Commands::Builder < Mrsk::Commands::Base delegate :create, :remove, :push, :clean, :pull, :info, to: :target def name - target.class.to_s.remove("Mrsk::Commands::Builder::").underscore + target.class.to_s.remove("Mrsk::Commands::Builder::").underscore.inquiry end def target @@ -34,26 +34,23 @@ def multiarch_remote @multiarch_remote ||= Mrsk::Commands::Builder::Multiarch::Remote.new(config) end - def native_and_local? - name == 'native' - end - def dependencies - if native_and_local? - docker_version + def ensure_dependencies_installed + if name.native? + ensure_docker_installed else combine \ - docker_version, - docker_buildx_version + ensure_docker_installed, + ensure_buildx_installed end end private - def docker_version + def ensure_docker_installed docker "--version" end - def docker_buildx_version + def ensure_buildx_installed docker :buildx, "version" end end diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index fa5c958d4..304f65f43 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -9,7 +9,7 @@ class CliBuildTest < CliTestCase end test "push" do - Mrsk::Cli::Build.any_instance.stubs(:dependencies).returns(true) + Mrsk::Cli::Build.any_instance.stubs(:verify_dependencies).returns(true) run_command("push").tap do |output| assert_match /docker buildx build --push --platform linux\/amd64,linux\/arm64 --builder mrsk-app-multiarch -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile \. as .*@localhost/, output end @@ -17,7 +17,7 @@ class CliBuildTest < CliTestCase test "push without builder" do stub_locking - Mrsk::Cli::Build.any_instance.stubs(:dependencies).returns(true) + Mrsk::Cli::Build.any_instance.stubs(:verify_dependencies).returns(true) SSHKit::Backend::Abstract.any_instance.stubs(:execute) .with { |arg| arg == :docker } .raises(SSHKit::Command::Failed.new("no builder")) @@ -70,9 +70,10 @@ class CliBuildTest < CliTestCase end end - test "dependencies" do - Mrsk::Commands::Builder.any_instance.stubs(:native_and_local?).returns(false) - run_command("dependencies").tap do |output| + test "verify dependencies" do + Mrsk::Commands::Builder.any_instance.stubs(:name).returns("remote".inquiry) + + run_command("verify_dependencies").tap do |output| assert_match /docker --version && docker buildx version/, output end end @@ -83,7 +84,7 @@ class CliBuildTest < CliTestCase .raises(SSHKit::Command::Failed.new("no buildx")) Mrsk::Commands::Builder.any_instance.stubs(:native_and_local?).returns(false) - assert_raises(Mrsk::Cli::Build::BuildError) { run_command("dependencies") } + assert_raises(Mrsk::Cli::Build::BuildError) { run_command("verify_dependencies") } end private diff --git a/test/configuration_test.rb b/test/configuration_test.rb index 2697673bb..eda85484e 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -48,7 +48,7 @@ class ConfigurationTest < ActiveSupport::TestCase end test "role" do - assert_equal "web", @config.role(:web).name + assert @config.role(:web).name.web? assert_equal "workers", @config_with_roles.role(:workers).name assert_nil @config.role(:missing) end From 98e7b995d577f56a51923db4f4784ee366542dc5 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 14:04:37 +0200 Subject: [PATCH 06/11] Distinguish from local dependency verification --- lib/mrsk/cli/main.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mrsk/cli/main.rb b/lib/mrsk/cli/main.rb index 0a3d2ce09..3c16f5336 100644 --- a/lib/mrsk/cli/main.rb +++ b/lib/mrsk/cli/main.rb @@ -17,7 +17,7 @@ def deploy invoke_options = deploy_options runtime = print_runtime do - say "Ensure curl and Docker are installed...", :magenta + say "Ensure curl and Docker are installed on servers...", :magenta invoke "mrsk:cli:server:bootstrap", [], invoke_options say "Log into image registry...", :magenta From c611a1616a5547dbe3b9da1ce40371a599bfe293 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 14:06:06 +0200 Subject: [PATCH 07/11] Distinguish from server dependencies --- lib/mrsk/cli/build.rb | 2 +- lib/mrsk/commands/builder.rb | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/mrsk/cli/build.rb b/lib/mrsk/cli/build.rb index 75558812c..9831f4e6e 100644 --- a/lib/mrsk/cli/build.rb +++ b/lib/mrsk/cli/build.rb @@ -87,7 +87,7 @@ def details def verify_dependencies run_locally do begin - execute *MRSK.builder.ensure_dependencies_installed + execute *MRSK.builder.ensure_local_dependencies_installed rescue SSHKit::Command::Failed => e build_error = e.message =~ /command not found/ ? "Docker is not installed locally" : diff --git a/lib/mrsk/commands/builder.rb b/lib/mrsk/commands/builder.rb index f7710b37c..58ca626e9 100644 --- a/lib/mrsk/commands/builder.rb +++ b/lib/mrsk/commands/builder.rb @@ -35,22 +35,22 @@ def multiarch_remote end - def ensure_dependencies_installed + def ensure_local_dependencies_installed if name.native? - ensure_docker_installed + ensure_local_docker_installed else combine \ - ensure_docker_installed, - ensure_buildx_installed + ensure_local_docker_installed, + ensure_local_buildx_installed end end private - def ensure_docker_installed + def ensure_local_docker_installed docker "--version" end - def ensure_buildx_installed + def ensure_local_buildx_installed docker :buildx, "version" end end From bb74a74dc48de7484a065bb12ceefe64f0f58569 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 14:07:30 +0200 Subject: [PATCH 08/11] Style --- lib/mrsk/commands/builder/base.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/mrsk/commands/builder/base.rb b/lib/mrsk/commands/builder/base.rb index 460cf1419..2d257e52b 100644 --- a/lib/mrsk/commands/builder/base.rb +++ b/lib/mrsk/commands/builder/base.rb @@ -1,9 +1,9 @@ class Mrsk::Commands::Builder::Base < Mrsk::Commands::Base - delegate :argumentize, to: Mrsk::Utils - class BuilderError < StandardError; end + delegate :argumentize, to: Mrsk::Utils + def clean docker :image, :rm, "--force", config.absolute_image end From a22e27dbf809bdd44247f20272a753dd0f8d0a45 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 14:07:47 +0200 Subject: [PATCH 09/11] Reveal configured dockerfile path --- lib/mrsk/commands/builder/base.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mrsk/commands/builder/base.rb b/lib/mrsk/commands/builder/base.rb index 2d257e52b..32dcba4fb 100644 --- a/lib/mrsk/commands/builder/base.rb +++ b/lib/mrsk/commands/builder/base.rb @@ -42,7 +42,7 @@ def build_dockerfile if Pathname.new(File.expand_path(dockerfile)).exist? argumentize "--file", dockerfile else - raise BuilderError, "Missing Dockerfile" + raise BuilderError, "Missing #{dockerfile}" end end From a2d99e48bf01eccabac94c6322bbea1a68308398 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 14:08:29 +0200 Subject: [PATCH 10/11] Naming --- test/cli/build_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index 304f65f43..d14c1cd4c 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -70,7 +70,7 @@ class CliBuildTest < CliTestCase end end - test "verify dependencies" do + test "verify local dependencies" do Mrsk::Commands::Builder.any_instance.stubs(:name).returns("remote".inquiry) run_command("verify_dependencies").tap do |output| @@ -78,7 +78,7 @@ class CliBuildTest < CliTestCase end end - test "dependencies with no buildx plugin" do + test "verify local dependencies with no buildx plugin" do SSHKit::Backend::Abstract.any_instance.stubs(:execute) .with(:docker, "--version", "&&", :docker, :buildx, "version") .raises(SSHKit::Command::Failed.new("no buildx")) From 263a24afe34441699e237eab1c7354b44fb256c5 Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Tue, 2 May 2023 14:09:10 +0200 Subject: [PATCH 11/11] Further distinguish dependency verification --- lib/mrsk/cli/build.rb | 4 ++-- test/cli/build_test.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/mrsk/cli/build.rb b/lib/mrsk/cli/build.rb index 9831f4e6e..0e6e9a7c0 100644 --- a/lib/mrsk/cli/build.rb +++ b/lib/mrsk/cli/build.rb @@ -16,7 +16,7 @@ def push run_locally do begin - if cli.verify_dependencies + if cli.verify_local_dependencies MRSK.with_verbosity(:debug) { execute *MRSK.builder.push } end rescue SSHKit::Command::Failed => e @@ -84,7 +84,7 @@ def details desc "", "" # Really a private method, but needed to be invoked from #push - def verify_dependencies + def verify_local_dependencies run_locally do begin execute *MRSK.builder.ensure_local_dependencies_installed diff --git a/test/cli/build_test.rb b/test/cli/build_test.rb index d14c1cd4c..0835262cc 100644 --- a/test/cli/build_test.rb +++ b/test/cli/build_test.rb @@ -9,7 +9,7 @@ class CliBuildTest < CliTestCase end test "push" do - Mrsk::Cli::Build.any_instance.stubs(:verify_dependencies).returns(true) + Mrsk::Cli::Build.any_instance.stubs(:verify_local_dependencies).returns(true) run_command("push").tap do |output| assert_match /docker buildx build --push --platform linux\/amd64,linux\/arm64 --builder mrsk-app-multiarch -t dhh\/app:999 -t dhh\/app:latest --label service="app" --file Dockerfile \. as .*@localhost/, output end @@ -17,7 +17,7 @@ class CliBuildTest < CliTestCase test "push without builder" do stub_locking - Mrsk::Cli::Build.any_instance.stubs(:verify_dependencies).returns(true) + Mrsk::Cli::Build.any_instance.stubs(:verify_local_dependencies).returns(true) SSHKit::Backend::Abstract.any_instance.stubs(:execute) .with { |arg| arg == :docker } .raises(SSHKit::Command::Failed.new("no builder")) @@ -73,7 +73,7 @@ class CliBuildTest < CliTestCase test "verify local dependencies" do Mrsk::Commands::Builder.any_instance.stubs(:name).returns("remote".inquiry) - run_command("verify_dependencies").tap do |output| + run_command("verify_local_dependencies").tap do |output| assert_match /docker --version && docker buildx version/, output end end @@ -84,7 +84,7 @@ class CliBuildTest < CliTestCase .raises(SSHKit::Command::Failed.new("no buildx")) Mrsk::Commands::Builder.any_instance.stubs(:native_and_local?).returns(false) - assert_raises(Mrsk::Cli::Build::BuildError) { run_command("verify_dependencies") } + assert_raises(Mrsk::Cli::Build::BuildError) { run_command("verify_local_dependencies") } end private