[Nagiosplug-devel] [Nagiosplug-help] check_ntp

[Thomas Guyot-Sionnest]

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

On 11/11/07 09:58 AM, Andreas Ericsson wrote:
> sdepThomas Guyot-Sionnest wrote:
>>
>> check_ntp's description in --help is:
>>
>> "This plugin checks the selected ntp server"
>>
>> To me that clearly doesn't mean we're checking the local time against
>> the given ntp server.
>>
> 
> So amend the message then, perhaps? Changing 400 lines of code (introducing

Isn't that as bad? A plugin doesn't do what it is advertised to do, so
change the description and get out with it? What about people that
actually relied on this description? Shouldn't they get what they think
they're getting?

> at least one segfault crash in the process) because a message is wrong
> doesn't seem like a stellar plan, tbh.

I didn't reviewed my code yet; I also planed some more thoughtful tests
and even putting check_ntpd in a full production system before release.

If you found any bug I'll be glad to fix them.

>>
>> If we go that way, it will stay with its current semantics, but it'll
>> say that it's deprecated and will be removed much later (Maybe next
>> major release).
>>
> 
> Still really bad. There's no real reason for it! Simply renaming it is
> NOT a valid reason, although I see you've also robbed check_ntp of some
> of its functionality in the check_time_ntp incantation.

Because checking jitter just make no sense when you want to check if
your local host is sync. This can be confusing too; some people may
thing it's the jitter calculated during the check and not reported my
the ntp server, while others may assume since there's a jitter option
that the offset is for the peer as well.

There's no point in checking two different things in the same plugin and
this is why I split them.

>> I'm not sure if you follow me well there. By "deprecating" check_ntp I
>> just mean we'll yell it all over the place without touching it (apart
>> from normal bugfixes). However people will have the option to use two
>> new checks that do what they should do.
>>
> 
> I understand perfectly well what "deprecating" means, thanks. What I
> mean is that there's no real reason for it. None. Zip. Zero. So what
> you'll end up doing is forcing people who are using a perfectly fine
> plugin to pick another one (that does less btw, as check_time_ntp can't
> check for jitter) in order to retain exactly the same setup as they
> already have. Can't you see how unforgivably ridiculous this is?

I totally disagree there. First check_ntp IS broken. "This plugin checks
the selected ntp server". Ok, I run ntpd on all my server, so I add a
service check for all my servers that do "check_ntp -H $HOSTADDRESS$
- --some-thresholds-here", right? WRONG.

So I tell them if you want to monitor your ntp servers, use check_ntpd.
if you want to check the time diff between a server and a remote clock,
run check_time_ntp on them. If you do it that way you don't have to
check the jitter.

Second, they can keep their setup by using check_ntpd. the other option
is broken by design. Checking local offset and remote jitter make no sense.

I also think we could start a release notes document that would cover
more in-depth the changes in NEWS and current issues in BUGS.

>> IMHO for many people it's already broken and they just don't realize it.
>> When I do a NTP check I check the $HOSTADDRESS$, which mean I want to
>> know about the health of the NTP server, not the offset between Nagios
>> and the server. Do you know many people that run check_ntp over nrpe and
>> specify a time server?
>>
> 
> Yes. I also know many people that want to be notified if their nagios
> server's clock starts drifting, and I know that everyone will simply
> hate whoever makes them sit down to fix some breakage in their nagios
> installation simply because a plugin was renamed. They'll be equally

Not renamed. I talking about creating NEW ones.

> pissed if a plugin that formerly did something now does something
> entirely different (although that particular disagreement seems to
> have been settled, since you named it check_ntpd instead of replacing
> check_ntp).

Yes and I mentioned creating two checks and not touching check_ntp two
posts ago, so I don't understand why it's still going on in the thread.

>> The newer check specifically warn if the server isn't sync (I may
>> add an option to make that configurable - ok/warn/crit/unknown).
>>
> 
> Which server? The one check_ntp runs on, or the one it's checking?

I mean it checks if the remote server has a sync.peer (noted with a star
in "ntpq -p" output. This might not be working well yet, but as I said
it's still work in progress...

Thomas


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

iD8DBQFHNzWC6dZ+Kt5BchYRAoT2AKDzpSOlktsQacehEdeY6pmgCV/NpwCeICMW
/C1cXPWDiKEEVntrRGyOFZU=
=try5
-----END PGP SIGNATURE-----