Grafana agent require only one outgoing relation to be Active (#205)

* now only 1 outgoing relation is necessary to be Active

* change the way we define mandatory relations

* fix tests according to new behaviour

* update libs

* change itest so now verifies for blocked status

* missing blocked status to check

* make mandatory_relation_pairs easier

* fix statuses of the tests

src/grafana_agent.py

@@ -10,7 +10,7 @@
 import shutil
 from collections import namedtuple
 from dataclasses import dataclass
-from typing import Any, Callable, Dict, List, Optional, Tuple, Union
+from typing import Any, Callable, Dict, List, Optional, Set, Tuple, Union
 
 import yaml
 from charms.grafana_cloud_integrator.v0.cloud_config_requirer import (
@@ -72,7 +72,7 @@ class GrafanaAgentCharm(CharmBase):
     # incur data loss.
     # Property to facilitate centralized status update.
     # 'outgoing' are OR-ed, 'incoming' are AND-ed.
-    mandatory_relation_pairs: List[Tuple[str, List[str]]]  # overridden
+    mandatory_relation_pairs: Dict[str, List[Set[str]]]  # overridden
 
     def __new__(cls, *args: Any, **kwargs: Dict[Any, Any]):
         """Forbid the usage of GrafanaAgentCharm directly."""
@@ -156,12 +156,13 @@ def __init__(self, *args):
         self.framework.observe(self.on.config_changed, self._on_config_changed)
 
         # Register status observers
-        for incoming, outgoings in self.mandatory_relation_pairs:
+        for incoming, outgoings in self.mandatory_relation_pairs.items():
             self.framework.observe(self.on[incoming].relation_joined, self._update_status)
             self.framework.observe(self.on[incoming].relation_broken, self._update_status)
-            for outgoing in outgoings:
-                self.framework.observe(self.on[outgoing].relation_joined, self._update_status)
-                self.framework.observe(self.on[outgoing].relation_broken, self._update_status)
+            for outgoing_list in outgoings:
+                for outgoing in outgoing_list:
+                    self.framework.observe(self.on[outgoing].relation_joined, self._update_status)
+                    self.framework.observe(self.on[outgoing].relation_broken, self._update_status)
 
     def _on_mandatory_relation_event(self, _event=None):
         """Event handler for any mandatory relation event."""
@@ -362,55 +363,70 @@ def _update_status(self, *_):
             self.unit.status = self.status.validation_error
             return
 
+        has_incoming = False
+        outgoing_rels = {"has": False, "message": ""}
+
         # Make sure every incoming relation has at least one matching outgoing relation
-        for incoming, outgoings in self.mandatory_relation_pairs:
+        for incoming, outgoings in self.mandatory_relation_pairs.items():
             if not self.model.relations.get(incoming):
                 continue
 
-            # FIXME: This is a workaround since on relation-broken the relation that is
-            # going away is still present in `self.model.relations`
-            # See: https://github.com/canonical/operator/issues/888
-            #
-            # Once this issue is fixed, we can have:
-            #
-            # has_outgoing = any(
-            #     (len(self.model.relations.get(outgoing, [])) for outgoing in outgoings)
-            # )
-            has_outgoing = self._has_outgoing(outgoings)
-
-            if not has_outgoing:
-                missing = "|".join(outgoings)
-                logger.warning(
-                    "An incoming '%s' relation does not yet have any "
-                    "matching outgoing relation(s): [%s]",
-                    incoming,
-                    missing,
-                )
-                self.unit.status = BlockedStatus(f"Missing relation: [{missing}]")
-                return
+            has_incoming = True
+            outgoing_rels = self._has_outgoings(outgoings)
+
+        if not has_incoming:
+            self.unit.status = BlockedStatus("Missing incoming ('requires') relation")
+            return
+
+        if not outgoing_rels["has"]:
+            self.unit.status = BlockedStatus(f"{outgoing_rels['message']}")
+            return
 
         if not self.is_ready:
             self.unit.status = WaitingStatus("waiting for the agent to start")
             return
 
-        self.unit.status = ActiveStatus()
+        self.unit.status = ActiveStatus(f"{outgoing_rels['message']}")
 
-    def _has_outgoing(self, outgoings) -> bool:
-        for outgoing in outgoings:
-            relation = self.model.relations.get(outgoing, [])
+    def _has_outgoings(self, outgoings: List[Set[str]]) -> Dict[str, Any]:
+        missing_rels = set()
+        active_rels = set()
 
-            if relation == []:
-                continue
+        for outgoing_list in outgoings:
+            for outgoing in outgoing_list:
+                relation = self.model.relations.get(outgoing, False)
+
+                if not relation:
+                    missing_rels.add(outgoing)
+                    continue
+
+                try:
+                    units = relation[0].units  # pyright: ignore
+                except IndexError:
+                    missing_rels.add(outgoing)
+                    units = None
+
+                if not units:
+                    missing_rels.add(outgoing)
+
+                if relation and units:
+                    active_rels.add(outgoing)
+
+        return {"has": bool(active_rels), "message": self._status_message(missing_rels)}
+
+    def _status_message(self, missing_relations: set) -> str:
+        if not missing_relations:
+            return ""
 
-            try:
-                units = relation[0].units
-            except IndexError:
-                units = None
+        # The grafana-cloud-config relation is  established
+        if "grafana-cloud-config" not in missing_relations:
+            return ""
 
-            if relation and units:
-                return True
+        # The other 3 relations (logs, metrics, dashboards) are established
+        if len(missing_relations) == 1 and "grafana-cloud-config" in missing_relations:
+            return ""
 
-        return False
+        return ", ".join([f"{x}: off" for x in missing_relations])
 
     def _update_config(self) -> None:
         if not self.is_ready:

src/k8s_charm.py

@@ -26,11 +26,20 @@
 class GrafanaAgentK8sCharm(GrafanaAgentCharm):
     """K8s version of the Grafana Agent charm."""
 
-    mandatory_relation_pairs = [
-        ("metrics-endpoint", ["send-remote-write", "grafana-cloud-config"]),
-        ("grafana-dashboards-consumer", ["grafana-dashboards-provider", "grafana-cloud-config"]),
-        ("logging-provider", ["logging-consumer", "grafana-cloud-config"]),
-    ]
+    mandatory_relation_pairs = {
+        "metrics-endpoint": [  # must be paired with:
+            {"send-remote-write"},  # or
+            {"grafana-cloud-config"},
+        ],
+        "logging-provider": [  # must be paired with:
+            {"logging-consumer"},  # or
+            {"grafana-cloud-config"},
+        ],
+        "grafana-dashboards-consumer": [  # must be paired with:
+            {"grafana-dashboards-provider"},  # or
+            {"grafana-cloud-config"},
+        ],
+    }
 
     def __init__(self, *args):
         super().__init__(*args)

src/machine_charm.py

@@ -148,13 +148,16 @@ class GrafanaAgentMachineCharm(GrafanaAgentCharm):
 
     service_name = "grafana-agent.grafana-agent"
 
-    mandatory_relation_pairs = [
-        ("cos-agent", ["send-remote-write", "grafana-cloud-config"]),
-        ("cos-agent", ["logging-consumer", "grafana-cloud-config"]),
-        ("cos-agent", ["grafana-dashboards-provider", "grafana-cloud-config"]),
-        ("juju-info", ["send-remote-write", "grafana-cloud-config"]),
-        ("juju-info", ["logging-consumer", "grafana-cloud-config"]),
-    ]
+    mandatory_relation_pairs = {
+        "cos-agent": [  # must be paired with:
+            {"grafana-cloud-config"},  # or
+            {"send-remote-write", "logging-consumer", "grafana-dashboards-provider"},
+        ],
+        "juju-info": [  # must be paired with:
+            {"grafana-cloud-config"},  # or
+            {"send-remote-write", "logging-consumer"},
+        ],
+    }
 
     def __init__(self, *args):
         super().__init__(*args)

tests/integration/test_charm.py

@@ -24,12 +24,16 @@ async def test_build_and_deploy(ops_test, grafana_agent_charm):
     await ops_test.model.deploy(grafana_agent_charm, resources=resources, application_name="agent")
 
     await ops_test.model.wait_for_idle(
-        apps=["agent"], status="active", timeout=300, idle_period=30
+        apps=["agent"], status="blocked", timeout=300, idle_period=30
     )
-    assert ops_test.model.applications["agent"].units[0].workload_status == "active"
+    assert ops_test.model.applications["agent"].units[0].workload_status == "blocked"
 
 
 async def test_relates_to_loki(ops_test):
     await ops_test.model.deploy("loki-k8s", channel="edge", application_name="loki", trust=True)
     await ops_test.model.add_relation("loki", "agent:logging-consumer")
-    await ops_test.model.wait_for_idle(apps=["loki", "agent"], status="active", timeout=1000)
+
+    await ops_test.model.wait_for_idle(
+        apps=["agent"], status="blocked", timeout=300  # Missing incoming ('requires') relation
+    )
+    await ops_test.model.wait_for_idle(apps=["loki"], status="active", timeout=300)

tests/integration/test_forwards_alerts.py

@@ -37,8 +37,8 @@ async def test_deploy(ops_test, grafana_agent_charm):
     # issuing placeholder update_status just to trigger an event
     await ops_test.model.set_config({"update-status-hook-interval": "10s"})
 
-    await ops_test.model.wait_for_idle(apps=[agent_name], status="active", timeout=300)
-    assert ops_test.model.applications[agent_name].units[0].workload_status == "active"
+    await ops_test.model.wait_for_idle(apps=[agent_name], status="blocked", timeout=300)
+    assert ops_test.model.applications[agent_name].units[0].workload_status == "blocked"
 
 
 async def test_relate_to_external_apps(ops_test):
@@ -53,7 +53,10 @@ async def test_relate_to_external_apps(ops_test):
         ops_test.model.add_relation(f"{prometheus_name}:receive-remote-write", agent_name),
     )
     await ops_test.model.wait_for_idle(
-        apps=[loki_name, prometheus_name, agent_name], status="active", timeout=300
+        apps=[loki_name, prometheus_name], status="active", timeout=300
+    )
+    await ops_test.model.wait_for_idle(
+        apps=[agent_name], status="blocked", timeout=300  # Missing incoming ('requires') relation
     )
 
 

tests/integration/test_kubectl_delete.py

@@ -25,7 +25,7 @@ async def test_deploy_from_local_path(ops_test, grafana_agent_charm):
     await ops_test.model.deploy(
         grafana_agent_charm, application_name=app_name, resources=resources
     )
-    await ops_test.model.wait_for_idle(apps=[app_name], status="active", timeout=1000)
+    await ops_test.model.wait_for_idle(apps=[app_name], status="blocked", timeout=1000)
 
 
 @pytest.mark.xfail

tests/scenario/test_setup_statuses.py

@@ -8,7 +8,7 @@
 import k8s_charm
 import machine_charm
 import pytest
-from ops import ActiveStatus, UnknownStatus, WaitingStatus, pebble
+from ops import BlockedStatus, UnknownStatus, WaitingStatus, pebble
 from ops.testing import CharmType
 from scenario import Container, ExecOutput, State, trigger
 
@@ -105,6 +105,6 @@ def test_k8s_charm_start_with_container(charm_type, substrate, vroot):
         charm_root=vroot,
     )
 
