nagios-charmers team mailing list archive

[Paul Goins <paul.goins@xxxxxxxxxxxxx>]

Review: Needs Fixing

I'm not sure the updated relations are correct.  Manual testing in a test model gives me errors when I try to wire things in the way done here.

Diff comments:

> diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
> index fafe7f0..d607a62 100644
> --- a/tests/functional/conftest.py
> +++ b/tests/functional/conftest.py
> @@ -237,8 +237,8 @@ async def deploy_app(relatives, model, series):
>              'enable_pagerduty': False
>          }
>      )
> -    await model.add_relation('{}:monitors'.format(app_name), 'mysql:monitors')
> -    await model.add_relation('{}:nagios'.format(app_name), 'mediawiki:juju-info')
> +    for relative in relatives.keys():
> +        await model.add_relation(app_name + ':nagios', relative + ':juju-info')

I'm not sure about this.

What I think would be more correct (and I have not tested this; it's based on what I see on other clouds though):
* Dropping nagios-<series>:monitors relationship with mysql:monitors: yes, agreed.  Instead, I would connect nagios-<series>:monitors with nrpe:monitors.
* Dropping nagios-<series>:nagios relationship with mediawiki:juju-info: I don't know.  I've not used this relationship; hard to judge.
* Adding relations from nagios-<series>:nagios to each relative's :juju-info (mediawiki, mysql, nrpe): When I try this in a test model, nagios has errors each time I try to add one of these relations.  I don't really know how this relation is supposed to work off hand, but the current implementation appears to be incorrect.  (Please push back if I'm wrong here.)

>      await model.add_relation('nrpe:monitors', '{}:monitors'.format(app_name))
>      await model.block_until(lambda: nagios_app.status == "active")
>      await model.block_until(lambda: all(


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