Merge pull request #7191 from Martchus/clone-job-retry

feat: Retry API calls in `clone-job` like already `openqa-cli` does

Author: mergify[bot]

lib/OpenQA/Script/CloneJob.pm

@@ -8,7 +8,7 @@ use Mojo::Base -strict, -signatures;
 use Cpanel::JSON::XS;
 use Data::Dump 'pp';
 use Exporter 'import';
-use OpenQA::Client;
+use OpenQA::Command;
 use OpenQA::Jobs::Constants;
 use Mojo::File 'path';
 use Mojo::URL;
@@ -17,6 +17,11 @@ use OpenQA::Script::CloneJobSUSE;
 use List::Util 'any';
 use HTTP::Status qw(:constants);
 
+package OpenQA::Script::CloneJob::Command {
+    use Mojo::Base 'OpenQA::Command', -signatures;
+    sub handle_result ($self, $tx, $orig_tx = undef) { $tx }
+}
+
 our @EXPORT = qw(
   clone_jobs
   clone_job_apply_settings
@@ -89,18 +94,23 @@ sub _handle_txn_error ($tx, $jobid, $ctx) {
     _handle_unexpected_return_code($tx) unless $tx->res->code == HTTP_OK;
 }
 
+sub _get_with_retry ($url_handler, $url, $jobid, $ctx, $options) {
+    my $remote = $url_handler->{remote};
+    my $tx = $url_handler->{command}->retry_tx($remote, $remote->get($url), $options->{retry});
+    _handle_txn_error($tx, $jobid, $ctx);
+    return $tx->res->json;
+}
+
 sub _get_vars ($jobid, $url_handler, $options) {
     my $url = $url_handler->{remote_url}->clone->path("/tests/$jobid/file/vars.json");
-    _handle_txn_error(my $tx = $url_handler->{remote}->max_redirects(3)->get($url), $jobid, 'vars.json of job');
-    return $tx->res->json;
+    return _get_with_retry($url_handler, $url, $jobid, 'vars.json of job', $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'};
-    _handle_txn_error(my $tx = $url_handler->{remote}->max_redirects(3)->get($url), $jobid, 'job');
-    my $job = $tx->res->json->{job};
+    my $job = _get_with_retry($url_handler, $url, $jobid, 'job', $options)->{job};
     print STDERR Cpanel::JSON::XS->new->pretty->encode($job) if $options->{verbose};
     $job->{vars} = _get_vars($jobid, $url_handler, $options) if $options->{reproduce};
     return $job;
@@ -219,7 +229,7 @@ sub split_jobid ($url_string) {
 }
 
 sub make_curl_arguments ($options) {
-    my @args = ('--follow', '--retry', ($options->{retry} // 5), '--retry-connrefused');
+    my @args = ('--follow', '--retry', $options->{retry}, '--retry-connrefused');
     push @args, '--no-progress-meter' unless $options->{'show-progress'};
     push @args, '--verbose' if $options->{verbose};
     return \@args;
@@ -236,22 +246,30 @@ sub create_url_handler ($options) {
     # configure user agent for destination host (usually localhost)
     my $local_url = OpenQA::Client::url_from_host($options->{host});
     $local_url->path('/api/v1/jobs');
-    my $local = OpenQA::Client->new(
-        api => $local_url->host,
+    my $command = OpenQA::Script::CloneJob::Command->new(
+        name => 'openqa-clone-job',
         apikey => $options->{apikey},
-        apisecret => $options->{apisecret});
+        apisecret => $options->{apisecret},
+        options => $options
+    );
+    my $local = $command->client($local_url)->max_redirects(3);
     die "API key/secret for '$options->{host}' missing. Check out '$0 --help' for the config file syntax/lookup.\n"
       if !$options->{'export-command'} && !($local->apikey && $local->apisecret);
 
+    # configure the default for the number of retries and use exponential backoff by default
+    $options->{retry} //= 5;
+    $ENV{OPENQA_CLI_RETRY_FACTOR} //= 2;
+
     # configure user agents for the source host (usually a remote host)
     my $remote_url = OpenQA::Client::url_from_host($options->{from});
     $remote_url->path('/api/v1/jobs');
     return {
         curl_args => make_curl_arguments($options),
         secrets => read_secrets($remote_url->host),
+        command => $command,
         local => $local,
         local_url => $local_url,
-        remote => OpenQA::Client->new(api => $remote_url->host),
+        remote => $command->client($remote_url)->apikey(undef)->apisecret(undef)->max_redirects(3),
         remote_url => $remote_url
     };
 }
@@ -316,8 +334,10 @@ sub clone_jobs ($jobid, $options) {
     clone_job($jobid, $url_handler, $options, my $post_params = {}, my $jobs = {});
     for my $counter (1 .. $repeat) {
         append_idx_to_test_name($counter, $digits, $post_params) if $repeat > 1;
-        my $tx = post_jobs($post_params, $url_handler, $options);
-        handle_tx($tx, $url_handler, $options, $jobs) if $tx;
+        if (my $tx = post_jobs($post_params, $url_handler, $options)) {
+            $tx = $url_handler->{command}->retry_tx($url_handler->{local}, $tx, $options->{retry});
+            handle_tx($tx, $url_handler, $options, $jobs);
+        }
     }
 }
 
@@ -397,7 +417,7 @@ sub post_jobs ($post_params, $url_handler, $options) {
         return undef;
     }
     print STDERR Cpanel::JSON::XS->new->pretty->encode(\%composed_params) if $options->{verbose};
-    return $local->max_redirects(3)->post($local_url, form => \%composed_params);
+    return $local->post($local_url, form => \%composed_params);
 }
 
 1;

script/openqa-clone-job

@@ -166,9 +166,8 @@ Will create 50 clones of the same job.
 
 =item B<--retry> N
 
-Retries up to N times with exponential backoff on transient errors when
-downloading assets. The default number of retries is 5. Can be set to 0 to
-disable retrying.
+Retries up to N times with exponential backoff on transient download errors. The
+default number of retries is 5. Can be set to 0 to disable retrying.
 
 =item B<--show-progress>
 

t/35-script_clone_job.t

@@ -27,6 +27,9 @@ package Test::FakeResult {
     has json => undef;
 }    # uncoverable statement
 
+my $command_mock = Test::MockModule->new('OpenQA::Command');
+$command_mock->redefine(retry_tx => sub ($self, $client, $tx, $retries) { $tx });
+
 my @argv = (
     qw(WORKER_CLASS=local HDD_1=new.qcow2 HDDSIZEGB=40 FOO=value:with:colon),
     'WORKER_CLASS:cre:ate+hpc#foo@bar+=-parent'
@@ -64,7 +67,10 @@ subtest 'getting job' => sub {
     my $fake_txn = Test::MockObject->new->set_always(res => $fake_res)->set_always(error => undef);
     my $fake_ua = Test::MockObject->new->set_always(get => $fake_txn);
     $fake_ua->set_always(max_redirects => $fake_ua);
-    my $url_handler = {remote => $fake_ua, remote_url => Mojo::URL->new('foo')};
+    my %fake_params = (host => 'host', from => 'from', apikey => 'key', apisecret => 'secret');
+    my $url_handler = OpenQA::Script::CloneJob::create_url_handler(\%fake_params);
+    $url_handler->{remote} = $fake_ua;
+    $url_handler->{remote_url} = Mojo::URL->new('foo');
     my $options = {'ignore-missing-assets' => 1, reproduce => 1};
     throws_ok { clone_job_get_job(42, $url_handler, $options) } qr/unexpected return code/,
       'unexpected return code handled';
@@ -494,7 +500,7 @@ subtest 'invoking curl passing auth headers' => sub {
         });
     note 'config path: ' . ($ENV{OPENQA_CONFIG} = "$FindBin::Bin/data");
 
-    my $args = OpenQA::Script::CloneJob::make_curl_arguments({});
+    my $args = OpenQA::Script::CloneJob::make_curl_arguments({retry => 5});
     my @expected_default_args = qw(--follow --retry 5 --retry-connrefused --no-progress-meter);
     is_deeply $args, \@expected_default_args, 'default arguments correct';
     my $secrets = OpenQA::Script::CloneJob::read_secrets('testapi');

t/40-openqa-clone-job.t

@@ -39,7 +39,7 @@ subtest 'usage' => sub {
 };
 
 subtest errors => sub {
-    local @ARGV = (@apiargs, 'http://openqa.local.foo/t1');
+    local @ARGV = (@apiargs, qw(--retry 0 http://openqa.local.foo/t1));
     throws_ok { main::main() } qr/failed to get job '1'/, 'fails with non existing host';
 };