Improve cleanup of temporary needle refs

* Use the settings key `temp_needle_refs_retention` which makes it clear that this setting is about temporary needle refs * Document settings key in default `openqa.ini` * Adapt the cleanup to be in accordance with the changed path for temporary needle refs * Rename cleanup task to `limit_temp_needle_refs` to be more in-line with existing cleanup tasks * Add systemd units to enqueue the cleanup task automatically * Change the default retention to two hours (from 30 minutes) * Use the modification time instead of the access time because the access time might be updated very to easily unintendedly * Use signal guard to retry cleanup in case the gru service is restarted * See https://progress.opensuse.org/issues/154783
os-autoinst · Oct 21, 2024 · b31f643 · b31f643
1 parent 5fbb935
commit b31f643
Show file tree

Hide file tree

Showing 12 changed files with 98 additions and 55 deletions.
diff --git a/etc/openqa/openqa.ini b/etc/openqa/openqa.ini
@@ -119,6 +119,8 @@
 #git_auto_update = no
 # whether openQA should attempt to display needles of the correct version in the web UI
 #checkout_needles_sha = no
+# retention for storing temporary needle refs in minutes
+#temp_needle_refs_retention = 120
 
 ## Authentication method to use for user management
 [auth]

diff --git a/lib/OpenQA/Git.pm b/lib/OpenQA/Git.pm
@@ -6,6 +6,7 @@ package OpenQA::Git;
 use Mojo::Base -base, -signatures;
 use Mojo::Util 'trim';
 use Cwd 'abs_path';
+use File::Touch;
 use OpenQA::Utils qw(run_cmd_with_log_return_error);
 
 has 'app';
