From 0f3b1d0e76781cfcca47cfea59451c7bb812069f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tina=20M=C3=BCller?= <tina.mueller@suse.com>
Date: Wed, 16 Oct 2024 19:25:43 +0200
Subject: [PATCH] Fix handling of job array in enqueue_git_clones

The job array should not be emptied because there are other
functions called after that that work with the job ids.

Issue: https://progress.opensuse.org/issues/164898
---
 lib/OpenQA/Shared/Plugin/Gru.pm | 16 +++++++---------
 t/14-grutasks-git.t             |  3 ---
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/lib/OpenQA/Shared/Plugin/Gru.pm b/lib/OpenQA/Shared/Plugin/Gru.pm
index 7db59ac80c1..2c47963b323 100644
--- a/lib/OpenQA/Shared/Plugin/Gru.pm
+++ b/lib/OpenQA/Shared/Plugin/Gru.pm
@@ -133,14 +133,13 @@ sub _find_existing_minion_job ($self, $task, $args, $job_ids) {
         $dtf->format_datetime(DateTime->now()->subtract(minutes => 1)),
         'git_clone', OpenQA::Schema::Result::GruTasks->encode_json_to_db($args));
     $sth->execute(@args);
-    return unless my $job = $sth->fetchrow_hashref;
+    return 0 unless my $job = $sth->fetchrow_hashref;
+    # same task was run less than 1 minute ago and finished, nothing to do
+    return 1 if $job->{state} eq 'finished';
+
     my $notes = decode_json $job->{notes};
-    if ($job->{state} eq 'finished') {
-        # same task was run less than 1 minute ago and finished, nothing to do
-        @$job_ids = ();
-        return;
-    }
     $self->_add_jobs_to_gru_task($notes->{gru_id}, $job_ids);
+    return 1;
 }
 
 sub _add_jobs_to_gru_task ($self, $gru_id, $job_ids) {
@@ -159,7 +158,6 @@ sub _add_jobs_to_gru_task ($self, $gru_id, $job_ids) {
             last;
         }
     }
-    @$job_ids = ();
 }
 
 sub enqueue ($self, $task, $args = [], $options = {}, $jobs = []) {
@@ -246,8 +244,8 @@ sub enqueue_git_clones ($self, $clones, $job_ids) {
     return unless OpenQA::App->singleton->config->{'scm git'}->{git_auto_clone} eq 'yes';
     # $clones is a hashref with paths as keys and git urls as values
     # $job_id is used to create entries in a related table (gru_dependencies)
-    $self->_find_existing_minion_job('git_clone', $clones, $job_ids);
-    $self->enqueue('git_clone', $clones, {priority => 10}, $job_ids) if @$job_ids;
+    my $found = $self->_find_existing_minion_job('git_clone', $clones, $job_ids);
+    $self->enqueue('git_clone', $clones, {priority => 10}, $job_ids) unless $found;
 }
 
 sub enqueue_and_keep_track {
diff --git a/t/14-grutasks-git.t b/t/14-grutasks-git.t
index 55f4990b840..c240302c9fc 100644
--- a/t/14-grutasks-git.t
+++ b/t/14-grutasks-git.t
@@ -249,7 +249,6 @@ subtest 'enqueue_git_clones' => sub {
         $mocked_gru->redefine(enqueue => sub (@) { $enq++; });
         my $jobs = [$j[2]->id];
         my $result = $t->app->gru->enqueue_git_clones($clones, $jobs);
-        is_deeply $jobs, [], 'job array was emptied';
         my @deps = $task->jobs;
         is scalar @deps, 3, 'job was added to GruTask';
         is $deps[2]->job_id, $j[2]->id, 'third job was added to existing GruTask';
@@ -266,7 +265,6 @@ subtest 'enqueue_git_clones' => sub {
         $mocked_gru->redefine(enqueue => sub (@) { $enq++; });
         my $jobs = [$j[3]->id];
         my $result = $t->app->gru->enqueue_git_clones($clones, $jobs);
-        is_deeply $jobs, [], 'job array was emptied';
         my @deps = $task->jobs;
         is scalar @deps, 3, 'no job was added to GruTask';
         is $enq, 0, 'enqueue was not called';
@@ -278,7 +276,6 @@ subtest 'enqueue_git_clones' => sub {
         stderr_like { $t->app->gru->_add_jobs_to_gru_task(999, $jobs) }
 qr{GruTask 999 already gone.*insert or update on table "gru_dependencies" violates foreign key constraint "gru_dependencies_fk_gru_task_id"},
           'expected log output if GruTask deleted in between';
-        is_deeply $jobs, [], 'job array was emptied';
         my @deps = $task->jobs;
         is scalar @deps, 3, 'no job was added to GruTask';
     };