fix: handle ingress relation-broken when event.app==None (#291)

* fix: handle ingress relation-broken if event.app==None

Fixes [this issue](#189).

While handling relation-broken events for ingress, we see either:
* event.app=the_departing_app
* event.app=None

The previous charm logic did not cover the second case, leading to an uncaught KeyError.

We aren't sure what controls which of the situations we encounter, but we add error handling here to at least log the event and avoid uncaught exceptions.

* tests: add unit test case for relation broken event

Adding a test case that confirms we call the logger when handling a relation broken event
and the event.app == None.
---------

Co-authored-by: Daniela Plascencia <[email protected]>

charms/istio-pilot/src/charm.py

@@ -431,10 +431,18 @@ def _get_ingress_data(self, event) -> dict:
         # Get all route data from the ingress interface
         routes = get_routes_from_ingress_interface(ingress_interface, self.app)
 
-        # The app-level data is still visible on a broken relation, but we
+        # The app-level data may still be visible on a broken relation, but we
         # shouldn't be keeping the VirtualService for that related app.
         if isinstance(event, RelationBrokenEvent) and event.relation.name == INGRESS_RELATION_NAME:
-            routes.pop((event.relation, event.app))
+            if event.app is None:
+                self.log.info(
+                    f"When handling event '{event}', event.app found to be None.  We cannot pop"
+                    f" the departing application's data from 'routes' because we do not know the"
+                    f" departing application's name.  We assume that the departing application's"
+                    f" is not in routes.keys='{list(routes.keys())}'."
+                )
+            else:
+                routes.pop((event.relation, event.app))
 
         return routes
 

charms/istio-pilot/tests/unit/test_charm.py

@@ -591,6 +591,21 @@ def test_get_ingress_data_for_broken_event(self, harness):
         this_relation = harness.model.get_relation("ingress", 0)
         assert ingress_data[(this_relation, this_relation.app)] == relation_info[0]["data"]
 
+    def test_get_ingress_data_for_broken_event_none_event_app(self, harness):
+        """Tests _get_ingress_data helper logs on RelationBroken event when event.app is None."""
+        harness.begin()
+        # Check for data while pretending this is a RelationBrokenEvent for relation[1] of the
+        # above relations.
+        mock_relation_broken_event = MagicMock(spec=RelationBrokenEvent)
+        mock_relation_broken_event.relation = harness.model.get_relation("ingress", 1)
+        mock_relation_broken_event.app = None
+
+        # Mock the logger
+        harness.charm.log = MagicMock()
+
+        harness.charm._get_ingress_data(mock_relation_broken_event)
+        assert harness.charm.log.info.call_count == 1
+
     def test_get_ingress_data_empty(self, harness):
         """Tests that the _get_ingress_data helper returns the correct empty relation data."""
         harness.begin()