Skip to content

Commit

Permalink
Fail early when attempting to clone a job with missing assets
Browse files Browse the repository at this point in the history
* Do the same check for assets we already do before restarting a job also
  before cloning
    * This is necessary because assets that do not exist anymore are not
      returned by the job API anymore at all. Thus the clone script would
      not even attempt to download them and also not fail early. (The error
      about the missing asset would only occur when trying to run the job.)
* Do not fail on missing assets that are created by a parent job (when that
  parent job is cloned as well)
* Allow to proceed via `--ignore-missing-assets` as it is already possible
  for download errors that happen on the fly
* See https://progress.opensuse.org/issues/151522
  • Loading branch information
Martchus committed Dec 1, 2023
1 parent 3c8b171 commit 6fa5591
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 6fa5591

Please sign in to comment.