@@ -146,7 +147,10 @@ sub is_workdir_clean ($self) {
 }
 
 sub cache_ref ($self, $ref, $relative_path, $output_file) {
-    return undef if -f $output_file;
+    if (-f $output_file) {
+        eval { touch $output_file };
+        return $@ ? $@ : undef;
+    }
     my @git = $self->_prepare_git_command;
     my $res = run_cmd_with_log_return_error [@git, 'show', "$ref:./$relative_path"], output_file => $output_file;
     return undef if $res->{status};

diff --git a/lib/OpenQA/Needles.pm b/lib/OpenQA/Needles.pm
@@ -13,10 +13,12 @@ use OpenQA::Log qw(log_error);
 use OpenQA::Utils qw(prjdir sharedir);
 use Mojo::File qw(path);
 
-our @EXPORT = qw(is_in_temp_dir needle_temp_dir locate_needle);
+our @EXPORT = qw(temp_dir is_in_temp_dir needle_temp_dir locate_needle);
 
 my $tmp_dir = prjdir() . '/webui/cache/needle-refs';
 
+sub temp_dir () { $tmp_dir }
+
 sub is_in_temp_dir ($file_path) { index($file_path, $tmp_dir) == 0 }
 
 sub needle_temp_dir ($dir, $ref) { path($tmp_dir, basename(dirname($dir)), $ref, 'needles') }

diff --git a/lib/OpenQA/Setup.pm b/lib/OpenQA/Setup.pm
@@ -70,7 +70,7 @@ sub read_config ($app) {
             git_auto_clone => 'yes',
             git_auto_update => 'no',
             checkout_needles_sha => 'no',
-            minimum_needle_retention_time => undef,
+            temp_needle_refs_retention => 120,
         },
         scheduler => {
             max_job_scheduled_time => 7,

diff --git a/lib/OpenQA/Shared/Plugin/Gru.pm b/lib/OpenQA/Shared/Plugin/Gru.pm
@@ -31,8 +31,10 @@ sub register_tasks ($self) {
       OpenQA::Task::Asset::Download
       OpenQA::Task::Asset::Limit
       OpenQA::Task::Git::Clone
-      OpenQA::Task::Needle::Scan OpenQA::Task::Needle::Save OpenQA::Task::Needle::Delete
-      qw(OpenQA::Task::Needle::Scan OpenQA::Task::Needle::Save OpenQA::Task::Needle::Delete OpenQA::Task::Needle::RemoveVersions),
+      OpenQA::Task::Needle::Scan
+      OpenQA::Task::Needle::Save
+      OpenQA::Task::Needle::Delete
+      OpenQA::Task::Needle::LimitTempRefs
       OpenQA::Task::Job::Limit
       OpenQA::Task::Job::ArchiveResults
       OpenQA::Task::Job::FinalizeResults

diff --git a/lib/OpenQA/Task/Needle/LimitTempRefs.pm b/lib/OpenQA/Task/Needle/LimitTempRefs.pm
@@ -0,0 +1,39 @@
+# Copyright SUSE LLC
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+package OpenQA::Task::Needle::LimitTempRefs;
+use Mojo::Base 'Mojolicious::Plugin', -signatures;
+
+use File::Find;
+use File::stat;
+use Fcntl qw(S_ISDIR);
+use OpenQA::Needles;
+use OpenQA::Task::SignalGuard;
+use Time::Seconds;
+
+my $retention;
+
+sub register ($self, $app, $job) {
+    $retention = $app->config->{'scm git'}->{temp_needle_refs_retention} * ONE_MINUTE;
+    $app->minion->add_task(limit_temp_needle_refs => sub ($job) { _limit($app, $job) });
+}
+
+sub _limit ($app, $job) {
+    my $ensure_task_retry_on_termination_signal_guard = OpenQA::Task::SignalGuard->new($job);
+
+    return $job->finish({error => 'Another job to remove needle versions is running. Try again later.'})
+      unless my $guard = $app->minion->guard('limit_needle_versions_task', 7200);
+
+    # remove all temporary needles which haven't been accessed in time period specified in config
+    my $temp_dir = OpenQA::Needles::temp_dir;
+    return undef unless -d $temp_dir;
+    my $now = time;
+    my $wanted = sub {
+        return undef unless my $lstat = lstat $File::Find::name;
+        return rmdir $File::Find::name if S_ISDIR($lstat->mode);    # remove all empty dirs
+        return unlink $File::Find::name if ($now - $lstat->mtime) > $retention;
+    };
+    find({no_chdir => 1, bydepth => 1, wanted => $wanted}, $temp_dir);
+}
+
+1;
diff --git a/lib/OpenQA/Task/Needle/RemoveVersions.pm b/lib/OpenQA/Task/Needle/RemoveVersions.pm
diff --git a/...pt/openqa-enqueue-needle-versions-cleanup → script/openqa-enqueue-needle-ref-cleanup b/...pt/openqa-enqueue-needle-versions-cleanup → script/openqa-enqueue-needle-ref-cleanup
@@ -1,2 +1,2 @@
 #!/bin/sh -e
-exec "$(dirname "$0")"/openqa eval -m production -V 'app->gru->enqueue(remove_needle_versions => [], {priority => 5, ttl => 172800, limit => 1})' "$@"
+exec "$(dirname "$0")"/openqa eval -m production -V 'app->gru->enqueue(limit_temp_needle_refs => [], {priority => 5, ttl => 172800, limit => 1})' "$@"
diff --git a/systemd/openqa-enqueue-needle-ref-cleanup.service b/systemd/openqa-enqueue-needle-ref-cleanup.service
@@ -0,0 +1,9 @@
+[Unit]
+Description=Enqueues a needle ref cleanup task for openQA.
+After=postgresql.service openqa-setup-db.service
+Wants=openqa-setup-db.service
+
+[Service]
+Type=oneshot
+User=geekotest
+ExecStart=/usr/share/openqa/script/openqa-enqueue-needle-ref-cleanup
diff --git a/systemd/openqa-enqueue-needle-ref-cleanup.timer b/systemd/openqa-enqueue-needle-ref-cleanup.timer
@@ -0,0 +1,9 @@
+[Unit]
+Description=Enqueues a needle refs task for openQA every day.
+
+[Timer]
+OnCalendar=hourly
+Persistent=true
+
+[Install]
+WantedBy=timers.target
diff --git a/t/14-grutasks.t b/t/14-grutasks.t
@@ -12,6 +12,7 @@ use OpenQA::Jobs::Constants;
 use OpenQA::JobDependencies::Constants;
 use OpenQA::JobGroupDefaults;
 use OpenQA::Schema::Result::Jobs;
+use OpenQA::Needles;
 use File::Copy;
 require OpenQA::Test::Database;
 use OpenQA::Test::Utils qw(run_gru_job perform_minion_jobs);
@@ -411,17 +412,29 @@ subtest 'limit_results_and_logs gru task cleans up logs' => sub {
     ok !-e $log_file_for_groupless_job, 'log file for groupless job got cleaned';
 };
 
-subtest 'remove_needle_versions gru task cleans up needle versions' => sub {
-    # Create a temporary needle file older than the configured expiry time (defaults to 30 minutes)
-    my $temp_needle_path = '/tmp/needle_dirs/test_repo/branch/needles';
-    File::Path->make_path($temp_needle_path);
-    my @needle_files = ($temp_needle_path . 'needle.png', $temp_needle_path . 'needle.json');
-    my $ref = File::Touch->new(atime => time - (30 * 60 + 1));
-    $ref->touch(@needle_files);
-
-    # Run cleanup
-    run_gru_job $t->app, 'remove_needle_versions';
-    ok !-e $_ for @needle_files;
+subtest 'limit_temp_needle_refs task cleans up temp needle refs exceeding retention' => sub {
+    my $temp_dir = path(OpenQA::Needles::temp_dir);
+    is $temp_dir, 't/data/openqa/webui/cache/needle-refs', 'needle temp dir determined as expected';
+    $temp_dir->child($_)->make_path for qw(ref1 ref2);
+    my @old_needle_files = ("$temp_dir/ref1/needle_old.png", "$temp_dir/ref1/needle_old.json");
+    my @new_needle_files = ("$temp_dir/ref2/needle_new.png", "$temp_dir/ref2/needle_new.json");
+    my $now = time;
+    File::Touch->new(time => $now - (120 * ONE_MINUTE + 1))->touch(@old_needle_files);
+    File::Touch->new(time => $now + ONE_MINUTE)->touch(@new_needle_files);
+
+    # enqueue and run cleanup
+    my $minion = $t->app->minion;
+    my $id = $minion->enqueue('limit_temp_needle_refs');
+    ok defined $id, 'job enqueued';
+    perform_minion_jobs $minion;
+    my $job_info = $minion->job($id)->info;
+
+    subtest 'cleanup result' => sub {
+        is $job_info->{state}, 'finished', 'job finished';
+        ok !-e $_, "old file '$_' cleaned up" for @old_needle_files;
+        ok !-e "$temp_dir/ref1", 'empty directory removed';
+        ok -e $_, "new file '$_' preserved" for @new_needle_files;
+    } or diag explain $job_info;
 };
 
 subtest 'limit audit events' => sub {

diff --git a/t/config.t b/t/config.t
@@ -68,6 +68,7 @@ subtest 'Test configuration default modes' => sub {
             git_auto_clone => 'yes',
             git_auto_update => 'no',
             checkout_needles_sha => 'no',
+            temp_needle_refs_retention => 120,
         },
         'scheduler' => {
             max_job_scheduled_time => 7,