thomas-krenn / check_ipmi_sensor_v3

Monitoring plugin to check IPMI sensors
https://www.thomas-krenn.com/en/wiki/IPMI_Sensor_Monitoring_Plugin
GNU General Public License v3.0
54 stars 21 forks source link

very unintuitive the cli misses the hostname argument #14

Closed sandrotosi closed 6 years ago

sandrotosi commented 7 years ago

Hello, when running the check without any argument, it's very unclear that you gotta pass the hostname (at least):

$ ./check_ipmi_sensor
Use of uninitialized value $ipmi_host in concatenation (.) or string at ./check_ipmi_sensor3 line 504.
Usage: host [-aCdlriTwv] [-c class] [-N ndots] [-t type] [-W time]
            [-R number] [-m flag] hostname [server]
       -a is equivalent to -v -t ANY
       -c specifies query class for non-IN data
       -C compares SOA records on authoritative nameservers
       -d is equivalent to -v
       -l lists all hosts in a domain, using AXFR
       -i IP6.INT reverse lookups
       -N changes the number of dots allowed before root lookup is done
       -r disables recursive processing
       -R specifies number of retries for UDP packets
       -s a SERVFAIL response should stop query
       -t specifies the query type
       -T enables TCP/IP mode
       -v enables verbose output
       -w specifies to wait forever for a reply
       -W specifies how long to wait for a reply
       -4 use IPv4 query transport only
       -6 use IPv6 query transport only
       -m set memory debugging flag (trace|record|usage)
IPMI Status: sensor not supported

$ ./check_ipmi_sensor -V
check_ipmi_sensor version 3.12
Copyright (C) 2009-2016 Thomas-Krenn.AG
Current updates at https://github.com/thomas-krenn/check_ipmi_sensor_v3.git

that error comes from host (likely called to resolve the hostname eventually passed via -H) but there's no cli args there.

I'd say the situation should be correctly identified and either default to localhost if -H is missing or print the help message and exit

thanks, Sandro

gschoenberger commented 6 years ago

I did a quick test, if the host is ommited localhost is assumed and sudo is added if you are not already root: ./check_ipmi_sensor IPMI Status: Critical [Intrusion = Critical] | 'System Temp'=23.00 'CPU1 Vcore'=0.95 'CPU2 Vcore'=0.95 'CPU1 DIMM'=1.58 'CPU2 DIMM'=1.58 '+1.5V'=1.50 '+3.3V'=3.26 '+3.3VSB'=3.22 '+5V'=5.06 '+12V'=12.14 'VBAT'=3.19 'Fan5'=3645.00 'Fan6'=3645.00

Lines 549 - 554:

    if(!(defined $ipmi_host) || ($ipmi_host eq 'localhost')){
        if(!defined($no_sudo)){
            # Only add sudo if not already root
            @basecmd = ($> != 0 ? 'sudo' : (), $IPMICOMMAND);
        }
    }

Can you check if your plugin contains those lines, are you using the latest version from git?

sandrotosi commented 6 years ago

it seems like it's working now, thanks and i'm closing this issue