From 4afad2a047a9883e52fd3b7af2947b7917ece2a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 19 Mar 2019 10:26:36 +0100 Subject: [PATCH 1/2] Correctly invoke stop.pl when start.pl fails MR !1141 broke the way stop.pl is invoked when start.pl fails: - start.pl changes the working directory to $testdir/$server before attempting to start $server, - commit 27ee629e6b583f60fea0ab78fb3ebd0d1d71d9d2 causes the $testdir variable in stop.pl to be determined using the $SYSTEMTESTTOP environment variable, which is set to ".." by all tests.sh scripts, - commit e227815af51c0656e22e5aebfe99e2399106b31c makes start.pl pass $test (the test's name) rather than $testdir (the path to the test's directory) to stop.pl when a given server fails to start. Thus, when a server is restarted from within a tests.sh script and such a restart fails, stop.pl attempts to look for the server directory in a nonexistent location ($testdir/$server/../$test, i.e. $testdir/$test, instead of $testdir/../$test). Fix the issue by changing the working directory before stop.pl is invoked in the scenario described above. --- bin/tests/system/start.pl | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/tests/system/start.pl b/bin/tests/system/start.pl index 30139c7ac3..869c1f0eb2 100755 --- a/bin/tests/system/start.pl +++ b/bin/tests/system/start.pl @@ -204,6 +204,7 @@ sub start_server { print "I:$test:Couldn't start server $command (pid=$child)\n"; print "I:$test:failed\n"; system "kill -9 $child" if ("$child" ne ""); + chdir "$testdir"; system "$PERL $topdir/stop.pl $test"; exit 1; } From c787a539d2a931ba9023677c1c269ed191455512 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 19 Mar 2019 10:26:36 +0100 Subject: [PATCH 2/2] Make stop.pl wait for lock file cleanup bin/tests/system/stop.pl only waits for the PID file to be cleaned up while named cleans up the lock file after the PID file. Thus, the aforementioned script may consider a named instance to be fully shut down when in fact it is not. Fix by also checking whether the lock file exists when determining a given instance's shutdown status. This change assumes that if a named instance uses a lock file, it is called "named.lock". Also rename clean_pid_file() to pid_file_exists(), so that it is called more appropriately (it does not clean up the PID file itself, it only returns the server's identifier if its PID file is not yet cleaned up). --- bin/tests/system/stop.pl | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/bin/tests/system/stop.pl b/bin/tests/system/stop.pl index 0a1358a275..1045ee374f 100644 --- a/bin/tests/system/stop.pl +++ b/bin/tests/system/stop.pl @@ -127,9 +127,19 @@ exit($errors); # Subroutines +# Return the full path to a given server's lock file. +sub server_lock_file { + my ( $server ) = @_; + + return $testdir . "/" . $server . "/named.lock" if ($server =~ /^ns/); + return if ($server =~ /^ans/); + + die "Unknown server type $server\n"; +} + # Return the full path to a given server's PID file. sub server_pid_file { - my($server) = @_; + my ( $server ) = @_; return $testdir . "/" . $server . "/named.pid" if ($server =~ /^ns/); return $testdir . "/" . $server . "/ans.pid" if ($server =~ /^ans/); @@ -228,7 +238,7 @@ sub stop_signal { return; } -sub clean_pid_file { +sub pid_file_exists { my ( $server ) = @_; my $pid_file = server_pid_file($server); @@ -250,6 +260,15 @@ sub clean_pid_file { return $server; } +sub lock_file_exists { + my ( $server ) = @_; + my $lock_file = server_lock_file($server); + + return unless defined($lock_file) && -f $lock_file; + + return $server; +} + sub wait_for_servers { my ( $timeout, @servers ) = @_; @@ -257,7 +276,7 @@ sub wait_for_servers { sleep 1 if (@servers > 0); @servers = grep { defined($_) } - map { clean_pid_file($_) } @servers; + map { pid_file_exists($_) || lock_file_exists($_) } @servers; $timeout--; }