[Nagiosplug-devel] Re: Issues on check_disk

[Karl DeBisschop]

Voon, Ton writes: 

> The code for clearing thresholds is already there! Use -1% or -1: 
> 
> $ ./check_disk -v -v -v -w 10% -c 5% -p /tmp -w 10000 -c 5000 -p /var
> DISK OK [846 MB (85%) free on /var] [1886 MB (93%) free on /tmp]
> 846 of 992 MB (85%) free on /dev/dsk/c0t0d0s3 (type ufs mounted on /var)
> warn:10000 crit:5000 warn%:10% crit%:5%
> 1886 of 2034 MB (93%) free on swap (type tmpfs mounted on /tmp) warn:-1
> crit:-1 warn%:10% crit%:5%

I didn't have a chance to check, but I'm not surprised. 

> $ ./check_disk -v -v -v -w 10% -c 5% -p /tmp -w 10000 -c 5000 -w -1% -c -1%
> -p /var
> DISK OK [846 MB (85%) free on /var] [1887 MB (93%) free on /tmp]
> 846 of 992 MB (85%) free on /dev/dsk/c0t0d0s3 (type ufs mounted on /var)
> warn:10000 crit:5000 warn%:-1% crit%:-1%
> 1887 of 2035 MB (93%) free on swap (type tmpfs mounted on /tmp) warn:-1
> crit:-1 warn%:10% crit%:5% 
> 
> Looks like any values are accepted, but checked at the end of all parameter
> parsing. It looks a nightmare to read though. 

I don't find it a nightmare to read. Not pretty, but a nightmare? Can you 
put you finger on what you find disconcerting? 

> Do you think it should be something else (-w C -c C?)

I knew the checking happened late, and thought about 'undef' -- that would 
be pretty clear to a wide audience. But 'undef%' seemed odd as would 'C%'. I 
don't mind accepting a short list of strings like 'undef' and 'null' however 
(but I fail to see hoe 'C' is intuitive). 

Also, -C as a option to clear all previous defaults is fine. It makes much 
more sense to me in the context of this framework. On its own, as I have 
expressed before, it was sort of ad hoc to me. 

I think we also need to make a clear statement on what a threshold becomes 
if it is not specified foir a drive -- is it the last one used, or is it a 
'default' thta would be specified before and paths are specified? 

> Also, this currently does not work: 
> 
> check_disk -w 10% -c 5% / /tmp /var 
> 
> You need to specify as: 
> 
> check_disk -w 10% -c 5% -p / -p /tmp -p /var 
> 
> I think it makes sense to do it the top way, but check_disk looks like it is
> expecting: 
> 
> check_disk warn crit path 
> 
> I seem to have broken this with my latest changes. Instead of fixing that,
> can I propose removing warn and crit and assume all additional parameters to
> check_disk are considered as paths?

'check_disk warn crit path' is the oldest form of usage, but was originally 
the only valid invocation. I would prefer to keep that as well, since I 
think it can be accepted without too much trouble, it does not violate 
POSIX, and my policy has been that reverse compatibility should be preserved 
if reasonably possible. Again, if there is a groundswell of disagreement, I 
will defer. But I do feel rather strenuously that old invocations should be 
supported and would be decidely less happy if we choose not to go that way. 
Release schedule has no core priciples attached, this does. 

 --
Karl