Merge pull request #594 from basecamp/match-primary-role-in-filters

Try to match primary_role when roles are filtered

lib/kamal/commander.rb

@@ -28,15 +28,32 @@ class Kamal::Commander
   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

test/commander_test.rb

@@ -24,8 +24,10 @@ 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
 
+    exception = assert_raises(ArgumentError) do
       @kamal.specific_hosts = [ "*miss" ]
-    assert_equal [], @kamal.hosts
+    end
+    assert_match /hosts match for \*miss/, exception.message
   end
 
   test "filtering hosts by filtering roles" do
@@ -33,6 +35,11 @@ class CommanderTest < ActiveSupport::TestCase
 
     @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
@@ -50,8 +57,10 @@ class CommanderTest < ActiveSupport::TestCase
     @kamal.specific_roles = [ "*" ]
     assert_equal [ "web", "workers" ], @kamal.roles.map(&:name)
 
+    exception = assert_raises(ArgumentError) do
       @kamal.specific_roles = [ "*miss" ]
-    assert_equal [], @kamal.roles.map(&:name)
+    end
+    assert_match /roles match for \*miss/, exception.message
   end
 
   test "filtering roles by filtering hosts" do
@@ -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|