Skip to content

Commit 29b0f7e

Browse files
[DPE-10370] fix(backups): reject standby backup actions and harden async replication (#1587)
* fix(backups): reject standby backup actions and harden async replication Fail create-backup, list-backups, and restore on standby clusters with explicit guidance to run those actions on the primary cluster. Also guard async replication relation handling when no remote units are present yet, preventing uncaught StopIteration during create-replication. Add unit coverage for both fixes, add a Juju3 Ceph-backed integration test for async replication backup behavior, and switch microceph TLS setup to a non-loopback host IP. Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * test(integration): simplify action assertions for Juju 3 Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * fix(async-replication): correct re-emission of relation-changed event Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * test(unit): fix async replication mocks Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * test(integration): migrate async replication backup test to jubilant Re-implement the standby-backup integration test on jubilant under the high_availability/ package, mirroring the jubilant implementation from - Replace ha_tests/test_async_replication_backups_ceph.py with a jubilant version under high_availability/, plus the package __init__ - Add high_availability_helpers_new.py with leader/units/primary lookups and wait_for_apps_status - Add a jubilant `juju` fixture to the integration conftest and depend on jubilant ^1.8.0 - Point the spread task at the new module path and drop the -juju29 variant - Use REPLICATION_CONSUMER_RELATION/REPLICATION_OFFER_RELATION constants in _is_standby_cluster instead of "replication"/"replication-offer" - Reword async replication action failures from "pod" to "unit" addresses and update the matching unit test Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * test(integration): fix async replication ceph backup test Get test_standby_backup_rejected_with_clear_message running green against real Juju + Ceph: - Pass the s3-integrator config at deploy time and wait for the app to reach blocked status (10m) before syncing credentials, instead of configuring it separately while the unit is still provisioning - Run create-backup on the primary unit: without TLS the charm only permits backups on the primary, and this test does not enable TLS (drop the now-unused get_app_units import) - Give the second_model fixture its own Juju() instance so it stops reassigning the shared temp-model's model name, which broke jubilant.temp_model teardown and leaked the model Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> * test(integration): exclude async replication backup test from Juju 2.9 Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com> --------- Signed-off-by: Marcelo Henrique Neppel <marcelo.neppel@canonical.com>
1 parent 17c6449 commit 29b0f7e

11 files changed

Lines changed: 529 additions & 22 deletions

File tree

poetry.lock

Lines changed: 17 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ pytest = "^9.1.0"
6666
pytest-operator = "^0.43.2"
6767
# renovate caret doesn't work: https://github.com/renovatebot/renovate/issues/26940
6868
juju = "<=3.6.1.3"
69+
jubilant = "^1.8.0"
6970
boto3 = "*"
7071
tenacity = "*"
7172
landscape-api-py3 = "^0.9.0"

src/backups.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
PGBACKREST_LOGS_PATH,
4646
POSTGRESQL_DATA_PATH,
4747
)
48+
from relations.async_replication import REPLICATION_CONSUMER_RELATION, REPLICATION_OFFER_RELATION
4849

4950
logger = logging.getLogger(__name__)
5051

@@ -60,6 +61,18 @@
6061
FAILED_TO_ACCESS_CREATE_BUCKET_ERROR_MESSAGE,
6162
FAILED_TO_INITIALIZE_STANZA_ERROR_MESSAGE,
6263
]
64+
STANDBY_CLUSTER_CREATE_BACKUP_ERROR_MESSAGE = (
65+
"Backups are not supported on a standby cluster. "
66+
"Run create-backup on the primary cluster instead."
67+
)
68+
STANDBY_CLUSTER_LIST_BACKUPS_ERROR_MESSAGE = (
69+
"Backups are not supported on a standby cluster. "
70+
"Run list-backups on the primary cluster instead."
71+
)
72+
STANDBY_CLUSTER_RESTORE_ERROR_MESSAGE = (
73+
"Restoring backups is not supported on a standby cluster. "
74+
"Run restore on the primary cluster instead."
75+
)
6376

6477

6578
class ListBackupsError(Exception):
@@ -153,6 +166,9 @@ def _can_initialise_stanza(self) -> bool:
153166

154167
def _can_unit_perform_backup(self) -> tuple[bool, str | None]:
155168
"""Validates whether this unit can perform a backup."""
169+
if self._is_standby_cluster():
170+
return False, STANDBY_CLUSTER_CREATE_BACKUP_ERROR_MESSAGE
171+
156172
if self.charm.is_blocked:
157173
return False, "Unit is in a blocking state"
158174

