Merge ~arif-ali/charm-nagios:nrpe_v2_fix_config_changed into charm-nagios:master

[Arif Ali]

Fix config-changed hook for nrpe_packet_version change

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

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

[Arif Ali (arif-ali)]

Below are the results from the lint and unit tests. The functional tests don't work, and hang after active/idle state of the model. Tried to fix that put the same issue with xenial. The xenial functional tests wouldn't work as the ch default channels didn't have xenial.

More work is required to get the functional tests to work. I may have a further dig, but if this can be reviewed without that, then that would help me

❱❱❱ tox -e lint | pastebinit
All done! ✨ 🍰 ✨
15 files would be left unchanged.
https://paste.ubuntu.com/p/JbhT49hKFz/

❱❱❱ tox -e unit | pastebinit
/home/arif/gitRepos/openstack/charm-nagios/.tox/unit/lib/python3.11/site-packages/coverage/inorout.py:503: CoverageWarning: Module lib was never imported. (module-not-imported)
  self.warn(f"Module {pkg} was never imported.", slug="module-not-imported")
/home/arif/gitRepos/openstack/charm-nagios/.tox/unit/lib/python3.11/site-packages/coverage/inorout.py:503: CoverageWarning: Module reactive was never imported. (module-not-imported)
  self.warn(f"Module {pkg} was never imported.", slug="module-not-imported")
/home/arif/gitRepos/openstack/charm-nagios/.tox/unit/lib/python3.11/site-packages/coverage/inorout.py:503: CoverageWarning: Module src was never imported. (module-not-imported)
  self.warn(f"Module {pkg} was never imported.", slug="module-not-imported")
https://paste.ubuntu.com/p/jDxK2GCDVn/

[Ashley James (dashmage)]

Changes LGTM! Thanks for sharing the test results as well. If the func tests aren't failing due to your changes then it's alright.

review: Approve

[Andrea Ieri (aieri)]

a couple of comments in-line

review: Needs Fixing

[Arif Ali (arif-ali)]

Can I get another review please, thanks

[Andrea Ieri (aieri)]

review: Approve

[Andrea Ieri (aieri)]

The fact that functional tests don't work is mildly terrifying, but it's on us to get them sorted out before we can promote to stable. We can push to edge for now.
I have rerun lint and unit tests and they pass.

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

Change successfully merged at revision a2aa5e8935b885fd5d415bf5b791744ad0d0c3bd

diff --git a/hooks/common.py b/hooks/common.py
index ee65735..9d23d68 100644
--- a/hooks/common.py
+++ b/hooks/common.py
@@ -6,6 +6,7 @@ import re
 import shutil
 import socket
 import subprocess
+import sys
 import tempfile
 import time
 
@@ -149,7 +150,7 @@ def refresh_hostgroups():  # noqa:C901
         except (ValueError, KeyError):
             pass
 
-    for hgroup_name, members in hgroups.iteritems():
+    for hgroup_name, members in hgroups.items():
         if os.path.exists(get_nagios_hostgroup_config_path(hgroup_name)):
             # Skip updating files unrelated to the hook at hand unless they were
             # deliberately removed with the intent of them being rewritten.
@@ -472,8 +473,13 @@ def apply_host_policy(target_id, owner_unit, owner_relation):
 def _replace_in_config(find_me, replacement):
     with open(INPROGRESS_CFG) as cf:
         with tempfile.NamedTemporaryFile(dir=INPROGRESS_DIR, delete=False) as new_cf:
+            py_version = sys.version_info[0]
             for line in cf:
-                new_cf.write(line.replace(find_me, replacement))
+                line_to_write = line.replace(find_me, replacement)
+                if py_version == 2:
+                    new_cf.write(line_to_write)
+                else:
+                    new_cf.write(line_to_write.encode("UTF-8"))
             new_cf.flush()
             os.chmod(new_cf.name, 0o644)
             os.unlink(INPROGRESS_CFG)
@@ -483,8 +489,13 @@ def _replace_in_config(find_me, replacement):
 def _commit_in_config(find_me, replacement):
     with open(MAIN_NAGIOS_CFG) as cf:
         with tempfile.NamedTemporaryFile(dir=MAIN_NAGIOS_DIR, delete=False) as new_cf:
+            py_version = sys.version_info[0]
             for line in cf:
-                new_cf.write(line.replace(find_me, replacement))
+                line_to_write = line.replace(find_me, replacement)
+                if py_version == 2:
+                    new_cf.write(line_to_write)
+                else:
+                    new_cf.write(line_to_write.encode("UTF-8"))
             new_cf.flush()
             os.chmod(new_cf.name, 0o644)
             os.unlink(MAIN_NAGIOS_CFG)
