Skip to content
New issue

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

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

Already on GitHub? # to your account

Add option to generate setfile with multiple hosts and roles #124

Conversation

h-haaks
Copy link
Contributor

@h-haaks h-haaks commented Apr 17, 2024

We have a few modules that override the modulesync generated ci.yml because they have integration tests that require multiple hosts with custom roles.
This PR add a new option to metadata2gha that can be used to generate setfile strings to bring up multiple hosts in gha.

@h-haaks h-haaks marked this pull request as draft April 17, 2024 22:07
@h-haaks h-haaks force-pushed the multihost-and-roles-in-setfile branch from 6ea3081 to dee14d1 Compare April 17, 2024 22:10
@h-haaks h-haaks marked this pull request as ready for review April 17, 2024 22:11
@h-haaks h-haaks force-pushed the multihost-and-roles-in-setfile branch from dee14d1 to 670fbea Compare April 17, 2024 22:16
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@h-haaks h-haaks force-pushed the multihost-and-roles-in-setfile branch from 670fbea to e4dd12d Compare April 18, 2024 06:34
Copy link
Member

@bastelfreak bastelfreak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code looks fine to me, but I would like to see another reviewer.

@bastelfreak bastelfreak requested a review from ekohl April 18, 2024 07:54
@h-haaks
Copy link
Contributor Author

h-haaks commented Apr 18, 2024

I didn't notice #111 until now ...
If we change my string format from '#HOST:ROLE,ROLE' to 'HOSTNAME:ROLE,ROLE' we could solve both hostname and role requirements.

ie gen strings like this for 'myhost1:role1;myhost2;role2'
`debian12-64role1.ma{hostname=myhost1-puppet7}-debian12-64role2.a{hostname=myhost2-puppet7}

Rename option to --beaker_hosts_and_roles ? or even just --hosts and leave it up to docs that roles are also supported.

bin/metadata2gha Outdated
opt.split(';').each do |node|
node_num, roles = node.split(':', 2)
options[:beaker_nodes_and_roles][node_num] = if roles
roles.split(',')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could just keep roles as a string. no need to split it into array.

@h-haaks
Copy link
Contributor Author

h-haaks commented Apr 18, 2024

I'll squash my commits before merge.

@bastelfreak bastelfreak added the enhancement New feature or request label Apr 19, 2024
@h-haaks
Copy link
Contributor Author

h-haaks commented Apr 19, 2024

@bastelfreak do I wait for @ekohl to approve before merging?

@ekohl
Copy link
Member

ekohl commented Apr 22, 2024

Currently traveling, so I didn't take too close of a look but at first glance it looks sensible.

@jhoblitt would this also serve your needs?

@jhoblitt
Copy link
Member

@jhoblitt would this also serve your needs?

I haven't tried it but at a glance I think it would work for me with changes to gha-puppet & modsync_config.

@h-haaks
Copy link
Contributor Author

h-haaks commented Apr 22, 2024

I put up a branch in puppet-mongodb to verify that my changes are working as expected
voxpupuli/puppet-mongodb#752

@h-haaks
Copy link
Contributor Author

h-haaks commented Apr 22, 2024

I haven't tried it but at a glance I think it would work for me with changes to gha-puppet & modsync_config.

Ah, I'll have to do something in modulesync_config as well to get my new input from .sync.yml to ci.yml
Or is there a chance voxpupuli/modulesync_config#858 will get approved?

@h-haaks
Copy link
Contributor Author

h-haaks commented Apr 25, 2024

I haven't tried it but at a glance I think it would work for me with changes to gha-puppet & modsync_config.

modulesync_config support fixed in version 7.5.0 (voxpupuli/modulesync_config#888)
gha-puppet voxpupuli/gha-puppet#53 will be merged and gha-puppet released when this is merged.

@ekohl @bastelfreak is it ok to merge and release this now?

@bastelfreak bastelfreak merged commit ea38f86 into voxpupuli:master Apr 25, 2024
6 checks passed
@h-haaks h-haaks deleted the multihost-and-roles-in-setfile branch April 25, 2024 11:37
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants