[Nagiosplug-devel] [Nagiosplug-help] check_ntp

[Thomas Guyot-Sionnest]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/11/07 10:15 AM, Andreas Ericsson wrote:
> Thomas Guyot-Sionnest wrote:
>>
>> No, that would become a mess because getting offset between ourself and
>> the server and getting the peers offset it a totally different operation
>> in NTP.
> 
> So? Users think "check_ntp checks stuff concerning the ntp protocol".

I mean from a code point of view. Also the usage would be less than
optimal or more complex... I personally prefer two more simple plugins,
especially since code duplication will be minimal: mainly some ntp
definitions (that can be moved to /lib if we like), argument processing,
output and performance data). As I said the two operations are fairly
different.

>> Also it doesn't make much sense to get the former along with
>> jitter or stratum.
>>
> 
> True. --check-peers would obviously have to complain if stupid options
> were given to it.
> 
>> I started rewriting check_ntp to use the offset from the peers, and also
>> removed the useless loop for getting 4 times the control packets (since
>> they all contain the same info).
> 
> You mean you're doing something along the lines of what --check-peers
> would do?

I'm removing plenty of stuff and check everything with NTP control
packets. The cleanup isn't finished yet; the plug-in is functional (I
just fixed the parsing function) and will likely need some polishing for
various cases (which I'll do soon...). So far here's what I got:

 check_ntp.c |  462
+++++++++++++++---------------------------------------------
 1 file changed, 117 insertions(+), 345 deletions(-)

>> I believe the best option is to also
>> rewrite check_ntp the other way round to get only offset using normal
>> NTP packets and name it something like check_time_ntp (that will be very
>> easy).
>>
> 
> NO! Seriously. check_ntp is RELEASED already. Giving it completely
> different
> semantics while retaining the name is BROKEN. It will cause real problems
> for real people. Doing that just because you want to name your entirely
> new plugin check_ntp as well is just stupid.

Well, the options for check_ntp will remain the same, the only
difference it that it will return the peer's offset instead of the
offset between localhost and the ntp server. It will also send 8 time
less packets...

We could call it check_ntpd to avoid any confusion, so we would end up
with check_ntp (marked ad deprecated), check_ntpd and check_time_ntp...
Or something like that.

I'm open to discussion but I've seen multiple times people wanting the
peer ofsset in check_ntp, and I feel this is the right thing anyways.

> So unless you add something along the lines of "Oh, the caller thinks I'm
> the old check_ntp, since it gives me arguments only that other check_ntp
> accepts, I'd better run that program instead to help them", you really
> should name your rewritten effort "check_ntp_peers" or some such instead.

What might happen if we don't change the name is that people expecting
to check the offset between their server and the remote will actually
monitor the remote server. OTOH in the past (before working on
check_ntp) I assumed that the offset was the peer's offset and I might
not be alone thinking that. So either way some people won't do the
RightThing(tm) with check_ntp...

Thomas

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFHNdqE6dZ+Kt5BchYRAj2XAJwJXs6DL23aYViMf8/5EW7AM7fJmACeNAPe
ZJ75BmtF5szPwMA57fKhhBM=
=1O8Y
-----END PGP SIGNATURE-----