-    assert out.status.unit == ActiveStatus("")
+    assert out.status.unit == BlockedStatus("Missing incoming ('requires') relation")
     agent_out = out.get_container("agent")
     assert agent_out.services["agent"].current == pebble.ServiceStatus.ACTIVE

tests/scenario/test_start_statuses.py

@@ -117,10 +117,10 @@ def test_start(charm_type, charm_meta, substrate, vroot, placeholder_cfg_path):
         written_cfg = placeholder_cfg_path.read_text()
         assert written_cfg  # check nonempty
 
-        assert out.status.unit == ("active", "")
+        assert out.status.unit.name == "blocked"
 
     else:
-        assert out.status.unit == ("unknown", "")
+        assert out.status.unit.name == "unknown"
 
 
 def test_k8s_charm_start_with_container(charm_type, charm_meta, substrate, vroot):
@@ -140,6 +140,6 @@ def test_k8s_charm_start_with_container(charm_type, charm_meta, substrate, vroot
     )
     out = ctx.run(state=State(containers=[agent]), event=agent.pebble_ready_event)
 
-    assert out.status.unit == ("active", "")
+    assert out.status.unit.name == "blocked"
     agent_out = out.get_container("agent")
     assert agent_out.services["agent"].current == pebble.ServiceStatus.ACTIVE

