Skip to content

Commit

Permalink
Improve cleanup of temporary needle refs
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
Martchus committed Mar 12, 2024
1 parent da01e51 commit 07e3a1f
Show file tree
Hide file tree
Showing 12 changed files with 95 additions and 54 deletions.
2 changes: 2 additions & 0 deletions etc/openqa/openqa.ini
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@
#do_push = 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]
Expand Down
6 changes: 5 additions & 1 deletion lib/OpenQA/Git.pm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package OpenQA::Git;

use Mojo::Base -base, -signatures;
use Cwd 'abs_path';
use File::Touch;
use OpenQA::Utils qw(run_cmd_with_log_return_error);

has 'app';
Expand Down Expand Up @@ -95,7 +96,10 @@ sub commit ($self, $args = undef) {
}

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;

Check warning on line 101 in lib/OpenQA/Git.pm

View check run for this annotation

Codecov / codecov/patch

lib/OpenQA/Git.pm#L98-L101

Added lines #L98 - L101 were not covered by tests
}
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};
Expand Down
4 changes: 3 additions & 1 deletion lib/OpenQA/Needles.pm
Original file line number Diff line number Diff line change
Expand Up @@ -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') }

Check warning on line 24 in lib/OpenQA/Needles.pm

View check run for this annotation

Codecov / codecov/patch

lib/OpenQA/Needles.pm#L24

Added line #L24 was not covered by tests
Expand Down
2 changes: 1 addition & 1 deletion lib/OpenQA/Setup.pm
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ sub read_config ($app) {
do_push => 'no',
do_cleanup => 'no',
checkout_needles_sha => 'no',
minimum_needle_retention_time => undef,
temp_needle_refs_retention => 120,
},
scheduler => {
max_job_scheduled_time => 7,
Expand Down
2 changes: 1 addition & 1 deletion lib/OpenQA/Shared/Plugin/Gru.pm
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ sub register_tasks ($self) {
for (
qw(OpenQA::Task::AuditEvents::Limit),
qw(OpenQA::Task::Asset::Download OpenQA::Task::Asset::Limit),
qw(OpenQA::Task::Needle::Scan OpenQA::Task::Needle::Save OpenQA::Task::Needle::Delete OpenQA::Task::Needle::RemoveVersions),
qw(OpenQA::Task::Needle::Scan OpenQA::Task::Needle::Save OpenQA::Task::Needle::Delete OpenQA::Task::Needle::LimitTempRefs),
qw(OpenQA::Task::Job::Limit),
qw(OpenQA::Task::Job::ArchiveResults),
qw(OpenQA::Task::Job::FinalizeResults),
Expand Down
39 changes: 39 additions & 0 deletions lib/OpenQA/Task/Needle/LimitTempRefs.pm
Original file line number Diff line number Diff line change
@@ -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;
38 changes: 0 additions & 38 deletions lib/OpenQA/Task/Needle/RemoveVersions.pm

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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})' "$@"
9 changes: 9 additions & 0 deletions systemd/openqa-enqueue-needle-ref-cleanup.service
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions systemd/openqa-enqueue-needle-ref-cleanup.timer
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[Unit]
Description=Enqueues a needle refs task for openQA every day.

[Timer]
OnCalendar=hourly
Persistent=true

[Install]
WantedBy=timers.target
35 changes: 24 additions & 11 deletions t/14-grutasks.t
Original file line number Diff line number Diff line change
Expand Up @@ -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;
use OpenQA::Test::Database;
use OpenQA::Test::Utils qw(run_gru_job perform_minion_jobs);
Expand Down Expand Up @@ -402,17 +403,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 {
Expand Down
1 change: 1 addition & 0 deletions t/config.t
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ subtest 'Test configuration default modes' => sub {
do_push => 'no',
do_cleanup => 'no',
checkout_needles_sha => 'no',
temp_needle_refs_retention => 120,
},
'scheduler' => {
max_job_scheduled_time => 7,
Expand Down

0 comments on commit 07e3a1f

Please sign in to comment.