From 14f5f81758077de852723187d9b9395f3df57981 Mon Sep 17 00:00:00 2001
From: Cris Dywa <cdywan@suse.de>
Date: Wed, 11 May 2022 13:34:47 +0200
Subject: [PATCH] Use script_retry instead of shell for loops

Fixes: https://progress.opensuse.org/issues/110803
---
 lib/utils.pm                     | 53 +++++++++++++++++++++++++++++++-
 tests/containers/build.pm        |  7 +++--
 tests/containers/setup_env.pm    |  2 +-
 tests/osautoinst/test_running.pm |  2 +-
 4 files changed, 58 insertions(+), 6 deletions(-)

diff --git a/lib/utils.pm b/lib/utils.pm
index 0a1c1a0d..300906e8 100644
--- a/lib/utils.pm
+++ b/lib/utils.pm
@@ -6,7 +6,7 @@ use strict;
 use testapi;
 use File::Basename qw(basename);
 
-our @EXPORT = qw(clear_root_console switch_to_x11 wait_for_desktop ensure_unlocked_desktop wait_for_container_log);
+our @EXPORT = qw(clear_root_console switch_to_x11 wait_for_desktop ensure_unlocked_desktop wait_for_container_log script_retry);
 
 sub clear_root_console {
     enter_cmd('clear');
@@ -106,4 +106,55 @@ sub wait_for_container_log {
     validate_script_output("$cmd logs $container 2>&1", qr/$text/);
 }
 
+=head2 script_retry
+
+ script_retry($cmd, [expect => $expect], [retry => $retry], [delay => $delay], [timeout => $timeout], [die => $die]);
+
+Repeat command until expected result or timeout.
+
+C<$expect> refers to the expected command exit code and defaults to C<0>.
+
+C<$retry> refers to the number of retries and defaults to C<10>.
+
+C<$delay> is the time between retries and defaults to C<30>.
+
+The command must return within C<$timeout> seconds (default: 25).
+
+If the command doesn't return C<$expect> after C<$retry> retries,
+this function will die, if C<$die> is set.
+
+Example:
+
+ script_retry('ping -c1 -W1 machine', retry => 5);
+
+=cut
+sub script_retry {
+    my ($cmd, %args) = @_;
+    my $ecode = $args{expect} // 0;
+    my $retry = $args{retry} // 10;
+    my $delay = $args{delay} // 30;
+    my $timeout = $args{timeout} // 30;
+    my $die = $args{die} // 1;
+
+    my $ret;
+
+    my $exec = "timeout $timeout $cmd";
+    # Exclamation mark needs to be moved before the timeout command, if present
+    if (substr($cmd, 0, 1) eq "!") {
+        $cmd = substr($cmd, 1);
+        $cmd =~ s/^\s+//;    # left trim spaces after the exclamation mark
+        $exec = "! timeout $timeout $cmd";
+    }
+    for (1 .. $retry) {
+        # timeout for script_run must be larger than for the 'timeout ...' command
+        $ret = script_run($exec, ($timeout + 3));
+        last if defined($ret) && $ret == $ecode;
+
+        die("Waiting for Godot: $cmd") if $retry == $_ && $die == 1;
+        sleep $delay if ($delay > 0);
+    }
+
+    return $ret;
+}
+
 1;
diff --git a/tests/containers/build.pm b/tests/containers/build.pm
index 23b29f34..c9995b94 100644
--- a/tests/containers/build.pm
+++ b/tests/containers/build.pm
@@ -1,13 +1,14 @@
 use strict;
 use base "openQAcoretest";
 use testapi;
+use utils;
 
 sub run {
     my ($self) = @_;
     assert_script_run("git clone https://github.com/os-autoinst/openQA.git", timeout => 300);
-    assert_script_run('for i in {1..3}; do docker build openQA/container/webui -t openqa_webui && break; done', timeout => 3600);
-    assert_script_run('for i in {1..3}; do docker build openQA/container/worker -t openqa_worker && break; done', timeout => 3600);
-    assert_script_run('for i in {1..3}; do docker build openQA/container/openqa_data -t openqa_data && break; done', timeout => 3600);
+    script_retry('docker build openQA/container/webui -t openqa_webui', retry => 3, delay => 60, timeout => 3600);
+    script_retry('docker build openQA/container/worker -t openqa_worker', retry => 3, delay => 60, timeout => 3600);
+    script_retry('docker build openQA/container/openqa_data -t openqa_data', retry => 3, delay => 60, timeout => 3600);
 }
 
 1;
diff --git a/tests/containers/setup_env.pm b/tests/containers/setup_env.pm
index afd6b68e..e5011823 100644
--- a/tests/containers/setup_env.pm
+++ b/tests/containers/setup_env.pm
@@ -7,7 +7,7 @@ sub run {
 
    assert_script_run("mkdir -p /root/data/factory/{iso,hdd,other} /root/data/tests");
    assert_script_run("docker network create testing");
-   assert_script_run("docker run --rm -d --network testing -e POSTGRES_PASSWORD=openqa -e POSTGRES_USER=openqa -e POSTGRES_DB=openqa --net-alias=db --name db postgres", timeout => 600);
+   script_retry("docker run --rm -d --network testing -e POSTGRES_PASSWORD=openqa -e POSTGRES_USER=openqa -e POSTGRES_DB=openqa --net-alias=db --name db postgres", retry => 3, delay => 60, timeout => 600);
    wait_for_container_log("db", "database system is ready to accept connections", "docker");
 }
 
diff --git a/tests/osautoinst/test_running.pm b/tests/osautoinst/test_running.pm
index 0bf78a24..0b1a426c 100644
--- a/tests/osautoinst/test_running.pm
+++ b/tests/osautoinst/test_running.pm
@@ -5,7 +5,7 @@ use utils;
 
 sub run {
     assert_script_run 'command -v ack >/dev/null || zypper --no-refresh -n in ack';
-    assert_script_run 'ret=false; for i in {1..5} ; do openqa-cli api jobs state=running state=done | ack --passthru --color "running|done" && ret=true && break ; sleep 30 ; done ; [ "$ret" = "true" ]', 300;
+    script_retry('openqa-cli api jobs state=running state=done | ack --passthru --color "running|done"', retry => 5, delay => 30, timeout => 300);
     save_screenshot;
     clear_root_console;
 }