tests/unit/k8s/test_relation_status.py

@@ -27,13 +27,13 @@ def setUp(self, *unused):
     def test_no_relations(self):
         # GIVEN no relations joined (see SetUp)
         # WHEN the charm starts (see SetUp)
-        # THEN status is "active"
-        self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
+        # THEN status is "blocked"
+        self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
 
         # AND WHEN "update-status" fires
         self.harness.charm.on.update_status.emit()
-        # THEN status is still "active"
-        self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
+        # THEN status is still "blocked"
+        self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
 
     def test_with_relations(self):
         for incoming, outgoing in [
@@ -43,16 +43,21 @@ def test_with_relations(self):
         ]:
             with self.subTest(incoming=incoming, outgoing=outgoing):
                 # WHEN an incoming relation is added
-                rel_id = self.harness.add_relation(incoming, "grafana-agent")
-                self.harness.add_relation_unit(rel_id, "grafana-agent/0")
-                self.harness.update_relation_data(rel_id, "grafana-agent/0", {"sample": "value"})
+                rel_incoming_id = self.harness.add_relation(incoming, "grafana-agent")
+                self.harness.add_relation_unit(rel_incoming_id, "grafana-agent/0")
+                self.harness.update_relation_data(
+                    rel_incoming_id, "grafana-agent/0", {"sample": "value"}
+                )
 
                 # THEN the charm goes into blocked status
                 self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
 
                 # AND WHEN an appropriate outgoing relation is added
