-
Notifications
You must be signed in to change notification settings - Fork 219
RFC: Allow multiple machines per job template #6714
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
base: master
Are you sure you want to change the base?
Conversation
2a8dab6
to
19f1c78
Compare
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (88.23%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #6714 +/- ##
==========================================
- Coverage 99.23% 99.22% -0.01%
==========================================
Files 398 398
Lines 40942 40957 +15
==========================================
+ Hits 40628 40641 +13
- Misses 314 316 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this makes sense. The array syntax would be enough so maybe it would be best to only allow that. But I also don't really care as the code for allowing @(…)
isn't really complicated.
elsif ($template->{machine}) { | ||
my $machine = delete $template->{machine}; | ||
if (ref $machine eq 'ARRAY') { | ||
@machines = @$machine; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also remove empty machines here for the sake of consistency:
@machines = @$machine; | |
@machines = grep { length } @$machine; |
my $job_template = $job_templates->{$key}; | ||
my $settings = $job_template->{settings} // {}; | ||
my $template = $job_templates->{$key}; | ||
my %new_template = (%$template, name => $key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is never used as a hash so we could avoid hashing:
my %new_template = (%$template, name => $key); | |
my @new_template = (%$template, name => $key); |
With "allow" do you mean to add the array syntax to schedule all specified variants? |
Yes. Unless you specify the machine on the command line. I haven't implemented/tested that yet. |
I think it would also be useful to have a default like in jobgroup templates, so that we don't have to specify any machine if it's the default. |
Issue: https://progress.opensuse.org/issues/188310
This is just a proof of concept on how we could allow multple machines per job template in SCENARIO_DEFINITIONs.
It came up because we possibly want to run openqa-in-openqa jobs for both machines, and it's a useful feature in general.
Here it how our scenario file would look like:
https://github.com/perlpunk/os-autoinst-distri-openQA/blob/multiple-machines/scenario-definitions.yaml
Two possibilities to define machines per job template: