Bugfix

[mbe-financial-com]

Hi,

I spent a few hours hacking on this check, resulting in the four commits here -- hope they're useful.

• "fix option parsing: more concise, check logic": you'll probably want this one, it fixes #78. The GetOptions() was unnecessarily verbose, and there was a logic error in the option checking, leading to the "Use of uninitialized value $strOption" warning.
• "fix a few logic/undef-var errors": perhaps these checks even worked accidentally but the parentheses were definitely not how they should be: defined($s && $s eq "foo") instead of defined($s) and $s eq "foo". Even where they were, in this function I changed && to the unambiguously low-precedence logical and. Guess you'll want this, too.
• "reduce boilerplate": Subrefs to the rescue! This is a pretty big change, eliminating the whole huge if-elsif construct that checks $strOption. Now it errors out if the option is unknown, and does so in just over a quarter of the lines. I didn't bother with the Hungarian notation so I totally understand if you don't want this, or want to re-style it.
• "eliminate repetitive child_add() calls": TBH this is completely untested in real life as we don't use that feature, but at least it gives the same results as before. It introduces two little helper functions to eliminate a whole bunch of function calls occurring in almost every get_* function. May be useful, maybe not.

I thought of adding support for Monitoring::Plugin as it would probably cut the LOC count in half and eliminate all the tedious status var juggling, but that would be too much for now ;)

[mbe-financial-com]

Sorry for the stupid title, I had to reload the page and it reset to my nondescript branch name ...

[willemdh]

Thanks @mbe-financial-com ! Looking good, we will try to test it asap. Could take a few weeks.

[willemdh]

@mbe-financial-com Sorry, but I can't merge this. We get the following error when running your version:

/usr/local/nagios/libexec/check_netapp_ontap9a.pl -H <netapphost> -u nagios -p <password> -o aggregate_health -c "failed, offline"
Unknown warnings category 'experimental::smartmatch' at /usr/local/nagios/libexec/check_netapp_ontap9a.pl line 28.
BEGIN failed--compilation aborted at /usr/local/nagios/libexec/check_netapp_ontap9a.pl line 28.

[mbe-financial-com]

@willemdh But that line is from your version, introduced in 255e9b9!? I didn't change anything about that. Is that a very old Perl you're testing on? It should fail with the original HEAD, too. But in any case, I'd say just drop the smartmatch. It's only used in three instances to do something that's more efficiently done with a hash anyway, and it has been deprecated in recent Perl versions because it wasn't a good feature to begin with. I'll make another commit.

[willemdh]

@mbe-financial-com Sorry it took me so long to test this. But it seems even with your latest commit, it still fails...

[willemdh]

@mbe-financial-com Do you still want to try resolve this, if so please resolve conflicts. Tx

[willemdh]

@mbe-financial-com Another pr merged, see https://github.com/OutsideIT/check_netapp_ontap/pull/89

[mbe-financial-com]

Did you test the version with the conflict marker?! Because that's the only way I could imagine how to get a compilation error on line 28. My version completely eliminates the whole smartmatch magic so the no if ... hackery isn't needed any more. I rebased my PR on the latest master so it includes the fixes from #87.

[willemdh]

@mbe-financial-com I copied your version to my pr server and launched some checks, which failed with the error in the screenshot. As your pr failed 3 times yet, the moment I find some spare time I will first test and merge https://github.com/OutsideIT/check_netapp_ontap/pull/90 I hope u understand.

[willemdh]

@mbe-financial-com Hello, Just merged @Elias481 's PR. Can you rebase pls? Thanks.

[willemdh]

@mbe-financial-com Merged https://github.com/OutsideIT/check_netapp_ontap/pull/91 rebase required if you still watn to see this merged.