From ce58da9ac0ecb1c74c688c44ce4dd95ffc54f228 Mon Sep 17 00:00:00 2001 From: Dragomir Penev <6687393+dragomirp@users.noreply.github.com> Date: Tue, 12 Nov 2024 18:40:21 +0200 Subject: [PATCH] [DPE-5934] Early exit on missing s3 rel (#762) * Early exit on missing s3 rel * Split of initial checks --- src/backups.py | 21 +++++++++++++++------ tests/unit/test_backups.py | 13 ++++++++++++- 2 files changed, 27 insertions(+), 7 deletions(-) diff --git a/src/backups.py b/src/backups.py index 12f32798c1..31a285b3af 100644 --- a/src/backups.py +++ b/src/backups.py @@ -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 ( @@ -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() diff --git a/tests/unit/test_backups.py b/tests/unit/test_backups.py index 3945335046..db70862e5a 100644 --- a/tests/unit/test_backups.py +++ b/tests/unit/test_backups.py @@ -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 @@ -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()