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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants