From 47f8725cf3f5d243e06fc73c6fd29387faf07bc8 Mon Sep 17 00:00:00 2001 From: Matthew Kent Date: Fri, 10 Nov 2023 16:35:25 -0800 Subject: [PATCH 1/6] Support a dynamic primary_web_role instead of assuming it's 'web'. This allows for more meaningful naming in roles. The only caution here is that we don't support the renaming of roles, so any migration is left to the user. --- lib/kamal/commands/healthcheck.rb | 2 +- lib/kamal/configuration.rb | 20 +++++++++++++++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/lib/kamal/commands/healthcheck.rb b/lib/kamal/commands/healthcheck.rb index 5adc0244..05379ab6 100644 --- a/lib/kamal/commands/healthcheck.rb +++ b/lib/kamal/commands/healthcheck.rb @@ -1,7 +1,7 @@ class Kamal::Commands::Healthcheck < Kamal::Commands::Base def run - web = config.role(:web) + web = config.role(config.primary_web_role) docker :run, "--detach", diff --git a/lib/kamal/configuration.rb b/lib/kamal/configuration.rb index fc881c06..64c74c63 100644 --- a/lib/kamal/configuration.rb +++ b/lib/kamal/configuration.rb @@ -90,14 +90,21 @@ class Kamal::Configuration end def primary_web_host - role(:web).primary_host + role(primary_web_role)&.primary_host + end + + def traefik_roles + roles.select(&:running_traefik?) + end + + def traefik_role_names + traefik_roles.flat_map(&:name) end def traefik_hosts - roles.select(&:running_traefik?).flat_map(&:hosts).uniq + traefik_roles.flat_map(&:hosts).uniq end - def repository [ raw_config.registry["server"], image ].compact.join("/") end @@ -199,6 +206,9 @@ class Kamal::Configuration raw_config.asset_path end + def primary_web_role + raw_config.primary_web_role || "web" + end def valid? ensure_destination_if_required && ensure_required_keys_present && ensure_valid_kamal_version @@ -253,6 +263,10 @@ class Kamal::Configuration end end + unless traefik_role_names.include?(primary_web_role) + raise ArgumentError, "Role #{primary_web_role} needs to have traefik enabled" + end + true end From 628a47ad88e0935c9fb75f5b843e07211007e255 Mon Sep 17 00:00:00 2001 From: Matthew Kent Date: Fri, 10 Nov 2023 16:39:06 -0800 Subject: [PATCH 2/6] Background for the new option. --- lib/kamal/cli/templates/deploy.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/kamal/cli/templates/deploy.yml b/lib/kamal/cli/templates/deploy.yml index 65b3062d..44fa2294 100644 --- a/lib/kamal/cli/templates/deploy.yml +++ b/lib/kamal/cli/templates/deploy.yml @@ -83,3 +83,11 @@ registry: # boot: # limit: 10 # Can also specify as a percentage of total hosts, such as "25%" # wait: 2 + +# Configure the role used to determine the primary_web_host. This host takes +# deploy locks, runs health checks during the deploy, and follow logs, etc. +# This role should have traefik enabled. +# +# Caution: there's no support for role renaming yet, so be careful to cleanup +# the previous role on the deployed hosts. +# primary_web_role: web From d0ac6507e760c67b2ae798f04e5cd75b81163288 Mon Sep 17 00:00:00 2001 From: Matthew Kent Date: Fri, 10 Nov 2023 16:49:37 -0800 Subject: [PATCH 3/6] Add test coverage. --- test/configuration_test.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/test/configuration_test.rb b/test/configuration_test.rb index e48659ed..63e1de4a 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -278,4 +278,22 @@ class ConfigurationTest < ActiveSupport::TestCase assert_nil @config.asset_path assert_equal "foo", Kamal::Configuration.new(@deploy.merge!(asset_path: "foo")).asset_path end + + test "primary web role" do + assert_equal "web", @config.primary_web_role + + config = Kamal::Configuration.new(@deploy_with_roles.deep_merge({ + servers: { "alternate_web" => { "hosts" => [ "1.1.1.4", "1.1.1.5" ] , "traefik" => true } }, + primary_web_role: "alternate_web" } )) + + assert_equal "alternate_web", config.primary_web_role + assert_equal "1.1.1.4", config.primary_web_host + end + + test "primary web role no traefik" do + error = assert_raises(ArgumentError) do + Kamal::Configuration.new(@deploy.merge(primary_web_role: "bar")) + end + assert_match /bar/, error.message + end end From 6898e8789e220714520393ce68a153a2bef6a87a Mon Sep 17 00:00:00 2001 From: Matthew Kent Date: Fri, 10 Nov 2023 17:17:16 -0800 Subject: [PATCH 4/6] Further test the override. --- test/cli/main_test.rb | 10 ++++++++++ .../deploy_primary_web_role_override.yml | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+) create mode 100644 test/fixtures/deploy_primary_web_role_override.yml diff --git a/test/cli/main_test.rb b/test/cli/main_test.rb index a19ff90e..07e47f8d 100644 --- a/test/cli/main_test.rb +++ b/test/cli/main_test.rb @@ -283,6 +283,16 @@ class CliMainTest < CliTestCase end end + test "config with primary web role override" do + run_command("config", config_file: "deploy_primary_web_role_override").tap do |output| + config = YAML.load(output) + + assert_equal ["web_chicago", "web_tokyo"], config[:roles] + assert_equal ["1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4"], config[:hosts] + assert_equal "1.1.1.3", config[:primary_host] + end + end + test "config with destination" do run_command("config", "-d", "world", config_file: "deploy_for_dest").tap do |output| config = YAML.load(output) diff --git a/test/fixtures/deploy_primary_web_role_override.yml b/test/fixtures/deploy_primary_web_role_override.yml new file mode 100644 index 00000000..264819fe --- /dev/null +++ b/test/fixtures/deploy_primary_web_role_override.yml @@ -0,0 +1,20 @@ +service: app +image: dhh/app +servers: + web_chicago: + traefik: enabled + hosts: + - 1.1.1.1 + - 1.1.1.2 + web_tokyo: + traefik: enabled + hosts: + - 1.1.1.3 + - 1.1.1.4 +env: + REDIS_URL: redis://x/y +registry: + server: registry.digitalocean.com + username: user + password: pw +primary_web_role: web_tokyo From a9cc7c73d2928260236dbdc5818e5b1fd901b64c Mon Sep 17 00:00:00 2001 From: Matthew Kent Date: Sat, 11 Nov 2023 12:57:31 -0800 Subject: [PATCH 5/6] Handle an undefined primary_web_role. --- lib/kamal/configuration.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/kamal/configuration.rb b/lib/kamal/configuration.rb index 64c74c63..f7f2233c 100644 --- a/lib/kamal/configuration.rb +++ b/lib/kamal/configuration.rb @@ -263,6 +263,10 @@ class Kamal::Configuration end end + unless role_names.include?(primary_web_role) + raise ArgumentError, "The primary_web_role #{primary_web_role} isn't defined" + end + unless traefik_role_names.include?(primary_web_role) raise ArgumentError, "Role #{primary_web_role} needs to have traefik enabled" end From 073f745677aad07d6829ea72cb78e15f8ec2073c Mon Sep 17 00:00:00 2001 From: Matthew Kent Date: Sat, 11 Nov 2023 12:57:52 -0800 Subject: [PATCH 6/6] Test for both undefined roles and missing traefik. --- test/configuration_test.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/configuration_test.rb b/test/configuration_test.rb index 63e1de4a..00612f24 100644 --- a/test/configuration_test.rb +++ b/test/configuration_test.rb @@ -291,9 +291,16 @@ class ConfigurationTest < ActiveSupport::TestCase end test "primary web role no traefik" do + error = assert_raises(ArgumentError) do + Kamal::Configuration.new(@deploy_with_roles.merge(primary_web_role: "workers")) + end + assert_match /workers needs to have traefik enabled/, error.message + end + + test "primary web role missing" do error = assert_raises(ArgumentError) do Kamal::Configuration.new(@deploy.merge(primary_web_role: "bar")) end - assert_match /bar/, error.message + assert_match /bar isn't defined/, error.message end end