Skip to content

Commit

Permalink
Merge pull request #3496 from Martchus/finalize-job
Browse files Browse the repository at this point in the history 
Prevent failed 'finalize_job_results' Minion jobs for skipped jobs
  • Loading branch information
@mergify
mergify[bot] committed Oct 30, 2020
2 parents 10ee48c + 0a7db04 commit 57a95a4
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 33 deletions.
35 changes: 18 additions & 17 deletions lib/OpenQA/Schema/Result/Jobs.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1783,27 +1783,26 @@ sub bugref {
return undef;
}

# extend to finish
sub store_column {
my ($self, %args) = @_;
if ($args{state} && grep { $args{state} eq $_ } FINAL_STATES) {
if (!$self->t_finished) {
# make sure we do not overwrite a t_finished from fixtures
# in normal operation it should be impossible to finish
# twice
$self->t_finished(now());
}
my ($self, $columnname, $value) = @_;

# handle transition to the final state
if ($columnname eq 'state' && grep { $value eq $_ } FINAL_STATES) {
# make sure we do not overwrite a t_finished from fixtures
# note: In normal operation it should be impossible to finish twice.
$self->t_finished(now()) unless $self->t_finished;
# make sure no modules are left running
$self->modules->search({result => RUNNING})->update({result => NONE});
# This function gets executed even when unit tests are setting up
# fixtures. $app and $self->id may not be set in that case. Additionally,
# the app might not be setup to have the "gru" method.
if (my $id = $self->id) {
my $gru = eval { OpenQA::App->singleton->gru };
$gru->enqueue(finalize_job_results => [$id]) if $gru;
}
}
return $self->SUPER::store_column(%args);

return $self->SUPER::store_column($columnname, $value);
}

sub enqueue_finalize_job_results {
my ($self) = @_;

my $gru = eval { OpenQA::App->singleton->gru }; # gru might not be present within tests
$gru->enqueue(finalize_job_results => [$self->id]) if $gru;
}

# used to stop jobs with some kind of dependency relationship to another
Expand Down Expand Up @@ -1992,6 +1991,7 @@ sub done {
$new_val{reason} = $reason;
}
$self->update(\%new_val);
$self->enqueue_finalize_job_results;

# stop other jobs in the cluster
if (defined $new_val{result} && !grep { $result eq $_ } OK_RESULTS) {
Expand All @@ -2013,6 +2013,7 @@ sub cancel {
return undef if $self->result ne NONE;
$self->release_networks;
$self->update({state => CANCELLED, result => ($obsoleted ? OBSOLETED : USER_CANCELLED)});
$self->enqueue_finalize_job_results;
my $count = 1;
if (my $worker = $self->assigned_worker) {
$worker->send_command(command => WORKER_COMMAND_CANCEL, job_id => $self->id);
Expand Down
39 changes: 23 additions & 16 deletions t/14-grutasks.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ use FindBin;
use lib "$FindBin::Bin/lib";
use OpenQA::Utils;
use OpenQA::Jobs::Constants;
use OpenQA::JobDependencies::Constants;
use OpenQA::Schema::Result::Jobs;
use File::Copy;
use OpenQA::Test::Database;
Expand Down Expand Up @@ -72,13 +73,14 @@ $assets_result_mock->redefine(
$assets_result_mock->redefine(refresh_size => sub { });

# setup test config and database
my $test_case = OpenQA::Test::Case->new(config_directory => "$FindBin::Bin/data/41-audit-log");
my $schema = $test_case->init_data(fixtures_glob => '01-jobs.pl 04-products.pl');
my $jobs = $schema->resultset('Jobs');
my $job_groups = $schema->resultset('JobGroups');
my $parent_groups = $schema->resultset('JobGroupParents');
my $assets = $schema->resultset('Assets');
my $job_assets = $schema->resultset('JobsAssets');
my $test_case = OpenQA::Test::Case->new(config_directory => "$FindBin::Bin/data/41-audit-log");
my $schema = $test_case->init_data(fixtures_glob => '01-jobs.pl 04-products.pl');
my $jobs = $schema->resultset('Jobs');
my $job_groups = $schema->resultset('JobGroups');
my $job_dependencies = $schema->resultset('JobDependencies');
my $parent_groups = $schema->resultset('JobGroupParents');
my $assets = $schema->resultset('Assets');
my $job_assets = $schema->resultset('JobsAssets');

# move group 1002 into a parent group
# note: This shouldn't change much because 1002 will be the only child and the same limit applies.
Expand Down Expand Up @@ -642,7 +644,10 @@ subtest 'finalize job results' => sub {
ARCH => 'x86_64',
TEST => 'minion',
);
my $job = $schema->resultset('Jobs')->create_from_settings(\%settings);
my $job = $jobs->create_from_settings(\%settings);
my $child_job = $jobs->create_from_settings(\%settings);
my @chained = (dependency => OpenQA::JobDependencies::Constants::CHAINED);
$job_dependencies->create({child_job_id => $child_job->id, parent_job_id => $job->id, @chained});
$job->discard_changes;
$job->insert_module({name => 'a', category => 'a', script => 'a', flags => {}});
$job->update_module('a', {result => 'ok', details => [{title => 'wait_serial', text => 'a-0.txt'}]});
Expand All @@ -661,18 +666,20 @@ subtest 'finalize job results' => sub {
my $a_txt = path($job->result_dir, 'a-0.txt')->spurt('Foo');
my $b_txt = path('t/data/14-module-b.txt')->copy_to($job->result_dir . '/b-0.txt');
$job->done;
$job->discard_changes;
is($job->result, OpenQA::Jobs::Constants::FAILED, 'job result is failed');
$_->discard_changes for ($job, $child_job);
is($job->result, FAILED, 'job result is failed');
is($child_job->state, CANCELLED, 'child job cancelled');
is($child_job->result, SKIPPED, 'child job skipped');
$minion->perform_jobs;
my $minion_jobs = $minion->jobs({tasks => ['finalize_job_results']});
is($minion_jobs->total, 1, 'one minion job executed')
is($minion_jobs->total, 1, 'one minion job created; no minion job for skipped child job created')
and is($minion_jobs->next->{state}, 'finished', 'the minion job succeeded');
ok(!-e $a_txt);
ok(!-e $b_txt);
ok(!-e $a_txt, 'extra txt file for module a actually gone');
ok(!-e $b_txt, 'extra txt file for module b actually gone');
my @modlist = $job->modules;
is($modlist[0]->results->{details}->[0]->{text_data}, 'Foo');
is($modlist[1]->results->{details}->[0]->{text_data}, "正解\n");
is($a_details->stat->mode & 0644, 0644, 'details JSON globally readable');
is($modlist[0]->results->{details}->[0]->{text_data}, 'Foo', 'text data for module a still present');
is($modlist[1]->results->{details}->[0]->{text_data}, "正解\n", 'text data for module b still present');
is($a_details->stat->mode & 0644, 0644, 'details JSON globally readable');
};

subtest 'enqueue finalize_job_results without job or job which (no longer) exists' => sub {
Expand Down

0 comments on commit 57a95a4

Please sign in to comment.