Skip to content

Commit

Permalink
Fix check for worker classes when creating jobs from settings
Browse files Browse the repository at this point in the history
* Consider all worker classes to verify that all jobs in a directly chained
  cluster have the same set of worker classes assigned
* Avoid DBIx warning in case a job with directly chained dependencies has
  more than one worker class assigned
* See https://progress.opensuse.org/issues/154735
  • Loading branch information
Martchus committed Sep 23, 2024
1 parent 87877b8 commit 5c31cf6
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 15 deletions.
23 changes: 9 additions & 14 deletions lib/OpenQA/Schema/ResultSet/Jobs.pm
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ sub create_from_settings {
my $dependency_type = $dependency_definition->{dependency_type};
for my $id (@$ids) {
if ($dependency_type eq OpenQA::JobDependencies::Constants::DIRECTLY_CHAINED) {
my $parent_worker_class = $job_settings->find({job_id => $id, key => 'WORKER_CLASS'});
_handle_directly_chained_dep($parent_worker_class, $id, \%settings);
my $parent_worker_classes = join(',', @{$job_settings->all_values_sorted($id, 'WORKER_CLASS')});
_handle_directly_chained_dep($parent_worker_classes, $id, \%settings);
}
push(@{$new_job_args{parents}}, {parent_job_id => $id, dependency => $dependency_type});
}
Expand Down Expand Up @@ -203,18 +203,13 @@ sub create_from_settings {
return $job;
}

sub _handle_directly_chained_dep ($parent_worker_class, $id, $settings) {
if ($parent_worker_class = $parent_worker_class ? $parent_worker_class->value : '') {
if (!$settings->{WORKER_CLASS}) {
# assume we want to use the worker class from the parent here (and not the default which
# is otherwise assumed)
$settings->{WORKER_CLASS} = $parent_worker_class;
}
elsif ($settings->{WORKER_CLASS} ne $parent_worker_class) {
die "Specified WORKER_CLASS ($settings->{WORKER_CLASS}) does not match the one from"
. " directly chained parent $id ($parent_worker_class)";
}
}
sub _handle_directly_chained_dep ($parent_classes, $id, $settings) {
# assume we want to use the worker class from the parent here (and not the default which is otherwise assumed)
return $settings->{WORKER_CLASS} = $parent_classes unless defined(my $classes = $settings->{WORKER_CLASS});

# raise error if the directly chained child has a different set of worker classes assigned than its parent
die "Specified WORKER_CLASS ($classes) does not match the one from directly chained parent $id ($parent_classes)"
unless $parent_classes eq join(',', sort split(m/,/, $classes));
}

sub _search_modules ($self, $module_re) {
Expand Down
3 changes: 2 additions & 1 deletion t/05-scheduler-dependencies.t
Original file line number Diff line number Diff line change
Expand Up @@ -1083,9 +1083,10 @@ subtest 'WORKER_CLASS validated when creating directly chained dependencies' =>
'bar', 'job B has different class bar (ok for regularly chained dependencies)');
$jobC = _job_create({%default_job_settings, TEST => 'chained-C'}, undef, [], [$jobB->id]);
is($jobC->settings->find({key => 'WORKER_CLASS'})->value, 'bar', 'job C inherits worker class from B');
$job_settings->create({job_id => $jobC->id, key => 'WORKER_CLASS', value => 'baz'});
throws_ok(
sub { _job_create({%default_job_settings, TEST => 'chained-D', WORKER_CLASS => 'foo'}, [], [], [$jobC->id]) },
qr/Specified WORKER_CLASS \(foo\) does not match the one from directly chained parent .* \(bar\)/,
qr/Specified WORKER_CLASS \(foo\) does not match the one from directly chained parent .* \(bar,baz\)/,
'creation of job with mismatching worker class prevented'
);
};
Expand Down

0 comments on commit 5c31cf6

Please sign in to comment.