From patchwork Thu Jun 20 16:06:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "plaisthos (Code Review)" X-Patchwork-Id: 3745 Return-Path: Delivered-To: patchwork@openvpn.net Received: by 2002:a05:7000:a501:b0:57d:b2cb:6cf with SMTP id hh1csp907331mab; Thu, 20 Jun 2024 09:06:50 -0700 (PDT) X-Forwarded-Encrypted: i=2; AJvYcCVp9S2gRi0Cncc7OfqMp07qtXSvSwmHpgDdeHTjTzv0gdk9buQ42Slq8YsW7UF2gYa3qS5z3JBY/Z9/M3klT+0UwQQl2QM= X-Google-Smtp-Source: AGHT+IGpZ1g87nMAPneym9Pt4E/ziTF3PqTmSC+4DcOyplT1J8SW1P1sBAxFVMYoYD0Y+N8THR+X X-Received: by 2002:a05:6a20:244c:b0:1b8:622a:cf74 with SMTP id adf61e73a8af0-1bcbb762357mr5811743637.6.1718899610498; Thu, 20 Jun 2024 09:06:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1718899610; cv=none; d=google.com; s=arc-20160816; b=fMqAfBj/KSiI9tn5dA68YMhGL6I0NJQWq6Ufyb+QyfsIbIV67xL2ZFCBa0DvY+pG6S yvCq0+V1g2+S40BGE6UN3o6nJgcirK6MUgJgQs/xmoCao4aklkX6QD5BLzWfXIn4cF3U t/ahXyth+bklUmTjLoDntX/tDM/G78tLmhA+6gZ3jGxTkZWxpp9//SYALXYGK7FxoLk8 C0PB3n6nPjnIQtEOzmbsVmkBJ1qbx8c40LiZWp3+FMHqkRC+e1OES4Dpjq5AT24dLtz1 wD2IvcqI6zDAslDs/KiCEsQIH7Ba/kh/SlAVT3MGhICVqB9sf1rzID/XjHHVlJ19SEEO x70w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:cc:reply-to:list-subscribe:list-help:list-post :list-archive:list-unsubscribe:list-id:precedence:subject:user-agent :mime-version:message-id:references:auto-submitted:to:date:from :dkim-signature:dkim-signature:dkim-signature; bh=MKf3LQK2L90hmZ83bpXndJd5OIyOk4+zaTs3zC+9z0w=; fh=U7wEyxtwz2o5+UdevFSA47vNeG9knhWH0KV//QhD5a0=; b=eLxhSZjSrf8melPOeBY4tU/E0fG8sN2KFVMRMpDblAb4YSKovogrf8Vb6TkC+Unvc5 SxrN31Sj9HShroz3r5C4idxUON6YPCLfQ8xo1nt1ZZKDK0/iOCxugT7blMRPEZChtKrq 7SfM6Jdc8XVG4+7MMT39x4PtXXamt0X7kpVmHeoUvTkSzHtRirck77tpw5NU/B0Y0caF Bf2gnHtXFCUQKu3taM2oC/udS8yzdlOe3j4FJBt2oC9J4lv390Mtqq01io5U7JcUj+iq 39Ih5omTmwaWeO65eUzM7Tng35QJx5+3xNZdI1g2U2DU/b0SNdq0D/k5pMANGNNUewO9 /tEA==; dara=google.com ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=aEUg4tRz; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=CxdS38U8; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=gdBao8x2; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net Received: from lists.sourceforge.net (lists.sourceforge.net. [216.105.38.7]) by mx.google.com with ESMTPS id d2e1a72fcca58-705ccb3ce0bsi14452767b3a.180.2024.06.20.09.06.50 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 20 Jun 2024 09:06:50 -0700 (PDT) Received-SPF: pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) client-ip=216.105.38.7; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@sourceforge.net header.s=x header.b=aEUg4tRz; dkim=neutral (body hash did not verify) header.i=@sf.net header.s=x header.b=CxdS38U8; dkim=neutral (body hash did not verify) header.i=@openvpn.net header.s=google header.b=gdBao8x2; spf=pass (google.com: domain of openvpn-devel-bounces@lists.sourceforge.net designates 216.105.38.7 as permitted sender) smtp.mailfrom=openvpn-devel-bounces@lists.sourceforge.net; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=openvpn.net Received: from [127.0.0.1] (helo=sfs-ml-4.v29.lw.sourceforge.com) by sfs-ml-4.v29.lw.sourceforge.com with esmtp (Exim 4.95) (envelope-from ) id 1sKKIu-0003cp-FE; Thu, 20 Jun 2024 16:06:28 +0000 Received: from [172.30.29.66] (helo=mx.sourceforge.net) by sfs-ml-4.v29.lw.sourceforge.com with esmtps (TLS1.2) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1sKKIr-0003cg-Ff for openvpn-devel@lists.sourceforge.net; Thu, 20 Jun 2024 16:06:25 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Content-Type:Content-Transfer-Encoding:MIME-Version :Message-ID:Reply-To:References:Subject:List-Unsubscribe:List-Id:Cc:To:Date: From:Sender:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help: List-Subscribe:List-Post:List-Owner:List-Archive; bh=QJCCPNhyUwE9j1maVF2xAh0n8ZrRjmBAH5+q3rDDAZs=; b=aEUg4tRz1Rsiuy2TZQLBYrER1R 51/fuqwdJ6XzVtnSqbrH122oJbaOp1OxLSjUEyREqK0C96pzvtyITqCBGaSaxS4lqkQUEKrH4rJcP v5OWKOiIh4LVtno7NgN+XnWcKsICVL5tw69uhDk3UD15Jpf/D5OTrhAuMhSnoRiAdaE8=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Content-Type:Content-Transfer-Encoding:MIME-Version:Message-ID:Reply-To: References:Subject:List-Unsubscribe:List-Id:Cc:To:Date:From:Sender:Content-ID :Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To: Resent-Cc:Resent-Message-ID:In-Reply-To:List-Help:List-Subscribe:List-Post: List-Owner:List-Archive; bh=QJCCPNhyUwE9j1maVF2xAh0n8ZrRjmBAH5+q3rDDAZs=; b=C xdS38U85eZWQHHEPr99PL3MzHH1IdgQUWaxLGfKMavj010X6H/54QcfXCAO4MYJY0RMGuCdbRtBBI ErNHFPwLl/NvH5qTJCz+HYn5ZYWF3Kp6Z39Nibg+kgvZ/CeSIReCoSEi4xdA3Mcg4yZQk8ez7RtKT 7F8XnHuuaCLMfZNg=; Received: from mail-wr1-f44.google.com ([209.85.221.44]) by sfi-mx-2.v28.lw.sourceforge.com with esmtps (TLS1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.95) id 1sKKIq-0006sO-Tp for openvpn-devel@lists.sourceforge.net; Thu, 20 Jun 2024 16:06:25 +0000 Received: by mail-wr1-f44.google.com with SMTP id ffacd0b85a97d-364ff42999eso731438f8f.3 for ; Thu, 20 Jun 2024 09:06:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=openvpn.net; s=google; t=1718899577; x=1719504377; darn=lists.sourceforge.net; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from:from:to:cc :subject:date:message-id:reply-to; bh=QJCCPNhyUwE9j1maVF2xAh0n8ZrRjmBAH5+q3rDDAZs=; b=gdBao8x2grfOuKPYXrgmDm5e58kymA2oT9Z1fKRBgdKSh9DSOXj4ISvYKJBHu1dVi1 FGjoSbofiAO7zgxKDFZ85TX29gDEOgPYc1XOLQxoLjsy2BzGgsU0O/roNTzUnsFQ1pzv Wbn11gwkJCqMUufhv2G3wVT1HEo5n0KyQcLBAj3/TaM0NhPODyI20GWBCTWlxuF2gZVS rLnq4WnaqI1u4XEFFIa9GHkdTpSN0VPVYiyAzKtT9cztvZRe4nMbcPj9bB1KODJZQMhb jFqUQoKx9Vf+KHxf9C0S1fdYVkNqpSdIcgB++sB1w7CrjjQVsLDq3zRBoFMnLO86Lowc wLiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718899577; x=1719504377; h=user-agent:content-disposition:content-transfer-encoding :mime-version:message-id:reply-to:references:subject :list-unsubscribe:list-id:auto-submitted:cc:to:date:from :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=QJCCPNhyUwE9j1maVF2xAh0n8ZrRjmBAH5+q3rDDAZs=; b=j6pZn0ps7f+24xOjDjWf7EDirw/FVIp2yWLhw58BQKhDsxxO3ShDaX/xNS2Nvas3HV 0489k+IdyldN8wLyrKbHG9Cg8M2EVdrl730JQe2m22blVLDmLBwo8ZeZ3dORl+v2wWGf Cnmx+x+m9zPkvVWgiJ5x2q1BpejVoDXBMV0HB8U4wpzF6JRBqPCEbYhzQxixokl/rcYY UX5iDO0XXn5zvP2UDTYiG42OkURHPTaAGBc3k8jFrGlwx+qFM1eG1VsBPt8bBm+CB/qq IEePxKFKeaq3EqP+m1YmXPJ2Br0ZlQZxU8NgBlhcgK/VVEI1gQwLXBFpM2uwcqdVoKEG LjiQ== X-Gm-Message-State: AOJu0YwQIpbjjcD3fo/pPRm9wKd5I5YinMi5d8MPKtl9U6LMPOqaWBy1 Yg9ywCOCFFt9dIhRDRWTu4HT8e1UnYVV8VZWPEe5fYIWn84sd1F6uIW5zXIQQq0CUh8YGzqDkDU g X-Received: by 2002:a5d:5270:0:b0:362:c7b3:7649 with SMTP id ffacd0b85a97d-363193c5166mr5706731f8f.56.1718899577353; Thu, 20 Jun 2024 09:06:17 -0700 (PDT) Received: from gerrit.openvpn.in (ec2-18-159-0-78.eu-central-1.compute.amazonaws.com. [18.159.0.78]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4247d1f6e60sm30370035e9.46.2024.06.20.09.06.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 20 Jun 2024 09:06:17 -0700 (PDT) From: "mattock (Code Review)" X-Google-Original-From: "mattock (Code Review)" X-Gerrit-PatchSet: 1 Date: Thu, 20 Jun 2024 16:06:16 +0000 To: plaisthos , flichtenheld Auto-Submitted: auto-generated X-Gerrit-MessageType: newchange X-Gerrit-Change-Id: I2cce8ad4e0d262e1404ab1eb6ff673d8590b6b3a X-Gerrit-Change-Number: 669 X-Gerrit-Project: openvpn X-Gerrit-ChangeURL: X-Gerrit-Commit: c175015020459912dd5c8ef046df1f8e27feaf6c References: Message-ID: <4711ccb96b777449406ef34ee3d2e6a27aeaf9e8-HTML@gerrit.openvpn.net> MIME-Version: 1.0 User-Agent: Gerrit/3.8.2 X-Spam-Score: -0.2 (/) X-Spam-Report: Spam detection software, running on the system "util-spamd-1.v13.lw.sourceforge.com", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld, I'd like you to do a code review. Please visit Content analysis details: (-0.2 points, 6.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: openvpn.net] 0.0 RCVD_IN_VALIDITY_RPBL_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.221.44 listed in bl.score.senderscore.com] 0.0 RCVD_IN_VALIDITY_SAFE_BLOCKED RBL: ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. [209.85.221.44 listed in sa-accredit.habeas.com] -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.44 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record 0.0 WEIRD_PORT URI: Uses non-standard port number for HTTP 0.0 HTML_MESSAGE BODY: HTML included in message -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.0 T_SCC_BODY_TEXT_LINE No description available. 0.0 T_KAM_HTML_FONT_INVALID Test for Invalidly Named or Formatted Colors in HTML X-Headers-End: 1sKKIq-0006sO-Tp Subject: [Openvpn-devel] [M] Change in openvpn[master]: t_server_null: multiple improvements and fixes X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: samuli@openvpn.net, arne-openvpn@rfc2549.org, openvpn-devel@lists.sourceforge.net, frank@lichtenheld.com Cc: openvpn-devel Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox X-GMAIL-THRID: =?utf-8?q?1802396878071769550?= X-GMAIL-MSGID: =?utf-8?q?1802396878071769550?= X-getmail-filter-classifier: gerrit message type newchange 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/+/669?usp=email to review the following change. Change subject: t_server_null: multiple improvements and fixes ...................................................................... t_server_null: multiple improvements and fixes - exit after a timeout if unable to kill servers - use sudo or equivalent only for server stop/start - use /bin/sh directly instead of through /usr/bin/env - simplify sudo call in the sample rc file - remove misleading and outdated documentation Change-Id: I2cce8ad4e0d262e1404ab1eb6ff673d8590b6b3a Signed-off-by: Samuli Seppänen --- M doc/t_server_null.rst M tests/t_server_null.rc-sample M tests/t_server_null.sh M tests/t_server_null_client.sh M tests/t_server_null_server.sh M tests/t_server_null_stress.sh 6 files changed, 41 insertions(+), 32 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/69/669/1 diff --git a/doc/t_server_null.rst b/doc/t_server_null.rst index e3a098a..7ce7a3f 100644 --- a/doc/t_server_null.rst +++ b/doc/t_server_null.rst @@ -73,13 +73,6 @@ * Waits until servers have launched. Then launch all clients, wait for them to exit and then check test results by parsing the client log files. Each client kills itself after some delay using an "--up" script. -Note that "make check" moves on once *t_server_null_client.sh* has exited. At -that point *t_server_null_server.sh* is still running, because it exists only -after waiting a few seconds for more client connections to potentially appear. -This is a feature and not a bug, but means that launching "make check" runs too -quickly might cause test failures or unexpected behavior such as leftover -OpenVPN server processes. - Configuration ------------- diff --git a/tests/t_server_null.rc-sample b/tests/t_server_null.rc-sample index 28c3773..98d7869 100644 --- a/tests/t_server_null.rc-sample +++ b/tests/t_server_null.rc-sample @@ -1,6 +1,5 @@ # Uncomment to run tests with sudo -#SUDO_EXEC=`which sudo` -#RUN_SUDO="${SUDO_EXEC} -E" +#RUN_SUDO="sudo -E" TEST_RUN_LIST="1 2 3 10 11" diff --git a/tests/t_server_null.sh b/tests/t_server_null.sh index 0e53ba4..7627edf 100755 --- a/tests/t_server_null.sh +++ b/tests/t_server_null.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env sh +#!/bin/sh # TSERVER_NULL_SKIP_RC="${TSERVER_NULL_SKIP_RC:-77}" @@ -57,12 +57,7 @@ srcdir="${srcdir:-.}" -if [ -z "${RUN_SUDO}" ]; then - "${srcdir}/t_server_null_server.sh" & -else - $RUN_SUDO "${srcdir}/t_server_null_server.sh" & -fi - +"${srcdir}/t_server_null_server.sh" & "${srcdir}/t_server_null_client.sh" retval=$? diff --git a/tests/t_server_null_client.sh b/tests/t_server_null_client.sh index 8890007..3b7dc6d 100755 --- a/tests/t_server_null_client.sh +++ b/tests/t_server_null_client.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env sh +#!/bin/sh launch_client() { test_name=$1 diff --git a/tests/t_server_null_server.sh b/tests/t_server_null_server.sh index 9bc0c88..037baf7 100755 --- a/tests/t_server_null_server.sh +++ b/tests/t_server_null_server.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env sh +#!/bin/sh launch_server() { server_name=$1 @@ -8,16 +8,23 @@ status="${server_name}.status" pid="${server_name}.pid" - # Ensure that old status, log and pid files are gone - rm -f "${status}" "${log}" "${pid}" - - "${server_exec}" \ - $server_conf \ - --status "${status}" 1 \ - --log "${log}" \ - --writepid "${pid}" \ - --explicit-exit-notify 3 - + if [ -z "${RUN_SUDO}" ]; then + rm -f "${status}" "${log}" "${pid}" + "${server_exec}" \ + $server_conf \ + --status "${status}" 1 \ + --log "${log}" \ + --writepid "${pid}" \ + --explicit-exit-notify 3 + else + $RUN_SUDO rm -f "${status}" "${log}" "${pid}" + $RUN_SUDO "${server_exec}" \ + $server_conf \ + --status "${status}" 1 \ + --log "${log}" \ + --writepid "${pid}" \ + --explicit-exit-notify 3 + fi } # Load base/default configuration @@ -64,15 +71,30 @@ echo "All clients have disconnected from all servers" +# Make sure that the server processes are truly dead before exiting. If a +# server process does not exit in 15 seconds assume it never will, move on and +# hope for the best. +echo "Waiting for servers to exit" for PID_FILE in $server_pid_files do SERVER_PID=$(cat "${PID_FILE}") - $KILL_EXEC "${SERVER_PID}" - # Make sure that the server processes are truly dead before exiting - while : + if [ -z "${RUN_SUDO}" ]; then + $KILL_EXEC "${SERVER_PID}" + else + $RUN_SUDO $KILL_EXEC "${SERVER_PID}" + fi + + count=0 + maxcount=75 + while [ $count -le $maxcount ] do ps -p "${SERVER_PID}" > /dev/null || break + count=$(( count + 1)) sleep 0.2 done + + if [ $count -ge $maxcount ]; then + echo "WARNING: could not kill server with pid ${SERVER_PID}!" + fi done diff --git a/tests/t_server_null_stress.sh b/tests/t_server_null_stress.sh index 1281397..0bb9452 100755 --- a/tests/t_server_null_stress.sh +++ b/tests/t_server_null_stress.sh @@ -1,4 +1,4 @@ -#!/usr/bin/env sh +#!/bin/sh # # Run this stress test as root to avoid sudo authorization from timing out.