From 5c31cf6872c857baf0221508acc0464a846b2f89 Mon Sep 17 00:00:00 2001
From: Marius Kittler <mkittler@suse.de>
Date: Mon, 23 Sep 2024 12:41:26 +0200
Subject: [PATCH] Fix check for worker classes when creating jobs from settings

* 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
---
 lib/OpenQA/Schema/ResultSet/Jobs.pm | 23 +++++++++--------------
 t/05-scheduler-dependencies.t       |  3 ++-
 2 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/lib/OpenQA/Schema/ResultSet/Jobs.pm b/lib/OpenQA/Schema/ResultSet/Jobs.pm
index ee03dc103903..494ddb1f98df 100644
--- a/lib/OpenQA/Schema/ResultSet/Jobs.pm
+++ b/lib/OpenQA/Schema/ResultSet/Jobs.pm
@@ -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});
         }
@@ -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) {
diff --git a/t/05-scheduler-dependencies.t b/t/05-scheduler-dependencies.t
index b354dff1f46a..b93eaf81c4b8 100644
--- a/t/05-scheduler-dependencies.t
+++ b/t/05-scheduler-dependencies.t
@@ -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'
     );
 };