[MISC] Clean-up duplicated Patroni config reloads (#682)

src/backups.py

@@ -658,8 +658,6 @@ def check_stanza(self) -> bool:
             # for that or else the s3 initialization sequence will fail.
             for attempt in Retrying(stop=stop_after_attempt(6), wait=wait_fixed(10), reraise=True):
                 with attempt:
-                    if self.charm._patroni.member_started:
-                        self.charm._patroni.reload_patroni_configuration()
                     return_code, _, stderr = self._execute_command([
                         PGBACKREST_EXECUTABLE,
                         PGBACKREST_CONFIGURATION_FILE,
@@ -708,9 +706,6 @@ def coordinate_stanza_fields(self) -> None:
             })
 
             self.charm.update_config()
-            if self.charm._patroni.member_started:
-                self.charm._patroni.reload_patroni_configuration()
-
             break
 
     @property
@@ -785,8 +780,6 @@ def _on_s3_credential_changed(self, event: CredentialsChangedEvent):
     def _on_s3_credential_changed_primary(self, event: HookEvent) -> bool:
         """Stanza must be cleared before calling this function."""
         self.charm.update_config()
-        if self.charm._patroni.member_started:
-            self.charm._patroni.reload_patroni_configuration()
 
         try:
             self._create_bucket_if_not_exists()

tests/unit/test_backups.py

@@ -719,7 +719,6 @@ def test_check_stanza(harness):
     with (
         patch("charm.PostgresqlOperatorCharm.update_config"),
         patch("backups.wait_fixed", return_value=wait_fixed(0)),
-        patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started,
         patch("charm.Patroni.reload_patroni_configuration") as _reload_patroni_configuration,
         patch("charm.PostgreSQLBackups._execute_command") as _execute_command,
         patch(
@@ -741,10 +740,8 @@ def test_check_stanza(harness):
                 {"s3-initialization-start": "test-stanza"},
             )
 
-        _member_started.return_value = False
         _execute_command.return_value = (49, "", "fake stderr")
         assert not harness.charm.backup.check_stanza()
-        _member_started.assert_called()
         _reload_patroni_configuration.assert_not_called()
         _set_primary_status_message.assert_not_called()
         _s3_initialization_set_failure.assert_called_once_with(
@@ -753,10 +750,8 @@ def test_check_stanza(harness):
 
         _execute_command.reset_mock()
         _s3_initialization_set_failure.reset_mock()
-        _member_started.return_value = True
         _execute_command.return_value = (0, "fake stdout", "")
         assert harness.charm.backup.check_stanza()
-        _reload_patroni_configuration.assert_called_once()
         _execute_command.assert_called_once()
         _set_primary_status_message.assert_called_once()
         _s3_initialization_set_failure.assert_not_called()
@@ -773,7 +768,6 @@ def test_check_stanza(harness):
 def test_coordinate_stanza_fields(harness):
     with (
         patch("charm.PostgresqlOperatorCharm.update_config") as _update_config,
-        patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started,
         patch("charm.Patroni.reload_patroni_configuration") as _reload_patroni_configuration,
     ):
         peer_rel_id = harness.model.get_relation(PEER).id
@@ -809,7 +803,6 @@ def test_coordinate_stanza_fields(harness):
 
         # Test with clear values.
         harness.charm.backup.coordinate_stanza_fields()
-        _member_started.assert_not_called()
         assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {}
         assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
         assert harness.get_relation_data(peer_rel_id, new_unit) == {}
@@ -819,7 +812,6 @@ def test_coordinate_stanza_fields(harness):
             harness.update_relation_data(peer_rel_id, new_unit_name, peer_data_primary_error)
         harness.charm.backup.coordinate_stanza_fields()
         _update_config.assert_not_called()
-        _member_started.assert_not_called()
         assert harness.get_relation_data(peer_rel_id, harness.charm.app) == {}
         assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
         assert harness.get_relation_data(peer_rel_id, new_unit) == peer_data_primary_error
@@ -831,26 +823,22 @@ def test_coordinate_stanza_fields(harness):
             )
         harness.charm.backup.coordinate_stanza_fields()
         _update_config.assert_not_called()
-        _member_started.assert_not_called()
         assert harness.get_relation_data(peer_rel_id, harness.charm.app) == peer_data_leader_start
         assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
         assert harness.get_relation_data(peer_rel_id, new_unit) == peer_data_primary_error
 
         # Leader should sync fail result from the primary.
-        _member_started.return_value = False
         with harness.hooks_disabled():
             harness.set_leader()
         harness.charm.backup.coordinate_stanza_fields()
         _update_config.assert_called_once()
-        _member_started.assert_called_once()
         _reload_patroni_configuration.assert_not_called()
         assert harness.get_relation_data(peer_rel_id, harness.charm.app) == peer_data_leader_error
         assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
         assert harness.get_relation_data(peer_rel_id, new_unit) == peer_data_primary_error
 
         # Test with successful result from the primary.
         _update_config.reset_mock()
-        _member_started.return_value = True
         with harness.hooks_disabled():
             harness.update_relation_data(peer_rel_id, harness.charm.app.name, peer_data_clean)
             harness.update_relation_data(
@@ -860,7 +848,6 @@ def test_coordinate_stanza_fields(harness):
             harness.update_relation_data(peer_rel_id, new_unit_name, peer_data_primary_ok)
         harness.charm.backup.coordinate_stanza_fields()
         _update_config.assert_called_once()
-        _reload_patroni_configuration.assert_called_once()
         assert harness.get_relation_data(peer_rel_id, harness.charm.app) == peer_data_leader_ok
         assert harness.get_relation_data(peer_rel_id, harness.charm.unit) == {}
         assert harness.get_relation_data(peer_rel_id, new_unit) == peer_data_primary_ok
@@ -1014,7 +1001,6 @@ def test_on_s3_credential_changed(harness):
 def test_on_s3_credential_changed_primary(harness):
     with (
         patch("charm.PostgresqlOperatorCharm.update_config"),
-        patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started,
         patch("charm.Patroni.reload_patroni_configuration") as _reload_patroni_configuration,
         patch(
             "charm.PostgreSQLBackups._create_bucket_if_not_exists"
@@ -1033,10 +1019,8 @@ def test_on_s3_credential_changed_primary(harness):
     ):
         mock_event = MagicMock()
 
-        _member_started.return_value = False
         _create_bucket_if_not_exists.side_effect = ValueError()
         assert not harness.charm.backup._on_s3_credential_changed_primary(mock_event)
-        _member_started.assert_called_once()
         _reload_patroni_configuration.assert_not_called()
         _create_bucket_if_not_exists.assert_called_once()
         _s3_initialization_set_failure.assert_called_once_with(
@@ -1045,11 +1029,9 @@ def test_on_s3_credential_changed_primary(harness):
         _can_use_s3_repository.assert_not_called()
 
         _s3_initialization_set_failure.reset_mock()
-        _member_started.return_value = True
         _create_bucket_if_not_exists.side_effect = None
         _can_use_s3_repository.return_value = (False, ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE)
         assert not harness.charm.backup._on_s3_credential_changed_primary(mock_event)
-        _reload_patroni_configuration.assert_called_once()
         _can_use_s3_repository.assert_called_once()
         _s3_initialization_set_failure.assert_called_once_with(
             ANOTHER_CLUSTER_REPOSITORY_ERROR_MESSAGE