[DPE-2546] Split stanza create and stanza check (#240)

* Split stanza create and stanza check

* Remove force param for pgbackrest server

* Code review tweaks

src/backups.py

@@ -299,12 +299,21 @@ def _initialise_stanza(self) -> None:
             self.charm.unit.status = BlockedStatus(FAILED_TO_INITIALIZE_STANZA_ERROR_MESSAGE)
             return
 
+        self.start_stop_pgbackrest_service()
+
         # Store the stanza name to be used in configurations updates.
-        self.charm.app_peer_data.update({"stanza": self.stanza_name})
+        self.charm.app_peer_data.update({"stanza": self.stanza_name, "init-pgbackrest": "True"})
+
+    def check_stanza(self) -> None:
+        """Runs the pgbackrest stanza validation."""
+        if not self.charm.unit.is_leader() or "init-pgbackrest" not in self.charm.app_peer_data:
+            return
 
         # Update the configuration to use pgBackRest as the archiving mechanism.
         self.charm.update_config()
 
+        self.charm.unit.status = MaintenanceStatus("checking stanza")
+
         try:
             # Check that the stanza is correctly configured.
             for attempt in Retrying(stop=stop_after_attempt(5), wait=wait_fixed(3)):
@@ -317,13 +326,14 @@ def _initialise_stanza(self) -> None:
             # If the check command doesn't succeed, remove the stanza name
             # and rollback the configuration.
             self.charm.app_peer_data.update({"stanza": ""})
+            self.charm.app_peer_data.pop("init-pgbackrest", None)
             self.charm.update_config()
 
             logger.exception(e)
             self.charm.unit.status = BlockedStatus(FAILED_TO_INITIALIZE_STANZA_ERROR_MESSAGE)
             return
 
-        return
+        self.charm.app_peer_data.pop("init-pgbackrest", None)
 
     @property
     def _is_primary_pgbackrest_service_running(self) -> bool:
@@ -372,8 +382,6 @@ def _on_s3_credential_changed(self, event: CredentialsChangedEvent):
 
         self._initialise_stanza()
 
-        self.start_stop_pgbackrest_service()
-
     def _on_create_backup_action(self, event) -> None:
         """Request that pgBackRest creates a backup."""
         can_unit_perform_backup, validation_message = self._can_unit_perform_backup()

src/charm.py

@@ -464,6 +464,8 @@ def _on_peer_relation_changed(self, event: HookEvent) -> None:
 
         self.postgresql_client_relation.update_read_only_endpoint()
 
+        self.backup.check_stanza()
+
         # Start or stop the pgBackRest TLS server service when TLS certificate change.
         if not self.backup.start_stop_pgbackrest_service():
             # Ping primary to start its TLS server.
@@ -474,7 +476,7 @@ def _on_peer_relation_changed(self, event: HookEvent) -> None:
             event.defer()
             return
         else:
-            self.unit_peer_data.pop("start-tls.server", None)
+            self.unit_peer_data.pop("start-tls-server", None)
 
         if not self.is_blocked:
             self.unit.status = ActiveStatus()

tests/unit/test_backups.py

@@ -517,35 +517,73 @@ def test_initialise_stanza(
             # Assert there is no stanza name in the application relation databag.
             self.assertEqual(self.harness.get_relation_data(self.peer_rel_id, self.charm.app), {})
 
-        # Test when the stanza creation succeeds, but the archiving is not working correctly
-        # (pgBackRest check command fails).
+        # Test when the archiving is working correctly (pgBackRest check command succeeds).
         _execute_command.reset_mock()
-        _execute_command.side_effect = [
-            None,
-            ExecError(
-                command=stanza_creation_command, exit_code=1, stdout="", stderr="fake error"
-            ),
+        _update_config.reset_mock()
+        _member_started.reset_mock()
+        _reload_patroni_configuration.reset_mock()
+        _execute_command.side_effect = None
+        self.charm.backup._initialise_stanza()
+        self.assertEqual(
+            self.harness.get_relation_data(self.peer_rel_id, self.charm.app),
+            {"stanza": "None.patroni-postgresql-k8s", "init-pgbackrest": "True"},
+        )
+        self.assertIsInstance(self.charm.unit.status, MaintenanceStatus)
+
+    @patch("charm.Patroni.reload_patroni_configuration")
+    @patch("charm.Patroni.member_started", new_callable=PropertyMock)
+    @patch("backups.wait_fixed", return_value=wait_fixed(0))
+    @patch("charm.PostgresqlOperatorCharm.update_config")
+    @patch("charm.PostgreSQLBackups._execute_command")
+    def test_check_stanza(
+        self, _execute_command, _update_config, _, _member_started, _reload_patroni_configuration
+    ):
+        # Set peer data flag
+        with self.harness.hooks_disabled():
+            self.harness.update_relation_data(
+                self.peer_rel_id,
+                self.charm.app.name,
+                {"init-pgbackrest": "True"},
+            )
+
+        # Test when the unit is not the leader.
+        self.charm.backup.check_stanza()
+        _execute_command.assert_not_called()
+
+        # Set the unit as leader
+        with self.harness.hooks_disabled():
+            self.harness.set_leader()
+
+        stanza_check_command = [
+            "pgbackrest",
+            f"--stanza={self.charm.backup.stanza_name}",
+            "check",
         ]
+        # Test when the archiving is not working correctly (pgBackRest check command fails).
+        _execute_command.side_effect = ExecError(
+            command=stanza_check_command, exit_code=1, stdout="", stderr="fake error"
+        )
         _member_started.return_value = True
-        self.charm.backup._initialise_stanza()
+        self.charm.backup.check_stanza()
         self.assertEqual(_update_config.call_count, 2)
-        self.assertEqual(self.harness.get_relation_data(self.peer_rel_id, self.charm.app), {})
         self.assertEqual(_member_started.call_count, 5)
         self.assertEqual(_reload_patroni_configuration.call_count, 5)
         self.assertIsInstance(self.charm.unit.status, BlockedStatus)
         self.assertEqual(self.charm.unit.status.message, FAILED_TO_INITIALIZE_STANZA_ERROR_MESSAGE)
 
         # Test when the archiving is working correctly (pgBackRest check command succeeds).
+        with self.harness.hooks_disabled():
+            self.harness.update_relation_data(
+                self.peer_rel_id,
+                self.charm.app.name,
+                {"init-pgbackrest": "True"},
+            )
         _execute_command.reset_mock()
         _update_config.reset_mock()
         _member_started.reset_mock()
         _reload_patroni_configuration.reset_mock()
         _execute_command.side_effect = None
-        self.charm.backup._initialise_stanza()
-        self.assertEqual(
-            self.harness.get_relation_data(self.peer_rel_id, self.charm.app),
-            {"stanza": self.charm.backup.stanza_name},
-        )
+        self.charm.backup.check_stanza()
         _update_config.assert_called_once()
         _member_started.assert_called_once()
         _reload_patroni_configuration.assert_called_once()
@@ -576,7 +614,6 @@ def test_is_primary_pgbackrest_service_running(
         self.assertEqual(self.charm.backup._is_primary_pgbackrest_service_running, True)
         _execute_command.assert_called_once()
 
-    @patch("charm.PostgreSQLBackups.start_stop_pgbackrest_service")
     @patch("charm.PostgreSQLBackups._initialise_stanza")
     @patch("charm.PostgreSQLBackups.can_use_s3_repository")
     @patch("charm.PostgreSQLBackups._create_bucket_if_not_exists")
@@ -589,7 +626,6 @@ def test_on_s3_credential_changed(
         _create_bucket_if_not_exists,
         _can_use_s3_repository,
         _initialise_stanza,
-        _start_stop_pgbackrest_service,
     ):
         # Test when the cluster was not initialised yet.
         self.relate_to_s3_integrator()
@@ -601,7 +637,6 @@ def test_on_s3_credential_changed(
         _create_bucket_if_not_exists.assert_not_called()
         _can_use_s3_repository.assert_not_called()
         _initialise_stanza.assert_not_called()
-        _start_stop_pgbackrest_service.assert_not_called()
 
         # Test when the cluster is already initialised, but the charm fails to render
         # the pgBackRest configuration file due to missing S3 parameters.
@@ -621,7 +656,6 @@ def test_on_s3_credential_changed(
         _create_bucket_if_not_exists.assert_not_called()
         _can_use_s3_repository.assert_not_called()
         _initialise_stanza.assert_not_called()
-        _start_stop_pgbackrest_service.assert_not_called()
 
         # Test when the charm render the pgBackRest configuration file, but fails to
         # access or create the S3 bucket.
@@ -654,7 +688,6 @@ def test_on_s3_credential_changed(
             )
             _can_use_s3_repository.assert_not_called()
             _initialise_stanza.assert_not_called()
-            _start_stop_pgbackrest_service.assert_not_called()
 
         # Test when it's not possible to use the S3 repository due to backups from another cluster.
         _create_bucket_if_not_exists.reset_mock()
@@ -668,7 +701,6 @@ def test_on_s3_credential_changed(
         _create_bucket_if_not_exists.assert_called_once()
         _can_use_s3_repository.assert_called_once()
         _initialise_stanza.assert_not_called()
-        _start_stop_pgbackrest_service.assert_not_called()
 
         # Test when the stanza can be initialised and the pgBackRest service can start.
         _can_use_s3_repository.reset_mock()
@@ -678,7 +710,6 @@ def test_on_s3_credential_changed(
         )
         _can_use_s3_repository.assert_called_once()
         _initialise_stanza.assert_called_once()
-        _start_stop_pgbackrest_service.assert_called_once()
 
     @patch("charm.PostgresqlOperatorCharm.update_config")
     @patch("charm.PostgreSQLBackups._change_connectivity_to_database")