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..573a2b1e 100755 --- a/content-cache/src/charm.py +++ b/content-cache/src/charm.py @@ -50,16 +50,19 @@ 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 +PEER_RELATION_NAME = "content-cache-peers" +PORT_MAP_FIELD = "port-map" +NEXT_OFFSET_FIELD = "next-offset" + class ContentCacheCharm(ops.CharmBase): """Charm the application.""" - _stored = ops.StoredState() - def __init__(self, framework: ops.Framework) -> None: """Initialize the object. @@ -68,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 @@ -87,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, @@ -150,12 +158,20 @@ 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._release_port(event.relation.id) + 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. + + 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: @@ -248,13 +264,67 @@ def _update_status_with_nginx(self) -> None: self.unit.status = ops.ActiveStatus() - def _load_nginx_config(self, tls_cert_removed: bool = False) -> None: + 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: + nginx_config: The valid per-relation nginx configuration. + 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 + yet established (in which case the unit status is set to waiting and backends + are cleared). + """ + if self._peer_relation() is None: + self.unit.status = ops.WaitingStatus(WAIT_FOR_PORT_MESSAGE) + 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) + + 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) + 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. @@ -264,10 +334,10 @@ 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() - } + 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 ( @@ -279,6 +349,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( @@ -300,9 +375,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() @@ -342,39 +419,106 @@ 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 sorted(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, sort_keys=True) + 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, sort_keys=True) + 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/integration/helpers.py b/content-cache/tests/integration/helpers.py index a331a28a..bef9df0e 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), None) + if local_ep is not None and 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_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}" 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) 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..cc576d8e 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,15 @@ } +def _peer_port_map(harness: Harness, charm: ContentCacheCharm) -> dict: + """Read the port_map JSON from the peer app databag.""" + 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 {} + + def test_start_no_relation(charm: ContentCacheCharm, mock_nginx_manager: MagicMock): """ arrange: A working charm. @@ -239,11 +252,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 +271,153 @@ 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_allocation_order_is_deterministic( + 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 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 +): + """ + 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 = 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, + 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_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 ): @@ -323,6 +459,59 @@ 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_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 ): diff --git a/docs/explanation/charm-design.md b/docs/explanation/charm-design.md index ee056edb..d3d3a79b 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 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. 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 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 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 IP addresses). ## Cache storage