On Mon, February 12, 2024 14:05, Jeremy Laidman wrote:
On Tue, 13 Feb 2024 at 00:07, Brian Scott <bscott at bunyatech.com.au> wrote:
i think using a discouraged wrapper like this makes no sense.
However, we still need some way to deal with the cases where the default is still the old ntp.
The attached rewritten patch implements this.
Would you mind if I tried to generalise your patch to automatically detect which version to use and pick the correct options accordingly? A quick look at the code suggests changes to configure.server, a new script under build/ to detect which type of ntp is installed, and possibly changes to the various build/Makefile.* to set a per-OS default. Also I notice that the defaults when nothing is set in xymonserver.cfg are established in lib/environ.c but they would be quite messy to tinker with.
I don't think this should be done at compile/build time. The build server might not use the same NTP package, or even the same version of distro, or the same distro, or even the same OS.
The xymonserver.cfg config file has NTPDATE and NTPDATEOPTS values that can be modified by the sysadmin, to use ntpdig, chronyd, or whatever is installed onto the Xymon server at the time. If ntpdate is deprecated, perhaps these variables should now be adjusted to reflect a more commonly installed NTP client.
It probably warrants a re-think of how we're doing ntp checks overall, but I agree that for the moment this probably should be handled at the distribution packaging level via environment variables.
I'm not sure if the Xymon server does anything with the results of the ntpdate query. If it does, then we need to be mindful that the parser might not recognise the format of ntpdig output. In which case, a wrapper might actually be the best solution. Perhaps the Xymon source should provide a few different wrapper scripts called ntpdate-chrony, ntpdate-ntpsec, etc, and the sysadmin can simply adjust NTPDATE to use the appropriate wrapper script.
IIRC chrony *should* work correct, but I don't know that ntpdig has been tested here.
The general process has been hacking the cl to present the same data so the xymond_client parser doesn't have to care, but with all the changes with ntp client-side over the years this might be one to clean up for sure. Especially with it already being special-cased.
-jc