Remove validation in install event handler

canonical · Mar 23, 2024 · 39da143 · 39da143
1 parent 16fd216
commit 39da143
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 24 deletions.
diff --git 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()
@@ -101,7 +96,6 @@ def _on_install_or_upgrade(self, event: ops.InstallEvent) -> None:
             logger.error(msg)
             self.model.unit.status = BlockedStatus(msg)
             return
-        logger.info("Successfully installed exporter")
         self._on_update_status(event)
 
     def _on_remove(self, _: EventBase) -> None:
@@ -125,15 +119,6 @@ def _on_update_status(self, _: EventBase) -> None:  # noqa: C901
             # The charm should be in BlockedStatus with install failed msg
             return  # type: ignore[unreachable]
 
-        if not self._stored.exporter_installed:  # type: ignore[truthy-function]
-            # The charm should be in BlockedStatus with exporter install failed msg
-            return  # type: ignore[unreachable]
-
-        config_valid, config_valid_message = self.validate_exporter_configs()
-        if not config_valid:
-            self.model.unit.status = BlockedStatus(config_valid_message)
-            return
-
         if not self.exporter_enabled:
             self.model.unit.status = BlockedStatus("Missing relation: [cos-agent]")
             return
@@ -142,6 +127,11 @@ def _on_update_status(self, _: EventBase) -> None:  # noqa: C901
             self.model.unit.status = BlockedStatus("Cannot relate to more than one grafana-agent")
             return
 
+        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()
         if not hw_tool_ok:
             self.model.unit.status = BlockedStatus(error_msg)

diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -149,7 +149,10 @@ def test_exporter_install_fail(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())
@@ -206,13 +209,6 @@ def test_update_status_exporter_crashed(self, mock_hw_tool_helper, mock_exporter
         self.harness.add_relation("cos-agent", "grafana-agent")
         self.harness.begin()
         self.harness.charm._stored.resource_installed = True
-        self.harness.charm._stored.exporter_installed = False
-
-        # no exception should be raised
-        self.harness.charm.on.update_status.emit()
-
-        # exception raised, exporter should crash
-        self.harness.charm._stored.exporter_installed = True
         with self.assertRaises(ExporterError):
             self.harness.charm.on.update_status.emit()
 
@@ -221,7 +217,6 @@ 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
-        self.harness.charm._stored.exporter_installed = True
 
         new_config = {"exporter-port": 80, "exporter-log-level": "DEBUG"}
         self.harness.update_config(new_config)
@@ -396,6 +391,10 @@ def test_install_redfish_enabled_with_incorrect_credential(
 
         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'"),