-                rel_id = self.harness.add_relation(outgoing, "grafana-agent")
-                self.harness.add_relation_unit(rel_id, "grafana-agent/0")
+                rel_outgoing_id = self.harness.add_relation(outgoing, "grafana-agent")
+                self.harness.add_relation_unit(rel_outgoing_id, "grafana-agent/0")
 
                 # THEN the charm goes into active status
                 self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
+
+                # Remove incoming relation.
+                self.harness.remove_relation(rel_incoming_id)

tests/unit/k8s/test_scrape_configuration.py

@@ -109,6 +109,10 @@ def test_remote_write_configuration(self):
 
         agent_container = self.harness.charm.unit.get_container("agent")
 
+        # Add incoming relation
+        rel_incoming_id = self.harness.add_relation("metrics-endpoint", "agent")
+        self.harness.add_relation_unit(rel_incoming_id, "agent/0")
+
         rel_id = self.harness.add_relation("send-remote-write", "prometheus")
 
         self.harness.add_relation_unit(rel_id, "prometheus/0")

tests/unit/machine/test_relation_status.py

@@ -36,13 +36,13 @@ def setUp(self, *unused):
     def test_no_relations(self):
         # GIVEN no relations joined (see SetUp)
         # WHEN the charm starts (see SetUp)
-        # THEN status is "active"
-        self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
+        # THEN status is "blocked"
+        self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
 
         # AND WHEN "update-status" fires
         self.harness.charm.on.update_status.emit()
-        # THEN status is still "active"
-        self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
+        # THEN status is still "blocked"
+        self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
 
     def test_cos_agent_with_relations(self):
         # WHEN an incoming relation is added
@@ -52,22 +52,19 @@ def test_cos_agent_with_relations(self):
         # THEN the charm goes into blocked status
         self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
 
-        # AND WHEN all the necessary outgoing relations are added
+        # AND WHEN at least one of the necessary outgoing relations is added
         for outgoing in ["send-remote-write", "logging-consumer", "grafana-dashboards-provider"]:
-            # Before the relation is added, the charm is still in blocked status
-            self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
-
             rel_id = self.harness.add_relation(outgoing, "grafana-agent")
             self.harness.add_relation_unit(rel_id, "grafana-agent/0")
 
-        # THEN the charm goes into active status
-        self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
+            # THEN the charm goes into active status when one mandatory relation is added
+            self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
 
-        # AND WHEN we remove one mandatory relation
+        # AND WHEN we remove one of the mandatory relations
         self.harness.remove_relation(rel_id)
 
-        # THEN the charm goes into blocked status again
-        self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
+        # THEN the charm keeps into active status
+        self.assertIsInstance(self.harness.charm.unit.status, ActiveStatus)
 
     def test_juju_info_with_relations(self):
         # WHEN an incoming relation is added
@@ -79,9 +76,6 @@ def test_juju_info_with_relations(self):
 
         # AND WHEN all the necessary outgoing relations are added
         for outgoing in ["send-remote-write", "logging-consumer"]:
-            # Before the relation is added, the charm is still in blocked status
-            self.assertIsInstance(self.harness.charm.unit.status, BlockedStatus)
-
             rel_id = self.harness.add_relation(outgoing, "grafana-agent")
             self.harness.add_relation_unit(rel_id, "grafana-agent/0")