diff --git a/hooks/monitors_relation_changed.py b/hooks/monitors_relation_changed.py
index 0bdb0cc..48b4e99 100755
--- a/hooks/monitors_relation_changed.py
+++ b/hooks/monitors_relation_changed.py
@@ -124,13 +124,13 @@ def main(argv, full_rewrite=False):  # noqa: C901
     # Hack to work around http://pad.lv/1025478
     targets_with_addresses = set()
 
-    for relid, units in all_relations.iteritems():
+    for relid, units in all_relations.items():
         for unit, relation_settings in units.items():
             if TARGET_ID_KEY in relation_settings:
                 targets_with_addresses.add(relation_settings.get(TARGET_ID_KEY))
     new_all_relations = {}
 
-    for relid, units in all_relations.iteritems():
+    for relid, units in all_relations.items():
         for unit, relation_settings in units.items():
             if relation_settings.get(TARGET_ID_KEY) in targets_with_addresses:
                 if relid not in new_all_relations:
@@ -196,7 +196,7 @@ def main(argv, full_rewrite=False):  # noqa: C901
         status_set("active", "ready")
 
     new_file_set = set()
-    for units in all_relations.itervalues():
+    for units in all_relations.values():
         apply_relation_config(units, all_hosts, new_file_set)
 
     cleanup_leftover_hosts(all_relations)
@@ -253,7 +253,7 @@ def compute_host_prefixes(model_ids):
 
 
 def apply_relation_config(units, all_hosts, new_file_set):  # noqa: C901
-    for relation_settings in units.itervalues():
+    for relation_settings in units.values():
         target_id = relation_settings[TARGET_ID_KEY]
         host_config_path = get_nagios_host_config_path(target_id)
         if os.path.exists(host_config_path) and host_config_path not in new_file_set:
@@ -305,8 +305,8 @@ def apply_relation_config(units, all_hosts, new_file_set):  # noqa: C901
             host.set_attribute("parents", parent_host)
         host.save()
 
-        for mon_family, mons in monitors["monitors"]["remote"].iteritems():
-            for mon_name, mon in mons.iteritems():
+        for mon_family, mons in monitors["monitors"]["remote"].items():
+            for mon_name, mon in mons.items():
                 service_name = "%s-%s" % (target_id, mon_name)
                 service = get_pynag_service(target_id, service_name)
                 try:
diff --git a/hooks/upgrade_charm.py b/hooks/upgrade_charm.py
index 3ea6453..f837d2e 100755
--- a/hooks/upgrade_charm.py
+++ b/hooks/upgrade_charm.py
@@ -37,6 +37,8 @@ from common import (
     update_notification_options,
 )
 
+config = hookenv.config()
+
 # Gather facts
 legacy_relations = hookenv.config("legacy")
 extra_config = hookenv.config("extraconfig")
@@ -698,6 +700,12 @@ def configure_livestatus_xinetd():
     hookenv.log("Livestatus xinetd configured.", "INFO")
 
 
+def update_nrpe():
+    from monitors_relation_changed import main  # noqa: E402
+
+    main([__file__], full_rewrite=True)
+
+
 warn_legacy_relations()
 write_extra_config()
 # enable_traps_config and enable_pagerduty_config modify forced_contactgroup_members
@@ -719,6 +727,8 @@ update_contacts()
 update_password("nagiosro", ro_password)
 configure_livestatus_xinetd()
 application_dashboard_relation_changed()
+if config.changed("nrpe_packet_version"):
+    update_nrpe()
 
 if password:
     update_password(nagiosadmin, password)
diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py
index f21d419..2393df5 100644
--- a/tests/functional/conftest.py
+++ b/tests/functional/conftest.py
@@ -218,8 +218,16 @@ def series(request):
 async def relatives(model, series):
     nrpe = "nrpe"
     nrpe_name = "nrpe-{}".format(series)
+    nrpe_channel = "latest/stable"
+    if series == "xenial":
+        nrpe_channel = "latest/beta"
     nrpe_app = await model.deploy(
-        "ch:" + nrpe, application_name=nrpe_name, series=series, config={}, num_units=0
+        "ch:" + nrpe,
+        application_name=nrpe_name,
+        channel=nrpe_channel,
+        series=series,
+        config={},
+        num_units=0,
     )
 
     mysql = "percona-cluster"