From 46282881e148d2f6cdf8d0e61e399a4ca187aa52 Mon Sep 17 00:00:00 2001
From: Iman Enami <iman.enami@canonical.com>
Date: Wed, 18 Dec 2024 13:57:07 +0400
Subject: [PATCH] some fixes

---
 src/core/models.py         | 16 +---------------
 src/events/controller.py   |  4 ++--
 src/managers/config.py     |  1 -
 src/managers/controller.py |  5 +++--
 tests/unit/test_charm.py   |  2 +-
 5 files changed, 7 insertions(+), 21 deletions(-)

diff --git a/src/core/models.py b/src/core/models.py
index 6235429f..9ace20d9 100644
--- a/src/core/models.py
+++ b/src/core/models.py
@@ -132,7 +132,7 @@ def __init__(
         self._balancer_uris = balancer_uris
         self._controller_password = controller_password
 
-    def _fetch_from_secrets(self, group, field):
+    def _fetch_from_secrets(self, group, field) -> str:
         if not self.relation:
             return ""
 
@@ -166,13 +166,6 @@ def broker_username(self) -> str:
 
         return self._fetch_from_secrets("broker", "broker-username")
 
-        return self.data_interface._fetch_relation_data_with_secrets(
-            component=self.relation.app,
-            req_secret_fields=BALANCER.requested_secrets,
-            relation=self.relation,
-            fields=BALANCER.requested_secrets,
-        ).get("broker-username", "")
-
     @property
     def broker_password(self) -> str:
         """The provided password for the broker application."""
@@ -184,13 +177,6 @@ def broker_password(self) -> str:
 
         return self._fetch_from_secrets("broker", "broker-password")
 
-        return self.data_interface._fetch_relation_data_with_secrets(
-            component=self.relation.app,
-            req_secret_fields=BALANCER.requested_secrets,
-            relation=self.relation,
-            fields=BALANCER.requested_secrets,
-        ).get("broker-password", "")
-
     @property
     def broker_uris(self) -> str:
         """The provided uris for the balancer application to connect to the broker application."""
diff --git a/src/events/controller.py b/src/events/controller.py
index d842b5c7..b002267d 100644
--- a/src/events/controller.py
+++ b/src/events/controller.py
@@ -80,7 +80,6 @@ def _on_start(self, event: StartEvent | PebbleReadyEvent) -> None:  # noqa: C901
             event.defer()
             return
 
-        self.broker.config_manager.set_server_properties()
         self._format_storages()
 
         # update status to add controller
@@ -97,7 +96,7 @@ def _init_kraft_mode(self) -> None:
         """Initialize the server when running controller mode."""
         # NOTE: checks for `runs_broker` in this method should be `is_cluster_manager` in
         # the large deployment feature.
-        if not self.model.unit.is_leader():
+        if not self.model.unit.is_leader() or not self.charm.state.kraft_mode:
             return
 
         if not self.charm.state.cluster.internal_user_credentials and self.charm.state.runs_broker:
@@ -143,6 +142,7 @@ def _format_storages(self) -> None:
         if not self.charm.state.kraft_mode:
             return
 
+        self.broker.config_manager.set_server_properties()
         if self.charm.state.runs_broker:
             credentials = self.charm.state.cluster.internal_user_credentials
         elif self.charm.state.runs_controller:
diff --git a/src/managers/config.py b/src/managers/config.py
index 2c9b3ba2..e2db5370 100644
--- a/src/managers/config.py
+++ b/src/managers/config.py
@@ -696,7 +696,6 @@ def controller_properties(self) -> list[str]:
         properties = [
             f"process.roles={','.join(roles)}",
             f"node.id={node_id}",
-            # f"controller.quorum.voters={self.state.peer_cluster.controller_quorum_uris}",
             f"controller.quorum.bootstrap.servers={self.state.peer_cluster.bootstrap_controller}",
             f"controller.listener.names={CONTROLLER_LISTENER_NAME}",
             *self.controller_scram_properties,
diff --git a/src/managers/controller.py b/src/managers/controller.py
index f36c8617..d68465f7 100644
--- a/src/managers/controller.py
+++ b/src/managers/controller.py
@@ -6,6 +6,7 @@
 
 import logging
 import os
+from subprocess import CalledProcessError
 from typing import TYPE_CHECKING
 
 from tenacity import retry, stop_after_attempt, wait_fixed
@@ -134,8 +135,8 @@ def remove_controller(
                     controller_directory_id,
                 ],
             )
-        except Exception as e:
-            error_details = getattr(e, "stderr")
+        except CalledProcessError as e:
+            error_details = e.stderr
             if "VoterNotFoundException" in error_details or "TimeoutException" in error_details:
                 # successful
                 return
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index ce8adbae..8e4cff8d 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -265,7 +265,7 @@ def test_start_defers_without_zookeeper(ctx: Context, base_state: State) -> None
     state_out = ctx.run(ctx.on.start(), state_in)
 
     # Then
-    assert len(state_out.deferred) == 1
+    assert len(state_out.deferred) == 2
     assert state_out.deferred[0].name == "start"