diff --git a/src/charm.py b/src/charm.py index 2616660d..e5d36422 100755 --- a/src/charm.py +++ b/src/charm.py @@ -86,11 +86,6 @@ def _on_install_or_upgrade(self, event: ops.InstallEvent) -> None: # Install exporter self.model.unit.status = MaintenanceStatus("Installing exporter...") - success, err_msg = self.validate_exporter_configs() - if not success: - self.model.unit.status = BlockedStatus(err_msg) - return - port = self.model.config.get("exporter-port", EXPORTER_DEFAULT_PORT) level = self.model.config.get("exporter-log-level", "INFO") redfish_conn_params = self.get_redfish_conn_params() @@ -132,9 +127,9 @@ def _on_update_status(self, _: EventBase) -> None: # noqa: C901 self.model.unit.status = BlockedStatus("Cannot relate to more than one grafana-agent") return - config_valied, confg_valid_message = self.validate_exporter_configs() - if not config_valied: - self.model.unit.status = BlockedStatus(confg_valid_message) + config_valid, config_valid_message = self.validate_exporter_configs() + if not config_valid: + self.model.unit.status = BlockedStatus(config_valid_message) return hw_tool_ok, error_msg = self.hw_tool_helper.check_installed() diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 3b4db824..a94bf051 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -59,16 +59,16 @@ def _get_notice_count(self, hook): notice_count += 1 return notice_count - def test_01_harness(self) -> None: - """Test charm initialise.""" + def test_harness(self) -> None: + """Test charm initialize.""" self.harness.begin() self.assertFalse(self.harness.charm._stored.resource_installed) self.assertTrue(isinstance(self.harness.charm._stored.config, ops.framework.StoredDict)) @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_02_install(self, mock_hw_tool_helper, mock_exporter) -> None: - """Test event install.""" + def test_install(self, mock_hw_tool_helper, mock_exporter) -> None: + """Test install event handler.""" mock_hw_tool_helper.return_value.install.return_value = (True, "") mock_exporter.return_value.install.return_value = True self.harness.begin() @@ -83,8 +83,8 @@ def test_02_install(self, mock_hw_tool_helper, mock_exporter) -> None: @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_03_upgrade_charm(self, mock_hw_tool_helper, mock_exporter) -> None: - """Test event upgrade_charm.""" + def test_upgrade_charm(self, mock_hw_tool_helper, mock_exporter) -> None: + """Test upgrade_charm event handler.""" mock_hw_tool_helper.return_value.install.return_value = (True, "") mock_exporter.return_value.install.return_value = True self.harness.begin() @@ -101,8 +101,8 @@ def test_03_upgrade_charm(self, mock_hw_tool_helper, mock_exporter) -> None: @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_04_install_missing_resources(self, mock_hw_tool_helper, mock_exporter) -> None: - """Test event install.""" + def test_install_missing_resources(self, mock_hw_tool_helper, mock_exporter) -> None: + """Test install event handler when resources are missing.""" mock_hw_tool_helper.return_value.install.return_value = ( False, "Missing resources: ['storcli-deb']", @@ -116,8 +116,8 @@ def test_04_install_missing_resources(self, mock_hw_tool_helper, mock_exporter) @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_05_install_redfish_unavailable(self, mock_hw_tool_helper, mock_exporter) -> None: - """Test event install.""" + def test_install_redfish_unavailable(self, mock_hw_tool_helper, mock_exporter) -> None: + """Test install event handler when redfish is unavailable.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, @@ -138,8 +138,8 @@ def test_05_install_redfish_unavailable(self, mock_hw_tool_helper, mock_exporter @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_06_install_failed(self, mock_hw_tool_helper, mock_exporter) -> None: - """Test event install.""" + def test_exporter_install_fail(self, mock_hw_tool_helper, mock_exporter) -> None: + """Test exporter install failure.""" mock_hw_tool_helper.return_value.install.return_value = (True, "") mock_exporter.return_value.install.return_value = False self.harness.begin() @@ -149,12 +149,15 @@ def test_06_install_failed(self, mock_hw_tool_helper, mock_exporter) -> None: self.assertTrue(self.harness.charm._stored.resource_installed) - self.assertEqual(self.harness.charm.unit.status, BlockedStatus("error")) + self.assertEqual( + self.harness.charm.unit.status, + BlockedStatus("Failed to install exporter, please refer to `juju debug-log`"), + ) @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_07_update_status_all_green(self, mock_hw_tool_helper, mock_exporter): - """Test update_status when everything is okay.""" + def test_update_status_all_green(self, mock_hw_tool_helper, mock_exporter): + """Test update_status event handler when everything is okay.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, @@ -173,8 +176,8 @@ def test_07_update_status_all_green(self, mock_hw_tool_helper, mock_exporter): @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_08_update_status_check_installed_false(self, mock_hw_tool_helper, mock_exporter): - """Test update_status when hw tool checks failed.""" + def test_update_status_check_installed_false(self, mock_hw_tool_helper, mock_exporter): + """Test update_status event handler when hw tool checks failed.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, @@ -193,7 +196,7 @@ def test_08_update_status_check_installed_false(self, mock_hw_tool_helper, mock_ @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_09_update_status_exporter_crashed(self, mock_hw_tool_helper, mock_exporter): + def test_update_status_exporter_crashed(self, mock_hw_tool_helper, mock_exporter): """Test update_status.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.IPMI_SENSOR, @@ -210,7 +213,7 @@ def test_09_update_status_exporter_crashed(self, mock_hw_tool_helper, mock_expor self.harness.charm.on.update_status.emit() @mock.patch("charm.Exporter", return_value=mock.MagicMock()) - def test_10_config_changed(self, mock_exporter): + def test_config_changed(self, mock_exporter): """Test config change event updates the charm's internal store.""" self.harness.begin() self.harness.charm._stored.resource_installed = True @@ -227,8 +230,8 @@ def test_10_config_changed(self, mock_exporter): ) @mock.patch("charm.Exporter", return_value=mock.MagicMock()) - def test_11_config_changed_before_install_complete(self, mock_exporter): - """Test: config change event is deferred if charm not installed.""" + def test_config_changed_before_install_complete(self, mock_exporter): + """Test config change event is deferred if charm not installed.""" self.harness.begin() self.harness.charm._stored.resource_installed = False @@ -237,8 +240,8 @@ def test_11_config_changed_before_install_complete(self, mock_exporter): @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_12_upgrade_force_reconfig_exporter(self, mock_hw_tool_helper, mock_exporter) -> None: - """Test event install.""" + def test_upgrade_force_reconfig_exporter(self, mock_hw_tool_helper, mock_exporter) -> None: + """Test upgrade event handler will reconfigure exporter.""" mock_hw_tool_helper.return_value.install.return_value = (True, "") mock_exporter.return_value.install.return_value = True self.harness.begin() @@ -256,8 +259,8 @@ def test_12_upgrade_force_reconfig_exporter(self, mock_hw_tool_helper, mock_expo @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_13_update_status_config_invalid(self, mock_hw_tool_helper, mock_exporter): - """Test update_status when everything is okay.""" + def test_update_status_config_invalid(self, mock_hw_tool_helper, mock_exporter): + """Test update_status event handler when config is invalid.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, @@ -279,7 +282,7 @@ def test_13_update_status_config_invalid(self, mock_hw_tool_helper, mock_exporte @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): + def test_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, @@ -315,8 +318,8 @@ 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): - """Test upgrade charm will update alert rule.""" + def test_upgrade_charm_update_alert_rules(self, mock_hw_tool_helper, mock_exporter): + """Test upgrade charm event updates alert rule.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.IPMI_SENSOR, HWTool.IPMI_SEL, @@ -351,10 +354,10 @@ def test_15_upgrade_charm_update_alert_rules(self, mock_hw_tool_helper, mock_exp @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) - def test_16_install_redfish_enabled_with_correct_credential( + def test_install_redfish_enabled_with_correct_credential( self, mock_hw_tool_helper, mock_exporter ) -> None: - """Test event install when redfish is available and credential is correct.""" + """Test install event when redfish is available and credential is correct.""" self.mock_bmc_hw_verifier.return_value = [ HWTool.REDFISH, ] @@ -373,7 +376,7 @@ def test_16_install_redfish_enabled_with_correct_credential( @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) @mock.patch("charm.redfish_client", return_value=mock.MagicMock()) - def test_17_install_redfish_enabled_with_incorrect_credential( + def test_install_redfish_enabled_with_incorrect_credential( self, test_exception, mock_redfish_client, mock_hw_tool_helper, mock_exporter ) -> None: """Test event install when redfish is available but credential is wrong.""" @@ -383,11 +386,17 @@ def test_17_install_redfish_enabled_with_incorrect_credential( mock_redfish_client.side_effect = test_exception() mock_hw_tool_helper.return_value.install.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.add_relation_unit(rid, "grafana-agent/0") self.harness.charm.on.install.emit() self.assertTrue(self.harness.charm._stored.resource_installed) + # ensure exporter is installed (not started/enabled) + # even when redfish credentials are wrong + mock_exporter.return_value.install.assert_called_once() + mock_exporter.reset_mock() self.assertEqual( self.harness.charm.unit.status, BlockedStatus("Invalid config: 'redfish-username' or 'redfish-password'"), @@ -397,7 +406,7 @@ def test_17_install_redfish_enabled_with_incorrect_credential( @mock.patch("charm.Exporter", return_value=mock.MagicMock()) @mock.patch("charm.HWToolHelper", return_value=mock.MagicMock()) @mock.patch("charm.redfish_client", return_value=mock.MagicMock()) - def test_18_config_changed_redfish_enabled_with_incorrect_credential( + def test_config_changed_redfish_enabled_with_incorrect_credential( self, test_exception, mock_redfish_client, mock_hw_tool_helper, mock_exporter ) -> None: """Test event config changed when redfish is available but credential is wrong."""