Consider ingress revoked in utests (#94)

* Consider traefik removal in utest

* Fix ingress (traefik lib changed locally)

* Update lib

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"
@@ -273,6 +273,16 @@ def _handle_relation_broken(self, event):
 
     def wipe_ingress_data(self, relation: Relation):
         """Clear ingress data from relation."""
+        assert self.unit.is_leader(), "only leaders can do this"
+        try:
+            relation.data
+        except ModelError as e:
+            log.warning(
+                "error {} accessing relation data for {!r}. "
+                "Probably a ghost of a dead relation is still "
+                "lingering around.".format(e, relation.name)
+            )
+            return
         del relation.data[self.app]["ingress"]
 
     def _get_requirer_data(self, relation: Relation) -> RequirerData:
@@ -435,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):
@@ -451,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
@@ -475,6 +490,7 @@ def provide_ingress_requirements(self, *, host: str = None, port: int):
         # require one unit to publish it -- it will not differ between units,
         # unlike in ingress-per-unit.
         assert self.unit.is_leader(), "only leaders should do this."
+        assert self.relation, "no relation"
 
         if not host:
             host = socket.getfqdn()
@@ -493,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:
@@ -519,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

pyproject.toml

@@ -52,14 +52,19 @@ allow_redefinition = true
 
 # Ignore libraries that do not have type hint nor stubs
 [[tool.mypy.overrides]]
-module = ["ops.*", "lightkube.*", "git.*", "pytest_operator.*", "validators.*"]
+module = ["ops.*", "lightkube.*", "git.*", "validators.*"]
 ignore_missing_imports = true
 
 [[tool.mypy.overrides]]
 module = ["charms.grafana_k8s.*", "charms.traefik_k8s.*", "charms.observability_libs.*"]
 follow_imports = "silent"
 warn_unused_ignores = false
 
+[[tool.mypy.overrides]]
+module = ["pytest_operator.*"]
+follow_imports = "skip"
+ignore_missing_imports = true
+
 [tool.pytest.ini_options]
 minversion = "6.0"
 log_cli_level = "INFO"

tests/unit/test_external_url.py

@@ -127,6 +127,17 @@ def test_config_option_overrides_traefik(self):
         self.assertEqual(self.get_url_cli_arg(), external_url_ingress)
         self.assertTrue(self.is_service_running())
 
+        # AND WHEN the traefik relation is removed
+        self.harness.remove_relation_unit(rel_id, "traefik-app/0")
+        self.harness.remove_relation(rel_id)
+
+        # NOTE intentionally not emptying out relation data manually
+        # app_data = {"ingress": ""}
+        # self.harness.update_relation_data(rel_id, "traefik-app", app_data)
+
+        # THEN the fqdn is used as external url
+        self.assertEqual(self.get_url_cli_arg(), self.fqdn_url)
+
     @patch.object(AlertmanagerCharm, "_check_config", lambda *a, **kw: ("ok", ""))
     @patch("socket.getfqdn", new=lambda *args: "fqdn")
     @k8s_resource_multipatch