Merge pull request #5834 from Martchus/jobs-api-prio

Allow specifying priority when creating a single set of jobs
os-autoinst · Aug 7, 2024 · 549ee3b · 549ee3b
2 parents 4770abe + 6fc4e62
commit 549ee3b
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 19 deletions.
diff --git a/lib/OpenQA/Schema/Result/ScheduledProducts.pm b/lib/OpenQA/Schema/Result/ScheduledProducts.pm
@@ -334,7 +334,6 @@ sub _schedule_iso {
         my %job_ids_by_test_machine;    # key: "TEST@MACHINE", value: "array of job ids"
 
         for my $settings (@{$jobs || []}) {
-            my $prio = delete $settings->{PRIO};
             $settings->{_GROUP_ID} = delete $settings->{GROUP_ID};
 
             # create a new job with these parameters and count if successful, do not send job notifies yet
@@ -348,10 +347,6 @@ sub _schedule_iso {
                 my $j_id = $job->id;
                 $job_ids_by_test_machine{_job_ref($settings)} //= [];
                 push @{$job_ids_by_test_machine{_job_ref($settings)}}, $j_id;
-
-                # set prio if defined explicitly (otherwise default prio is used)
-                $job->update({priority => $prio}) if (defined($prio));
-
                 $self->_create_download_lists(\%tmp_downloads, $download_list, $j_id);
             }
             catch {
@@ -603,7 +598,7 @@ sub _generate_jobs {
             my $error = OpenQA::JobSettings::generate_settings(\%params);
             $error_message .= $error if defined $error;
 
-            $settings{PRIO} = defined($priority) ? $priority : $job_template->prio;
+            $settings{_PRIORITY} = $priority // $job_template->prio;
             $settings{GROUP_ID} = $job_template->group_id;
 
             _populate_wanted_jobs_for_test_arg($args, \%settings, \%wanted);
@@ -793,13 +788,13 @@ sub _schedule_from_yaml ($self, $args, $skip_chained_deps, $include_children, @l
                 my $machine_settings = $mach->{settings} // {};
                 _merge_settings_and_worker_classes($machine_settings, $settings, \@worker_class);
                 $settings->{BACKEND} = $mach->{backend} if $mach->{backend};
-                $settings->{PRIO} = $mach->{priority} // DEFAULT_JOB_PRIORITY;
+                $settings->{_PRIORITY} = $mach->{priority} // DEFAULT_JOB_PRIORITY;
             }
         }
 
         # set priority of job if specified
         if (my $priority = $job_template->{priority}) {
-            $settings->{PRIO} = $priority;
+            $settings->{_PRIORITY} = $priority;
         }
 
         # handle further settings

diff --git a/lib/OpenQA/Schema/ResultSet/Jobs.pm b/lib/OpenQA/Schema/ResultSet/Jobs.pm
@@ -16,6 +16,8 @@ use OpenQA::Utils 'testcasedir';
 use Mojo::File 'path';
 use Mojo::JSON 'encode_json';
 use Mojo::URL;
+use Mojolicious::Validator;
+use Mojolicious::Validator::Validation;
 use Time::HiRes 'time';
 use DateTime;
 
@@ -119,13 +121,21 @@ sub create_from_settings {
 
     my @invalid_keys = grep { $_ =~ /^(PUBLISH_HDD|FORCE_PUBLISH_HDD|STORE_HDD)\S+(\d+)$/ && $settings{$_} =~ /\// }
       keys %settings;
-    die 'The ' . join(',', @invalid_keys) . ' cannot include / in value' if @invalid_keys;
+    die 'The ' . join(',', @invalid_keys) . " cannot include / in value\n" if @invalid_keys;
+
+    # validate special settings
+    my %special_settings = (_PRIORITY => delete $settings{_PRIORITY});
+    my $validator = Mojolicious::Validator->new;
+    my $v = Mojolicious::Validator::Validation->new(validator => $validator, input => \%special_settings);
+    my $prio = $v->optional('_PRIORITY')->num->param;
+    die 'The following settings are invalid: ' . join(', ', @{$v->failed}) . "\n" if $v->has_error;
 
     # assign group ID and priority
     my ($group_args, $group) = OpenQA::Schema::Result::Jobs::extract_group_args_from_settings(\%settings);
+    $new_job_args{priority} = $prio if defined $prio;
     if ($group) {
         $new_job_args{group_id} = $group->id;
-        $new_job_args{priority} = $group->default_priority;
+        $new_job_args{priority} //= $group->default_priority;
     }
 
     # handle dependencies

diff --git a/lib/OpenQA/WebAPI/Controller/API/V1/Job.pm b/lib/OpenQA/WebAPI/Controller/API/V1/Job.pm
@@ -487,11 +487,12 @@ Sets priority for a given job.
 
 sub prio ($self) {
     return unless my $job = $self->find_job_or_render_not_found($self->stash('jobid'));
-    my $res = $job->set_prio($self->param('prio'));
+    my $v = $self->validation;
+    my $prio = $v->required('prio')->num->param;
+    return $self->reply->validation_error({format => 'json'}) if $v->has_error;
 
-    # Referencing the scalar will result in true or false
-    # (see http://mojolicio.us/perldoc/Mojo/JSON)
-    $self->render(json => {result => \$res});
+    # Referencing the scalar will result in true or false (see http://mojolicio.us/perldoc/Mojo/JSON)
+    $self->render(json => {result => \$job->set_prio($prio)});
 }
 
 =over 4

diff --git a/t/api/04-jobs.t b/t/api/04-jobs.t
@@ -911,24 +911,39 @@ subtest 'WORKER_CLASS correctly assigned when posting job' => sub {
     is $jobs->find($id)->settings_hash->{WORKER_CLASS}, 'svirt', 'specified WORKER_CLASS assigned';
 };
 
-subtest 'default priority correctly assigned when posting job' => sub {
+subtest 'priority correctly assigned when posting job' => sub {
     # post new job and check default priority
     $t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(200);
     $t->get_ok('/api/v1/jobs/' . $t->tx->res->json->{id})->status_is(200);
-    $t->json_is('/job/group', 'opensuse');
-    $t->json_is('/job/priority', 50);
+    $t->json_is('/job/group', 'opensuse', 'group assigned (1)');
+    $t->json_is('/job/priority', 50, 'global default priority assigned');
 
     # post new job in job group with customized default priority
     $schema->resultset('JobGroups')->find({name => 'opensuse test'})->update({default_priority => 42});
     $jobs_post_params{_GROUP} = 'opensuse test';
     $t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(200);
     $t->get_ok('/api/v1/jobs/' . $t->tx->res->json->{id})->status_is(200);
-    $t->json_is('/job/group', 'opensuse test');
-    $t->json_is('/job/priority', 42);
+    $t->json_is('/job/group', 'opensuse test', 'group assigned (2)');
+    $t->json_is('/job/priority', 42, 'default priority from group assigned');
+
+    # post new job with explicitely specified _PRIORITY setting
+    $jobs_post_params{_PRIORITY} = 43;
+    $t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(200);
+    $t->get_ok('/api/v1/jobs/' . $t->tx->res->json->{id})->status_is(200);
+    $t->json_is('/job/group', 'opensuse test', 'group assigned (3)');
+    $t->json_is('/job/priority', 43, 'explicitely specified priority assigned');
+    $t->json_is('/job/settings/_PRIORITY', undef, 'priority not added as setting');
+
+    # post new job with invalid explicitely specified _PRIORITY setting
+    $jobs_post_params{_PRIORITY} = 43.5;
+    $t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(400);
+    $t->json_like('/error', qr/settings.*invalid.*_PRIORITY/, 'error returned');
+    $t->json_is('/id', undef, 'no job ID returned');
 };
 
 subtest 'specifying group by ID' => sub {
     delete $jobs_post_params{_GROUP};
+    delete $jobs_post_params{_PRIORITY};
     $jobs_post_params{_GROUP_ID} = 1002;
     $t->post_ok('/api/v1/jobs', form => \%jobs_post_params)->status_is(200);
     $t->get_ok('/api/v1/jobs/' . $t->tx->res->json->{id})->status_is(200);