From c48ef211818d6fa9f563d68ce76a4b29a39a41ef Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Wed, 14 Aug 2024 14:28:03 -0400 Subject: [PATCH 1/5] feat: add unit test that confirms prometheus_remote_write config is correct As shown in [this issue](https://github.com/canonical/grafana-cloud-integrator/issues/12), we previously did not have test coverage to assert that prometheus_remote_write config was written successfully on non-leader units. The current commit adds a test to assert these configs are available for both leader and non-leader units. --- tests/unit/test_cloud_integration.py | 72 ++++++++++++++++++++++++++++ 1 file changed, 72 insertions(+) create mode 100644 tests/unit/test_cloud_integration.py diff --git a/tests/unit/test_cloud_integration.py b/tests/unit/test_cloud_integration.py new file mode 100644 index 0000000..427fc5e --- /dev/null +++ b/tests/unit/test_cloud_integration.py @@ -0,0 +1,72 @@ +import tempfile +from pathlib import Path +import unittest +from unittest.mock import patch + +import yaml +from ops import BlockedStatus, ActiveStatus +from ops.testing import Harness + +from charm import GrafanaAgentMachineCharm as GrafanaAgentCharm + + +class TestUpdateStatus(unittest.TestCase): + def setUp(self, *unused): + patcher = patch.object(GrafanaAgentCharm, "_agent_version", property(lambda *_: "0.0.0")) + self.mock_version = patcher.start() + self.addCleanup(patcher.stop) + + temp_config_path = tempfile.mkdtemp() + "/grafana-agent.yaml" + # otherwise will attempt to write to /etc/grafana-agent.yaml + patcher = patch("grafana_agent.CONFIG_PATH", temp_config_path) + self.config_path_mock = patcher.start() + self.addCleanup(patcher.stop) + + patcher = patch("charm.snap") + self.mock_snap = patcher.start() + self.addCleanup(patcher.stop) + + patcher = patch.object(GrafanaAgentCharm, "_install") + self.mock_install = patcher.start() + self.addCleanup(patcher.stop) + + def test_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(self): + """Asserts that the prometheus remote write config is written correctly when the charm is a leader.""" + harness = Harness(GrafanaAgentCharm) + harness.set_model_name(self.__class__.__name__) + + harness.set_leader(True) + harness.begin_with_initial_hooks() + + self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(harness) + + def test_prometheus_remote_write_config_with_grafana_cloud_integrator_on_non_leader(self): + """Asserts that the prometheus remote write config is written correctly when the charm is not a leader.""" + harness = Harness(GrafanaAgentCharm) + harness.set_model_name(self.__class__.__name__) + + harness.set_leader(False) + harness.begin_with_initial_hooks() + + self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(harness) + + def _subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(self, harness): + """Helper for all shared code between prometheus_remote_write_config tests.""" + # WHEN an incoming relation is added + rel_id = harness.add_relation("juju-info", "grafana-agent") + harness.add_relation_unit(rel_id, "grafana-agent/0") + + # THEN the charm goes into blocked status + assert isinstance(harness.charm.unit.status, BlockedStatus) + + # AND WHEN all the necessary outgoing relations are added + # for outgoing in ["send-remote-write", "logging-consumer"]: + harness.add_relation("grafana-cloud-config", "grafana-cloud-integrator", app_data={"prometheus_url": "http://some.domain.name:9090/api/v1/write"}) + + # THEN the charm goes into active status + assert isinstance(harness.charm.unit.status, ActiveStatus) + + # THEN with the expected prometheus endpoint settings are written to the config file + config = yaml.safe_load(Path(self.config_path_mock).read_text()) + assert len(config["integrations"]["prometheus_remote_write"]) == 1 + assert config["integrations"]["prometheus_remote_write"][0]["url"] == "http://some.domain.name:9090/api/v1/write" From 9f358e3f044912e6a2e7f45b681197e9bfd0f9e7 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Wed, 14 Aug 2024 15:19:54 -0400 Subject: [PATCH 2/5] fix: bump grafana_cloud_integrator lib to fix issue with non-leader units --- .../grafana_cloud_integrator/v0/cloud_config_requirer.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/charms/grafana_cloud_integrator/v0/cloud_config_requirer.py b/lib/charms/grafana_cloud_integrator/v0/cloud_config_requirer.py index 8fd8550..96d36a1 100644 --- a/lib/charms/grafana_cloud_integrator/v0/cloud_config_requirer.py +++ b/lib/charms/grafana_cloud_integrator/v0/cloud_config_requirer.py @@ -57,7 +57,7 @@ def _on_relation_changed(self, event): def _on_relation_broken(self, event): self.on.cloud_config_revoked.emit() # pyright: ignore - + def _is_not_empty(self, s): return bool(s and not s.isspace()) @@ -124,7 +124,7 @@ def prometheus_endpoint(self) -> dict: """Return the prometheus endpoint dict.""" if not self.prometheus_ready: return {} - + endpoint = {} endpoint["url"] = self.prometheus_url if self.credentials: From f1c478e5ec1979628094b277028e9338bb44bc82 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Wed, 14 Aug 2024 15:21:30 -0400 Subject: [PATCH 3/5] fix: formatting in test_cloud_integration.py --- tests/unit/test_cloud_integration.py | 30 +++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/tests/unit/test_cloud_integration.py b/tests/unit/test_cloud_integration.py index 427fc5e..d95457a 100644 --- a/tests/unit/test_cloud_integration.py +++ b/tests/unit/test_cloud_integration.py @@ -1,13 +1,12 @@ import tempfile -from pathlib import Path import unittest +from pathlib import Path from unittest.mock import patch import yaml -from ops import BlockedStatus, ActiveStatus -from ops.testing import Harness - from charm import GrafanaAgentMachineCharm as GrafanaAgentCharm +from ops import ActiveStatus, BlockedStatus +from ops.testing import Harness class TestUpdateStatus(unittest.TestCase): @@ -38,7 +37,9 @@ def test_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader( harness.set_leader(True) harness.begin_with_initial_hooks() - self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(harness) + self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader( + harness + ) def test_prometheus_remote_write_config_with_grafana_cloud_integrator_on_non_leader(self): """Asserts that the prometheus remote write config is written correctly when the charm is not a leader.""" @@ -48,9 +49,13 @@ def test_prometheus_remote_write_config_with_grafana_cloud_integrator_on_non_lea harness.set_leader(False) harness.begin_with_initial_hooks() - self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(harness) + self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader( + harness + ) - def _subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(self, harness): + def _subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader( + self, harness + ): """Helper for all shared code between prometheus_remote_write_config tests.""" # WHEN an incoming relation is added rel_id = harness.add_relation("juju-info", "grafana-agent") @@ -61,7 +66,11 @@ def _subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_lea # AND WHEN all the necessary outgoing relations are added # for outgoing in ["send-remote-write", "logging-consumer"]: - harness.add_relation("grafana-cloud-config", "grafana-cloud-integrator", app_data={"prometheus_url": "http://some.domain.name:9090/api/v1/write"}) + harness.add_relation( + "grafana-cloud-config", + "grafana-cloud-integrator", + app_data={"prometheus_url": "http://some.domain.name:9090/api/v1/write"}, + ) # THEN the charm goes into active status assert isinstance(harness.charm.unit.status, ActiveStatus) @@ -69,4 +78,7 @@ def _subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_lea # THEN with the expected prometheus endpoint settings are written to the config file config = yaml.safe_load(Path(self.config_path_mock).read_text()) assert len(config["integrations"]["prometheus_remote_write"]) == 1 - assert config["integrations"]["prometheus_remote_write"][0]["url"] == "http://some.domain.name:9090/api/v1/write" + assert ( + config["integrations"]["prometheus_remote_write"][0]["url"] + == "http://some.domain.name:9090/api/v1/write" + ) From a3db5c95c88afa9e3ad7514748f950cf9b121c7c Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Fri, 16 Aug 2024 14:50:31 -0400 Subject: [PATCH 4/5] refactor: test_cloud_integration.py based on review feedback --- tests/unit/test_cloud_integration.py | 86 +++++++++++----------------- 1 file changed, 33 insertions(+), 53 deletions(-) diff --git a/tests/unit/test_cloud_integration.py b/tests/unit/test_cloud_integration.py index d95457a..f295d5d 100644 --- a/tests/unit/test_cloud_integration.py +++ b/tests/unit/test_cloud_integration.py @@ -29,56 +29,36 @@ def setUp(self, *unused): self.mock_install = patcher.start() self.addCleanup(patcher.stop) - def test_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader(self): - """Asserts that the prometheus remote write config is written correctly when the charm is a leader.""" - harness = Harness(GrafanaAgentCharm) - harness.set_model_name(self.__class__.__name__) - - harness.set_leader(True) - harness.begin_with_initial_hooks() - - self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader( - harness - ) - - def test_prometheus_remote_write_config_with_grafana_cloud_integrator_on_non_leader(self): - """Asserts that the prometheus remote write config is written correctly when the charm is not a leader.""" - harness = Harness(GrafanaAgentCharm) - harness.set_model_name(self.__class__.__name__) - - harness.set_leader(False) - harness.begin_with_initial_hooks() - - self._subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader( - harness - ) - - def _subtest_prometheus_remote_write_config_with_grafana_cloud_integrator_on_leader( - self, harness - ): - """Helper for all shared code between prometheus_remote_write_config tests.""" - # WHEN an incoming relation is added - rel_id = harness.add_relation("juju-info", "grafana-agent") - harness.add_relation_unit(rel_id, "grafana-agent/0") - - # THEN the charm goes into blocked status - assert isinstance(harness.charm.unit.status, BlockedStatus) - - # AND WHEN all the necessary outgoing relations are added - # for outgoing in ["send-remote-write", "logging-consumer"]: - harness.add_relation( - "grafana-cloud-config", - "grafana-cloud-integrator", - app_data={"prometheus_url": "http://some.domain.name:9090/api/v1/write"}, - ) - - # THEN the charm goes into active status - assert isinstance(harness.charm.unit.status, ActiveStatus) - - # THEN with the expected prometheus endpoint settings are written to the config file - config = yaml.safe_load(Path(self.config_path_mock).read_text()) - assert len(config["integrations"]["prometheus_remote_write"]) == 1 - assert ( - config["integrations"]["prometheus_remote_write"][0]["url"] - == "http://some.domain.name:9090/api/v1/write" - ) + def test_prometheus_remote_write_config_with_grafana_cloud_integrator(self): + """Asserts that the prometheus remote write config is written correctly for leaders and non-leaders.""" + for leader in (True, False): + with self.subTest(leader=leader): + harness = Harness(GrafanaAgentCharm) + harness.set_model_name(self.__class__.__name__) + harness.set_leader(True) + harness.begin_with_initial_hooks() + + # WHEN an incoming relation is added + rel_id = harness.add_relation("juju-info", "grafana-agent") + harness.add_relation_unit(rel_id, "grafana-agent/0") + + # THEN the charm goes into blocked status + assert isinstance(harness.charm.unit.status, BlockedStatus) + + # AND WHEN the necessary outgoing relations are added + harness.add_relation( + "grafana-cloud-config", + "grafana-cloud-integrator", + app_data={"prometheus_url": "http://some.domain.name:9090/api/v1/write"}, + ) + + # THEN the charm goes into active status + assert isinstance(harness.charm.unit.status, ActiveStatus) + + # THEN with the expected prometheus endpoint settings are written to the config file + config = yaml.safe_load(Path(self.config_path_mock).read_text()) + assert len(config["integrations"]["prometheus_remote_write"]) == 1 + self.assertEqual( + config["integrations"]["prometheus_remote_write"][0]["url"], + "http://some.domain.name:9090/api/v1/write", + ) From 7696acb3448f39506639857d1a7fdab3625a9148 Mon Sep 17 00:00:00 2001 From: Andrew Scribner Date: Mon, 19 Aug 2024 09:10:29 -0400 Subject: [PATCH 5/5] fix: linting --- tests/unit/test_cloud_integration.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_cloud_integration.py b/tests/unit/test_cloud_integration.py index f295d5d..06030ff 100644 --- a/tests/unit/test_cloud_integration.py +++ b/tests/unit/test_cloud_integration.py @@ -4,10 +4,11 @@ from unittest.mock import patch import yaml -from charm import GrafanaAgentMachineCharm as GrafanaAgentCharm from ops import ActiveStatus, BlockedStatus from ops.testing import Harness +from charm import GrafanaAgentMachineCharm as GrafanaAgentCharm + class TestUpdateStatus(unittest.TestCase): def setUp(self, *unused):