From 26e89bc64953dd3402da0d3036fbabb87f4cecf4 Mon Sep 17 00:00:00 2001 From: Weii Wang Date: Sun, 24 Nov 2024 20:58:59 +0800 Subject: [PATCH] IngressPerAppRequirer: retry when IP is not available --- lib/charms/traefik_k8s/v2/ingress.py | 22 ++++++++++++++++++---- tests/scenario/test_ingress_per_app.py | 19 ++++++++++++++++++- tests/unit/test_lib_per_app_requires.py | 12 +++--------- 3 files changed, 39 insertions(+), 14 deletions(-) diff --git a/lib/charms/traefik_k8s/v2/ingress.py b/lib/charms/traefik_k8s/v2/ingress.py index 4c7c12ff..13b9406a 100644 --- a/lib/charms/traefik_k8s/v2/ingress.py +++ b/lib/charms/traefik_k8s/v2/ingress.py @@ -84,7 +84,7 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 14 +LIBPATCH = 15 PYDEPS = ["pydantic"] @@ -338,6 +338,10 @@ class DataValidationError(IngressError): """Raised when data validation fails on IPU relation data.""" +class IpNotAvailableError(IngressError): + """Raised when retrieving ip information from juju failed.""" + + class _IngressPerAppBase(Object): """Base class for IngressPerUnit interface classes.""" @@ -710,7 +714,12 @@ def __init__( def _handle_relation(self, event): # created, joined or changed: if we have auto data: publish it - self._publish_auto_data() + try: + self._publish_auto_data() + except IpNotAvailableError as e: + log.error(e) + event.defer() + return if self.is_ready(): # Avoid spurious events, emit only when there is a NEW URL available new_url = ( @@ -728,7 +737,12 @@ def _handle_relation_broken(self, event): def _handle_upgrade_or_leader(self, event): """On upgrade/leadership change: ensure we publish the data we have.""" - self._publish_auto_data() + try: + self._publish_auto_data() + except IpNotAvailableError as e: + log.error(e) + event.defer() + return def is_ready(self): """The Requirer is ready if the Provider has sent valid data.""" @@ -794,7 +808,7 @@ def _publish_unit_data( ): ip = str(bind_address) else: - log.error("failed to retrieve ip information from juju") + raise IpNotAvailableError("failed to retrieve ip information from juju") unit_databag = relation.data[self.unit] try: diff --git a/tests/scenario/test_ingress_per_app.py b/tests/scenario/test_ingress_per_app.py index 878fef1d..572e99af 100644 --- a/tests/scenario/test_ingress_per_app.py +++ b/tests/scenario/test_ingress_per_app.py @@ -15,7 +15,7 @@ IngressRequirerUnitData, ) from ops import CharmBase, Framework -from scenario import Context, Model, Mount, Relation, State +from scenario import Context, Model, Mount, Network, Relation, State from tests.scenario._utils import create_ingress_relation from tests.scenario.conftest import MOCK_LB_ADDRESS @@ -188,6 +188,23 @@ def test_ingress_per_app_requirer_with_auto_data(host, ip, port, model, evt_name } +@pytest.mark.parametrize("evt_name", ("joined", "changed")) +@pytest.mark.parametrize("leader", (True, False)) +def test_ingress_per_app_requirer_retry_without_ip(model, evt_name, leader): + ipa = Relation("ingress") + state = State( + model=model, + leader=leader, + relations=[ipa], + networks={"ingress": Network(bind_addresses=[], egress_subnets=[], ingress_addresses=[])}, + ) + requirer_ctx = get_requirer_ctx("10.0.0.1", None, 80) + state_out = requirer_ctx.run(getattr(ipa, evt_name + "_event"), state) + + assert len(state_out.deferred) == 1 + assert state_out.deferred[0].name == f"ingress_relation_{evt_name}" + + def test_ingress_per_app_cleanup_on_remove(model, traefik_ctx, traefik_container): """Check that config file is removed when a relation is.""" ipa = create_ingress_relation() diff --git a/tests/unit/test_lib_per_app_requires.py b/tests/unit/test_lib_per_app_requires.py index 5652f6b2..adf68142 100644 --- a/tests/unit/test_lib_per_app_requires.py +++ b/tests/unit/test_lib_per_app_requires.py @@ -12,6 +12,7 @@ IngressPerAppReadyEvent, IngressPerAppRequirer, IngressPerAppRevokedEvent, + IpNotAvailableError, ) from ops.charm import CharmBase from ops.testing import Harness @@ -157,12 +158,5 @@ def test_ipa_events_juju_binding_failure(self): self.harness.begin_with_initial_hooks() self.rel_id = self.harness.add_relation("ingress", "traefik-app") self.harness.add_relation_unit(self.rel_id, "traefik-app/0") - self.harness.charm.ipa.provide_ingress_requirements(port=80) - - self.assertEqual( - # None is default so it gets omitted - self.harness.get_relation_data(self.rel_id, "ipa-requirer/0").get( - "ip", "" - ), - "", - ) + with self.assertRaises(IpNotAvailableError): + self.harness.charm.ipa.provide_ingress_requirements(port=80)