diff --git a/src/backups.py b/src/backups.py index eb75df42b1..1324b50791 100644 --- a/src/backups.py +++ b/src/backups.py @@ -669,7 +669,7 @@ def _on_s3_credentials_checks(self, event: CredentialsChangedEvent) -> bool: logger.debug("_on_s3_credential_changed early exit: no connection info") return False - if "cluster_initialised" not in self.charm.app_peer_data: + if not self.charm.is_cluster_initialised: logger.debug("Cannot set pgBackRest configurations, PostgreSQL has not yet started.") event.defer() return False @@ -681,10 +681,7 @@ def _on_s3_credentials_checks(self, event: CredentialsChangedEvent) -> bool: return False # Prevents S3 change in the middle of restoring backup and patroni / pgbackrest errors caused by that. - if ( - "restoring-backup" in self.charm.app_peer_data - or "restore-to-time" in self.charm.app_peer_data - ): + if self.charm.is_cluster_restoring_backup or self.charm.is_cluster_restoring_to_time: logger.info("Cannot change S3 configuration during restore") event.defer() return False @@ -1014,7 +1011,7 @@ def _on_restore_action(self, event): # noqa: C901 ) except ApiError as e: # If previous PITR restore was unsuccessful, there are no such endpoints. - if "restore-to-time" not in self.charm.app_peer_data: + if not self.charm.is_cluster_restoring_to_time: error_message = f"Failed to remove previous cluster information with error: {e!s}" logger.error(f"Restore failed: {error_message}") event.fail(error_message) diff --git a/src/charm.py b/src/charm.py index a77f2790df..283a82070e 100755 --- a/src/charm.py +++ b/src/charm.py @@ -388,6 +388,26 @@ def is_cluster_initialised(self) -> bool: """Returns whether the cluster is already initialised.""" return "cluster_initialised" in self.app_peer_data + @property + def is_cluster_restoring_backup(self) -> bool: + """Returns whether the cluster is restoring a backup.""" + return "restoring-backup" in self.app_peer_data + + @property + def is_cluster_restoring_to_time(self) -> bool: + """Returns whether the cluster is restoring a backup to a specific time.""" + return "restore-to-time" in self.app_peer_data + + @property + def is_unit_departing(self) -> bool: + """Returns whether the unit is departing.""" + return "departing" in self.unit_peer_data + + @property + def is_unit_stopped(self) -> bool: + """Returns whether the unit is stopped.""" + return "stopped" in self.unit_peer_data + @property def postgresql(self) -> PostgreSQL: """Returns an instance of the object used to interact with the database.""" @@ -456,7 +476,7 @@ def _on_peer_relation_departed(self, event: RelationDepartedEvent) -> None: if not self.unit.is_leader() or event.departing_unit == self.unit: return - if "cluster_initialised" not in self._peers.data[self.app]: + if not self.is_cluster_initialised: logger.debug( "Deferring on_peer_relation_departed: Cluster must be initialized before members can leave" ) @@ -521,7 +541,7 @@ def _on_peer_relation_changed(self, event: HookEvent) -> None: # noqa: C901 """Reconfigure cluster members.""" # The cluster must be initialized first in the leader unit # before any other member joins the cluster. - if "cluster_initialised" not in self._peers.data[self.app]: + if not self.is_cluster_initialised: if self.unit.is_leader(): if self._initialize_cluster(event): logger.debug("Deferring on_peer_relation_changed: Leader initialized cluster") @@ -566,7 +586,7 @@ def _on_peer_relation_changed(self, event: HookEvent) -> None: # noqa: C901 services = container.pebble.get_services(names=[self._postgresql_service]) if ( - ("restoring-backup" in self.app_peer_data or "restore-to-time" in self.app_peer_data) + (self.is_cluster_restoring_backup or self.is_cluster_restoring_to_time) and len(services) > 0 and not self._was_restore_successful(container, services[0]) ): @@ -769,7 +789,7 @@ def _add_members(self, event) -> None: # Reconfiguration can be successful only if the cluster is initialised # (i.e. first unit has bootstrap the cluster). - if "cluster_initialised" not in self._peers.data[self.app]: + if not self.is_cluster_initialised: return try: @@ -941,7 +961,7 @@ def _on_postgresql_pebble_ready(self, event: WorkloadEvent) -> None: # Otherwise, each unit will create a different cluster and # any update in the members list on the units won't have effect # on fixing that. - if not self.unit.is_leader() and "cluster_initialised" not in self._peers.data[self.app]: + if not self.unit.is_leader() and not self.is_cluster_initialised: logger.debug( "Deferring on_postgresql_pebble_ready: Not leader and cluster not initialized" ) @@ -1171,8 +1191,8 @@ def _has_non_restore_waiting_status(self) -> bool: """Returns whether the unit is in a waiting state and there is no restore process ongoing.""" return ( isinstance(self.unit.status, WaitingStatus) - and "restoring-backup" not in self.app_peer_data - and "restore-to-time" not in self.app_peer_data + and not self.is_cluster_restoring_backup + and not self.is_cluster_restoring_to_time ) def _on_get_password(self, event: ActionEvent) -> None: @@ -1420,9 +1440,9 @@ def _on_update_status(self, _) -> None: return if ( - "restoring-backup" not in self.app_peer_data - and "restore-to-time" not in self.app_peer_data - and "stopped" not in self.unit_peer_data + not self.is_cluster_restoring_backup + and not self.is_cluster_restoring_to_time + and not self.is_unit_stopped and services[0].current != ServiceStatus.ACTIVE ): logger.warning( @@ -1438,7 +1458,7 @@ def _on_update_status(self, _) -> None: return if ( - "restoring-backup" in self.app_peer_data or "restore-to-time" in self.app_peer_data + self.is_cluster_restoring_backup or self.is_cluster_restoring_to_time ) and not self._was_restore_successful(container, services[0]): return @@ -1454,7 +1474,7 @@ def _on_update_status(self, _) -> None: def _was_restore_successful(self, container: Container, service: ServiceInfo) -> bool: """Checks if restore operation succeeded and S3 is properly configured.""" - if "restore-to-time" in self.app_peer_data and all(self.is_pitr_failed(container)): + if self.is_cluster_restoring_to_time and all(self.is_pitr_failed(container)): logger.error( "Restore failed: database service failed to reach point-in-time-recovery target. " "You can launch another restore with different parameters" diff --git a/src/relations/async_replication.py b/src/relations/async_replication.py index 838b1bba20..7b6ecf8bc0 100644 --- a/src/relations/async_replication.py +++ b/src/relations/async_replication.py @@ -467,7 +467,7 @@ def is_primary_cluster(self) -> bool: return self.charm.app == self.get_primary_cluster() def _on_async_relation_broken(self, _) -> None: - if self.charm._peers is None or "departing" in self.charm._peers.data[self.charm.unit]: + if self.charm._peers is None or self.charm.is_unit_departing: logger.debug("Early exit on_async_relation_broken: Skipping departing unit.") return @@ -509,11 +509,11 @@ def _on_async_relation_changed(self, event: RelationChangedEvent) -> None: if not self._stop_database(event): return - if not all( + if not (self.charm.is_unit_stopped or self._is_following_promoted_cluster()) or not all( "stopped" in self.charm._peers.data[unit] or self.charm._peers.data[unit].get("unit-promoted-cluster-counter") == self._get_highest_promoted_cluster_counter_value() - for unit in {*self.charm._peers.units, self.charm.unit} + for unit in self.charm._peers.units ): self.charm.unit.status = WaitingStatus( "Waiting for the database to be stopped in all units" @@ -692,10 +692,7 @@ def _set_app_status(self) -> None: def _stop_database(self, event: RelationChangedEvent) -> bool: """Stop the database.""" - if ( - "stopped" not in self.charm._peers.data[self.charm.unit] - and not self._is_following_promoted_cluster() - ): + if not self.charm.is_unit_stopped and not self._is_following_promoted_cluster(): if not self.charm.unit.is_leader() and not self.container.exists(POSTGRESQL_DATA_PATH): logger.debug("Early exit on_async_relation_changed: following promoted cluster.") return False diff --git a/src/relations/db.py b/src/relations/db.py index dee16696aa..b4f437b54c 100644 --- a/src/relations/db.py +++ b/src/relations/db.py @@ -81,10 +81,7 @@ def _on_relation_changed(self, event: RelationChangedEvent) -> None: Generate password and handle user and database creation for the related application. """ # Check for some conditions before trying to access the PostgreSQL instance. - if ( - "cluster_initialised" not in self.charm._peers.data[self.charm.app] - or not self.charm._patroni.member_started - ): + if not self.charm.is_cluster_initialised or not self.charm._patroni.member_started: logger.debug( "Deferring on_relation_changed: Cluster not initialized or patroni not running" ) @@ -271,10 +268,7 @@ def _on_relation_departed(self, event: RelationDepartedEvent) -> None: Remove unit name from allowed_units key. """ # Check for some conditions before trying to access the PostgreSQL instance. - if ( - "cluster_initialised" not in self.charm._peers.data[self.charm.app] - or not self.charm._patroni.member_started - ): + if not self.charm.is_cluster_initialised or not self.charm._patroni.member_started: logger.debug( "Deferring on_relation_departed: Cluster not initialized or patroni not running" ) @@ -309,17 +303,14 @@ def _on_relation_departed(self, event: RelationDepartedEvent) -> None: def _on_relation_broken(self, event: RelationBrokenEvent) -> None: """Remove the user created for this relation.""" # Check for some conditions before trying to access the PostgreSQL instance. - if ( - "cluster_initialised" not in self.charm._peers.data[self.charm.app] - or not self.charm._patroni.member_started - ): + if not self.charm.is_cluster_initialised or not self.charm._patroni.member_started: logger.debug( "Deferring on_relation_broken: Cluster not initialized or patroni not running" ) event.defer() return - if "departing" in self.charm._peers.data[self.charm.unit]: + if self.charm.is_unit_departing: logger.debug("Early exit on_relation_broken: Skipping departing unit") return diff --git a/src/relations/postgresql_provider.py b/src/relations/postgresql_provider.py index 8301b067f7..bd37fa1bee 100644 --- a/src/relations/postgresql_provider.py +++ b/src/relations/postgresql_provider.py @@ -71,10 +71,7 @@ def _on_database_requested(self, event: DatabaseRequestedEvent) -> None: Generate password and handle user and database creation for the related application. """ # Check for some conditions before trying to access the PostgreSQL instance. - if ( - "cluster_initialised" not in self.charm._peers.data[self.charm.app] - or not self.charm._patroni.member_started - ): + if not self.charm.is_cluster_initialised or not self.charm._patroni.member_started: logger.debug( "Deferring on_database_requested: Cluster must be initialized before database can be requested" ) @@ -159,7 +156,7 @@ def _on_relation_broken(self, event: RelationBrokenEvent) -> None: # Check for some conditions before trying to access the PostgreSQL instance. if ( not self.charm._peers - or "cluster_initialised" not in self.charm._peers.data[self.charm.app] + or not self.charm.is_cluster_initialised or not self.charm._patroni.member_started ): logger.debug( @@ -170,7 +167,7 @@ def _on_relation_broken(self, event: RelationBrokenEvent) -> None: self._update_unit_status(event.relation) - if "departing" in self.charm._peers.data[self.charm.unit]: + if self.charm.is_unit_departing: logger.debug("Early exit on_relation_broken: Skipping departing unit") return diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index c26f9eae36..338c43a7c9 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -663,7 +663,9 @@ def test_on_peer_relation_departed(harness): patch( "charm.PostgresqlOperatorCharm._get_endpoints_to_remove" ) as _get_endpoints_to_remove, - patch("charm.PostgresqlOperatorCharm._peers", new_callable=PropertyMock) as _peers, + patch( + "charm.PostgresqlOperatorCharm.app_peer_data", new_callable=PropertyMock + ) as _app_peer_data, patch( "charm.PostgresqlOperatorCharm._get_endpoints_to_remove", return_value=sentinel.units ) as _get_endpoints_to_remove, @@ -680,7 +682,7 @@ def test_on_peer_relation_departed(harness): harness.charm._on_peer_relation_departed(event) event.defer.assert_called_once_with() - _peers.return_value.data = {harness.charm.app: {"cluster_initialised": True}} + _app_peer_data.return_value = {"cluster_initialised": True} harness.charm._on_peer_relation_departed(event) _get_endpoints_to_remove.assert_called_once_with() _remove_from_endpoints.assert_called_once_with(sentinel.units)