From b57449ea408b1e4feddbdf66d78a43f30007595f Mon Sep 17 00:00:00 2001 From: Leon <82407168+sed-i@users.noreply.github.com> Date: Tue, 30 Aug 2022 15:55:53 +0000 Subject: [PATCH] [IPA] Make sure `ingress.url` returns an up-to-date value (#84) --- lib/charms/traefik_k8s/v1/ingress.py | 28 ++++++++++---- tests/unit/test_lib_per_app_requires.py | 50 +++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 7 deletions(-) diff --git a/lib/charms/traefik_k8s/v1/ingress.py b/lib/charms/traefik_k8s/v1/ingress.py index 03934930..fbf611ee 100644 --- a/lib/charms/traefik_k8s/v1/ingress.py +++ b/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 diff --git a/tests/unit/test_lib_per_app_requires.py b/tests/unit/test_lib_per_app_requires.py index f92b7a33..0a47ff62 100644 --- a/tests/unit/test_lib_per_app_requires.py +++ b/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)