Merge ~vultaire/charm-nagios:skip-dedupe-prefix-for-same-model into charm-nagios:master

[Paul Goins]

Fixed deduping to skip the prefix if not needed

Deduplication is only performed if there are 2 or more related units
with the same target hostname from different models. If there are
"duplicates" but they are from the same model (e.g. multiple nrpe units
associated with different principal apps on the same host), it is
probably preferable to have those "coalesce" into a single host, rather
than having distinct records with redundant checks.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

FAILED: Continuous integration, rev:bfe338c0d3570047847d00ef66ed8b2660ad1c7f
https://jenkins.canonical.com/bootstack/job/lp-charm-nagios-ci/24/
Executed test runs:
    FAILURE: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/91/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/462/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nagios-ci/24//rebuild

review: Needs Fixing (continuous-integration)

[Paul Goins (vultaire)]

Results of testing on serverstack, adding a second nrpe app to the same principal host app:

* Without the changes, I end up with no duplicate hosts, but I end up with the prefix.
* With the changes, the prefix disappears as well.
* The prefix does still show up if I have a duplicate on a different model, i.e. the intended case for adding the prefix still works.

[🤖 prod-jenkaas-bootstack (prod-jenkaas-bootstack)]

PASSED: Continuous integration, rev:bfe338c0d3570047847d00ef66ed8b2660ad1c7f
https://jenkins.canonical.com/bootstack/job/lp-charm-nagios-ci/28/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/bootstack/job/lp-charm-test-functest/97/
    None: https://jenkins.canonical.com/bootstack/job/lp-update-mp/468/

Click here to trigger a rebuild:
https://jenkins.canonical.com/bootstack/job/lp-charm-nagios-ci/28//rebuild

review: Approve (continuous-integration)

[Xav Paice (xavpaice)]

LGTM

review: Approve

[Garrett Neugent (thogarre)]

nice! LGTM as well.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 1425e19f775dfcdad94518e79848f5fa88d29e37

diff --git a/hooks/monitors_relation_changed.py b/hooks/monitors_relation_changed.py
index 2a201c6..4c86a63 100755
--- a/hooks/monitors_relation_changed.py
+++ b/hooks/monitors_relation_changed.py
@@ -160,9 +160,16 @@ def main(argv, full_rewrite=False):  # noqa: C901
     duplicate_hostnames = set()
     all_hosts = {}
     for target_id, relation_settings_list in hosts_to_settings.items():
+        related_model_ids = set(
+            [
+                relation_settings.get(MODEL_ID_KEY)
+                for relation_settings in relation_settings_list
+            ]
+        )
+        deduping_required = len(related_model_ids) > 1
         for relation_settings in relation_settings_list:
             model_id = relation_settings.get(MODEL_ID_KEY)
-            if model_id and len(relation_settings_list) > 1:
+            if model_id and deduping_required:
                 duplicate_hostnames.add(target_id)
                 unique_prefix = host_prefixes[model_id]
                 relation_settings[TARGET_ID_KEY] = "{}_{}".format(