diff --git a/src/charm.py b/src/charm.py index 4005b48e0f..f91864a5ed 100755 --- a/src/charm.py +++ b/src/charm.py @@ -4,7 +4,6 @@ """Charmed Machine Operator for the PostgreSQL database.""" -import contextlib import json import logging import os @@ -49,7 +48,6 @@ SecretRemoveEvent, StartEvent, ) -from ops.framework import EventBase from ops.model import ( ActiveStatus, BlockedStatus, @@ -136,6 +134,10 @@ class CannotConnectError(Exception): """Cannot run smoke check on connected Database.""" +class StorageUnavailableError(Exception): + """Cannot find storage mountpoint.""" + + @trace_charm( tracing_endpoint="tracing_endpoint", extra_types=( @@ -1078,9 +1080,7 @@ def _on_cluster_topology_change(self, _): def _on_install(self, event: InstallEvent) -> None: """Install prerequisites for the application.""" logger.debug("Install start time: %s", datetime.now()) - if not self._is_storage_attached(): - self._reboot_on_detached_storage(event) - return + self._check_detached_storage() self.unit.status = MaintenanceStatus("installing PostgreSQL") @@ -1280,9 +1280,7 @@ def _get_ips_to_remove(self) -> set[str]: def _can_start(self, event: StartEvent) -> bool: """Returns whether the workload can be started on this unit.""" - if not self._is_storage_attached(): - self._reboot_on_detached_storage(event) - return False + self._check_detached_storage() # Safeguard against starting while upgrading. if not self.upgrade.idle: @@ -1927,19 +1925,20 @@ def clean_ca_file_from_workload(self, secret_name: str) -> bool: logger.exception("CA file failed to clean. Error in config update") return False - def _reboot_on_detached_storage(self, event: EventBase) -> None: - """Reboot on detached storage. + def _check_detached_storage(self) -> None: + """Wait for storage to become available. Workaround for lxd containers not getting storage attached on startups. Args: event: the event that triggered this handler """ - event.defer() - logger.error("Data directory not attached. Reboot unit.") - self.unit.status = WaitingStatus("Data directory not attached") - with contextlib.suppress(subprocess.CalledProcessError): - subprocess.check_call(["/usr/bin/systemctl", "reboot"]) + for attempt in Retrying(stop=stop_after_attempt(10), wait=wait_fixed(1), reraise=True): + with attempt: + if not self._is_storage_attached(): + logger.error("Data directory not attached.") + self.unit.status = WaitingStatus("Data directory not attached") + raise StorageUnavailableError() def _restart(self, event: RunWithLock) -> None: """Restart PostgreSQL.""" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 1747cf08a2..08f9ae1df2 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -34,6 +34,7 @@ EXTENSIONS_DEPENDENCY_MESSAGE, PRIMARY_NOT_REACHABLE_MESSAGE, PostgresqlOperatorCharm, + StorageUnavailableError, ) from cluster import ( NotReadyError, @@ -70,17 +71,12 @@ def test_on_install(harness): patch("charm.subprocess.check_call") as _check_call, patch("charm.snap.SnapCache") as _snap_cache, patch("charm.PostgresqlOperatorCharm._install_snap_packages") as _install_snap_packages, - patch( - "charm.PostgresqlOperatorCharm._reboot_on_detached_storage" - ) as _reboot_on_detached_storage, + patch("charm.PostgresqlOperatorCharm._check_detached_storage"), patch( "charm.PostgresqlOperatorCharm._is_storage_attached", side_effect=[False, True, True], ) as _is_storage_attached, ): - # Test without storage. - harness.charm.on.install.emit() - _reboot_on_detached_storage.assert_called_once() pg_snap = _snap_cache.return_value[POSTGRESQL_SNAP_NAME] # Test without adding Patroni resource. @@ -105,18 +101,13 @@ def test_on_install_failed_to_create_home(harness): patch("charm.subprocess.check_call") as _check_call, patch("charm.snap.SnapCache") as _snap_cache, patch("charm.PostgresqlOperatorCharm._install_snap_packages") as _install_snap_packages, - patch( - "charm.PostgresqlOperatorCharm._reboot_on_detached_storage" - ) as _reboot_on_detached_storage, + patch("charm.PostgresqlOperatorCharm._check_detached_storage"), patch( "charm.PostgresqlOperatorCharm._is_storage_attached", side_effect=[False, True, True], ) as _is_storage_attached, patch("charm.logger.exception") as _logger_exception, ): - # Test without storage. - harness.charm.on.install.emit() - _reboot_on_detached_storage.assert_called_once() pg_snap = _snap_cache.return_value[POSTGRESQL_SNAP_NAME] _check_call.side_effect = [subprocess.CalledProcessError(-1, ["test"])] @@ -611,9 +602,7 @@ def test_on_start(harness): patch("charm.Patroni.bootstrap_cluster") as _bootstrap_cluster, patch("charm.PostgresqlOperatorCharm._replication_password") as _replication_password, patch("charm.PostgresqlOperatorCharm._get_password") as _get_password, - patch( - "charm.PostgresqlOperatorCharm._reboot_on_detached_storage" - ) as _reboot_on_detached_storage, + patch("charm.PostgresqlOperatorCharm._check_detached_storage"), patch("upgrade.PostgreSQLUpgrade.idle", return_value=True) as _idle, patch( "charm.PostgresqlOperatorCharm._is_storage_attached", @@ -622,10 +611,6 @@ def test_on_start(harness): ): _get_postgresql_version.return_value = "14.0" - # Test without storage. - harness.charm.on.start.emit() - _reboot_on_detached_storage.assert_called_once() - # Test before the passwords are generated. _member_started.return_value = False _get_password.return_value = None @@ -1198,13 +1183,15 @@ def test_is_storage_attached(harness): assert not (is_storage_attached) -def test_reboot_on_detached_storage(harness): - with patch("subprocess.check_call") as _check_call: - mock_event = MagicMock() - harness.charm._reboot_on_detached_storage(mock_event) - mock_event.defer.assert_called_once() +def test_check_detached_storage(harness): + with ( + patch("charm.PostgresqlOperatorCharm._is_storage_attached") as _is_storage_attached, + patch("charm.wait_fixed", return_value=wait_fixed(0)), + ): + _is_storage_attached.return_value = False + with pytest.raises(StorageUnavailableError): + harness.charm._check_detached_storage() assert isinstance(harness.charm.unit.status, WaitingStatus) - _check_call.assert_called_once_with(["/usr/bin/systemctl", "reboot"]) def test_restart(harness):