diff --git a/src/grafana_agent.py b/src/grafana_agent.py index 504d1697..98552e63 100644 --- a/src/grafana_agent.py +++ b/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: diff --git a/src/k8s_charm.py b/src/k8s_charm.py index 650db5eb..207ff757 100755 --- a/src/k8s_charm.py +++ b/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) diff --git a/src/machine_charm.py b/src/machine_charm.py index b6a17992..0caeaaed 100755 --- a/src/machine_charm.py +++ b/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) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index ccf5c58f..783a37b2 100755 --- a/tests/integration/test_charm.py +++ b/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) diff --git a/tests/integration/test_forwards_alerts.py b/tests/integration/test_forwards_alerts.py index 84090689..2d674f8e 100755 --- a/tests/integration/test_forwards_alerts.py +++ b/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 ) diff --git a/tests/integration/test_kubectl_delete.py b/tests/integration/test_kubectl_delete.py index a2040f84..c47929aa 100644 --- a/tests/integration/test_kubectl_delete.py +++ b/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 diff --git a/tests/scenario/test_setup_statuses.py b/tests/scenario/test_setup_statuses.py index b359ef4b..2e0ff8a6 100644 --- a/tests/scenario/test_setup_statuses.py +++ b/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 diff --git a/tests/scenario/test_start_statuses.py b/tests/scenario/test_start_statuses.py index 15a88f5c..669cc195 100644 --- a/tests/scenario/test_start_statuses.py +++ b/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 diff --git a/tests/unit/k8s/test_relation_status.py b/tests/unit/k8s/test_relation_status.py index 19c2f88c..441887c7 100644 --- a/tests/unit/k8s/test_relation_status.py +++ b/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) diff --git a/tests/unit/k8s/test_scrape_configuration.py b/tests/unit/k8s/test_scrape_configuration.py index 0460c1bc..dc3f3282 100644 --- a/tests/unit/k8s/test_scrape_configuration.py +++ b/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") diff --git a/tests/unit/machine/test_relation_status.py b/tests/unit/machine/test_relation_status.py index 54415015..1c1b7b6a 100644 --- a/tests/unit/machine/test_relation_status.py +++ b/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")