[Nagiosplug-devel] [Nagiosplug-help] check_ntp

[Andreas Ericsson]

Thomas Guyot-Sionnest wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> On 10/11/07 10:15 AM, Andreas Ericsson wrote:
>> Thomas Guyot-Sionnest wrote:
> 
>>> 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,

Sounds good, although I still prefer check_ntp_peers, since that makes
it obvious from a directory listing which plugin to use depending on
ones needs.

> so we would end up
> with check_ntp (marked ad deprecated), check_ntpd and check_time_ntp...
> Or something like that.
> 

Sounds bad.

check_ntp really should stay with its current semantics. check_ntp{d,_peers}
can do the peer checking.

Deprecating check_ntp to give it a new name is nearly as user-unfriendly as
changing its semantics, but not quite.

Think of the plugins as an API that people actively use in programs. If
memcpy() all of a sudden was renamed to memcopy(), whole battallions of
programs would just stop working over-night.

Changing its semantics but maintaining the arguments would be just as
broken as if malloc() all of a sudden started returning a pointer to
the end of the newly allocated buffer.

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

The problem is that you only hear from people who want the behaviour to
change. Those who are happy with the way check_ntp works today are much
more likely to not say anything. Until it breaks, that is.

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

Hence "check_ntp" and "check_ntp_peers". A directory listing will tell
them which plugin to use based on their needs.

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

In my experience, it's much more common that random servers get out of
sync with the stratum >1 server than that server getting out of sync
with its peers. I've always expected check_ntp to see if the server it's
being run on is in sync with the ntp server.

-- 
Andreas Ericsson                   andreas.ericsson at op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231