[1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running
Message ID | 20250317195123.2092-1-adolf.belka@ipfire.org |
---|---|
State | New |
Headers |
Return-Path: <development+bounces-40-patchwork=ipfire.org@lists.ipfire.org> Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mail01.haj.ipfire.org", Issuer "R10" (verified OK)) by web04.haj.ipfire.org (Postfix) with ESMTPS id 4ZGlwS24Xjz3xFJ for <patchwork@web04.haj.ipfire.org>; Mon, 17 Mar 2025 19:51:36 +0000 (UTC) Received: from mail02.haj.ipfire.org (mail02.haj.ipfire.org [172.28.1.201]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) client-signature ECDSA (secp384r1)) (Client CN "mail02.haj.ipfire.org", Issuer "E5" (verified OK)) by mail01.ipfire.org (Postfix) with ESMTPS id 4ZGlwP1l5Zz5c6 for <patchwork@ipfire.org>; Mon, 17 Mar 2025 19:51:33 +0000 (UTC) Received: from mail02.haj.ipfire.org (localhost [127.0.0.1]) by mail02.haj.ipfire.org (Postfix) with ESMTP id 4ZGlwP10RLz34PV for <patchwork@ipfire.org>; Mon, 17 Mar 2025 19:51:33 +0000 (UTC) X-Original-To: development@lists.ipfire.org Received: from mail01.ipfire.org (mail01.haj.ipfire.org [172.28.1.202]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mail01.haj.ipfire.org", Issuer "R10" (verified OK)) by mail02.haj.ipfire.org (Postfix) with ESMTPS id 4ZGlwL0bVxz2xc2 for <development@lists.ipfire.org>; Mon, 17 Mar 2025 19:51:30 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id 4ZGlwK0ns9z4nJ; Mon, 17 Mar 2025 19:51:29 +0000 (UTC) DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003ed25519; t=1742241089; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=1Zx11q7i2Jcb3y/Xz+uCuMGZ0k26q3eZ6tUM4Tj4kMY=; b=Ur/iiY69ZLfLyLGMoibBIe3/Eowv+3RhBW6bY2O1mJu3sm1p6YVkeqcqksvs4g5gULKG1w 7Fr+UsUCxYXFXkAA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ipfire.org; s=202003rsa; t=1742241089; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding; bh=1Zx11q7i2Jcb3y/Xz+uCuMGZ0k26q3eZ6tUM4Tj4kMY=; b=MrXaOid5ybkndBWDFFrK31Dp1Hy3u4bIY2oPFWkBlf9wITofiwlDZRe3M4Vo5/hgUlI5aX Awx7azjKM53t+ujM7HNaT5b38jRiuLrP8hBNggSv6fgYNQSLP/c7WpftpN9XUHygyn69k1 pyIyVPueLc+Ztr8eqgHkPx/V4/fRn2XckaXae8f2DtjOu/sgkF1EMw46gY12SCZv68fH+n RsA/v0LZ6XHdRWgjb8e6IC53HJcBiYeUlIpUTCiu97PF0Ti4fBfTrvNJrgR23LxKcHbcOF ugL0Hufs3tTJ5v3G9WDNfBPJHUrTJXQvujwZVO3c55kQ0uANZ0EbDguVxweg3Q== From: Adolf Belka <adolf.belka@ipfire.org> To: development@lists.ipfire.org Cc: Adolf Belka <adolf.belka@ipfire.org> Subject: [PATCH 1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running Date: Mon, 17 Mar 2025 20:51:21 +0100 Message-ID: <20250317195123.2092-1-adolf.belka@ipfire.org> Precedence: list List-Id: <development.lists.ipfire.org> List-Subscribe: <https://lists.ipfire.org/>, <mailto:development+subscribe@lists.ipfire.org?subject=subscribe> List-Unsubscribe: <https://lists.ipfire.org/>, <mailto:development+unsubscribe@lists.ipfire.org?subject=unsubscribe> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development+help@lists.ipfire.org?subject=help> Sender: <development@lists.ipfire.org> Mail-Followup-To: <development@lists.ipfire.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit |
Series |
[1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running
|
|
Commit Message
Adolf Belka
March 17, 2025, 7:51 p.m. UTC
- Added code to check if openvpn.pid exists and only then uncomment the include openvpn plugin line in collectd.conf - Tested out on my vm testbed and the include openvpn plugin line in collectd.conf is only uncommented if the openvpn server is run ning and has a pid. Fixes: Bug13832 Tested-by: Adolf Belka <adolf.belka@ipfire.org> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> --- src/initscripts/system/collectd | 7 +++++++ 1 file changed, 7 insertions(+)
Comments
Hello Adolf, In this patch you are checking whether OpenVPN is actually running. Should we not check whether it is enabled? That would avoid any kind of races that we might see when collectd is being started before OpenVPN. -Michael > On 17 Mar 2025, at 19:51, Adolf Belka <adolf.belka@ipfire.org> wrote: > > - Added code to check if openvpn.pid exists and only then uncomment the include openvpn > plugin line in collectd.conf > - Tested out on my vm testbed and the include openvpn plugin line in collectd.conf is > only uncommented if the openvpn server is run ning and has a pid. > > Fixes: Bug13832 > Tested-by: Adolf Belka <adolf.belka@ipfire.org> > Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> > --- > src/initscripts/system/collectd | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/src/initscripts/system/collectd b/src/initscripts/system/collectd > index 263511fc7..f86b64e9d 100644 > --- a/src/initscripts/system/collectd > +++ b/src/initscripts/system/collectd > @@ -143,6 +143,13 @@ case "$1" in > sed -i -e "s|^#LoadPlugin swap|LoadPlugin swap|g" /etc/collectd.conf > fi > > + # Enable openvpn plugin if openvpn.pid found > + if [ ! -e /var/run/openvpn.pid ]; then > + sed -i -e 's|^include "/etc/collectd.vpn"$|#include "/etc/collectd.vpn"|g' /etc/collectd.conf > + else > + sed -i -e 's|^#include "/etc/collectd.vpn"$|include "/etc/collectd.vpn"|g' /etc/collectd.conf > + fi > + > if [ $(date +%Y) -gt 2011 ]; then > boot_mesg "Starting Collection daemon..." > /usr/sbin/collectd -C /etc/collectd.conf > -- > 2.49.0 > >
Hi Michael, On 19/03/2025 16:56, Michael Tremer wrote: > Hello Adolf, > > In this patch you are checking whether OpenVPN is actually running. > > Should we not check whether it is enabled? That would avoid any kind of races that we might see when collectd is being started before OpenVPN. I did look at the enabled bit first but for a new OpenVPN installation, you could end up with OpenVPN enabled but the Server not started for the first time. In that case the /var/run/ovpnserver.log file would still be empty and so you would get the error messages that flagged up the problem in the first place. The ovpnserver.log gets its contents defined when the server is started and not when it is enabled but not started. Once the OpenVPN server has been started once then the ovpnserver.log file contents satisfy collectd and those contents stay there even if the OpenVPN server is stopped and disabled. So the other option could be to fill the required contents into ovpnserver.log, when red, or blue or orange are enabled and before the server is started. If that change was made then the include openvpn plugin line in collectd.conf could be uncommented based on at least one of the enable options being selected. Should I look at doing the approach of filling the ovpnserver.log with its contents when the openvpn server is enabled on at least one of the interfaces specified. What I don't know is when the openvpn server is running and the openvpn plugin is loaded, are there any issues or error messages when the user does not yet have any client connections defined. I didn't find any when I tested it out on a vm system with the root/host certificate set created, the openvpn server enabled on red, the openvpn server started and the openvpn plugin uncommented in the collectd.conf file but with no client configurations created yet so I don't believe there should be any problem. Regards, Adolf. > > -Michael > >> On 17 Mar 2025, at 19:51, Adolf Belka <adolf.belka@ipfire.org> wrote: >> >> - Added code to check if openvpn.pid exists and only then uncomment the include openvpn >> plugin line in collectd.conf >> - Tested out on my vm testbed and the include openvpn plugin line in collectd.conf is >> only uncommented if the openvpn server is run ning and has a pid. >> >> Fixes: Bug13832 >> Tested-by: Adolf Belka <adolf.belka@ipfire.org> >> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> >> --- >> src/initscripts/system/collectd | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/src/initscripts/system/collectd b/src/initscripts/system/collectd >> index 263511fc7..f86b64e9d 100644 >> --- a/src/initscripts/system/collectd >> +++ b/src/initscripts/system/collectd >> @@ -143,6 +143,13 @@ case "$1" in >> sed -i -e "s|^#LoadPlugin swap|LoadPlugin swap|g" /etc/collectd.conf >> fi >> >> + # Enable openvpn plugin if openvpn.pid found >> + if [ ! -e /var/run/openvpn.pid ]; then >> + sed -i -e 's|^include "/etc/collectd.vpn"$|#include "/etc/collectd.vpn"|g' /etc/collectd.conf >> + else >> + sed -i -e 's|^#include "/etc/collectd.vpn"$|include "/etc/collectd.vpn"|g' /etc/collectd.conf >> + fi >> + >> if [ $(date +%Y) -gt 2011 ]; then >> boot_mesg "Starting Collection daemon..." >> /usr/sbin/collectd -C /etc/collectd.conf >> -- >> 2.49.0 >> >> > >
Hello, > On 19 Mar 2025, at 17:09, Adolf Belka <adolf.belka@ipfire.org> wrote: > > Hi Michael, > > On 19/03/2025 16:56, Michael Tremer wrote: >> Hello Adolf, >> In this patch you are checking whether OpenVPN is actually running. >> Should we not check whether it is enabled? That would avoid any kind of races that we might see when collectd is being started before OpenVPN. > > I did look at the enabled bit first but for a new OpenVPN installation, you could end up with OpenVPN enabled but the Server not started for the first time. > > In that case the /var/run/ovpnserver.log file would still be empty and so you would get the error messages that flagged up the problem in the first place. Yes, I assume so, but I thought this was a better and more robust option than the other way around? > The ovpnserver.log gets its contents defined when the server is started and not when it is enabled but not started. > > Once the OpenVPN server has been started once then the ovpnserver.log file contents satisfy collectd and those contents stay there even if the OpenVPN server is stopped and disabled. > > So the other option could be to fill the required contents into ovpnserver.log, when red, or blue or orange are enabled and before the server is started. > If that change was made then the include openvpn plugin line in collectd.conf could be uncommented based on at least one of the enable options being selected. > > Should I look at doing the approach of filling the ovpnserver.log with its contents when the openvpn server is enabled on at least one of the interfaces specified. Just write some dummy data into it? Possible, but again seems to be a workaround rather than a fix. Should we not try to upstream the change that at least nothing is being logged whenever the file exists but is empty? > What I don't know is when the openvpn server is running and the openvpn plugin is loaded, are there any issues or error messages when the user does not yet have any client connections defined. > I didn't find any when I tested it out on a vm system with the root/host certificate set created, the openvpn server enabled on red, the openvpn server started and the openvpn plugin uncommented in the collectd.conf file but with no client configurations created yet so I don't believe there should be any problem. > > Regards, > Adolf. > >> -Michael >>> On 17 Mar 2025, at 19:51, Adolf Belka <adolf.belka@ipfire.org> wrote: >>> >>> - Added code to check if openvpn.pid exists and only then uncomment the include openvpn >>> plugin line in collectd.conf >>> - Tested out on my vm testbed and the include openvpn plugin line in collectd.conf is >>> only uncommented if the openvpn server is run ning and has a pid. >>> >>> Fixes: Bug13832 >>> Tested-by: Adolf Belka <adolf.belka@ipfire.org> >>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> >>> --- >>> src/initscripts/system/collectd | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/src/initscripts/system/collectd b/src/initscripts/system/collectd >>> index 263511fc7..f86b64e9d 100644 >>> --- a/src/initscripts/system/collectd >>> +++ b/src/initscripts/system/collectd >>> @@ -143,6 +143,13 @@ case "$1" in >>> sed -i -e "s|^#LoadPlugin swap|LoadPlugin swap|g" /etc/collectd.conf >>> fi >>> >>> + # Enable openvpn plugin if openvpn.pid found >>> + if [ ! -e /var/run/openvpn.pid ]; then >>> + sed -i -e 's|^include "/etc/collectd.vpn"$|#include "/etc/collectd.vpn"|g' /etc/collectd.conf >>> + else >>> + sed -i -e 's|^#include "/etc/collectd.vpn"$|include "/etc/collectd.vpn"|g' /etc/collectd.conf >>> + fi >>> + >>> if [ $(date +%Y) -gt 2011 ]; then >>> boot_mesg "Starting Collection daemon..." >>> /usr/sbin/collectd -C /etc/collectd.conf >>> -- >>> 2.49.0 >>> >>> >
Hello, > On 19 Mar 2025, at 17:09, Adolf Belka <adolf.belka@ipfire.org> wrote: > > Hi Michael, > > On 19/03/2025 16:56, Michael Tremer wrote: >> Hello Adolf, >> In this patch you are checking whether OpenVPN is actually running. >> Should we not check whether it is enabled? That would avoid any kind of races that we might see when collectd is being started before OpenVPN. > > I did look at the enabled bit first but for a new OpenVPN installation, you could end up with OpenVPN enabled but the Server not started for the first time. > > In that case the /var/run/ovpnserver.log file would still be empty and so you would get the error messages that flagged up the problem in the first place. Yes, I assume so, but I thought this was a better and more robust option than the other way around? > The ovpnserver.log gets its contents defined when the server is started and not when it is enabled but not started. > > Once the OpenVPN server has been started once then the ovpnserver.log file contents satisfy collectd and those contents stay there even if the OpenVPN server is stopped and disabled. > > So the other option could be to fill the required contents into ovpnserver.log, when red, or blue or orange are enabled and before the server is started. > If that change was made then the include openvpn plugin line in collectd.conf could be uncommented based on at least one of the enable options being selected. > > Should I look at doing the approach of filling the ovpnserver.log with its contents when the openvpn server is enabled on at least one of the interfaces specified. Just write some dummy data into it? Possible, but again seems to be a workaround rather than a fix. Should we not try to upstream the change that at least nothing is being logged whenever the file exists but is empty? > What I don't know is when the openvpn server is running and the openvpn plugin is loaded, are there any issues or error messages when the user does not yet have any client connections defined. > I didn't find any when I tested it out on a vm system with the root/host certificate set created, the openvpn server enabled on red, the openvpn server started and the openvpn plugin uncommented in the collectd.conf file but with no client configurations created yet so I don't believe there should be any problem. > > Regards, > Adolf. > >> -Michael >>> On 17 Mar 2025, at 19:51, Adolf Belka <adolf.belka@ipfire.org> wrote: >>> >>> - Added code to check if openvpn.pid exists and only then uncomment the include openvpn >>> plugin line in collectd.conf >>> - Tested out on my vm testbed and the include openvpn plugin line in collectd.conf is >>> only uncommented if the openvpn server is run ning and has a pid. >>> >>> Fixes: Bug13832 >>> Tested-by: Adolf Belka <adolf.belka@ipfire.org> >>> Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> >>> --- >>> src/initscripts/system/collectd | 7 +++++++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git a/src/initscripts/system/collectd b/src/initscripts/system/collectd >>> index 263511fc7..f86b64e9d 100644 >>> --- a/src/initscripts/system/collectd >>> +++ b/src/initscripts/system/collectd >>> @@ -143,6 +143,13 @@ case "$1" in >>> sed -i -e "s|^#LoadPlugin swap|LoadPlugin swap|g" /etc/collectd.conf >>> fi >>> >>> + # Enable openvpn plugin if openvpn.pid found >>> + if [ ! -e /var/run/openvpn.pid ]; then >>> + sed -i -e 's|^include "/etc/collectd.vpn"$|#include "/etc/collectd.vpn"|g' /etc/collectd.conf >>> + else >>> + sed -i -e 's|^#include "/etc/collectd.vpn"$|include "/etc/collectd.vpn"|g' /etc/collectd.conf >>> + fi >>> + >>> if [ $(date +%Y) -gt 2011 ]; then >>> boot_mesg "Starting Collection daemon..." >>> /usr/sbin/collectd -C /etc/collectd.conf >>> -- >>> 2.49.0 >>> >>> >
diff --git a/src/initscripts/system/collectd b/src/initscripts/system/collectd index 263511fc7..f86b64e9d 100644 --- a/src/initscripts/system/collectd +++ b/src/initscripts/system/collectd @@ -143,6 +143,13 @@ case "$1" in sed -i -e "s|^#LoadPlugin swap|LoadPlugin swap|g" /etc/collectd.conf fi + # Enable openvpn plugin if openvpn.pid found + if [ ! -e /var/run/openvpn.pid ]; then + sed -i -e 's|^include "/etc/collectd.vpn"$|#include "/etc/collectd.vpn"|g' /etc/collectd.conf + else + sed -i -e 's|^#include "/etc/collectd.vpn"$|include "/etc/collectd.vpn"|g' /etc/collectd.conf + fi + if [ $(date +%Y) -gt 2011 ]; then boot_mesg "Starting Collection daemon..." /usr/sbin/collectd -C /etc/collectd.conf