t: Prevent use of occupied port in all full-stack/scalability tests

Sporadically tests can fail with "Connection refused" when already occupied ports have been selected for services in tests as we only looked for a single free port and tried to use the ports next to it without checking if they are actually free. This commit is inspired from the idea in the scalability test extended to all services and used in all relevant full-stack and scalability tests. By mocking the function "service_port" from OpenQA::Utils whenever the new function "mock_service_ports" is called we can provide a consistent but dynamically defined set of ports that should all be free to use during the course of each test. Related progress issue: https://progress.opensuse.org/issues/59043
os-autoinst · May 30, 2020 · 61ffabd · 61ffabd
1 parent 7441700
commit 61ffabd
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 40 deletions.
diff --git a/t/05-scheduler-full.t b/t/05-scheduler-full.t
@@ -38,6 +38,7 @@ use Mojo::IOLoop::Server;
 use Mojo::File qw(path tempfile);
 use Time::HiRes 'sleep';
 use OpenQA::Test::Utils qw(
+  mock_service_ports
   setup_fullstack_temp_dir create_user_for_workers
   create_webapi wait_for_worker setup_share_dir create_websocket_server
   stop_service unstable_worker
@@ -57,8 +58,9 @@ my $api_key         = $api_credentials->key;
 my $api_secret      = $api_credentials->secret;
 
 # create web UI and websocket server
-my $mojoport = $ENV{OPENQA_BASE_PORT} = Mojo::IOLoop::Server->generate_port();
-my $ws       = create_websocket_server($mojoport + 1, 0, 1, 1);
+mock_service_ports;
+my $mojoport = service_port 'webui';
+my $ws       = create_websocket_server(undef, 0, 1, 1);
 my $webapi   = create_webapi($mojoport, sub { });
 my @workers;
 
@@ -258,7 +260,7 @@ subtest 'Websocket server - close connection test' => sub {
 
     my $log;
     # create unstable ws
-    $ws      = create_websocket_server($mojoport + 1, 1, 0);
+    $ws      = create_websocket_server(undef, 1, 0);
     @workers = create_worker($api_key, $api_secret, "http://localhost:$mojoport", 2, \$log);
 
     my $found_connection_closed_in_log = 0;

diff --git a/t/33-developer_mode.t b/t/33-developer_mode.t
@@ -40,6 +40,7 @@ use Fcntl ':mode';
 use DBI;
 use File::Path qw(make_path remove_tree);
 use Module::Load::Conditional 'can_load';
+use OpenQA::Utils qw(service_port);
 use OpenQA::Test::Utils qw(
   create_websocket_server create_scheduler create_live_view_handler setup_share_dir setup_fullstack_temp_dir
   start_worker stop_service
@@ -92,11 +93,11 @@ $users->create(
 ok(Mojolicious::Commands->start_app('OpenQA::WebAPI', 'eval', '1+0'));
 
 # start Selenium test driver and other daemons
-my $mojoport = Mojo::IOLoop::Server->generate_port;
-my $driver   = call_driver(sub { }, {mojoport => $mojoport});
-$ws          = create_websocket_server($mojoport + 1, 0, 0);
-$scheduler   = create_scheduler($mojoport + 3);
-$livehandler = create_live_view_handler($mojoport);
+my $port   = service_port 'webui';
+my $driver = call_driver(sub { }, {mojoport => $port});
+$ws          = create_websocket_server(undef, 0, 0);
+$scheduler   = create_scheduler;
+$livehandler = create_live_view_handler;
 
 # login
 $driver->title_is('openQA', 'on main page');

diff --git a/t/43-scheduling-and-worker-scalability.t b/t/43-scheduling-and-worker-scalability.t
@@ -27,9 +27,11 @@ use IPC::Run qw(start);
 use FindBin;
 use lib "$FindBin::Bin/lib";
 use OpenQA::Scheduler::Model::Jobs;
+use OpenQA::Utils qw(service_port);
 use OpenQA::Test::Database;
 use OpenQA::Jobs::Constants;
 use OpenQA::Test::Utils
+  qw(mock_service_ports),
   qw(create_user_for_workers create_webapi setup_share_dir create_websocket_server),
   qw(stop_service setup_fullstack_temp_dir);
 
@@ -55,22 +57,7 @@ BAIL_OUT 'invalid SCALABILITY_TEST_WORKER_COUNT/SCALABILITY_TEST_JOB_COUNT'
 note("Running scalability test with $worker_count worker(s) and $job_count job(s).");
 note('Set SCALABILITY_TEST_WORKER_COUNT/SCALABILITY_TEST_JOB_COUNT to adjust this.');
 
-# generate free ports for the web UI and the web socket server and mock service_port to use them
-# FIXME: So far the fullstack tests only generate the web UI port and derive the required port for other services
-#        from it. Apparently sometimes these derived ports are sometimes not available leading to the error "Address
-#        already in use in ...". This manual effort should prevent that. It should likely be generalized and used
-#        in the other fullstack tests as well.
-my %ports      = (webui => Mojo::IOLoop::Server->generate_port, websocket => Mojo::IOLoop::Server->generate_port);
-my $utils_mock = Test::MockModule->new('OpenQA::Utils');
-$utils_mock->redefine(
-    service_port => sub {
-        my ($service) = @_;
-        my $port = $ports{$service};
-        BAIL_OUT("Service_port was called for unexpected/unknown service $service") unless defined $port;
-        note("Mocking service port for $service to be $port");
-        return $port;
-    });
-note('Used ports: ' . dumper(\%ports));
+mock_service_ports;
 
 # setup basedir, config dir and database
 my $tempdir = setup_fullstack_temp_dir('scalability');
@@ -79,16 +66,17 @@ my $workers = $schema->resultset('Workers');
 my $jobs    = $schema->resultset('Jobs');
 
 # create web UI and websocket server
-my $web_socket_server = create_websocket_server($ports{websocket}, 0, 1, 1);
-my $webui             = create_webapi($ports{webui}, sub { });
+my $web_socket_server = create_websocket_server(undef, 0, 1, 1);
+my $webui             = create_webapi(undef, sub { });
 
 # prepare spawning workers
 my $sharedir        = setup_share_dir($ENV{OPENQA_BASEDIR});
 my $resultdir       = path($ENV{OPENQA_BASEDIR}, 'openqa', 'testresults')->make_path;
 my $api_credentials = create_user_for_workers;
 my $api_key         = $api_credentials->key;
 my $api_secret      = $api_credentials->secret;
-my $webui_host      = "http://localhost:$ports{webui}";
+my $webui_port      = service_port 'webui';
+my $webui_host      = "http://localhost:$webui_port";
 my $worker_path     = path($FindBin::Bin)->child('../script/worker');
 my $isotovideo_path = path($FindBin::Bin)->child('dummy-isotovideo.sh');
 my @worker_args     = (

diff --git a/t/full-stack.t b/t/full-stack.t
@@ -43,14 +43,15 @@ use Fcntl ':mode';
 use DBI;
 use Mojo::File 'path';
 use Mojo::IOLoop::ReadWriteProcess::Session 'session';
+use OpenQA::Utils qw(service_port);
 use OpenQA::SeleniumTest;
 session->enable;
 
 use File::Path qw(make_path remove_tree);
 use Module::Load::Conditional 'can_load';
 use OpenQA::Test::Utils
   qw(create_websocket_server create_live_view_handler setup_share_dir),
-  qw(cache_minion_worker cache_worker_service setup_fullstack_temp_dir),
+  qw(cache_minion_worker cache_worker_service mock_service_ports setup_fullstack_temp_dir),
   qw(start_worker stop_service);
 use OpenQA::Test::FullstackUtils;
 
@@ -78,10 +79,11 @@ my $sharedir = setup_share_dir($ENV{OPENQA_BASEDIR});
 # initialize database, start daemons
 my $schema = OpenQA::Test::Database->new->create(skip_fixtures => 1, schema_name => 'public', drop_schema => 1);
 ok(Mojolicious::Commands->start_app('OpenQA::WebAPI', 'eval', '1+0'), 'assets are prefetched');
-my $mojoport = Mojo::IOLoop::Server->generate_port;
-$ws = create_websocket_server($mojoport + 1, 0, 0);
-my $driver = call_driver(sub { }, {mojoport => $mojoport});
-$livehandler = create_live_view_handler($mojoport);
+mock_service_ports;
+my $mojoport = service_port 'websocket';
+$ws = create_websocket_server($mojoport, 0, 0);
+my $driver = call_driver(sub { }, {mojoport => service_port 'webui'});
+$livehandler = create_live_view_handler;
 
 my $resultdir = path($ENV{OPENQA_BASEDIR}, 'openqa', 'testresults')->make_path;
 ok(-d $resultdir, "resultdir \"$resultdir\" exists");

diff --git a/t/lib/OpenQA/Test/Utils.pm b/t/lib/OpenQA/Test/Utils.pm
@@ -19,13 +19,15 @@ use OpenQA::Scheduler;
 use OpenQA::Scheduler::Client;
 use Mojo::Home;
 use Mojo::File qw(path tempfile tempdir);
+use Mojo::Util 'dumper';
 use Cwd qw(abs_path getcwd);
 use IPC::Run qw(start);
 use Mojo::Util 'gzip';
 use Test::Output 'combined_like';
 use Mojo::IOLoop;
 use Mojo::IOLoop::ReadWriteProcess 'process';
 use Mojo::Server::Daemon;
+use Mojo::IOLoop::Server;
 use Test::MockModule;
 use Time::HiRes 'sleep';
 
@@ -42,6 +44,7 @@ BEGIN {
 
 our (@EXPORT, @EXPORT_OK);
 @EXPORT_OK = (
+    qw(mock_service_ports),
     qw(redirect_output standard_worker create_user_for_workers),
     qw(create_webapi create_websocket_server create_scheduler create_live_view_handler),
     qw(unresponsive_worker broken_worker rejective_worker wait_for_worker setup_share_dir setup_fullstack_temp_dir run_gru_job),
@@ -50,6 +53,28 @@ our (@EXPORT, @EXPORT_OK);
     qw(run_cmd test_cmd)
 );
 
+# The function OpenQA::Utils::service_port method hardcodes ports in a
+# sequential range starting with OPENQA_BASE_PORT. This can cause problems
+# especially in repeated testing if any of the port in that range is already
+# occupied, so we inject random, free ports for the services here
+#
+# Potential point for
+# later improvement: In Mojo::IOLoop::Server::generate_port keep the sock
+# object on the port and reuse it in listen to prevent race condition
+#
+# Potentially this approach can also be used in production code.
+sub mock_service_ports {
+    my %ports;
+    Test::MockModule->new('OpenQA::Utils')->redefine(
+        service_port => sub {
+            my ($service) = @_;
+            my $port = $ports{$service} //= Mojo::IOLoop::Server->generate_port;
+            note("Mocking service port for $service to be $port");
+            return $port;
+        });
+    note('Used ports: ' . dumper(\%ports));
+}
+
 sub cache_minion_worker {
     process(
         sub {
@@ -197,17 +222,17 @@ sub wait_for_worker {
 }
 
 sub create_webapi {
-    my ($mojoport, $schema_hook) = @_;
-    die 'No port specified '       unless $mojoport;
+    my ($port, $schema_hook) = @_;
+    $port //= service_port 'webui';
     die 'No schema hook specified' unless $schema_hook;
-    note("Starting WebUI service. Port: $mojoport");
+    note("Starting WebUI service. Port: $port");
 
     my $h = start sub {
         $0 = 'openqa-webapi';
         $schema_hook->();
 
         local $ENV{MOJO_MODE} = 'test';
-        my $daemon = Mojo::Server::Daemon->new(listen => ["http://127.0.0.1:$mojoport"], silent => 1);
+        my $daemon = Mojo::Server::Daemon->new(listen => ["http://127.0.0.1:$port"], silent => 1);
         $daemon->build_app('OpenQA::WebAPI');
         $daemon->run;
         Devel::Cover::report() if Devel::Cover->can('report');
@@ -218,7 +243,7 @@ sub create_webapi {
         my $t      = time;
         my $socket = IO::Socket::INET->new(
             PeerHost => '127.0.0.1',
-            PeerPort => $mojoport,
+            PeerPort => $port,
             Proto    => 'tcp',
         );
         last    if $socket;
@@ -229,6 +254,7 @@ sub create_webapi {
 
 sub create_websocket_server {
     my ($port, $bogus, $nowait, $with_embedded_scheduler) = @_;
+    $port //= service_port 'websocket';
 
     note("Starting WebSocket service. Port: $port");
     note("Bogus: $bogus | No wait: $nowait");
@@ -294,6 +320,7 @@ sub create_websocket_server {
 
 sub create_scheduler {
     my ($port, $no_stale_job_detection) = @_;
+    $port //= service_port 'scheduler';
     note("Starting Scheduler service. Port: $port");
     OpenQA::Scheduler::Client->singleton->port($port);
     start sub {
@@ -309,11 +336,11 @@ sub create_scheduler {
 }
 
 sub create_live_view_handler {
-    my ($mojoport) = @_;
+    my ($port) = @_;
+    $port //= service_port 'livehandler';
     start sub {
-        my $livehandlerport = $mojoport + 2;
         $0 = 'openqa-livehandler';
-        my $daemon = Mojo::Server::Daemon->new(listen => ["http://127.0.0.1:$livehandlerport"], silent => 1);
+        my $daemon = Mojo::Server::Daemon->new(listen => ["http://127.0.0.1:$port"], silent => 1);
         $daemon->build_app('OpenQA::LiveHandler');
         $daemon->run;
         Devel::Cover::report() if Devel::Cover->can('report');