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

[Michael Tremer <michael.tremer@ipfire.org>]

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