Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to match primary_role when roles are filtered #594

Merged
merged 3 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions lib/kamal/commander.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,32 @@ def specific_primary!
end

def specific_roles=(role_names)
@specific_roles = Kamal::Utils.filter_specific_items(role_names, config.roles) if role_names.present?
if role_names.present?
@specific_roles = Kamal::Utils.filter_specific_items(role_names, config.roles)

if @specific_roles.empty?
raise ArgumentError, "No --roles match for #{role_names.join(',')}"
end

@specific_roles
end
end

def specific_hosts=(hosts)
@specific_hosts = Kamal::Utils.filter_specific_items(hosts, config.all_hosts) if hosts.present?
if hosts.present?
@specific_hosts = Kamal::Utils.filter_specific_items(hosts, config.all_hosts)

if @specific_hosts.empty?
raise ArgumentError, "No --hosts match for #{hosts.join(',')}"
end

@specific_hosts
end
end

def primary_host
specific_hosts&.first || specific_roles&.first&.primary_host || config.primary_host
# Given a list of specific roles, make an effort to match up with the primary_role
specific_hosts&.first || specific_roles&.detect { |role| role.name == config.primary_role }&.primary_host || specific_roles&.first&.primary_host || config.primary_host
end

def primary_role
Expand Down
26 changes: 22 additions & 4 deletions test/commander_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,22 @@ class CommanderTest < ActiveSupport::TestCase
@kamal.specific_hosts = [ "*" ]
assert_equal [ "1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4" ], @kamal.hosts

@kamal.specific_hosts = [ "*miss" ]
assert_equal [], @kamal.hosts
exception = assert_raises(ArgumentError) do
@kamal.specific_hosts = [ "*miss" ]
end
assert_match /hosts match for \*miss/, exception.message
end

test "filtering hosts by filtering roles" do
assert_equal [ "1.1.1.1", "1.1.1.2", "1.1.1.3", "1.1.1.4" ], @kamal.hosts

@kamal.specific_roles = [ "web" ]
assert_equal [ "1.1.1.1", "1.1.1.2" ], @kamal.hosts

exception = assert_raises(ArgumentError) do
@kamal.specific_roles = [ "*miss" ]
end
assert_match /roles match for \*miss/, exception.message
end

test "filtering roles" do
Expand All @@ -50,8 +57,10 @@ class CommanderTest < ActiveSupport::TestCase
@kamal.specific_roles = [ "*" ]
assert_equal [ "web", "workers" ], @kamal.roles.map(&:name)

@kamal.specific_roles = [ "*miss" ]
assert_equal [], @kamal.roles.map(&:name)
exception = assert_raises(ArgumentError) do
@kamal.specific_roles = [ "*miss" ]
end
assert_match /roles match for \*miss/, exception.message
end

test "filtering roles by filtering hosts" do
Expand Down Expand Up @@ -100,6 +109,15 @@ class CommanderTest < ActiveSupport::TestCase
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)

@kamal.specific_roles = [ "web_*" ]
assert_equal [ "web_chicago", "web_tokyo" ], @kamal.roles.map(&:name)
assert_equal "web_tokyo", @kamal.primary_role
assert_equal "1.1.1.3", @kamal.primary_host
end

private
def configure_with(variant)
@kamal = Kamal::Commander.new.tap do |kamal|
Expand Down
Loading