nagios-charmers team mailing list archive

[Stuart Bishop <stuart.bishop@xxxxxxxxxxxxx>]

Review: Approve

Looks good. One minor comment inline which can be ignored or adjusted before landing.

Diff comments:

> diff --git a/src/files/megacli/check_megacli.py b/src/files/megacli/check_megacli.py
> index ab3d99f..052927c 100755
> --- a/src/files/megacli/check_megacli.py
> +++ b/src/files/megacli/check_megacli.py
> @@ -8,6 +8,35 @@ from nagios_plugin3 import CriticalError, WarnError, try_check
>  INPUT_FILE = '/var/lib/nagios/megacli.out'
>  
>  
> +def handle_results(nlines, match, critical, errors, num_ldrive, num_pdrive, policy):
> +    if nlines == 0:
> +        raise WarnError('WARNING: controller not found')
> +    elif not match:
> +        raise WarnError('WARNING: error parsing megacli output')
> +    elif critical:
> +        if len(errors) > 0:
> +            msg = ', '.join([
> +                '{}({})'.format(cnt, vars()[cnt])
> +                for cnt in ('failed_ld', 'wrg_policy_ld') if vars().get(cnt, 0) > 0
> +            ])
> +            msg += '; '.join(errors)
> +        else:
> +            msg = 'failure caught but no output available'
> +        raise CriticalError('CRITICAL: {}'.format(msg))
> +    elif len(errors) > 0:
> +        raise WarnError('WARNING: {}'.format('; '.join(errors)))
> +
> +    else:
> +        if num_ldrive == 0:
> +            msg = 'OK: no disks configured for RAID'
> +        else:
> +            msg = 'OK: Optimal, ldrives[{}], pdrives[{}]'
> +            if policy:
> +                msg += ', policy[{}]'
> +            msg = msg.format(num_ldrive, num_pdrive, policy)

Hrm... this would be a lint error if the checker was smart enough. The format string has either 2 or 3 placeholders. I guess this is fine for Python. Maybe better though if the assignment on 74 was msg = 'OK: Optimal, ldrives[{}]. pdrives[{}'.format(num_ldrive, num_pdrive), and 76 msg += ', policy[{}]'.format(policy).

(or f-strings, but I don't think we have them available in this version of Python)

> +        print(msg)
> +
> +
>  def parse_output(policy=False):
>      noadapter_re = r'^Adapter \d+: No Virtual Drive Configured'
>      adapter_re = r'^Adapter (\d+) -- Virtual Drive Information:'


-- 
https://code.launchpad.net/~peter-sabaini/hw-health-charm/+git/hw-health-charm/+merge/378151
Your team Nagios Charm developers is subscribed to branch hw-health-charm:master.