From 4a644b2783b5313343c7b11fa3e29345493b1c95 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 10:53:37 +0700 Subject: [PATCH 01/10] feat(port-sync): declare content-cache peer relation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/metadata.yaml | 4 ++++ content-cache/src/charm.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/content-cache/metadata.yaml b/content-cache/metadata.yaml index 82632944..65e85e61 100644 --- a/content-cache/metadata.yaml +++ b/content-cache/metadata.yaml @@ -38,3 +38,7 @@ requires: interface: tls-certificates receive-ca-cert: interface: certificate_transfer + +peers: + content-cache-peers: + interface: content-cache-peers diff --git a/content-cache/src/charm.py b/content-cache/src/charm.py index adc90939..7e1fb2b0 100755 --- a/content-cache/src/charm.py +++ b/content-cache/src/charm.py @@ -54,6 +54,10 @@ NGINX_PORT_RANGE_START = 30000 NGINX_PORT_RANGE_SIZE = 200 +PEER_RELATION_NAME = "content-cache-peers" +PORT_MAP_FIELD = "port_map" +NEXT_OFFSET_FIELD = "next_offset" + class ContentCacheCharm(ops.CharmBase): """Charm the application.""" From 55198fd658a9542b5c7d3d2a2838ad027ec23a4b Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 10:56:44 +0700 Subject: [PATCH 02/10] feat(port-sync): allocate shared port via peer databag (leader) and read on followers Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/src/charm.py | 150 +++++++++++++++++++------ content-cache/tests/unit/conftest.py | 12 ++ content-cache/tests/unit/test_charm.py | 95 +++++++++++++--- 3 files changed, 207 insertions(+), 50 deletions(-) diff --git a/content-cache/src/charm.py b/content-cache/src/charm.py index 7e1fb2b0..a042a2f7 100755 --- a/content-cache/src/charm.py +++ b/content-cache/src/charm.py @@ -50,6 +50,7 @@ CERTIFICATE_TRANSFER_INTEGRATION_NAME = "receive-ca-cert" CERTIFICATE_INTEGRATION_NAME = "certificates" WAIT_FOR_TLS_CERT_MESSAGE = "Waiting for TLS certificate" +WAIT_FOR_PORT_MESSAGE = "Waiting for port assignment" NGINX_PORT_RANGE_START = 30000 NGINX_PORT_RANGE_SIZE = 200 @@ -62,8 +63,6 @@ class ContentCacheCharm(ops.CharmBase): """Charm the application.""" - _stored = ops.StoredState() - def __init__(self, framework: ops.Framework) -> None: """Initialize the object. @@ -72,9 +71,6 @@ def __init__(self, framework: ops.Framework) -> None: """ super().__init__(framework) - self._stored.set_default(port_map={}) - self._stored.set_default(next_port_offset=0) - self._cos_agent = COSAgentProvider(charm=self) self._certificate_transfer = CertificateTransferRequires( self, CERTIFICATE_TRANSFER_INTEGRATION_NAME @@ -91,6 +87,14 @@ def __init__(self, framework: ops.Framework) -> None: framework.observe(self.on.start, self._on_start) framework.observe(self.on.stop, self._on_stop) framework.observe(self.on.update_status, self._on_update_status) + framework.observe( + self.on[PEER_RELATION_NAME].relation_created, + self._on_peer_relation_changed, + ) + framework.observe( + self.on[PEER_RELATION_NAME].relation_changed, + self._on_peer_relation_changed, + ) framework.observe( self.on[CACHE_CONFIG_INTEGRATION_NAME].relation_changed, self._on_cache_config_relation_changed, @@ -154,12 +158,11 @@ def _on_cache_config_relation_changed(self, _: ops.RelationChangedEvent) -> None def _on_cache_config_relation_broken(self, event: ops.RelationBrokenEvent) -> None: """Handle config relation broken event.""" - port_map: dict[str, int] = self._stored.port_map # type: ignore[assignment] - port_map.pop(str(event.relation.id), None) - if not port_map: - self._stored.next_port_offset = 0 - self.unit.set_ports(*port_map.values()) event.relation.data[self.unit]["cache-backend"] = "" + self._load_nginx_config(broken_relation_id=event.relation.id) + + def _on_peer_relation_changed(self, _: ops.RelationChangedEvent) -> None: + """Handle peer relation changed: re-derive nginx from the shared port map.""" self._load_nginx_config() def _rebuild_ca_bundle(self) -> None: @@ -252,13 +255,17 @@ def _update_status_with_nginx(self) -> None: self.unit.status = ops.ActiveStatus() - def _load_nginx_config(self, tls_cert_removed: bool = False) -> None: + def _load_nginx_config( + self, tls_cert_removed: bool = False, broken_relation_id: int | None = None + ) -> None: """Validate the configuration and load to integration. Args: tls_cert_removed: Set to True when called from the certificates relation-broken handler. Bypasses the "waiting for TLS cert" guard so nginx is reconfigured back to HTTP even though the departing relation is still visible to ops. + broken_relation_id: When called from cache-config relation-broken, the id of the + departing relation, so its port is pruned even though ops may still list it. Raises: NginxFileError: File operation errors while updating nginx configuration files. @@ -268,10 +275,28 @@ def _load_nginx_config(self, tls_cert_removed: bool = False) -> None: self._clear_cache_backend() return - ported_config = { - rel_id: (self._get_port_for_relation(rel_id), config) - for rel_id, config in nginx_config.items() - } + if self._peer_relation() is None: + self.unit.status = ops.WaitingStatus(WAIT_FOR_PORT_MESSAGE) + self._clear_cache_backend() + return + + existing_ids = {rel.id for rel in self.model.relations[CACHE_CONFIG_INTEGRATION_NAME]} + if broken_relation_id is not None: + existing_ids.discard(broken_relation_id) + + if self.unit.is_leader(): + port_map = self._ensure_ports(set(nginx_config), existing_ids) + else: + port_map = self._read_port_map() + + ported_config = {} + awaiting_port = False + for rel_id, config in nginx_config.items(): + port = port_map.get(str(rel_id)) + if port is None: + awaiting_port = True + continue + ported_config[rel_id] = (port, config) cache_cert_path = self._get_cache_cert_path() if ( @@ -283,6 +308,11 @@ def _load_nginx_config(self, tls_cert_removed: bool = False) -> None: self._clear_cache_backend() return + if not ported_config: + self.unit.status = ops.WaitingStatus(WAIT_FOR_PORT_MESSAGE) + self._clear_cache_backend() + return + status_message = "" try: nginx_manager.update_and_load_config( @@ -304,9 +334,11 @@ def _load_nginx_config(self, tls_cert_removed: bool = False) -> None: self._update_status_with_nginx() if isinstance(self.unit.status, ops.ActiveStatus): - self.unit.status = ops.ActiveStatus(status_message) - port_map: dict[str, int] = self._stored.port_map # type: ignore[assignment] - self.unit.set_ports(*port_map.values()) + if awaiting_port: + self.unit.status = ops.WaitingStatus(WAIT_FOR_PORT_MESSAGE) + else: + self.unit.status = ops.ActiveStatus(status_message) + self.unit.set_ports(*{port for port, _ in ported_config.values()}) self._write_cache_backends(ported_config, cache_cert_path) else: self._clear_cache_backend() @@ -346,39 +378,85 @@ def _get_config_and_update_status(self) -> NginxConfig | None: self.unit.status = ops.MaintenanceStatus(RECEIVED_NGINX_CONFIG_MESSAGE) return nginx_config - def _get_port_for_relation(self, relation_id: int) -> int: - """Get the nginx listening port assigned to a relation, allocating one if needed. - - Port assignments are persisted in StoredState so the same port is returned - across charm restarts for the same relation. - - New ports are allocated monotonically (like Linux PIDs) to maximise the time - interval before a port number is reused after a relation is removed. + def _peer_relation(self) -> ops.Relation | None: + """Return the peer relation, or None if it is not yet established.""" + return self.model.get_relation(PEER_RELATION_NAME) + + def _read_port_map(self) -> dict[str, int]: + """Read the relation-id -> port map from the peer app databag.""" + rel = self._peer_relation() + if rel is None: + return {} + raw = rel.data[self.app].get(PORT_MAP_FIELD, "") + return json.loads(raw) if raw else {} + + def _read_next_offset(self) -> int: + """Read the monotonic allocation cursor from the peer app databag.""" + rel = self._peer_relation() + if rel is None: + return 0 + raw = rel.data[self.app].get(NEXT_OFFSET_FIELD, "") + return int(raw) if raw else 0 + + def _ensure_ports( + self, valid_relation_ids: set[int], existing_relation_ids: set[int] + ) -> dict[str, int]: + """Leader-only: allocate ports for valid relations, prune removed ones. + + The peer app databag is the single source of truth. Allocation is monotonic + (PID-like) to delay port reuse. Only the leader may call this. Args: - relation_id: The Juju relation ID. + valid_relation_ids: Relation ids that currently have valid config and need a port. + existing_relation_ids: Relation ids that still exist (used to prune stale entries). Returns: - The allocated port number. + The updated relation-id(str) -> port map. + + Raises: + RuntimeError: If the port range is exhausted. """ - key = str(relation_id) - port_map: dict[str, int] = self._stored.port_map # type: ignore[assignment] - if key not in port_map: - used_ports = set(port_map.values()) - next_offset: int = self._stored.next_port_offset # type: ignore[assignment] + rel = self._peer_relation() + if rel is None: + return {} + + port_map = self._read_port_map() + next_offset = self._read_next_offset() + changed = False + + existing_keys = {str(rid) for rid in existing_relation_ids} + for key in list(port_map): + if key not in existing_keys: + del port_map[key] + changed = True + + used = set(port_map.values()) + for rid in valid_relation_ids: + key = str(rid) + if key in port_map: + continue for i in range(NGINX_PORT_RANGE_SIZE): offset = (next_offset + i) % NGINX_PORT_RANGE_SIZE candidate = NGINX_PORT_RANGE_START + offset - if candidate not in used_ports: + if candidate not in used: port_map[key] = candidate - self._stored.next_port_offset = (offset + 1) % NGINX_PORT_RANGE_SIZE + used.add(candidate) + next_offset = (offset + 1) % NGINX_PORT_RANGE_SIZE + changed = True break else: raise RuntimeError( f"Port range exhausted: all {NGINX_PORT_RANGE_SIZE} ports " f"starting at {NGINX_PORT_RANGE_START} are in use" ) - return port_map[key] + + if not port_map: + next_offset = 0 + + if changed: + rel.data[self.app][PORT_MAP_FIELD] = json.dumps(port_map) + rel.data[self.app][NEXT_OFFSET_FIELD] = str(next_offset) + return port_map def _nginx_initialize(self) -> None: """Initialize the nginx instance. diff --git a/content-cache/tests/unit/conftest.py b/content-cache/tests/unit/conftest.py index fdfca269..b2fc515f 100644 --- a/content-cache/tests/unit/conftest.py +++ b/content-cache/tests/unit/conftest.py @@ -89,6 +89,7 @@ def harness_fixture(monkeypatch, mock_nginx_manager: MagicMock) -> Iterator[Harn """ harness = Harness(ContentCacheCharm) harness.add_network("10.0.0.1", endpoint="certificates") + harness.set_leader(True) harness.begin_with_initial_hooks() yield harness harness.cleanup() @@ -98,3 +99,14 @@ def harness_fixture(monkeypatch, mock_nginx_manager: MagicMock) -> Iterator[Harn def charm_fixture(harness: Harness) -> ContentCacheCharm: """The charm fixture.""" return harness.charm + + +@pytest.fixture(name="follower_harness", scope="function") +def follower_harness_fixture(monkeypatch, mock_nginx_manager: MagicMock) -> Iterator[Harness]: + """A non-leader harness for follower-side behavior.""" + harness = Harness(ContentCacheCharm) + harness.add_network("10.0.0.1", endpoint="certificates") + harness.set_leader(False) + harness.begin_with_initial_hooks() + yield harness + harness.cleanup() diff --git a/content-cache/tests/unit/test_charm.py b/content-cache/tests/unit/test_charm.py index c1c20111..d7c506cc 100644 --- a/content-cache/tests/unit/test_charm.py +++ b/content-cache/tests/unit/test_charm.py @@ -3,6 +3,7 @@ """Unit test for the charm.""" +import json from unittest.mock import MagicMock import ops @@ -14,6 +15,9 @@ CACHE_CONFIG_INTEGRATION_NAME, CERTIFICATE_INTEGRATION_NAME, NGINX_NOT_READY_MESSAGE, + NGINX_PORT_RANGE_START, + PEER_RELATION_NAME, + PORT_MAP_FIELD, WAIT_FOR_CONFIG_MESSAGE, WAIT_FOR_TLS_CERT_MESSAGE, ContentCacheCharm, @@ -26,6 +30,13 @@ } +def _peer_port_map(harness: Harness, charm: ContentCacheCharm) -> dict: + """Read the port_map JSON from the peer app databag.""" + peer_rel_id = harness.model.get_relation(PEER_RELATION_NAME).id + raw = harness.get_relation_data(peer_rel_id, charm.app.name).get(PORT_MAP_FIELD, "") + return json.loads(raw) if raw else {} + + def test_start_no_relation(charm: ContentCacheCharm, mock_nginx_manager: MagicMock): """ arrange: A working charm. @@ -239,11 +250,13 @@ def test_get_nginx_config_returns_flat_per_relation_dict( assert config[relation_id].backends[1].host == "10.10.2.2" -def test_unique_port_allocated_per_relation(harness: Harness, charm: ContentCacheCharm): +def test_unique_port_allocated_per_relation( + harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock +): """ - arrange: Charm with two different cache-config integrations. - act: Add both integrations and query their ports. - assert: Each relation gets a unique port in the expected range. + arrange: A leader charm with two different cache-config integrations. + act: Add both integrations (each triggers reconcile). + assert: Each relation gets a unique port in the peer databag, in range. """ rel_id_1 = harness.add_relation( CACHE_CONFIG_INTEGRATION_NAME, @@ -256,32 +269,86 @@ def test_unique_port_allocated_per_relation(harness: Harness, charm: ContentCach app_data=SAMPLE_INTEGRATION_DATA, ) - port_1 = charm._get_port_for_relation(rel_id_1) - port_2 = charm._get_port_for_relation(rel_id_2) + port_map = _peer_port_map(harness, charm) + port_1 = port_map[str(rel_id_1)] + port_2 = port_map[str(rel_id_2)] assert port_1 != port_2 - assert port_1 >= 8080 - assert port_2 >= 8080 + assert port_1 >= NGINX_PORT_RANGE_START + assert port_2 >= NGINX_PORT_RANGE_START -def test_port_stable_for_same_relation(harness: Harness, charm: ContentCacheCharm): +def test_port_stable_for_same_relation( + harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock +): """ - arrange: Charm with a cache-config integration. - act: Query the port for the same relation twice. - assert: Same port is returned both times (stable allocation). + arrange: A leader charm with a cache-config integration. + act: Reconcile twice (add relation, then update-status). + assert: The same port is retained for that relation. """ rel_id = harness.add_relation( CACHE_CONFIG_INTEGRATION_NAME, remote_app="config", app_data=SAMPLE_INTEGRATION_DATA, ) + port_first = _peer_port_map(harness, charm)[str(rel_id)] - port_first = charm._get_port_for_relation(rel_id) - port_second = charm._get_port_for_relation(rel_id) + harness.charm.on.update_status.emit() + port_second = _peer_port_map(harness, charm)[str(rel_id)] assert port_first == port_second +def test_follower_uses_shared_port_from_peer_databag( + follower_harness: Harness, mock_nginx_manager: MagicMock +): + """ + arrange: A non-leader charm with a peer databag pre-seeded with a port for a relation. + act: Add a cache-config relation with valid data (triggers reconcile). + assert: The follower configures nginx with the shared port and does not mutate the map. + """ + harness = follower_harness + charm = harness.charm + peer_rel_id = harness.model.get_relation(PEER_RELATION_NAME).id + + rel_id = harness.add_relation( + CACHE_CONFIG_INTEGRATION_NAME, + remote_app="config", + app_data=SAMPLE_INTEGRATION_DATA, + ) + shared_port = NGINX_PORT_RANGE_START + 5 + harness.update_relation_data( + peer_rel_id, charm.app.name, {PORT_MAP_FIELD: json.dumps({str(rel_id): shared_port})} + ) + harness.charm.on.update_status.emit() + + args, _ = mock_nginx_manager.update_and_load_config.call_args + ported_config = args[0] + assert ported_config[rel_id][0] == shared_port + raw = harness.get_relation_data(peer_rel_id, charm.app.name).get(PORT_MAP_FIELD, "") + assert json.loads(raw) == {str(rel_id): shared_port} + + +def test_follower_waits_when_port_not_yet_assigned( + follower_harness: Harness, mock_nginx_manager: MagicMock +): + """ + arrange: A non-leader charm with an empty peer databag. + act: Add a cache-config relation with valid data. + assert: The unit is in WaitingStatus and writes no cache-backend. + """ + harness = follower_harness + charm = harness.charm + rel_id = harness.add_relation( + CACHE_CONFIG_INTEGRATION_NAME, + remote_app="config", + app_data=SAMPLE_INTEGRATION_DATA, + ) + + assert isinstance(charm.unit.status, ops.WaitingStatus) + assert not harness.get_relation_data(rel_id, charm.unit.name).get("cache-backend") + + def test_load_nginx_config_writes_cache_backend( harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock ): From ed1f537989bbb8ce4158ba90f2d7d50001c91bf0 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 10:57:59 +0700 Subject: [PATCH 03/10] feat(port-sync): release peer port on relation-broken Fix pruning when the last relation is removed: reconcile early-returns before _ensure_ports when no valid config remains, so free the port explicitly in the relation-broken handler (leader-only). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/src/charm.py | 22 ++++++++++++++++++++++ content-cache/tests/unit/test_charm.py | 21 +++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/content-cache/src/charm.py b/content-cache/src/charm.py index a042a2f7..1b3c931f 100755 --- a/content-cache/src/charm.py +++ b/content-cache/src/charm.py @@ -159,6 +159,7 @@ def _on_cache_config_relation_changed(self, _: ops.RelationChangedEvent) -> None def _on_cache_config_relation_broken(self, event: ops.RelationBrokenEvent) -> None: """Handle config relation broken event.""" event.relation.data[self.unit]["cache-backend"] = "" + self._release_port(event.relation.id) self._load_nginx_config(broken_relation_id=event.relation.id) def _on_peer_relation_changed(self, _: ops.RelationChangedEvent) -> None: @@ -458,6 +459,27 @@ def _ensure_ports( rel.data[self.app][NEXT_OFFSET_FIELD] = str(next_offset) return port_map + def _release_port(self, relation_id: int) -> None: + """Leader-only: remove a relation's port from the peer map. + + Called on cache-config relation-broken so the port is freed even when no + valid config remains (in which case reconcile early-returns before + ``_ensure_ports`` would prune it). + + Args: + relation_id: The id of the departing cache-config relation. + """ + rel = self._peer_relation() + if rel is None or not self.unit.is_leader(): + return + port_map = self._read_port_map() + if str(relation_id) not in port_map: + return + del port_map[str(relation_id)] + next_offset = 0 if not port_map else self._read_next_offset() + rel.data[self.app][PORT_MAP_FIELD] = json.dumps(port_map) + rel.data[self.app][NEXT_OFFSET_FIELD] = str(next_offset) + def _nginx_initialize(self) -> None: """Initialize the nginx instance. diff --git a/content-cache/tests/unit/test_charm.py b/content-cache/tests/unit/test_charm.py index d7c506cc..da99c68f 100644 --- a/content-cache/tests/unit/test_charm.py +++ b/content-cache/tests/unit/test_charm.py @@ -390,6 +390,27 @@ def test_relation_broken_clears_cache_backends( assert charm.unit.status == ops.BlockedStatus(WAIT_FOR_CONFIG_MESSAGE) +def test_relation_broken_prunes_peer_port_map( + harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock +): + """ + arrange: A leader charm with a cache-config relation that has a port allocated. + act: Remove the relation. + assert: The port is removed from the peer databag map and the charm blocks. + """ + relation_id = harness.add_relation( + CACHE_CONFIG_INTEGRATION_NAME, + remote_app="config", + app_data=SAMPLE_INTEGRATION_DATA, + ) + assert str(relation_id) in _peer_port_map(harness, charm) + + harness.remove_relation(relation_id) + + assert str(relation_id) not in _peer_port_map(harness, charm) + assert charm.unit.status == ops.BlockedStatus(WAIT_FOR_CONFIG_MESSAGE) + + def test_cache_backend_cleared_when_config_fails( harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock ): From b36c8536fdef900ff4e5d279419a8a280124f860 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 11:00:38 +0700 Subject: [PATCH 04/10] test(port-sync): leader-change continuity and peer-absent guard Also extract _resolve_ported_config to keep _load_nginx_config within the complexity limit, and assert peer relation presence in tests for mypy. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/src/charm.py | 56 ++++++++++++++++++-------- content-cache/tests/unit/test_charm.py | 55 ++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 19 deletions(-) diff --git a/content-cache/src/charm.py b/content-cache/src/charm.py index 1b3c931f..1573ec25 100755 --- a/content-cache/src/charm.py +++ b/content-cache/src/charm.py @@ -256,30 +256,26 @@ def _update_status_with_nginx(self) -> None: self.unit.status = ops.ActiveStatus() - def _load_nginx_config( - self, tls_cert_removed: bool = False, broken_relation_id: int | None = None - ) -> None: - """Validate the configuration and load to integration. + def _resolve_ported_config( + self, nginx_config: NginxConfig, broken_relation_id: int | None + ) -> tuple[dict, bool] | None: + """Resolve each relation's config to its shared port. + + The leader allocates/prunes ports in the peer databag; followers read them. Args: - tls_cert_removed: Set to True when called from the certificates relation-broken - handler. Bypasses the "waiting for TLS cert" guard so nginx is reconfigured - back to HTTP even though the departing relation is still visible to ops. - broken_relation_id: When called from cache-config relation-broken, the id of the - departing relation, so its port is pruned even though ops may still list it. + nginx_config: The valid per-relation nginx configuration. + broken_relation_id: Id of a departing relation to exclude from pruning, if any. - Raises: - NginxFileError: File operation errors while updating nginx configuration files. + Returns: + A ``(ported_config, awaiting_port)`` tuple, or None if the peer relation is not + yet established (in which case the unit status is set to waiting and backends + are cleared). """ - nginx_config = self._get_config_and_update_status() - if nginx_config is None: - self._clear_cache_backend() - return - if self._peer_relation() is None: self.unit.status = ops.WaitingStatus(WAIT_FOR_PORT_MESSAGE) self._clear_cache_backend() - return + return None existing_ids = {rel.id for rel in self.model.relations[CACHE_CONFIG_INTEGRATION_NAME]} if broken_relation_id is not None: @@ -298,6 +294,32 @@ def _load_nginx_config( awaiting_port = True continue ported_config[rel_id] = (port, config) + return ported_config, awaiting_port + + def _load_nginx_config( + self, tls_cert_removed: bool = False, broken_relation_id: int | None = None + ) -> None: + """Validate the configuration and load to integration. + + Args: + tls_cert_removed: Set to True when called from the certificates relation-broken + handler. Bypasses the "waiting for TLS cert" guard so nginx is reconfigured + back to HTTP even though the departing relation is still visible to ops. + broken_relation_id: When called from cache-config relation-broken, the id of the + departing relation, so its port is pruned even though ops may still list it. + + Raises: + NginxFileError: File operation errors while updating nginx configuration files. + """ + nginx_config = self._get_config_and_update_status() + if nginx_config is None: + self._clear_cache_backend() + return + + resolved = self._resolve_ported_config(nginx_config, broken_relation_id) + if resolved is None: + return + ported_config, awaiting_port = resolved cache_cert_path = self._get_cache_cert_path() if ( diff --git a/content-cache/tests/unit/test_charm.py b/content-cache/tests/unit/test_charm.py index da99c68f..228fd574 100644 --- a/content-cache/tests/unit/test_charm.py +++ b/content-cache/tests/unit/test_charm.py @@ -32,7 +32,9 @@ def _peer_port_map(harness: Harness, charm: ContentCacheCharm) -> dict: """Read the port_map JSON from the peer app databag.""" - peer_rel_id = harness.model.get_relation(PEER_RELATION_NAME).id + peer_rel = harness.model.get_relation(PEER_RELATION_NAME) + assert peer_rel is not None + peer_rel_id = peer_rel.id raw = harness.get_relation_data(peer_rel_id, charm.app.name).get(PORT_MAP_FIELD, "") return json.loads(raw) if raw else {} @@ -309,7 +311,9 @@ def test_follower_uses_shared_port_from_peer_databag( """ harness = follower_harness charm = harness.charm - peer_rel_id = harness.model.get_relation(PEER_RELATION_NAME).id + peer_rel = harness.model.get_relation(PEER_RELATION_NAME) + assert peer_rel is not None + peer_rel_id = peer_rel.id rel_id = harness.add_relation( CACHE_CONFIG_INTEGRATION_NAME, @@ -349,6 +353,53 @@ def test_follower_waits_when_port_not_yet_assigned( assert not harness.get_relation_data(rel_id, charm.unit.name).get("cache-backend") +def test_leader_change_preserves_existing_ports( + harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock +): + """ + arrange: A leader charm with a relation and an allocated port. + act: Add a second relation (later reconcile). + assert: The first relation keeps its port; the second gets a different one. + """ + rel_id = harness.add_relation( + CACHE_CONFIG_INTEGRATION_NAME, + remote_app="config", + app_data=SAMPLE_INTEGRATION_DATA, + ) + original_port = _peer_port_map(harness, charm)[str(rel_id)] + + rel_id_2 = harness.add_relation( + CACHE_CONFIG_INTEGRATION_NAME, + remote_app="config2", + app_data=SAMPLE_INTEGRATION_DATA, + ) + port_map = _peer_port_map(harness, charm) + assert port_map[str(rel_id)] == original_port + assert port_map[str(rel_id_2)] != original_port + + +def test_waiting_when_peer_relation_absent( + harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock +): + """ + arrange: A leader charm whose peer relation has been removed. + act: Add a cache-config relation with valid data. + assert: The unit waits for port assignment and does not crash. + """ + peer_rel = harness.model.get_relation(PEER_RELATION_NAME) + assert peer_rel is not None + peer_rel_id = peer_rel.id + harness.remove_relation(peer_rel_id) + + harness.add_relation( + CACHE_CONFIG_INTEGRATION_NAME, + remote_app="config", + app_data=SAMPLE_INTEGRATION_DATA, + ) + + assert isinstance(charm.unit.status, ops.WaitingStatus) + + def test_load_nginx_config_writes_cache_backend( harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock ): From 7f260f181fa2da1a150a11c210d33a30888864dc Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 11:02:45 +0700 Subject: [PATCH 05/10] test(port-sync): assert equal port and distinct IPs across content-cache units Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/tests/integration/test_basic.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/content-cache/tests/integration/test_basic.py b/content-cache/tests/integration/test_basic.py index 3d51199d..1366212e 100644 --- a/content-cache/tests/integration/test_basic.py +++ b/content-cache/tests/integration/test_basic.py @@ -5,6 +5,7 @@ import json from asyncio import sleep +from urllib.parse import urlparse import pytest from juju.application import Application @@ -272,3 +273,15 @@ async def test_cache_backends_published( assert backend.startswith("http://") assert ":30000" in backend or ":30001" in backend + + # Scale to 2 units: every unit must serve this relation on the SAME port + # (with distinct IPs), proving cross-unit port synchronization via the peer relation. + await app.add_unit(count=1) + await model.wait_for_idle([app.name], status="active", timeout=10 * 60) + + backends = [await get_cache_backend(cache_unit) for cache_unit in app.units] + assert len(backends) >= 2, f"expected >= 2 content-cache units, got {backends}" + ports = {urlparse(cache_backend).port for cache_backend in backends} + hosts = {urlparse(cache_backend).hostname for cache_backend in backends} + assert len(ports) == 1, f"expected one shared port, got {ports} from {backends}" + assert len(hosts) == len(backends), f"expected distinct IPs per unit, got {hosts}" From 9ea558f8982baeef2538f92432baf6b116e974a0 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 11:03:46 +0700 Subject: [PATCH 06/10] docs(port-sync): describe shared cache-config port across units Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/explanation/charm-design.md | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/docs/explanation/charm-design.md b/docs/explanation/charm-design.md index ee056edb..db1d2b56 100644 --- a/docs/explanation/charm-design.md +++ b/docs/explanation/charm-design.md @@ -70,23 +70,33 @@ directive (set at the server block level) ties this location to its dedicated ca The charm allocates a unique TCP port to each `cache-config` relation. Ports are assigned from a fixed range starting at `30000` and are stable across charm restarts. The same -relation always receives the same port for the lifetime of that relation, stored via Juju's -`StoredState`. +relation always receives the same port for its lifetime. + +To keep the backend list consumers see uniform, **all units of the content-cache +application serve a given relation on the same port**. Port assignments are coordinated +through a peer relation (`content-cache-peers`): the **leader** unit is the sole allocator +and writes the relation-to-port map into the peer relation's application databag, which is +the single source of truth. Every unit (leader and followers) reads that map and configures +its nginx to listen on the assigned port. A follower that has not yet observed a port for a +relation reports a waiting status until the leader publishes it. Ports are allocated monotonically, so that when a relation is removed and a new one is added, the new relation receives the next port in sequence rather than immediately reusing the freed port. This maximises the time before a port number is reused, -reducing the risk of ingress routing conflicts during rapid relation cycling. +reducing the risk of ingress routing conflicts during rapid relation cycling. Because the +allocation state lives in the peer application databag, it survives leader changes: a newly +elected leader continues from the existing map without reallocating ports. -This means each configured backend is reachable at a distinct port on the content-cache -unit's IP address: +This means each configured backend is reachable at the same port across every unit, on that +unit's own IP address: - `http://:30000` contains the backends for the first `cache-config` relation - `http://:30001` contains the backends for the second `cache-config` relation An ingress component (such as `haproxy` with the `ingress-configurator` charm) is expected -to sit in front of the content-cache unit and route incoming requests to the appropriate -port based on hostname or path rules. +to sit in front of the content-cache units and route incoming requests to the appropriate +port based on hostname or path rules, load-balancing across the units of a relation (same +port, distinct IPs). ## Cache storage From 0ada4174fb2bb6fb73d1cd9c5de42cedf1333a86 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 14:07:03 +0700 Subject: [PATCH 07/10] fix(port-sync): exclude departing relation from port resolve, guard peer event before nginx install Address Copilot review: _resolve_ported_config now filters broken_relation_id out of nginx_config itself (not just the pruning existing_ids set), so a relation-broken hook can never re-allocate a port or republish backends for the relation that is departing, even if remote data for it is still visible. Also guard _on_peer_relation_changed to skip reconciling when nginx has not been installed yet: on unit add, the peers relation-created event can fire before the start hook runs, and calling _load_nginx_config() that early tried to restart a not-yet-installed nginx service (observed as an integration test failure: cache/1 unit in error state, 'Failed to restart nginx.service: Unit nginx.service not found'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/src/charm.py | 22 ++++++++++++++++-- content-cache/tests/unit/test_charm.py | 32 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/content-cache/src/charm.py b/content-cache/src/charm.py index 1573ec25..af9f5ebc 100755 --- a/content-cache/src/charm.py +++ b/content-cache/src/charm.py @@ -163,7 +163,15 @@ def _on_cache_config_relation_broken(self, event: ops.RelationBrokenEvent) -> No self._load_nginx_config(broken_relation_id=event.relation.id) def _on_peer_relation_changed(self, _: ops.RelationChangedEvent) -> None: - """Handle peer relation changed: re-derive nginx from the shared port map.""" + """Handle peer relation changed: re-derive nginx from the shared port map. + + Peer relation-created can fire before the ``start`` hook has installed nginx + (e.g. on unit add). Skip reconciling in that case; ``_on_start`` will load the + config once nginx is installed, and later cache-config/peer events will pick up + any port map changes missed in the meantime. + """ + if not Path(nginx_manager.NGINX_BIN).exists(): + return self._load_nginx_config() def _rebuild_ca_bundle(self) -> None: @@ -265,7 +273,10 @@ def _resolve_ported_config( Args: nginx_config: The valid per-relation nginx configuration. - broken_relation_id: Id of a departing relation to exclude from pruning, if any. + broken_relation_id: Id of a departing relation to exclude from allocation, + publication and pruning, if any. During relation-broken, remote data for + this relation may still be visible to ops, so it must be excluded + explicitly rather than relying on it being absent from ``nginx_config``. Returns: A ``(ported_config, awaiting_port)`` tuple, or None if the peer relation is not @@ -277,6 +288,13 @@ def _resolve_ported_config( self._clear_cache_backend() return None + if broken_relation_id is not None: + nginx_config = { + rel_id: config + for rel_id, config in nginx_config.items() + if rel_id != broken_relation_id + } + existing_ids = {rel.id for rel in self.model.relations[CACHE_CONFIG_INTEGRATION_NAME]} if broken_relation_id is not None: existing_ids.discard(broken_relation_id) diff --git a/content-cache/tests/unit/test_charm.py b/content-cache/tests/unit/test_charm.py index 228fd574..b7c8bb00 100644 --- a/content-cache/tests/unit/test_charm.py +++ b/content-cache/tests/unit/test_charm.py @@ -462,6 +462,38 @@ def test_relation_broken_prunes_peer_port_map( assert charm.unit.status == ops.BlockedStatus(WAIT_FOR_CONFIG_MESSAGE) +def test_relation_broken_excludes_stale_config_for_departing_relation( + harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock +): + """ + arrange: A leader charm with a port already allocated for a relation, then release it + as `_on_cache_config_relation_broken` would. + act: Resolve the ported config with a stale ``nginx_config`` entry that still contains + the departing relation, as can happen when remote data for the breaking relation + is still visible. + assert: The departing relation is excluded from both port allocation and the returned + config (i.e. its port is not re-added to the peer map and it is not republished). + """ + relation_id = harness.add_relation( + CACHE_CONFIG_INTEGRATION_NAME, + remote_app="config", + app_data=SAMPLE_INTEGRATION_DATA, + ) + assert str(relation_id) in _peer_port_map(harness, charm) + + charm._release_port(relation_id) + assert str(relation_id) not in _peer_port_map(harness, charm) + + stale_nginx_config = {relation_id: MagicMock()} + resolved = charm._resolve_ported_config(stale_nginx_config, broken_relation_id=relation_id) + + assert resolved is not None + ported_config, awaiting_port = resolved + assert relation_id not in ported_config + assert awaiting_port is False + assert str(relation_id) not in _peer_port_map(harness, charm) + + def test_cache_backend_cleared_when_config_fails( harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock ): From 9e3ffd93ea2fb5836efa35ff005529092c9ab4eb Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 14:07:11 +0700 Subject: [PATCH 08/10] fix(port-sync): avoid python-libjuju crash on peer relations in integration tests Application.related_applications() unconditionally unpacks Relation.endpoints as a 2-tuple, but a peer relation only ever has a single endpoint entry. Now that content-cache declares the content-cache-peers peer relation, any call to app.related_applications() on the content-cache app raises 'ValueError: not enough values to unpack (expected 2, got 1)', which broke test_tls_cert.py's certificate_transfer and certificates cleanup checks in CI. Add has_related_application(), a peer-relation-safe replacement, and use it in place of the two affected app.related_applications() calls. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/tests/integration/helpers.py | 25 +++++++++++++++++++ .../tests/integration/test_tls_cert.py | 5 ++-- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/content-cache/tests/integration/helpers.py b/content-cache/tests/integration/helpers.py index a331a28a..a5bd848f 100644 --- a/content-cache/tests/integration/helpers.py +++ b/content-cache/tests/integration/helpers.py @@ -433,6 +433,31 @@ def _start_server(self, cert_pem: str): return app +def has_related_application(app: Application, endpoint_name: str) -> bool: + """Check whether the app has a relation on the given local endpoint. + + A peer-relation-safe replacement for ``Application.related_applications``, + which raises ``ValueError: not enough values to unpack`` when the app (such as + content-cache, which declares a ``content-cache-peers`` peer relation) has any + peer relation: python-libjuju unconditionally unpacks ``Relation.endpoints`` as + a 2-tuple, but peer relations only ever have a single endpoint entry. + + Args: + app: The application to check. + endpoint_name: The local endpoint name to look for. + + Returns: + True if the app has an active relation on the given endpoint. + """ + for rel in app.relations: + if rel.is_peer: + continue + local_ep = next(ep for ep in rel.endpoints if ep.application_name == app.name) + if local_ep.name == endpoint_name: + return True + return False + + async def get_app_ip(app: Application) -> str: """Get the IP for a unit of the application. diff --git a/content-cache/tests/integration/test_tls_cert.py b/content-cache/tests/integration/test_tls_cert.py index 1b4f3831..eecc64ee 100644 --- a/content-cache/tests/integration/test_tls_cert.py +++ b/content-cache/tests/integration/test_tls_cert.py @@ -17,6 +17,7 @@ CacheTester, get_app_ip, get_cache_backend, + has_related_application, run_in_unit, ) from juju.application import Application @@ -97,7 +98,7 @@ async def test_certificate_transfer_full_lifecycle( response.status_code == 502 ), "Expected 502 after cert-transfer removal: CA untrusted" finally: - if app.related_applications(CERTIFICATE_TRANSFER_INTEGRATION_NAME): + if has_related_application(app, CERTIFICATE_TRANSFER_INTEGRATION_NAME): await app.remove_relation( CERTIFICATE_TRANSFER_INTEGRATION_NAME, https_cert_ok_app.name ) @@ -164,5 +165,5 @@ async def test_tls_termination_full_lifecycle( finally: # Ensure the certificates relation is removed even if the test fails. # Do NOT use block_until_done=True here — it has no timeout and can hang forever. - if app.related_applications(CERTIFICATES_INTEGRATION_NAME): + if has_related_application(app, CERTIFICATES_INTEGRATION_NAME): await app.remove_relation(CERTIFICATES_INTEGRATION_NAME, cache_lego_app.name) From b6548b93b26f38d3bce8c3e1ec918797cfdcd231 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 14:07:15 +0700 Subject: [PATCH 09/10] docs(port-sync): fix Vale spelling check failures (databag, IPs) Reword to 'data bag' and 'IP addresses' to satisfy the rtd-docs-checks spell check, which flagged 'databag' and 'IPs' as misspelled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/explanation/charm-design.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/explanation/charm-design.md b/docs/explanation/charm-design.md index db1d2b56..d3d3a79b 100644 --- a/docs/explanation/charm-design.md +++ b/docs/explanation/charm-design.md @@ -75,7 +75,7 @@ relation always receives the same port for its lifetime. To keep the backend list consumers see uniform, **all units of the content-cache application serve a given relation on the same port**. Port assignments are coordinated through a peer relation (`content-cache-peers`): the **leader** unit is the sole allocator -and writes the relation-to-port map into the peer relation's application databag, which is +and writes the relation-to-port map into the peer relation's application data bag, which is the single source of truth. Every unit (leader and followers) reads that map and configures its nginx to listen on the assigned port. A follower that has not yet observed a port for a relation reports a waiting status until the leader publishes it. @@ -84,7 +84,7 @@ Ports are allocated monotonically, so that when a relation is removed and a new one is added, the new relation receives the next port in sequence rather than immediately reusing the freed port. This maximises the time before a port number is reused, reducing the risk of ingress routing conflicts during rapid relation cycling. Because the -allocation state lives in the peer application databag, it survives leader changes: a newly +allocation state lives in the peer application data bag, it survives leader changes: a newly elected leader continues from the existing map without reallocating ports. This means each configured backend is reachable at the same port across every unit, on that @@ -96,7 +96,7 @@ unit's own IP address: An ingress component (such as `haproxy` with the `ingress-configurator` charm) is expected to sit in front of the content-cache units and route incoming requests to the appropriate port based on hostname or path rules, load-balancing across the units of a relation (same -port, distinct IPs). +port, distinct IP addresses). ## Cache storage From 40509985eb0708c1ee434a17d80a5a0b5cdcaf90 Mon Sep 17 00:00:00 2001 From: florentianayuwono Date: Mon, 7 Sep 2026 15:20:54 +0700 Subject: [PATCH 10/10] fix(port-sync): deterministic port order, stable JSON encoding, safer endpoint lookup Address second-pass Copilot review and weiiwang01's review comments: - _ensure_ports now allocates ports for newly-valid relations in sorted (ascending) relation-id order instead of iterating a set, so allocation is reproducible when several relations become valid in the same reconcile (e.g. after a leader restart). - json.dumps(..., sort_keys=True) for the peer port_map field, for stable encoding. - Rename PORT_MAP_FIELD/NEXT_OFFSET_FIELD to kebab-case ('port-map', 'next-offset') for consistency with other relation data field naming. - has_related_application() no longer raises StopIteration when a relation has no endpoint matching the local app name; it now safely returns False for that relation instead of blowing up test cleanup (finally blocks) and leaking relations. - Add test_port_allocation_order_is_deterministic to cover the ordering fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- content-cache/src/charm.py | 10 +++++----- content-cache/tests/integration/helpers.py | 4 ++-- content-cache/tests/unit/test_charm.py | 18 ++++++++++++++++++ 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/content-cache/src/charm.py b/content-cache/src/charm.py index af9f5ebc..573a2b1e 100755 --- a/content-cache/src/charm.py +++ b/content-cache/src/charm.py @@ -56,8 +56,8 @@ NGINX_PORT_RANGE_SIZE = 200 PEER_RELATION_NAME = "content-cache-peers" -PORT_MAP_FIELD = "port_map" -NEXT_OFFSET_FIELD = "next_offset" +PORT_MAP_FIELD = "port-map" +NEXT_OFFSET_FIELD = "next-offset" class ContentCacheCharm(ops.CharmBase): @@ -472,7 +472,7 @@ def _ensure_ports( changed = True used = set(port_map.values()) - for rid in valid_relation_ids: + for rid in sorted(valid_relation_ids): key = str(rid) if key in port_map: continue @@ -495,7 +495,7 @@ def _ensure_ports( next_offset = 0 if changed: - rel.data[self.app][PORT_MAP_FIELD] = json.dumps(port_map) + rel.data[self.app][PORT_MAP_FIELD] = json.dumps(port_map, sort_keys=True) rel.data[self.app][NEXT_OFFSET_FIELD] = str(next_offset) return port_map @@ -517,7 +517,7 @@ def _release_port(self, relation_id: int) -> None: return del port_map[str(relation_id)] next_offset = 0 if not port_map else self._read_next_offset() - rel.data[self.app][PORT_MAP_FIELD] = json.dumps(port_map) + rel.data[self.app][PORT_MAP_FIELD] = json.dumps(port_map, sort_keys=True) rel.data[self.app][NEXT_OFFSET_FIELD] = str(next_offset) def _nginx_initialize(self) -> None: diff --git a/content-cache/tests/integration/helpers.py b/content-cache/tests/integration/helpers.py index a5bd848f..bef9df0e 100644 --- a/content-cache/tests/integration/helpers.py +++ b/content-cache/tests/integration/helpers.py @@ -452,8 +452,8 @@ def has_related_application(app: Application, endpoint_name: str) -> bool: for rel in app.relations: if rel.is_peer: continue - local_ep = next(ep for ep in rel.endpoints if ep.application_name == app.name) - if local_ep.name == endpoint_name: + local_ep = next((ep for ep in rel.endpoints if ep.application_name == app.name), None) + if local_ep is not None and local_ep.name == endpoint_name: return True return False diff --git a/content-cache/tests/unit/test_charm.py b/content-cache/tests/unit/test_charm.py index b7c8bb00..cc576d8e 100644 --- a/content-cache/tests/unit/test_charm.py +++ b/content-cache/tests/unit/test_charm.py @@ -280,6 +280,24 @@ def test_unique_port_allocated_per_relation( assert port_2 >= NGINX_PORT_RANGE_START +def test_port_allocation_order_is_deterministic( + harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock +): + """ + arrange: A leader charm with no ports allocated yet. + act: Allocate ports for several relations that all become valid in one reconcile, + passing their ids in a deliberately unsorted order. + assert: Ports are assigned by ascending relation id, regardless of input/set + iteration order, so allocation is reproducible. + """ + relation_ids = {30, 10, 20} + + port_map = charm._ensure_ports(relation_ids, relation_ids) + + ports_by_id = {rid: port_map[str(rid)] for rid in relation_ids} + assert ports_by_id[10] < ports_by_id[20] < ports_by_id[30] + + def test_port_stable_for_same_relation( harness: Harness, charm: ContentCacheCharm, mock_nginx_manager: MagicMock ):