Attention is currently required from: flichtenheld, plaisthos.

Hello plaisthos, flichtenheld,

I'd like you to do a code review.
Please visit

    http://gerrit.openvpn.net/c/openvpn/+/1024?usp=email

to review the following change.


Change subject: t_server_null: print error when server startup fails
......................................................................

t_server_null: print error when server startup fails

The --daemon option has to be at the end of the command-line.  Moreover,
if a pid-file is not found or is empty, launch a new server instance
without --log or --daemon so that the error message is printed properly.

Change-Id: I39dc6b08952a06dae7901e468f9487c8541d83c3
Signed-off-by: Samuli Seppänen <samuli.seppa...@gmail.com>
---
M tests/t_server_null_default.rc
M tests/t_server_null_server.sh
2 files changed, 22 insertions(+), 11 deletions(-)



  git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/24/1024/1

diff --git a/tests/t_server_null_default.rc b/tests/t_server_null_default.rc
index 5b74761..1f2fa2c 100755
--- a/tests/t_server_null_default.rc
+++ b/tests/t_server_null_default.rc
@@ -38,7 +38,7 @@
 MAX_CLIENTS="10"
 CLIENT_MATCH="Test-Client"
 SERVER_EXEC="${top_builddir}/src/openvpn/openvpn"
-SERVER_BASE_OPTS="--daemon --dev tun --topology subnet --max-clients 
$MAX_CLIENTS --persist-tun --verb 3 --duplicate-cn"
+SERVER_BASE_OPTS="--dev tun --topology subnet --max-clients $MAX_CLIENTS 
--persist-tun --verb 3 --duplicate-cn"
 SERVER_BIND_OPTS="--local 127.0.0.1"
 SERVER_CIPHER_OPTS=""
 SERVER_CERT_OPTS="--ca ${CA} --dh ${DH} --cert ${SERVER_CERT} --key 
${SERVER_KEY} --tls-auth ${TA} 0"
diff --git a/tests/t_server_null_server.sh b/tests/t_server_null_server.sh
index acf8479..d8fd1c4 100755
--- a/tests/t_server_null_server.sh
+++ b/tests/t_server_null_server.sh
@@ -11,20 +11,31 @@
     # Allow reading this file even umask values are strict
     touch "$log"

-    if [ -z "${RUN_SUDO}" ]; then
-        "${server_exec}" \
-         $server_conf \
-         --status "${status}" 1 \
-         --log "${log}" \
-         --writepid "${pid}" \
-         --explicit-exit-notify 3
-    else
+    # Try to launch the server daemonized
+    $RUN_SUDO "${server_exec}" \
+               $server_conf \
+               --status "${status}" 1 \
+               --log "${log}" \
+               --writepid "${pid}" \
+               --explicit-exit-notify 3 \
+               --daemon
+
+    sleep 1
+
+    # With --daemon OpenVPN's exit code is always 0, even when it failed to
+    # start. Therefore we check the pid-file and if it is missing or is empty
+    # we run OpenVPN without --daemon and --log to catch the underlying error.
+    # By backgrounding the process in the shell we ensure that we don't block
+    # the test run in the (unlikely) case that OpenVPN startup actually
+    # succeeds.
+    if ! [ -r "$pid" ] || [ -z "$pid" ]; then
+        echo "ERROR: failed to start server $server_name"
+
         $RUN_SUDO "${server_exec}" \
                    $server_conf \
                    --status "${status}" 1 \
-                   --log "${log}" \
                    --writepid "${pid}" \
-                   --explicit-exit-notify 3
+                   --explicit-exit-notify 3 &
     fi
 }


--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1024?usp=email
To unsubscribe, or for help writing mail filters, visit 
http://gerrit.openvpn.net/settings

Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I39dc6b08952a06dae7901e468f9487c8541d83c3
Gerrit-Change-Number: 1024
Gerrit-PatchSet: 1
Gerrit-Owner: mattock <sas...@proton.me>
Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com>
Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org>
Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net>
Gerrit-Attention: plaisthos <arne-open...@rfc2549.org>
Gerrit-Attention: flichtenheld <fr...@lichtenheld.com>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to