Fix scale up with S3 and TLS relations (#480)

Signed-off-by: Marcelo Henrique Neppel <[email protected]>

Author: marceloneppel

src/backups.py

@@ -96,6 +96,22 @@ def _are_backup_settings_ok(self) -> Tuple[bool, Optional[str]]:
 
         return True, None
 
+    @property
+    def _can_initialise_stanza(self) -> bool:
+        """Validates whether this unit can initialise a stanza."""
+        # Don't allow stanza initialisation if this unit hasn't started the database
+        # yet and either hasn't joined the peer relation yet or hasn't configured TLS
+        # yet while other unit already has TLS enabled.
+        if not self.charm._patroni.member_started and (
+            (len(self.charm._peers.data.keys()) == 2)
+            or (
+                "tls" not in self.charm.unit_peer_data
+                and any("tls" in unit_data for _, unit_data in self.charm._peers.data.items())
+            )
+        ):
+            return False
+        return True
+
     def _can_unit_perform_backup(self) -> Tuple[bool, Optional[str]]:
         """Validates whether this unit can perform a backup."""
         if self.charm.is_blocked:
@@ -512,6 +528,11 @@ def _on_s3_credential_changed(self, event: CredentialsChangedEvent):
             logger.debug("Cannot set pgBackRest configurations, missing configurations.")
             return
 
+        if not self._can_initialise_stanza:
+            logger.debug("Cannot initialise stanza yet.")
+            event.defer()
+            return
+
         # Verify the s3 relation only on the primary.
         if not self.charm.is_primary:
             return

tests/unit/test_backups.py

@@ -73,6 +73,39 @@ def test_are_backup_settings_ok(harness):
         )
 
 
+def test_can_initialise_stanza(harness):
+    with patch("charm.Patroni.member_started", new_callable=PropertyMock) as _member_started:
+        # Test when Patroni or PostgreSQL hasn't started yet
+        # and the unit hasn't joined the peer relation yet.
+        _member_started.return_value = False
+        tc.assertEqual(
+            harness.charm.backup._can_initialise_stanza,
+            False,
+        )
+
+        # Test when the unit hasn't configured TLS yet while other unit already has TLS enabled.
+        harness.add_relation_unit(
+            harness.model.get_relation(PEER).id, f"{harness.charm.app.name}/1"
+        )
+        with harness.hooks_disabled():
+            harness.update_relation_data(
+                harness.model.get_relation(PEER).id,
+                f"{harness.charm.app.name}/1",
+                {"tls": "enabled"},
+            )
+        tc.assertEqual(
+            harness.charm.backup._can_initialise_stanza,
+            False,
+        )
+
+        # Test when everything is ok to initialise the stanza.
+        _member_started.return_value = True
+        tc.assertEqual(
+            harness.charm.backup._can_initialise_stanza,
+            True,
+        )
+
+
 @patch_network_get(private_address="1.1.1.1")
 def test_can_unit_perform_backup(harness):
     with (
@@ -926,6 +959,9 @@ def test_on_s3_credential_changed(harness):
         patch(
             "charm.PostgresqlOperatorCharm.is_primary", new_callable=PropertyMock
         ) as _is_primary,
+        patch(
+            "charm.PostgreSQLBackups._can_initialise_stanza", new_callable=PropertyMock
+        ) as _can_initialise_stanza,
         patch(
             "charm.PostgreSQLBackups._render_pgbackrest_conf_file"
         ) as _render_pgbackrest_conf_file,
@@ -953,31 +989,42 @@ def test_on_s3_credential_changed(harness):
                 {"cluster_initialised": "True"},
             )
         _render_pgbackrest_conf_file.return_value = False
-        _is_primary.return_value = False
         harness.charm.backup.s3_client.on.credentials_changed.emit(
             relation=harness.model.get_relation(S3_PARAMETERS_RELATION, s3_rel_id)
         )
         _defer.assert_not_called()
         _render_pgbackrest_conf_file.assert_called_once()
+        _can_initialise_stanza.assert_not_called()
         _create_bucket_if_not_exists.assert_not_called()
         _can_use_s3_repository.assert_not_called()
         _initialise_stanza.assert_not_called()
 
+        # Test when it's not possible to initialise the stanza in this unit.
+        _render_pgbackrest_conf_file.return_value = True
+        _can_initialise_stanza.return_value = False
+        harness.charm.backup.s3_client.on.credentials_changed.emit(
+            relation=harness.model.get_relation(S3_PARAMETERS_RELATION, s3_rel_id)
+        )
+        _defer.assert_called_once()
+        _can_initialise_stanza.assert_called_once()
+        _is_primary.assert_not_called()
+
         # Test that followers will not initialise the bucket
         harness.charm.unit.status = ActiveStatus()
         _render_pgbackrest_conf_file.reset_mock()
+        _can_initialise_stanza.return_value = True
+        _is_primary.return_value = False
         with harness.hooks_disabled():
             harness.update_relation_data(
                 peer_rel_id,
                 harness.charm.app.name,
                 {"cluster_initialised": "True"},
             )
-        _render_pgbackrest_conf_file.return_value = True
-
         harness.charm.backup.s3_client.on.credentials_changed.emit(
             relation=harness.model.get_relation(S3_PARAMETERS_RELATION, s3_rel_id)
         )
         _render_pgbackrest_conf_file.assert_called_once()
+        _is_primary.assert_called_once()
         _create_bucket_if_not_exists.assert_not_called()
         tc.assertIsInstance(harness.charm.unit.status, ActiveStatus)
         _can_use_s3_repository.assert_not_called()