Remove validation in install event handler

canonical · Mar 23, 2024 · e75cbaf · e75cbaf
1 parent c04077b
commit e75cbaf
Show file tree

Hide file tree

Showing 2 changed files with 8 additions and 18 deletions.
diff --git a/src/charm.py b/src/charm.py
@@ -93,11 +93,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")
         scrape_timeout = self.model.config.get("scrape-timeout", EXPORTER_DEFAULT_COLLECT_TIMEOUT)
@@ -133,10 +128,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)

diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
@@ -150,7 +150,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())
@@ -207,13 +210,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()
 
@@ -222,7 +218,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)
@@ -400,6 +395,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'"),