Rewrite to beaker-hosts option with roles

README.md

@@ -104,19 +104,16 @@ This results in the following JSON data
 ]
 ```
 
-If you need multiple hosts in your integration tests this could be achived by using the --beaker_nodes_and_roles option
+If you need custom hostname or multiple hosts in your integration tests this could be achived by using the --beaker-hosts option
 
-Option argument is '#HOST:ROLE,ROLE,...;#HOST:..;..' where
+Option argument is 'HOSTNAME:ROLES;HOSTNAME:..;..' where
 - hosts are separated by ';'
 - host number and roles are separated by ':'
-- Roles are separated by ','
+- Roles follow beaker-hostgenerator syntax
 If you don't need any extra roles use '1;2;..'
 
-The 'master' and 'agent' roles will be automatically added to the first host.
-The 'agent' role will be automatically added to the rest.
-
 ```console
-$ metadata2gha --beaker_nodes_and_roles '1:role1,other1;2:role2,other2'
+$ metadata2gha --beaker-hosts 'foo:primary.ma;bar:secondary.a'
 ```
 
 This results in the following JSON data
@@ -126,7 +123,7 @@ This results in the following JSON data
     "name": "Puppet 7 - Debian 12",
     "env": {
       "BEAKER_PUPPET_COLLECTION": "puppet7",
-      "BEAKER_SETFILE": "debian12-64role1,other1.ma{hostname=debian12-64-puppet7-1}-debian12-64role2,other2.a{hostname=debian12-64-puppet7-2}"
+      "BEAKER_SETFILE": "debian12-64primary.ma{hostname=foo-puppet7}-debian12-64secondary.a{hostname=bar-puppet7}"
     }
   }
 ]

bin/metadata2gha

@@ -11,6 +11,7 @@ options = {
   domain: nil,
   minimum_major_puppet_version: nil,
   beaker_fact: nil,
+  beaker_hosts: nil,
 }
 
 OptionParser.new do |opts|
@@ -40,16 +41,12 @@ OptionParser.new do |opts|
       options[:beaker_facter] = [fact, label, values.split(',')]
     end
   end
-  opts.on('--beaker_nodes_and_roles #NODE:ROLES;#NODE:ROLES;...', 'Expand the setfile string to create multiple nodes with custom roles. Separate roles using commas') do |opt|
-    options[:beaker_nodes_and_roles] = {}
+  opts.on('--beaker-hosts HOSTNAME:ROLES;HOSTNAME:ROLES;...', 'Expand the setfile string to create multiple hosts with custom roles. Roles string; see beaker-hostgenerator') do |opt|
+    options[:beaker_hosts] = {}
     if opt != 'false'
-      opt.split(';').each do |node|
-        node_num, roles = node.split(':', 2)
-        options[:beaker_nodes_and_roles][node_num] = if roles
-                                                       roles.split(',')
-                                                     else
-                                                       []
-                                                     end
+      opt.split(';').each do |host|
+        hostname, roles = host.split(':', 2)
+        options[:beaker_hosts][hostname] = roles
       end
     end
   end

lib/puppet_metadata/beaker.rb

@@ -55,53 +55,69 @@ def adjusted_os(os)
       #   Enforce a domain to be appended to the hostname, making it an FQDN
       # @param [Optional[String]] puppet_version
       #   The desired puppet version. Will be appended to the hostname
+      # @param [Optional[Hash]] hosts
+      #   Key: hostname, Value: roles (roles string as defined by beaker-hostgenerator )
+      #   Override the automatically generated hostname and optionally add roles
+      #   If more than one entry this will generate multiple hosts in the setfile
+      #   The domain may still be set via the `domain` param.
       #
       # @return [nil] If no setfile is available
       # @return [Array<(String, String)>] The beaker setfile description with a readable name
-      def os_release_to_setfile(os, release, use_fqdn: false, pidfile_workaround: false, domain: nil, puppet_version: nil, nodes_and_roles: nil)
+      def os_release_to_setfile(os, release, use_fqdn: false, pidfile_workaround: false, domain: nil, puppet_version: nil, hosts: nil)
         return unless os_supported?(os)
 
         aos = adjusted_os(os)
-
         name = "#{aos}#{release.tr('.', '')}-64"
+        human_name = "#{os} #{release}"
         domain ||= 'example.com' if use_fqdn
-        hostname = (puppet_version.nil? || puppet_version == 'none') ? name : "#{name}-#{puppet_version}"
-        options = {}
-        if domain || (puppet_version && puppet_version != 'none')
-          options[:hostname] = domain ? "#{hostname}.#{domain}" : hostname
+
+        hosts_settings = []
+        if hosts
+          hosts.each do |hostname, roles|
+            hosts_settings << {
+              'name' => if roles
+                          name + roles
+                        elsif hosts.size > 1
+                          hosts_settings.empty? ? "#{name}.ma" : "#{name}.a"
+                        else
+                          name
+                        end,
+              'hostname' => ((puppet_version.nil? || puppet_version == 'none') ? hostname : "#{hostname}-#{puppet_version}") + (domain ? ".#{domain}" : ''),
+            }
+          end
+        else
+          hosts_settings << {
+            'name' => name,
+            'hostname' => if puppet_version && puppet_version != 'none'
+                            "#{name}-#{puppet_version}" + (domain ? ".#{domain}" : '')
+                          elsif domain
+                            name + (domain ? ".#{domain}" : '')
+                          else
+                            ''
+                          end,
+          }
         end
 
+        options = {}
         # Docker messes up cgroups and some systemd versions can't deal with
         # that when PIDFile is used.
+        image_to_use = nil
         if pidfile_workaround?(pidfile_workaround, os)
           return if PIDFILE_INCOMPATIBLE[os]&.include?(release)
 
           if (image = PIDFILE_COMPATIBLE_IMAGES.dig(os, release))
-            options[:image] = image
+            image_to_use = image
           end
         end
 
-        human_name = "#{os} #{release}"
-
-        if nodes_and_roles
-          names = []
-          nodes_and_roles.each do |node, roles|
-            roles.map!(&:strip)
-            n = "#{name}#{roles.join(',')}"
-            n += if names.empty?
-                   # add master role to first node
-                   '.ma'
-                 else
-                   '.a'
-                 end
-            options[:hostname] = hostname + "-#{node.strip}" if nodes_and_roles.size > 1
-            options[:hostname] = "#{options[:hostname]}.#{domain}" if options[:hostname] && domain
-            names << build_setfile(n, options)
-          end
-          [names.join('-'), human_name]
-        else
-          [build_setfile(name, options), human_name]
+        setfile_parts = []
+        hosts_settings.each do |host_settings|
+          options[:hostname] = host_settings['hostname'] unless host_settings['hostname'].empty?
+          options[:image] = image_to_use if image_to_use
+          setfile_parts << build_setfile(host_settings['name'], options)
         end
+
+        [setfile_parts.join('-'), human_name]
       end
 
       # Return whether a Beaker setfile can be generated for the given OS

lib/puppet_metadata/github_actions.rb

@@ -144,7 +144,7 @@ def os_release_to_beaker_setfile(os, release, puppet_collection)
         pidfile_workaround: options[:beaker_pidfile_workaround],
         domain: options[:domain],
         puppet_version: puppet_collection,
-        nodes_and_roles: options[:beaker_nodes_and_roles],
+        hosts: options[:beaker_hosts],
       )
     end
   end

spec/beaker_spec.rb

@@ -21,7 +21,7 @@
 
     it { expect(described_class.os_release_to_setfile('SLES', '11')).to be_nil }
 
-    describe 'pidfile_workaround' do
+    context 'with pidfile_workaround' do
       describe 'true' do
         [
           ['CentOS', '6', ['centos6-64', 'CentOS 6']],
@@ -58,14 +58,13 @@
       end
     end
 
-    describe 'domain' do
+    context 'with domain' do
       it {
-        expect(described_class.os_release_to_setfile('CentOS', '7', domain: 'mydomain.org',
-                                                                    use_fqdn: true)).to eq(['centos7-64{hostname=centos7-64.mydomain.org}', 'CentOS 7'])
+        expect(described_class.os_release_to_setfile('CentOS', '7', domain: 'mydomain.org')).to eq(['centos7-64{hostname=centos7-64.mydomain.org}', 'CentOS 7'])
       }
     end
 
-    describe 'puppet_version' do
+    context 'with puppet_version' do
       [
         ['CentOS', '7', 'none', ['centos7-64', 'CentOS 7']],
         ['CentOS', '7', 'puppet7', ['centos7-64{hostname=centos7-64-puppet7}', 'CentOS 7']],
@@ -74,22 +73,41 @@
       end
     end
 
-    describe 'nodes_and_roles' do
+    context 'with hosts and no roles' do
       [
-        ['CentOS', '7', { '1' => ['role1', 'role2'] }, ['centos7-64role1,role2.ma', 'CentOS 7']],
-        ['CentOS', '7', { '1' => [], '2' => [] }, ['centos7-64.ma{hostname=centos7-64-1}-centos7-64.a{hostname=centos7-64-2}', 'CentOS 7']],
-        ['CentOS', '7', { '1' => ['role1'], '2' => ['role2'] }, ['centos7-64role1.ma{hostname=centos7-64-1}-centos7-64role2.a{hostname=centos7-64-2}', 'CentOS 7']],
-      ].each do |os, release, nodes, expected|
-        it { expect(described_class.os_release_to_setfile(os, release, nodes_and_roles: nodes)).to eq(expected) }
+        ['Debian', '12', { 'foo' => nil }, ['debian12-64{hostname=foo}', 'Debian 12']],
+        ['Debian', '12', { 'foo' => nil, 'bar' => nil }, ['debian12-64.ma{hostname=foo}-debian12-64.a{hostname=bar}', 'Debian 12']],
+      ].each do |os, release, hosts, expected|
+        it { expect(described_class.os_release_to_setfile(os, release, hosts: hosts)).to eq(expected) }
       end
     end
 
-    describe 'domain, puppet_version and nodes_and_roles' do
+    context 'with hosts and roles' do
       [
-        ['CentOS', '7', 'mydomain.org', 'puppet7', { '1' => ['role1'], '2' => ['role2'] },
-         ['centos7-64role1.ma{hostname=centos7-64-puppet7-1.mydomain.org}-centos7-64role2.a{hostname=centos7-64-puppet7-2.mydomain.org}', 'CentOS 7'],],
-      ].each do |os, release, domain, puppet_version, nodes, expected|
-        it { expect(described_class.os_release_to_setfile(os, release, domain: domain, puppet_version: puppet_version, nodes_and_roles: nodes)).to eq(expected) }
+        ['Debian', '12', { 'foo' => 'myrole.ma' }, ['debian12-64myrole.ma{hostname=foo}', 'Debian 12']],
+        ['Debian', '12', { 'foo' => 'myrole,primary.ma' }, ['debian12-64myrole,primary.ma{hostname=foo}', 'Debian 12']],
+        ['Debian', '12', { 'foo' => 'myrole,primary.ma', 'bar' => 'myrole,secondary.a' },
+         ['debian12-64myrole,primary.ma{hostname=foo}-debian12-64myrole,secondary.a{hostname=bar}', 'Debian 12'],],
+      ].each do |os, release, hosts, expected|
+        it { expect(described_class.os_release_to_setfile(os, release, hosts: hosts)).to eq(expected) }
+      end
+    end
+
+    context 'with hosts, roles and domain' do
+      [
+        ['Debian', '12', 'mydomain.org', { 'foo' => 'myrole,primary.ma', 'bar' => 'myrole,secondary.a' },
+         ['debian12-64myrole,primary.ma{hostname=foo.mydomain.org}-debian12-64myrole,secondary.a{hostname=bar.mydomain.org}', 'Debian 12'],],
+      ].each do |os, release, domain, hosts, expected|
+        it { expect(described_class.os_release_to_setfile(os, release, domain: domain, hosts: hosts)).to eq(expected) }
+      end
+    end
+
+    context 'with hosts, roles, domain and puppet_version' do
+      [
+        ['Debian', '12', 'mydomain.org', 'puppet7', { 'foo' => 'myrole,primary.ma', 'bar' => 'myrole,secondary.a' },
+         ['debian12-64myrole,primary.ma{hostname=foo-puppet7.mydomain.org}-debian12-64myrole,secondary.a{hostname=bar-puppet7.mydomain.org}', 'Debian 12'],],
+      ].each do |os, release, domain, puppet_version, hosts, expected|
+        it { expect(described_class.os_release_to_setfile(os, release, domain: domain, puppet_version: puppet_version, hosts: hosts)).to eq(expected) }
       end
     end
   end

spec/github_actions_spec.rb

@@ -161,32 +161,32 @@
         end
       end
 
-      context 'with option beaker_nodes_and_roles set to one node with custom roles' do
-        let(:options) { super().merge(beaker_nodes_and_roles: { '1' => ['role1', 'role2'] }) }
+      context 'with option beaker_hosts set to one node with custom roles' do
+        let(:options) { super().merge(beaker_hosts: { 'foo' => 'myrole,primary.ma' }) }
 
-        it 'is expected to contain supported os / puppet version / setfile with roles' do
+        it 'is expected to contain supported os / puppet version / custom hostname / custom roles' do
           expect(subject).to include(
-            { name: 'Distro Puppet - Archlinux rolling', env: { 'BEAKER_PUPPET_COLLECTION' => 'none', 'BEAKER_SETFILE' => 'archlinuxrolling-64role1,role2.ma' } },
+            { name: 'Distro Puppet - Archlinux rolling', env: { 'BEAKER_PUPPET_COLLECTION' => 'none', 'BEAKER_SETFILE' => 'archlinuxrolling-64myrole,primary.ma{hostname=foo}' } },
           )
         end
       end
 
-      context 'with option beaker_nodes_and_roles set to two node without custom roles' do
-        let(:options) { super().merge(beaker_nodes_and_roles: { '1' => [], '2' => [] }) }
+      context 'with option beaker_hosts set to two node without custom roles' do
+        let(:options) { super().merge(beaker_hosts: { 'foo' => nil, 'bar' => nil }) }
 
-        it 'is expected to contain supported os / puppet version / setfile with nodes' do
+        it 'is expected to contain supported os / puppet version / custom hostnames / required roles for multihost' do
           expect(subject).to include(
-            { name: 'Distro Puppet - Archlinux rolling', env: { 'BEAKER_PUPPET_COLLECTION' => 'none', 'BEAKER_SETFILE' => 'archlinuxrolling-64.ma{hostname=archlinuxrolling-64-1}-archlinuxrolling-64.a{hostname=archlinuxrolling-64-2}' } },
+            { name: 'Distro Puppet - Archlinux rolling', env: { 'BEAKER_PUPPET_COLLECTION' => 'none', 'BEAKER_SETFILE' => 'archlinuxrolling-64.ma{hostname=foo}-archlinuxrolling-64.a{hostname=bar}' } },
           )
         end
       end
 
-      context 'with option beaker_nodes_and_roles set to two node with custom roles' do
-        let(:options) { super().merge(beaker_nodes_and_roles: { '1' => ['role1'], '2' => ['role2'] }) }
+      context 'with option beaker_hosts set to two node with custom roles' do
+        let(:options) { super().merge(beaker_hosts: { 'foo' => 'primary.ma', 'bar' => 'secondary.a' }) }
 
-        it 'is expected to contain supported os / puppet version / setfile with nodes and roles' do
+        it 'is expected to contain supported os / puppet version / custom hostnames / custom roles' do
           expect(subject).to include(
-            { name: 'Distro Puppet - Archlinux rolling', env: { 'BEAKER_PUPPET_COLLECTION' => 'none', 'BEAKER_SETFILE' => 'archlinuxrolling-64role1.ma{hostname=archlinuxrolling-64-1}-archlinuxrolling-64role2.a{hostname=archlinuxrolling-64-2}' } },
+            { name: 'Distro Puppet - Archlinux rolling', env: { 'BEAKER_PUPPET_COLLECTION' => 'none', 'BEAKER_SETFILE' => 'archlinuxrolling-64primary.ma{hostname=foo}-archlinuxrolling-64secondary.a{hostname=bar}' } },
           )
         end
       end