Skip to content

[DPE-8615] Wait for storage to become available #1277

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[DPE-8615] Wait for storage to become available #1277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 14 additions & 15 deletions src/charm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@

"""Charmed Machine Operator for the PostgreSQL database."""

import contextlib
import json
import logging
import os
Expand Down Expand Up @@ -49,7 +48,6 @@
SecretRemoveEvent,
StartEvent,
)
from ops.framework import EventBase
from ops.model import (
ActiveStatus,
BlockedStatus,
Expand Down Expand Up @@ -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=(
Expand Down Expand Up @@ -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")

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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."""
Expand Down
37 changes: 12 additions & 25 deletions tests/unit/test_charm.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
EXTENSIONS_DEPENDENCY_MESSAGE,
PRIMARY_NOT_REACHABLE_MESSAGE,
PostgresqlOperatorCharm,
StorageUnavailableError,
)
from cluster import (
NotReadyError,
Expand Down Expand Up @@ -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.
Expand All @@ -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"])]

Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down
Loading