[PATCH 1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running

[PATCH 1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running

[Adolf Belka]

- 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

[PATCH 2/3] collectd.conf: Fixes bug13832 - include openvpn plugin line commented out by default

[Adolf Belka]

- The include openvpn plugin line in collectd.conf is changed to be commented out by
   default.
- When booting if the openvpn server is running then the collectd initscript will
   uncomment the include openvpn plugin line.

Fixes: bug13832
Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
---
 config/collectd/collectd.conf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/config/collectd/collectd.conf b/config/collectd/collectd.conf
index 020604e74..16f7dae09 100644
--- a/config/collectd/collectd.conf
+++ b/config/collectd/collectd.conf
@@ -83,5 +83,5 @@ include "/etc/collectd.precache"
 </Plugin>
 
 #include "/etc/collectd.thermal"
-include "/etc/collectd.vpn"
+#include "/etc/collectd.vpn"
 include "/etc/collectd.d/*"
-- 
2.49.0

[PATCH 3/3] ovpnmain.cgi: Fixes bug13832 - restart collectd initscript on openvpn server status change

[Adolf Belka]

- When the openvpn server status is changed from RUNNING to STOPPED or vice versa then
   the collectd initscript is restarted to ensure that the include openvpn plugin line
   in the collectd.conf file is correctly uncommented or not.

Fixes: bug13832
Tested-by: Adolf Belka <adolf.belka@ipfire.org>
Signed-off-by: Adolf Belka <adolf.belka@ipfire.org>
---
 html/cgi-bin/ovpnmain.cgi | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/html/cgi-bin/ovpnmain.cgi b/html/cgi-bin/ovpnmain.cgi
index 20f256f4b..f16ba0591 100644
--- a/html/cgi-bin/ovpnmain.cgi
+++ b/html/cgi-bin/ovpnmain.cgi
@@ -782,6 +782,10 @@ if ($cgiparams{'ACTION'} eq $Lang::tr{'start ovpn server'} ||
 	&General::system("/usr/local/bin/openvpnctrl", "-k");
 	&emptyserverlog();
     }
+
+    # restart collectd to uncomment include openvpn plugin command if openvpn pid exists
+    &General::system("/usr/local/bin/collectdctrl", "restart");
+
 #    #restart openvpn server
 #    if ($cgiparams{'ACTION'} eq $Lang::tr{'restart ovpn server'}){
 #workarund, till SIGHUP also works when running as nobody
-- 
2.49.0

Re: [PATCH 1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running

[Michael Tremer]

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
> 
> 

Re: [PATCH 1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running

[Adolf Belka]

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
>>
>>
> 
> 

Re: [PATCH 1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running

[Michael Tremer]

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
>>> 
>>> 
> 

Re: [PATCH 1/3] collectd: Fixes bug13832 - uncomments include openvpn plugin only if openvpn is running

[Michael Tremer]

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
>>> 
>>> 
>