nagios-charmers team mailing list archive

[Chris Sanders <chris.sanders@xxxxxxxxxxxxx>]

Review: Needs Fixing

One comment in line

Diff comments:

> diff --git a/hooks/upgrade-charm b/hooks/upgrade-charm
> index a80743b..905c459 100755
> --- a/hooks/upgrade-charm
> +++ b/hooks/upgrade-charm
> @@ -160,8 +160,38 @@ def enable_pagerduty_config():
>          if os.path.isfile(pagerduty_cron):
>              os.remove(pagerduty_cron)
>  
> -    # Update contacts for admin
> +    # Multiple Email Contacts
>      contactgroup_members = hookenv.config("contactgroup-members")
> +    contacts = []
> +    admin_email = list(
> +        filter(None, set(hookenv.config('admin_email').split(',')))
> +    )
> +    if len(admin_email) == 0:
> +        hookenv.log("admin_email is unset, this isn't valid config")
> +        hookenv.status_set("blocked", "admin_email is not configured")
> +        exit(0)

Should we really be doing an exit(0) here? That's fairly atypical of a charm, although I see this isn't a reactive charm I don't see that happening anywhere else in this charm either.

Shouldn't this just be a return to allow the rest of the configuration to happen?

> +    if len(admin_email) == 1:
> +        hookenv.log("Setting one admin email address '%s'" % admin_email[0])
> +        contacts = [{
> +            'contact_name': 'root',
> +            'alias': 'Root',
> +            'email': admin_email[0]
> +        }]
> +    elif len(admin_email) > 1:
> +        hookenv.log("Setting %d admin email addresses" % len(admin_email))
> +        contacts = [
> +            {
> +                'contact_name': email,
> +                'alias': email,
> +                'email': email
> +            }
> +            for email in admin_email
> +        ]
> +        contactgroup_members = ', '.join([
> +            c['contact_name'] for c in contacts
> +        ])
> +
> +    # Update contacts for admin
>      if enable_pagerduty:
>          # avoid duplicates
>          if "pagerduty" not in contactgroup_members:


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