nagios-charmers team mailing list archive

[James Hebden <james.hebden@xxxxxxxxxxxxx>]

Review: Needs Fixing

Just one comment on the name of the service, but other than that, the technical aspects of the change make sense and LGTM.

Diff comments:

> diff --git a/hooks/templates/localhost_nagios2.cfg.tmpl b/hooks/templates/localhost_nagios2.cfg.tmpl
> index 3cd69e1..b5a6153 100644
> --- a/hooks/templates/localhost_nagios2.cfg.tmpl
> +++ b/hooks/templates/localhost_nagios2.cfg.tmpl
> @@ -20,6 +20,14 @@ define host{
>          icon_image              base/ubuntu.png
>          }
>  
> +
> +
> +# 'check_disks_custom' command definition
> +define command{
> +        command_name    check_disks_custom
> +        command_line    /usr/lib/nagios/plugins/check_disk -w '$ARG1$' -c '$ARG2$' -e -X squashfs
> +        }

It always unreasonably bothers me that the braces in these checks and commands sit flush with the name and that we write them so they don't sit flush against the newline when being closed. Yes, I feel better for having said it. No, this is not a voting comment.

> +
>  # Define a service to check the disk space of the root partition
>  # on the local machine.  Warning if < 20% free, critical if
>  # < 10% free space on partition.
> @@ -31,7 +39,7 @@ define service{
>  {%- if is_container %}
>          check_command                   check_disk!20%!10%!/
>  {%- else %}
> -        check_command                   check_all_disks!20%!10%
> +        check_command                   check_disks_custom!20%!10%

Perhaps make it clear in the check name that it excluses squashfs
Something like 'check_all_disks_no_squashfs'

>  {%- endif %}
>          }
>  


-- 
https://code.launchpad.net/~xavpaice/nagios-charm/+git/nagios-charm/+merge/366740
Your team Nagios Charm developers is subscribed to branch nagios-charm:master.