[IPA] Make sure ingress.url returns an up-to-date value (#84)

lib/charms/traefik_k8s/v1/ingress.py

@@ -57,7 +57,7 @@ def _on_ingress_revoked(self, event: IngressPerAppRevokedEvent):
 from typing import Any, Dict, Optional, Tuple
 
 import yaml
-from ops.charm import CharmBase, RelationEvent
+from ops.charm import CharmBase, RelationBrokenEvent, RelationEvent
 from ops.framework import EventSource, Object, ObjectEvents, StoredState
 from ops.model import ModelError, Relation
 
@@ -69,7 +69,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 = 2
+LIBPATCH = 3
 
 DEFAULT_RELATION_NAME = "ingress"
 RELATION_INTERFACE = "ingress"
@@ -445,12 +445,17 @@ def _handle_relation(self, event):
 
         if self.is_ready():
             # Avoid spurious events, emit only when there is a NEW URL available
-            new_url = self.url
+            new_url = (
+                None
+                if isinstance(event, RelationBrokenEvent)
+                else self._get_url_from_relation_data()
+            )
             if self._stored.current_url != new_url:
                 self._stored.current_url = new_url
                 self.on.ready.emit(event.relation, new_url)
 
     def _handle_relation_broken(self, event):
+        self._stored.current_url = None
         self.on.revoked.emit(event.relation)
 
     def _handle_upgrade_or_leader(self, event):
@@ -461,7 +466,7 @@ def _handle_upgrade_or_leader(self, event):
     def is_ready(self):
         """The Requirer is ready if the Provider has sent valid data."""
         try:
-            return bool(self.url)
+            return bool(self._get_url_from_relation_data())
         except DataValidationError as e:
             log.warning("Requirer not ready; validation error encountered: %s" % str(e))
             return False
@@ -504,11 +509,10 @@ def relation(self):
         """The established Relation instance, or None."""
         return self.relations[0] if self.relations else None
 
-    @property
-    def url(self) -> Optional[str]:
+    def _get_url_from_relation_data(self) -> Optional[str]:
         """The full ingress URL to reach the current unit.
 
-        May return None if the URL isn't available yet.
+        Returns None if the URL isn't available yet.
         """
         relation = self.relation
         if not relation:
@@ -530,3 +534,13 @@ def url(self) -> Optional[str]:
         ingress: ProviderIngressData = yaml.safe_load(raw)
         _validate_data({"ingress": ingress}, INGRESS_PROVIDES_APP_SCHEMA)
         return ingress["url"]
+
+    @property
+    def url(self) -> Optional[str]:
+        """The full ingress URL to reach the current unit.
+
+        Returns None if the URL isn't available yet.
+        """
+        data = self._stored.current_url or None  # type: ignore
+        assert isinstance(data, (str, type(None)))  # for static checker
+        return data

tests/unit/test_lib_per_app_requires.py

@@ -1,5 +1,6 @@
 # Copyright 2022 Canonical Ltd.
 # See LICENSE file for licensing details.
+import unittest
 from textwrap import dedent
 
 import pytest
@@ -9,6 +10,7 @@
     DataValidationError,
     IngressPerAppReadyEvent,
     IngressPerAppRequirer,
+    IngressPerAppRevokedEvent,
 )
 from ops.charm import CharmBase
 from ops.testing import Harness
@@ -89,3 +91,51 @@ def test_validator(requirer: IngressPerAppRequirer, harness, auto_data, ok):
     else:
         host, port = auto_data
         requirer.provide_ingress_requirements(host=host, port=port)
+
+
+class TestIPAEventsEmission(unittest.TestCase):
+    class _RequirerCharm(CharmBase):
+        META = dedent(
+            """\
+            name: ipa-requirer
+            requires:
+              ingress:
+                interface: ingress
+                limit: 1
+            """
+        )
+
+        def __init__(self, *args, **kwargs):
+            super().__init__(*args, **kwargs)
+            self.ipa = IngressPerAppRequirer(self, relation_name="ingress", port=80)
+
+    def setUp(self):
+        self.harness = Harness(self._RequirerCharm, meta=self._RequirerCharm.META)
+        self.addCleanup(self.harness.cleanup)
+
+        self.harness.set_model_name(self.__class__.__name__)
+        self.harness.begin_with_initial_hooks()
+
+    def test_ipa_events(self):
+        # WHEN an ingress relation is formed
+        # THEN the ready event is emitted
+        with capture(self.harness.charm, IngressPerAppReadyEvent):
+            self.rel_id = self.harness.add_relation("ingress", "traefik-app")
+            self.harness.add_relation_unit(self.rel_id, "traefik-app/0")
+
+            # AND an ingress is in effect
+            data = {"url": "http://a.b/c"}
+            self.harness.update_relation_data(
+                self.rel_id, "traefik-app", {"ingress": yaml.safe_dump(data)}
+            )
+            self.assertEqual(self.harness.charm.ipa.url, "http://a.b/c")
+
+        # WHEN a relation with traefik is removed
+        # THEN a revoked event fires
+        with capture(self.harness.charm, IngressPerAppRevokedEvent):
+            self.harness.remove_relation_unit(self.rel_id, "traefik-app/0")
+            self.harness.remove_relation(self.rel_id)
+            # NOTE intentionally not emptying out relation data manually
+
+        # AND ingress.url returns a false-y value
+        self.assertFalse(self.harness.charm.ipa.url)