@@ -183,6 +199,16 @@ def _can_unit_perform_backup(self) -> tuple[bool, str | None]:
183199

184200
return self._are_backup_settings_ok()
185201

202+
def _is_standby_cluster(self) -> bool:
203+
"""Return whether this unit belongs to a standby cluster."""
204+
if (
205+
self.model.get_relation(REPLICATION_CONSUMER_RELATION) is None
206+
and self.model.get_relation(REPLICATION_OFFER_RELATION) is None
207+
):
208+
return False
209+
210+
return not self.charm.async_replication.is_primary_cluster()
211+
186212
def can_use_s3_repository(self) -> tuple[bool, str | None]:
187213
"""Returns whether the charm was configured to use another cluster repository."""
188214
# Check model uuid
@@ -1060,6 +1086,11 @@ def _run_backup(
10601086

10611087
def _on_list_backups_action(self, event) -> None:
10621088
"""List the previously created backups."""
1089+
if self._is_standby_cluster():
1090+
logger.warning(STANDBY_CLUSTER_LIST_BACKUPS_ERROR_MESSAGE)
1091+
event.fail(STANDBY_CLUSTER_LIST_BACKUPS_ERROR_MESSAGE)
1092+
return
1093+
10631094
are_backup_settings_ok, validation_message = self._are_backup_settings_ok()
10641095
if not are_backup_settings_ok:
10651096
logger.warning(validation_message)
@@ -1239,6 +1270,11 @@ def _pre_restore_checks(self, event: ActionEvent) -> bool:
12391270
Returns:
12401271
a boolean indicating whether restore should be run.
12411272
"""
1273+
if self._is_standby_cluster():
1274+
logger.error(f"Restore failed: {STANDBY_CLUSTER_RESTORE_ERROR_MESSAGE}")
1275+
event.fail(STANDBY_CLUSTER_RESTORE_ERROR_MESSAGE)
1276+
return False
1277+
12421278
are_backup_settings_ok, validation_message = self._are_backup_settings_ok()
12431279
if not are_backup_settings_ok:
12441280
logger.error(f"Restore failed: {validation_message}")

src/relations/async_replication.py

Lines changed: 29 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -434,14 +434,25 @@ def _handle_replication_change(self, event: ActionEvent) -> bool:
434434
return False
435435

436436
relation = self._relation
437+
if relation is None:
438+
event.fail("Replication relation not found")
439+
return False
437440

438-
# Check if all units from the other cluster published their pod IPs in the relation data.
439-
# If not, fail the action telling that all units must publish their pod addresses in the
441+
# Ensure the relation has at least one remote unit before trying to process unit data.
442+
remote_units = [unit for unit in relation.units if unit.app == relation.app]
443+
if len(remote_units) == 0:
444+
event.fail(
445+
"All units from the other cluster must publish their unit addresses in the relation data."
446+
)
447+
return False
448+
449+
# Check if all units from the other cluster published their IPs in the relation data.
450+
# If not, fail the action telling that all units must publish their unit addresses in the
440451
# relation data.
441-
for unit in relation.units:
452+
for unit in remote_units:
442453
if "unit-address" not in relation.data[unit]:
443454
event.fail(
444-
"All units from the other cluster must publish their pod addresses in the relation data."
455+
"All units from the other cluster must publish their unit addresses in the relation data."
445456
)
446457
return False
447458

@@ -648,12 +659,20 @@ def _primary_cluster_endpoint(self) -> str:
648659

649660
def _re_emit_async_relation_changed_event(self) -> None:
650661
"""Re-emit the async relation changed event."""
651-
relation = self._relation
652-
getattr(self.charm.on, f"{relation.name.replace('-', '_')}_relation_changed").emit(
653-
relation,
654-
app=relation.app,
655-
unit=next(unit for unit in relation.units if unit.app == relation.app),
656-
)
662+
if relation := self._relation:
663+
relation_unit = next(
664+
(unit for unit in relation.units if unit.app == relation.app), None
665+
)
666+
if relation_unit is None:
667+
logger.debug(
668+
"Skipping re-emitting relation-changed event: no related units found yet."
669+
)
670+
return
671+
getattr(self.charm.on, f"{relation.name.replace('-', '_')}_relation_changed").emit(
672+
relation,
673+
app=relation.app,
674+
unit=relation_unit,
675+
)
657676

658677
def _reinitialise_pgdata(self) -> None:
659678
"""Reinitialise the pgdata folder."""

tests/integration/conftest.py

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,11 @@
44
import json
55
import logging
66
import os
7-
import socket
87
import subprocess
98
import uuid
109

1110
import boto3
11+
import jubilant
1212
import pytest
1313
from pytest_operator.plugin import OpsTest
1414

@@ -21,6 +21,17 @@
2121
logger = logging.getLogger(__name__)
2222

2323

24+
def _get_non_loopback_host_ip() -> str:
25+
"""Return the host IP reachable by LXD containers."""
26+
output = subprocess.run(
27+
["ip", "-4", "route", "get", "1.1.1.1"],
28+
check=True,
29+
capture_output=True,
30+
text=True,
31+
).stdout
32+
return output.split("src", maxsplit=1)[1].split()[0]
33+
34+
2435
@pytest.fixture(scope="session")
2536
def charm():
2637
# Return str instead of pathlib.Path since python-libjuju's model.deploy(), juju deploy, and
@@ -101,6 +112,20 @@ async def gcp_cloud_configs(ops_test: OpsTest) -> None:
101112
cleanup_cloud(config, credentials)
102113

103114

115+
@pytest.fixture(scope="module")
116+
def juju(request: pytest.FixtureRequest):
117+
"""Pytest fixture that wraps jubilant.temp_model."""
118+
model = request.config.getoption("--model")
119+
keep_models = bool(request.config.getoption("--keep-models"))
120+
121+
if model:
122+
juju = jubilant.Juju(model=model)
123+
yield juju
124+
else:
125+
with jubilant.temp_model(keep=keep_models) as juju:
126+
yield juju
127+
128+
104129
@dataclasses.dataclass(frozen=True)
105130
class ConnectionInformation:
106131
access_key_id: str
@@ -151,7 +176,7 @@ def microceph():
151176
],
152177
check=True,
153178
)
154-
host_ip = socket.gethostbyname(socket.gethostname())
179+
host_ip = _get_non_loopback_host_ip()
155180
subprocess.run(
156181
f'echo "subjectAltName = IP:{host_ip}" > ./extfile.cnf',
157182
shell=True,
@@ -209,7 +234,7 @@ def microceph():
209234
key_id = key["access_key"]
210235
secret_key = key["secret_key"]
211236
logger.info("Set up microceph")
212-
host_ip = socket.gethostbyname(socket.gethostname())
237+
host_ip = _get_non_loopback_host_ip()
213238
result = subprocess.run(
214239
"base64 -w0 ./ca.crt", shell=True, check=True, stdout=subprocess.PIPE, text=True
215240
)
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
# Copyright 2026 Canonical Ltd.
2+
# See LICENSE file for licensing details.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
#!/usr/bin/env python3
2+
# Copyright 2026 Canonical Ltd.
3+
# See LICENSE file for licensing details.
4+
5+
from collections.abc import Callable
6+
7+
import jubilant
8+
from jubilant import Juju
9+
from jubilant.statustypes import Status, UnitStatus
10+
11+
MINUTE_SECS = 60
12+
13+
JujuModelStatusFn = Callable[[Status], bool]
14+
JujuAppsStatusFn = Callable[[Status, str], bool]
15+
16+
17+
def get_app_leader(juju: Juju, app_name: str) -> str:
18+
"""Get the leader unit for the given application."""
19+
app_status = juju.status().apps[app_name]
20+
for name, status in app_status.units.items():
21+
if status.leader:
22+
return name
23+
24+
raise Exception("No leader unit found")
25+
26+
27+
def get_app_units(juju: Juju, app_name: str) -> dict[str, UnitStatus]:
28+
"""Get the units for the given application."""
29+
return juju.status().apps[app_name].units
30+
31+
32+
def get_db_primary_unit(juju: Juju, app_name: str) -> str:
33+
"""Get the current primary node of the cluster."""
34+
postgresql_primary = get_app_leader(juju, app_name)
35+
task = juju.run(unit=postgresql_primary, action="get-primary", wait=5 * MINUTE_SECS)
36+
task.raise_on_failure()
37+
38+
primary = task.results.get("primary")
39+
if primary != "None":
40+
return primary
41+
42+
raise Exception("No primary node found")
43+
44+
45+
def wait_for_apps_status(jubilant_status_func: JujuAppsStatusFn, *apps: str) -> JujuModelStatusFn:
46+
"""Wait for Juju agents to be idle and apps to reach a target status."""
47+
return lambda status: all((
48+
jubilant.all_agents_idle(status, *apps),
49+
jubilant_status_func(status, *apps),
50+
))

0 commit comments

Comments
 (0)