From 32fc3bd43a3e7cd31a30a920b8775ca8b1434f53 Mon Sep 17 00:00:00 2001 From: Chi Wai Chan Date: Fri, 26 Jan 2024 16:13:57 +0800 Subject: [PATCH 1/3] Add refresh_events to COSAgentProvider to update relation data when config changed (default behavior) and upgrade charm. --- src/apt_helpers.py | 1 + src/charm.py | 1 + src/config.py | 1 + src/hardware.py | 1 + src/hw_tools.py | 1 + src/service.py | 1 + tests/unit/test_charm.py | 73 ++++++++++++++++++++++++++++++++++++++++ 7 files changed, 79 insertions(+) diff --git a/src/apt_helpers.py b/src/apt_helpers.py index c89ca81b..9c384370 100644 --- a/src/apt_helpers.py +++ b/src/apt_helpers.py @@ -1,4 +1,5 @@ """Apt helper module for missing features in operator_libs_linux.""" + import re from subprocess import PIPE, CalledProcessError, check_output from typing import Optional diff --git a/src/charm.py b/src/charm.py index 7a9ed508..3169e0b1 100755 --- a/src/charm.py +++ b/src/charm.py @@ -33,6 +33,7 @@ def __init__(self, *args: Any) -> None: self.cos_agent_provider = COSAgentProvider( self, + refresh_events=[self.on.config_changed, self.on.upgrade_charm], metrics_endpoints=[ {"path": "/metrics", "port": int(self.model.config["exporter-port"])} ], diff --git a/src/config.py b/src/config.py index d8f97813..15f1a273 100644 --- a/src/config.py +++ b/src/config.py @@ -1,4 +1,5 @@ """Config.""" + import typing as t from enum import Enum from pathlib import Path diff --git a/src/hardware.py b/src/hardware.py index 586b23f5..a1a19e23 100644 --- a/src/hardware.py +++ b/src/hardware.py @@ -1,4 +1,5 @@ """Hardware support config and command helper.""" + import json import logging import subprocess diff --git a/src/hw_tools.py b/src/hw_tools.py index b4c557cf..e3e379d1 100644 --- a/src/hw_tools.py +++ b/src/hw_tools.py @@ -2,6 +2,7 @@ Define strategy for install, remove and verifier for different hardwares. """ + import logging import os import shutil diff --git a/src/service.py b/src/service.py index 42833c67..f0caa8ac 100644 --- a/src/service.py +++ b/src/service.py @@ -1,4 +1,5 @@ """Exporter service helper.""" + from functools import wraps from logging import getLogger from pathlib import Path diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index f7c70576..6e40df83 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3,6 +3,7 @@ # # Learn more about testing at: https://juju.is/docs/sdk/testing +import json import unittest from unittest import mock @@ -268,3 +269,75 @@ def test_13_update_status_config_invalid(self, mock_hw_tool_helper, mock_exporte self.harness.charm.on.update_status.emit() self.assertEqual(self.harness.charm.unit.status, BlockedStatus("config fail message")) + + @mock.patch("charm.Exporter", return_value=mock.MagicMock()) + @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) + def test_14_config_changed_update_alert_rules(self, mock_hw_tool_helper, mock_exporter): + """Test config changed will update alert rule.""" + self.mock_bmc_hw_verifier.return_value = [ + HWTool.IPMI_SENSOR, + HWTool.IPMI_SEL, + HWTool.IPMI_DCMI, + ] + mock_hw_tool_helper.return_value.install.return_value = (True, "") + mock_hw_tool_helper.return_value.check_installed.return_value = (True, "") + mock_exporter.return_value.install.return_value = True + rid = self.harness.add_relation("cos-agent", "grafana-agent") + self.harness.begin() + self.harness.charm.on.install.emit() + self.harness.add_relation_unit(rid, "grafana-agent/0") + self.harness.charm.on.update_status.emit() + self.assertEqual(self.harness.charm.unit.status, ActiveStatus("Unit is ready")) + + relation_data = self.harness.get_relation_data(rid, "hardware-observer/0") + metrics_alert_rules = json.loads(relation_data["config"]).get("metrics_alert_rules") + + with mock.patch( + "charm.COSAgentProvider._metrics_alert_rules", new_callable=mock.PropertyMock + ) as mock_alert_rules: + fake_metrics_alert_rules = {} # just make it empty + mock_alert_rules.return_value = fake_metrics_alert_rules + self.harness.charm.on.config_changed.emit() + + relation_data = self.harness.get_relation_data(rid, "hardware-observer/0") + updated_metrics_alert_rules = json.loads(relation_data["config"]).get( + "metrics_alert_rules" + ) + self.assertEqual(updated_metrics_alert_rules, fake_metrics_alert_rules) + self.assertNotEqual(updated_metrics_alert_rules, metrics_alert_rules) + + @mock.patch("charm.Exporter", return_value=mock.MagicMock()) + @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) + def test_15_upgrade_charm_update__alert_rules(self, mock_hw_tool_helper, mock_exporter): + """Test upgrade charm will update alert rule.""" + self.mock_bmc_hw_verifier.return_value = [ + HWTool.IPMI_SENSOR, + HWTool.IPMI_SEL, + HWTool.IPMI_DCMI, + ] + mock_hw_tool_helper.return_value.install.return_value = (True, "") + mock_hw_tool_helper.return_value.check_installed.return_value = (True, "") + mock_exporter.return_value.install.return_value = True + rid = self.harness.add_relation("cos-agent", "grafana-agent") + self.harness.begin() + self.harness.charm.on.install.emit() + self.harness.add_relation_unit(rid, "grafana-agent/0") + self.harness.charm.on.update_status.emit() + self.assertEqual(self.harness.charm.unit.status, ActiveStatus("Unit is ready")) + + relation_data = self.harness.get_relation_data(rid, "hardware-observer/0") + metrics_alert_rules = json.loads(relation_data["config"]).get("metrics_alert_rules") + + with mock.patch( + "charm.COSAgentProvider._metrics_alert_rules", new_callable=mock.PropertyMock + ) as mock_alert_rules: + fake_metrics_alert_rules = {} # just make it empty + mock_alert_rules.return_value = fake_metrics_alert_rules + self.harness.charm.on.upgrade_charm.emit() + + relation_data = self.harness.get_relation_data(rid, "hardware-observer/0") + updated_metrics_alert_rules = json.loads(relation_data["config"]).get( + "metrics_alert_rules" + ) + self.assertEqual(updated_metrics_alert_rules, fake_metrics_alert_rules) + self.assertNotEqual(updated_metrics_alert_rules, metrics_alert_rules) From 953d8da279643f87ac839e568ff0bbbb5ba61f5c Mon Sep 17 00:00:00 2001 From: Chi Wai Chan Date: Mon, 29 Jan 2024 14:51:24 +0800 Subject: [PATCH 2/3] Add comment for refresh_events. --- src/charm.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/charm.py b/src/charm.py index 3169e0b1..0410fbff 100755 --- a/src/charm.py +++ b/src/charm.py @@ -31,6 +31,9 @@ def __init__(self, *args: Any) -> None: super().__init__(*args) self.hw_tool_helper = HWToolHelper() + # Add refresh_events to COSAgentProvider to update relation data when + # config changed (default behavior) and upgrade charm. This is useful + # for updating alert rules. self.cos_agent_provider = COSAgentProvider( self, refresh_events=[self.on.config_changed, self.on.upgrade_charm], From 6191c5d441490c1aec316335720b2b0f03f0cba3 Mon Sep 17 00:00:00 2001 From: Chi Wai Chan Date: Mon, 29 Jan 2024 15:23:21 +0800 Subject: [PATCH 3/3] Remove comments and fix typo. --- tests/unit/test_charm.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 6e40df83..eb3cedae 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -295,7 +295,7 @@ def test_14_config_changed_update_alert_rules(self, mock_hw_tool_helper, mock_ex with mock.patch( "charm.COSAgentProvider._metrics_alert_rules", new_callable=mock.PropertyMock ) as mock_alert_rules: - fake_metrics_alert_rules = {} # just make it empty + fake_metrics_alert_rules = {} mock_alert_rules.return_value = fake_metrics_alert_rules self.harness.charm.on.config_changed.emit() @@ -308,7 +308,7 @@ def test_14_config_changed_update_alert_rules(self, mock_hw_tool_helper, mock_ex @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_15_upgrade_charm_update__alert_rules(self, mock_hw_tool_helper, mock_exporter): + def test_15_upgrade_charm_update_alert_rules(self, mock_hw_tool_helper, mock_exporter): """Test upgrade charm will update alert rule.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.IPMI_SENSOR, @@ -331,7 +331,7 @@ def test_15_upgrade_charm_update__alert_rules(self, mock_hw_tool_helper, mock_ex with mock.patch( "charm.COSAgentProvider._metrics_alert_rules", new_callable=mock.PropertyMock ) as mock_alert_rules: - fake_metrics_alert_rules = {} # just make it empty + fake_metrics_alert_rules = {} mock_alert_rules.return_value = fake_metrics_alert_rules self.harness.charm.on.upgrade_charm.emit()