Skip to content

Commit

Permalink
Merge pull request #5383 from Martchus/missing-assets
Browse files Browse the repository at this point in the history
Fail early when attempting to clone a job with missing assets
  • Loading branch information
mergify[bot] authored Dec 6, 2023
2 parents 9fa738d + 6fa5591 commit 83dffeb
Show file tree
Hide file tree
Showing 5 changed files with 56 additions and 2 deletions.
1 change: 1 addition & 0 deletions lib/OpenQA/Schema/Result/Jobs.pm
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ sub to_hash ($job, %args) {
$j->{parent_group} = $parent_group->name;
}
}
$j->{missing_assets} = $job->missing_assets if $args{check_assets};
return $j;
}

Expand Down
18 changes: 18 additions & 0 deletions lib/OpenQA/Script/CloneJob.pm
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ sub clone_job_apply_settings ($argv, $depth, $settings, $options) {
sub clone_job_get_job ($jobid, $url_handler, $options) {
my $url = $url_handler->{remote_url}->clone;
$url->path("jobs/$jobid");
$url->query->merge(check_assets => 1) unless $options->{'ignore-missing-assets'};
my $tx = $url_handler->{remote}->max_redirects(3)->get($url);
if ($tx->error) {
my $err = $tx->error;
Expand Down Expand Up @@ -113,8 +114,25 @@ sub _job_publishes_uefi_vars ($job, $file) {
$job->{settings}->{UEFI} && _job_setting_is $job, PUBLISH_PFLASH_VARS => $file;
}

sub _check_for_missing_assets ($job, $parents, $options) {
return undef if $options->{'ignore-missing-assets'};
my $missing_assets = $job->{missing_assets};
return undef unless ref $missing_assets eq 'ARRAY'; # most likely an old version of the web UI
my @relevant_missing_assets;
for my $missing_asset (@$missing_assets) {
my ($type, $name) = split '/', $missing_asset, 2;
push @relevant_missing_assets, $missing_asset
unless _is_asset_generated_by_cloned_jobs $job, $parents, $name, $options;
}
return undef unless @relevant_missing_assets;
my $relevant_missing_assets = join("\n - ", @relevant_missing_assets);
my $note = 'Use --ignore-missing-assets or --skip-download to proceed regardless.';
die "The following assets are missing:\n - $relevant_missing_assets\n$note\n";
}

sub clone_job_download_assets ($jobid, $job, $url_handler, $options) {
my $parents = _get_chained_parents($job, $url_handler, $options);
_check_for_missing_assets($job, $parents, $options);
my $ua = $url_handler->{ua};
my $remote_url = $url_handler->{remote_url};
for my $type (keys %{$job->{assets}}) {
Expand Down
4 changes: 3 additions & 1 deletion lib/OpenQA/WebAPI/Controller/API/V1/Job.pm
Original file line number Diff line number Diff line change
Expand Up @@ -445,9 +445,11 @@ settings, state and times of startup and finish of the job.
sub show ($self) {
my $job_id = int($self->stash('jobid'));
my $details = $self->stash('details') || 0;
my $check_assets = !!$self->param('check_assets');
my $job = $self->schema->resultset("Jobs")->find($job_id, {prefetch => 'settings'});
return $self->reply->not_found unless $job;
$self->render(json => {job => $job->to_hash(assets => 1, deps => 1, details => $details, parent_group => 1)});
$job = $job->to_hash(assets => 1, check_assets => $check_assets, deps => 1, details => $details, parent_group => 1);
$self->render(json => {job => $job});
}

=over 4
Expand Down
22 changes: 21 additions & 1 deletion t/35-script_clone_job.t
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use lib "$FindBin::Bin/lib", "$FindBin::Bin/../external/os-autoinst-common/lib";
use OpenQA::Test::TimeLimit '6';
use Mojo::Base -signatures;
use Test::Exception;
use Test::Output qw(combined_like output_from);
use Test::Output qw(combined_like combined_unlike output_from);
use Test::MockObject;
use Test::MockModule;
use OpenQA::Script::CloneJob;
Expand Down Expand Up @@ -95,6 +95,7 @@ subtest 'asset download' => sub {
my %url_handler = (remote_url => Mojo::URL->new('http://foo'), ua => $fake_ua);
my %options = (dir => $temp_assetdir);
my $job_id = 1;
my @missing_assets;
my %job = (
id => $job_id,
parents => {Chained => [2]},
Expand All @@ -104,6 +105,7 @@ subtest 'asset download' => sub {
# HDDs which are not downloaded because generated by (indirect) parent (which is cloned as well)
hdd => [qw(some.qcow2 uefi-vars.qcow2 another.qcow2)],
},
missing_assets => \@missing_assets
);
my $clone_mock = Test::MockModule->new('OpenQA::Script::CloneJob');
$clone_mock->redefine(
Expand All @@ -123,6 +125,7 @@ subtest 'asset download' => sub {
throws_ok { clone_job_download_assets($job_id, \%job, \%url_handler, \%options) }
qr/can't write $temp_assetdir/, 'error because folder does not exist';

# assume an asset download fails
$temp_assetdir->child($_)->make_path for qw(iso hdd);
$fake_ua->missing(1);
throws_ok {
Expand All @@ -131,6 +134,23 @@ subtest 'asset download' => sub {
}
qr/Can't clone due to missing assets: some status/, 'error if asset does not exist';

# assume openQA reports that an asset is missing
@missing_assets = ('iso/foo.iso', 'hdd/some.qcow2');
throws_ok {
combined_unlike { clone_job_download_assets($job_id, \%job, \%url_handler, \%options) }
qr|downloading|s, 'download not attempted if openQA reports missing assets';
}
qr|assets are missing.*iso/foo.iso|s, 'error about missing asset shown';
unlike $@, qr|hdd/some.qcow2|, 'no error about qcow2 image which is generated by parent job';

$options{'skip-deps'} = 1;
throws_ok {
combined_unlike { clone_job_download_assets($job_id, \%job, \%url_handler, \%options) }
qr|downloading|s, 'download not attempted if openQA reports missing assets with --skip-deps';
}
qr|assets are missing.*iso/foo.iso.*hdd/some.qcow2|s, 'error about missing qcow2 with --skip-deps';

$options{'skip-deps'} = 0;
$options{'ignore-missing-assets'} = 1;
combined_like { clone_job_download_assets($job_id, \%job, \%url_handler, \%options) }
qr/downloading.*foo.*to.*failed.*some status.*downloading.*bar.*failed/s, 'download error logged but ignored';
Expand Down
13 changes: 13 additions & 0 deletions t/api/04-jobs.t
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,19 @@ subtest 'job details' => sub {

};

subtest 'check for missing assets' => sub {
$t->get_ok('/api/v1/jobs/99939?check_assets=1')->status_is(200, 'status for job with missing asset');
$t->json_is('/job/assets/hdd/0', 'openSUSE-13.1-x86_64.hda', 'present asset returned');
$t->json_is(
'/job/missing_assets/0',
'iso/openSUSE-Factory-DVD-x86_64-Build0048-Media.iso',
'missing asset returned'
);

$t->get_ok('/api/v1/jobs/99927?check_assets=1')->status_is(200, 'status for job without missing assets');
$t->json_is('/job/missing_assets/0', undef, 'no missing asset returned for 99927');
};

subtest 'update job and job settings' => sub {
# check defaults
$t->get_ok('/api/v1/jobs/99926')->status_is(200);
Expand Down

0 comments on commit 83dffeb

Please sign in to comment.