From 6fa55913c13fc54fa666e0a223a01b7b22d47ea5 Mon Sep 17 00:00:00 2001 From: Marius Kittler Date: Fri, 1 Dec 2023 15:05:28 +0100 Subject: [PATCH] Fail early when attempting to clone a job with missing assets * 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 --- lib/OpenQA/Schema/Result/Jobs.pm | 1 + lib/OpenQA/Script/CloneJob.pm | 18 ++++++++++++++++++ lib/OpenQA/WebAPI/Controller/API/V1/Job.pm | 4 +++- t/35-script_clone_job.t | 22 +++++++++++++++++++++- t/api/04-jobs.t | 13 +++++++++++++ 5 files changed, 56 insertions(+), 2 deletions(-) diff --git a/lib/OpenQA/Schema/Result/Jobs.pm b/lib/OpenQA/Schema/Result/Jobs.pm index bfb6b7c5b47..66f732cd7d1 100644 --- a/lib/OpenQA/Schema/Result/Jobs.pm +++ b/lib/OpenQA/Schema/Result/Jobs.pm @@ -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; } diff --git a/lib/OpenQA/Script/CloneJob.pm b/lib/OpenQA/Script/CloneJob.pm index 5081d2ff3de..1fd05807454 100644 --- a/lib/OpenQA/Script/CloneJob.pm +++ b/lib/OpenQA/Script/CloneJob.pm @@ -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; @@ -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}}) { diff --git a/lib/OpenQA/WebAPI/Controller/API/V1/Job.pm b/lib/OpenQA/WebAPI/Controller/API/V1/Job.pm index 6c8de8ae5dd..299baa9e5d1 100644 --- a/lib/OpenQA/WebAPI/Controller/API/V1/Job.pm +++ b/lib/OpenQA/WebAPI/Controller/API/V1/Job.pm @@ -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 diff --git a/t/35-script_clone_job.t b/t/35-script_clone_job.t index 1c9b4bd382a..a6326a4cc1a 100644 --- a/t/35-script_clone_job.t +++ b/t/35-script_clone_job.t @@ -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; @@ -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]}, @@ -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( @@ -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 { @@ -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'; diff --git a/t/api/04-jobs.t b/t/api/04-jobs.t index 732116d959d..ee42e341908 100644 --- a/t/api/04-jobs.t +++ b/t/api/04-jobs.t @@ -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);