glensc / nagios-plugin-check_raid

Nagios/Icinga/Sensu plugin to check current server's RAID status ⛺
144 stars 84 forks source link

hpacucli plugin fix case with missing controllers #207

Closed Ashark closed 3 years ago

Ashark commented 3 years ago

Currently (check_raid version 4.0.10) in hpacucli plugin, checking of missing controllers is done incorrectly.

In my server if I install hpacucli at any server without a controller (non-hp server is also ok), I get that plugin erroring in output:

UNKNOWN: hpacucli:[[ERROR, The controller must be identified by slot, chassisname, wwn,, Battery: missing]]:

After my change it will be:

WARNING: hpacucli:[No Controllers were found on this machine]:

The text for no controllers was already there, but it did not applied, because scan_targets function returned something (instead of returning null) and scan_luns was next invoked, which will not happen in correct case.

Sorry, I could not run tests successfully, because perl complained it misses aliased.pm even when I used PERL_USE_UNSAFE_INC=1.

I will write the output of hpacucli here for the commands runned by plugin:

The first command

root@server# hpacucli controller all show status

Error: No controllers detected.

root@server#

The second command

root@server# hpacucli controller * logicaldrive all show

Error: The controller must be identified by slot, chassisname, wwn,
       serialnumber, or IPF information.

root@server#

Version of hpacucli

root@server# hpacucli version

   ACU CLI Version: 9.20.9.0
   SoulAPI Version: 6.1.11.0

root@server#

Debug output (after fixing):

# perl ./check_raid.pl -d
check_raid 4.0.10
Visit <https://github.com/glensc/nagios-plugin-check_raid#reporting-bugs> how to report bugs
Please include output of **ALL** commands in bugreport

DEBUG EXEC: /sbin/dmsetup status --noflush at ./check_raid.pl line 503.
DEBUG EXEC: /proc/mdstat at ./check_raid.pl line 503.
DEBUG EXEC: /usr/sbin/hpacucli controller all show status at ./check_raid.pl line 503.
DEBUG EXEC: /proc/mdstat at ./check_raid.pl line 503.
WARNING: hpacucli:[No Controllers were found on this machine]; mdstat:[md127(167.05 GiB raid1):UU]

Debug output (before fixing):

# perl ./check_raid.pl -d
check_raid 4.0.10
Visit <https://github.com/glensc/nagios-plugin-check_raid#reporting-bugs> how to report bugs
Please include output of **ALL** commands in bugreport

DEBUG EXEC: /sbin/dmsetup status --noflush at ./check_raid.pl line 503.
DEBUG EXEC: /proc/mdstat at ./check_raid.pl line 503.
DEBUG EXEC: /usr/sbin/hpacucli controller all show status at ./check_raid.pl line 503.
Use of uninitialized value $target in hash element at ./check_raid.pl line 3012, <$fh> line 2.
Use of uninitialized value in substitution iterator at ./check_raid.pl line 484.
DEBUG EXEC: /usr/sbin/hpacucli controller  logicaldrive all show at ./check_raid.pl line 503.
Use of uninitialized value in quotemeta at ./check_raid.pl line 3078, <$fh> line 3.
Use of uninitialized value in string ne at ./check_raid.pl line 3149.
Use of uninitialized value $name in concatenation (.) or string at ./check_raid.pl line 3176.
DEBUG EXEC: /proc/mdstat at ./check_raid.pl line 503.
UNKNOWN: hpacucli:[[ERROR, The controller must be identified by slot, chassisname, wwn,]]; mdstat:[md127(167.05 GiB raid1):UU]
glensc commented 3 years ago

can you add the command outputs to unit tests please?

Ashark commented 3 years ago

I am not at the pc right now. Do I need to run tests successfully? Or it is sufficient to just add the output to tests?

glensc commented 3 years ago

@Ashark what an odd question!

you need to add them to tests and tests need to pass! what would be the point of adding to tests and let tests fail?

Ashark commented 3 years ago

I needed to install perl-aliased from AUR. I have Added the data. All tests are running ok.

But I am not sure which status the plugin should mark it. Currently it is warning. Probably, it should be ok, because user may just install hpacucli utility without having controllers.

glensc commented 3 years ago

I think a warning is okay.

Is this ready for merge from your side now?

Ashark commented 3 years ago

Yes, you can merge it.