Do not keep stale targets in csv file

.gitignore

@@ -1,3 +1,4 @@
+.idea/
 venv/
 build/
 *.charm

README.md

@@ -1,5 +1,10 @@
 # COS Proxy charm
 
+[![Charmhub Badge](https://charmhub.io/cos-proxy/badge.svg)](https://charmhub.io/cos-proxy)
+[![Release](https://github.com/canonical/cos-proxy-operator/actions/workflows/release.yaml/badge.svg)](https://github.com/canonical/cos-proxy-operator/actions/workflows/release.yaml)
+[![Discourse Status](https://img.shields.io/discourse/status?server=https%3A%2F%2Fdiscourse.charmhub.io&style=flat&label=CharmHub%20Discourse)](https://discourse.charmhub.io)
+
+
 This Juju machine charm that provides a single integration point in the machine world
 with the Kubernetes-based [COS bundle](https://charmhub.io/cos-lite).
 

metadata.yaml

@@ -3,32 +3,25 @@
 name: cos-proxy
 
 description: |
-  This Juju machine charm that provides a single integration point in the machine world with Kubernetes-based Canonical Observability Stack
+  This Juju machine charm that provides a single integration point in the machine world with Kubernetes-based Canonical
+  Observability Stack
 
 summary: |
   Single integration point in the machine world with Kubernetes-based COS
 docs: https://discourse.charmhub.io/t/cos-proxy-operator-docs-index/5611
 
-tags:
-  - observability
-  - lma
-  - cos
-  - monitoring
-  - logging
-  - alerting
-  - grafana
-  - nrpe
-
 series:
   - focal
   - jammy
+
 provides:
   downstream-grafana-dashboard:
     interface: grafana_dashboard
   downstream-prometheus-scrape:
     interface: prometheus_scrape
   filebeat:
     interface: elastic-beats
+
 requires:
   dashboards:
     interface: grafana-dashboard

src/charm.py

@@ -202,7 +202,11 @@ def _on_stop(self, _):
 
     def _nrpe_relation_joined(self, _):
         self._stored.have_nrpe = True
+        self._setup_nrpe_exporter()
+        self._start_vector()
+        self._set_status()
 
+    def _setup_nrpe_exporter(self):
         # Make sure the exporter binary is present with a systemd service
         if not Path("/usr/local/bin/nrpe-exporter").exists():
             arch = platform.machine()
@@ -242,9 +246,6 @@ def _nrpe_relation_joined(self, _):
             # so it will survive reboots
             service_resume("nrpe-exporter.service")
 
-        self._start_vector()
-        self._set_status()
-
     def _nrpe_relation_broken(self, _):
         self._stored.have_nrpe = False
         self._set_status()
@@ -316,48 +317,60 @@ def _modify_enrichment_file(self, endpoints: Optional[List[Dict[str, Any]]] = No
                 writer = DictWriter(f, fieldnames=fieldnames)
                 writer.writeheader()
 
-        if endpoints:
-            contents = []
-            current = [
+        if not endpoints:
+            return
+
+        current = []
+        for endpoint in endpoints:
+            target = (
                 f"{endpoint['target'][next(iter(endpoint['target']))]['hostname']}_"
                 + f"{endpoint['additional_fields']['updates']['params']['command'][0]}"
-                for endpoint in endpoints
-            ]
-            with path.open(newline="") as f:
-                reader = DictReader(f)
-                contents = [r for r in reader if r["composite_key"] in current]
-
-                for endpoint in endpoints:
-                    unit = next(
-                        iter(
-                            [
-                                c["replacement"]
-                                for c in endpoint["additional_fields"]["relabel_configs"]
-                                if c["target_label"] == "juju_unit"
-                            ]
-                        )
+            )
+
+            unit_name = next(
+                filter(
+                    lambda d: "target_label" in d and d["target_label"] == "juju_unit",  # type: ignore
+                    endpoint["additional_fields"]["relabel_configs"],
+                )
+            )["replacement"]
+
+            # Out of all the fieldnames in the csv file, "composite_key" and "juju_unit" are sufficient to uniquely
+            # identify targets. This is needed for calculating an up-to-date list of targets on relation changes.
+            current.append((target, unit_name))
+
+        with path.open(newline="") as f:
+            reader = DictReader(f)
+            contents = [r for r in reader if (r["composite_key"], r["juju_unit"]) in current]
+
+            for endpoint in endpoints:
+                unit = next(
+                    iter(
+                        [
+                            c["replacement"]
+                            for c in endpoint["additional_fields"]["relabel_configs"]
+                            if c["target_label"] == "juju_unit"
+                        ]
                     )
-                    entry = {
-                        "composite_key": f"{endpoint['target'][next(iter(endpoint['target']))]['hostname']}_"
-                        + f"{endpoint['additional_fields']['updates']['params']['command'][0]}",
-                        "juju_application": re.sub(r"^(.*?)/\d+$", r"\1", unit),
-                        "juju_unit": unit,
-                        "command": endpoint["additional_fields"]["updates"]["params"]["command"][
-                            0
-                        ],
-                        "ipaddr": f"{endpoint['target'][next(iter(endpoint['target']))]['hostname']}",
-                    }
-
-                    if entry not in contents:
-                        contents.append(entry)
-
-            if contents:
-                with path.open("w", newline="") as f:
-                    writer = DictWriter(f, fieldnames=fieldnames)
-                    writer.writeheader()
-
-                    for c in contents:
-                        writer.writerow(c)
+                )
+                entry = {
+                    "composite_key": f"{endpoint['target'][next(iter(endpoint['target']))]['hostname']}_"
+                    + f"{endpoint['additional_fields']['updates']['params']['command'][0]}",
+                    "juju_application": re.sub(r"^(.*?)/\d+$", r"\1", unit),
+                    "juju_unit": unit,
+                    "command": endpoint["additional_fields"]["updates"]["params"]["command"][0],
+                    "ipaddr": f"{endpoint['target'][next(iter(endpoint['target']))]['hostname']}",
+                }
+
+                if entry not in contents:
+                    contents.append(entry)
+
+        if contents:
+            with path.open("w", newline="") as f:
+                writer = DictWriter(f, fieldnames=fieldnames)
+                writer.writeheader()
+
+                for c in contents:
+                    writer.writerow(c)
 
     def _write_vector_config(self, _):
         if not Path("/var/lib/vector").exists():

tests/unit/test_relation_monitors.py

@@ -0,0 +1,83 @@
+import tempfile
+import unittest
+from pathlib import Path
+from unittest.mock import patch
+
+from charm import COSProxyCharm
+from ops.testing import Harness
+
+
+class TestRelationMonitors(unittest.TestCase):
+    def setUp(self):
+        self.mock_enrichment_file = Path(tempfile.mktemp())
+
+        # The unit data below were obtained from the output of:
+        # juju show-unit \
+        #   cos-proxy/0 --format json | jq '."cos-proxy/0"."relation-info"[0]."related-units"."nrpe/0".data'
+        self.default_unit_data = {
+            "egress-subnets": "10.41.168.226/32",
+            "ingress-address": "10.41.168.226",
+            "machine_id": "1",
+            "model_id": "fe2c9bbb-58ab-40e4-8f70-f27480093fca",
+            "monitors": "{'monitors': {'remote': {'nrpe': {'check_conntrack': 'check_conntrack', 'check_systemd_scopes': 'check_systemd_scopes', 'check_reboot': 'check_reboot'}}}, 'version': '0.3'}",
+            "private-address": "10.41.168.226",
+            "target-address": "10.41.168.226",
+            "target-id": "ubuntu-0",
+        }
+
+        for p in [
+            patch("charm.remove_package"),
+            patch.object(COSProxyCharm, "_setup_nrpe_exporter"),
+            patch.object(COSProxyCharm, "_start_vector"),
+            patch.object(COSProxyCharm, "path", property(lambda *_: self.mock_enrichment_file)),
+        ]:
+            p.start()
+            self.addCleanup(p.stop)
+
+        self.harness = Harness(COSProxyCharm)
+        self.addCleanup(self.harness.cleanup)
+        self.harness.add_network("10.41.168.226")
+        self.harness.set_model_info(name="mymodel", uuid="fe2c9bbb-58ab-40e4-8f70-f27480093fca")
+        self.harness.set_leader(True)
+
+    def tearDown(self):
+        self.mock_enrichment_file.unlink(missing_ok=True)
+
+    def test_monitors_changed(self):
+        # GIVEN a post-startup charm
+        self.harness.begin_with_initial_hooks()
+
+        # WHEN a "monitors" relation joins
+        rel_id = self.harness.add_relation("monitors", "nrpe")
+        self.harness.add_relation_unit(rel_id, "nrpe/0")
+        self.harness.update_relation_data(rel_id, "nrpe/0", self.default_unit_data)
+
+        # THEN the csv file contains corresponding targets
+        expected = "\n".join(
+            [
+                "composite_key,juju_application,juju_unit,command,ipaddr",
+                "10.41.168.226_check_conntrack,ubuntu,ubuntu/0,check_conntrack,10.41.168.226",
+                "10.41.168.226_check_systemd_scopes,ubuntu,ubuntu/0,check_systemd_scopes,10.41.168.226",
+                "10.41.168.226_check_reboot,ubuntu,ubuntu/0,check_reboot,10.41.168.226",
+                "",
+            ]
+        )
+        self.assertEqual(expected, self.mock_enrichment_file.read_text())
+
+        # AND WHEN the relation data updates with a different prefix
+        # The following simulates `juju config nrpe nagios_host_context="context-1"`
+        self.harness.update_relation_data(
+            rel_id, "nrpe/0", {**self.default_unit_data, **{"target-id": "context-1-ubuntu-0"}}
+        )
+
+        # THEN the csv file is replaced with targets with the modified prefix
+        expected = "\n".join(
+            [
+                "composite_key,juju_application,juju_unit,command,ipaddr",
+                "10.41.168.226_check_conntrack,context-1-ubuntu,context-1-ubuntu/0,check_conntrack,10.41.168.226",
+                "10.41.168.226_check_systemd_scopes,context-1-ubuntu,context-1-ubuntu/0,check_systemd_scopes,10.41.168.226",
+                "10.41.168.226_check_reboot,context-1-ubuntu,context-1-ubuntu/0,check_reboot,10.41.168.226",
+                "",
+            ]
+        )
+        self.assertEqual(expected, self.mock_enrichment_file.read_text())