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 5adc02445..05379ab6c 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 fc881c069..64c74c63c 100644 --- a/lib/kamal/configuration.rb +++ b/lib/kamal/configuration.rb @@ -90,13 +90,20 @@ def all_hosts end def primary_web_host - role(:web).primary_host + role(primary_web_role)&.primary_host end - def traefik_hosts - roles.select(&:running_traefik?).flat_map(&:hosts).uniq + def traefik_roles + roles.select(&:running_traefik?) + end + + def traefik_role_names + traefik_roles.flat_map(&:name) end + def traefik_hosts + traefik_roles.flat_map(&:hosts).uniq + end def repository [ raw_config.registry["server"], image ].compact.join("/") @@ -199,6 +206,9 @@ def asset_path 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 @@ def ensure_required_keys_present 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 65b3062d7..44fa22941 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 e48659ed7..63e1de4a0 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 a19ff90e7..07e47f8de 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 000000000..264819fe1 --- /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 64c74c63c..f7f2233c0 100644 --- a/lib/kamal/configuration.rb +++ b/lib/kamal/configuration.rb @@ -263,6 +263,10 @@ def ensure_required_keys_present 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 63e1de4a0..00612f24a 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