Skip to content

Commit

Permalink
[DPE-5934] Early exit on missing s3 rel (canonical#762)
Browse files Browse the repository at this point in the history
* Early exit on missing s3 rel

* Split of initial checks
  • Loading branch information
dragomirp authored Nov 12, 2024
1 parent 2aab7da commit ce58da9
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 7 deletions.
21 changes: 15 additions & 6 deletions src/backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -675,18 +675,21 @@ def _is_primary_pgbackrest_service_running(self) -> bool:

return True

def _on_s3_credential_changed(self, event: CredentialsChangedEvent):
"""Call the stanza initialization when the credentials or the connection info change."""
def _on_s3_credentials_checks(self, event: CredentialsChangedEvent) -> bool:
if not self.s3_client.get_s3_connection_info():
logger.debug("_on_s3_credential_changed early exit: no connection info")
return False

if "cluster_initialised" not in self.charm.app_peer_data:
logger.debug("Cannot set pgBackRest configurations, PostgreSQL has not yet started.")
event.defer()
return
return False

# Prevents config change in bad state, so DB peer relations change event will not cause patroni related errors.
if self.charm.unit.status.message == CANNOT_RESTORE_PITR:
logger.info("Cannot change S3 configuration in bad PITR restore status")
event.defer()
return
return False

# Prevents S3 change in the middle of restoring backup and patroni / pgbackrest errors caused by that.
if (
Expand All @@ -695,15 +698,21 @@ def _on_s3_credential_changed(self, event: CredentialsChangedEvent):
):
logger.info("Cannot change S3 configuration during restore")
event.defer()
return
return False

if not self._render_pgbackrest_conf_file():
logger.debug("Cannot set pgBackRest configurations, missing configurations.")
return
return False

if not self._can_initialise_stanza:
logger.debug("Cannot initialise stanza yet.")
event.defer()
return False
return True

def _on_s3_credential_changed(self, event: CredentialsChangedEvent) -> bool | None:
"""Call the stanza initialization when the credentials or the connection info change."""
if not self._on_s3_credentials_checks(event):
return

self.start_stop_pgbackrest_service()
Expand Down
13 changes: 12 additions & 1 deletion tests/unit/test_backups.py
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,9 @@ def test_on_s3_credential_changed(harness):
patch(
"charm.PostgreSQLBackups._on_s3_credential_changed_primary"
) as _on_s3_credential_changed_primary,
patch(
"backups.S3Requirer.get_s3_connection_info", return_value={}
) as _get_s3_connection_info,
patch("ops.framework.EventBase.defer") as _defer,
patch(
"charm.PostgresqlOperatorCharm.is_standby_leader", new_callable=PropertyMock
Expand All @@ -977,11 +980,19 @@ def test_on_s3_credential_changed(harness):
patch("time.asctime", return_value="Thu Feb 24 05:00:00 2022"),
):
peer_rel_id = harness.model.get_relation(PEER).id
# Test when the cluster was not initialised yet.
# Early exit when no s3 creds
s3_rel_id = harness.add_relation(S3_PARAMETERS_RELATION, "s3-integrator")
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_not_called()

# Test when the cluster was not initialised yet.
_get_s3_connection_info.return_value = {"creds": "value"}
harness.charm.backup.s3_client.on.credentials_changed.emit(
relation=harness.model.get_relation(S3_PARAMETERS_RELATION, s3_rel_id)
)
_defer.assert_called_once()
_render_pgbackrest_conf_file.assert_not_called()

Expand Down

0 comments on commit ce58da9

Please sign in to comment.