fix: handle multiple istio-ingress-route relations (#336)

* fix: handle multiple istio-ingress-route relations

Use model.relations[...] (a list) instead of model.get_relation(...) when
checking ambient vs. sidecar ingress, so the charm no longer raises
TooManyRelatedAppsError when more than one istio-ingress-route relation is
present. submit_config already publishes the same config to every related
ingress provider.

Add unit and integration coverage for multiple ambient ingress relations.

* test: widen metallb CIDR for multiple Istio ingress gateways

Integration tests now deploy a second Istio ingress gateway, which needs its
own load-balancer IP. Provide at least two IPs via a /31 CIDR.

* test: strengthen istio-ingress-route unit tests

Assert every istio-ingress-route relation receives a valid HTTPRoute config,
assert the charm reconciles to active (rather than just 'not blocked') with
multiple ambient relations, and factor the relation/app names into constants
(reusing the charmed-kubeflow-chisme testing constants).

Author: NohaIhab

concierge.yaml

@@ -13,7 +13,7 @@ providers:
       load-balancer:
         enabled: true
         l2-mode: true
-        cidrs: 10.64.140.43/32
+        cidrs: 10.64.140.42/31  # NOTE: at least two IPs required for integration tests: one for each of the two Istio ingress gateways
     bootstrap-constraints:
       root-disk: 2G
 

poetry.lock

@@ -98,6 +98,7 @@ optional = true
 juju = "<4.0"
 charmed-kubeflow-chisme = ">=0.4.18"
 aiohttp = "^3.10.11"
+tenacity = "^9.0.0"
 lightkube = "^0.15.6"
 ops = "^2.17.1"
 pytest = "^8.3.4"

pyproject.toml

@@ -289,15 +289,26 @@ def _ambient_mesh_ingress(self):
             ],
         )
 
+        # submit_config publishes this same config to every istio-ingress-route
+        # relation, so all related ingress providers are (re)configured at once.
         if self.unit.is_leader():
             self.ingress.submit_config(config)
 
     def _check_istio_relations(self):
-        """Check that both ambient and sidecar relations are not present simultaneously."""
-        ambient_relation = self.model.get_relation("istio-ingress-route")
-        sidecar_relation = self.model.get_relation("ingress")
+        """Validate the charm's ingress relations are mutually exclusive.
 
-        if ambient_relation and sidecar_relation:
+        The charm supports both ambient ingress (via the 'istio-ingress-route'
+        endpoint) and sidecar ingress (via the 'ingress' endpoint), but the two
+        cannot be used at the same time. Each endpoint may hold any number of relations,
+        so this inspects the full list of relations on each endpoint.
+
+        Raises:
+            CheckFailed: with BlockedStatus if relations exist on both endpoints.
+        """
+        ambient_relations = self.model.relations["istio-ingress-route"]
+        sidecar_relations = self.model.relations["ingress"]
+
+        if ambient_relations and sidecar_relations:
             self.logger.error(
                 "Both 'istio-ingress-route' and 'ingress' relations are present, "
                 "remove one to unblock."

src/charm.py

@@ -8,9 +8,12 @@
 
 import pytest
 import pytest_asyncio
+import tenacity
 import yaml
 from charmed_kubeflow_chisme.testing import (
     GRAFANA_AGENT_APP,
+    ISTIO_INGRESS_K8S_APP,
+    ISTIO_INGRESS_ROUTE_ENDPOINT,
     assert_grafana_dashboards,
     assert_logging,
     assert_metrics_endpoint,
@@ -26,6 +29,7 @@
 from charms_dependencies import KUBEFLOW_PROFILES
 from dashboard_links_requirer_tester_charm.src.charm import generate_links_for_location
 from lightkube import Client
+from lightkube.generic_resource import create_namespaced_resource
 from lightkube.resources.core_v1 import ConfigMap
 from pytest_operator.plugin import OpsTest
 
@@ -53,6 +57,28 @@
 HTTP_PATH = "/volumes/"
 KUBEFLOW_PROFILES_RELATION_NAME = "kubeflow-profiles"
 
+# A second istio-ingress-k8s instance used to verify multiple-ingress support.
+SECOND_INGRESS_APP = "istio-ingress-k8s-alt"
+INGRESS_CHANNEL = "2/stable"
+# Name of the HTTPRoute submitted by kubeflow-dashboard (see charm._ambient_mesh_ingress).
+INGRESS_ROUTE_NAME = "http-ingress"
+# Gateway listener section for cleartext HTTP on port 80.
+HTTP_SECTION_NAME = "http-80"
+# Path matched by the dashboard HTTPRoute.
+INGRESS_ROUTE_PATH = "/"
+# Gateway API generic resources, resolved at runtime via lightkube.
+HTTPROUTE_RESOURCE = create_namespaced_resource(
+    "gateway.networking.k8s.io", "v1", "HTTPRoute", "httproutes"
+)
+GATEWAY_RESOURCE = create_namespaced_resource(
+    "gateway.networking.k8s.io", "v1", "Gateway", "gateways"
+)
+RETRY_120_SECONDS = tenacity.Retrying(
+    stop=tenacity.stop_after_delay(120),
+    wait=tenacity.wait_fixed(2),
+    reraise=True,
+)
+
 log = logging.getLogger(__name__)
 
 
@@ -366,8 +392,7 @@ async def assert_links_in_configmap_by_text_value(
     return links_texts
 
 
-@pytest.mark.abort_on_fail
-async def test_ui_is_accessible(ops_test: OpsTest):
+async def assert_ui_is_accessible(ops_test: OpsTest):
     """Verify that UI is accessible through the ingress gateway."""
     await assert_path_reachable_through_ingress(
         http_path=HTTP_PATH,
@@ -378,6 +403,93 @@ async def test_ui_is_accessible(ops_test: OpsTest):
     )
 
 
+@pytest.mark.abort_on_fail
+async def test_ui_is_accessible(ops_test: OpsTest):
+    """Verify that UI is accessible through the ingress gateway before the second ingress."""
+    await assert_ui_is_accessible(ops_test)
+
+
+@pytest.mark.abort_on_fail
+async def test_deploy_and_relate_second_ingress(ops_test: OpsTest):
+    """Deploy a second istio-ingress-k8s and relate it to kubeflow-dashboard.
+
+    kubeflow-dashboard must accept more than one istio-ingress-route relation without
+    erroring, so it should remain active after the second ingress is related.
+    """
+    await ops_test.model.deploy(
+        ISTIO_INGRESS_K8S_APP,
+        application_name=SECOND_INGRESS_APP,
+        channel=INGRESS_CHANNEL,
+        trust=True,
+    )
+    await ops_test.model.wait_for_idle(
+        [SECOND_INGRESS_APP],
+        raise_on_blocked=False,
+        raise_on_error=False,
+        wait_for_active=True,
+        timeout=60 * 15,
+    )
+
+    await ops_test.model.integrate(
+        f"{SECOND_INGRESS_APP}:{ISTIO_INGRESS_ROUTE_ENDPOINT}",
+        f"{CHARM_NAME}:{ISTIO_INGRESS_ROUTE_ENDPOINT}",
+    )
+    await ops_test.model.wait_for_idle(
+        [CHARM_NAME, SECOND_INGRESS_APP],
+        status="active",
+        raise_on_blocked=False,
+        raise_on_error=False,
+        timeout=60 * 10,
+        idle_period=30,
+    )
+
+    assert ops_test.model.applications[CHARM_NAME].units[0].workload_status == "active"
+
+
+async def test_httproute_attached_to_second_gateway(ops_test: OpsTest, lightkube_client: Client):
+    """Verify the HTTPRoute for the second ingress is created and bound to its Gateway.
+
+    The istio-ingress-k8s charm names each route
+    ``{source_app}-{route_name}-httproute-{section}-{ingress_app}`` and binds it to a
+    Gateway named after the ingress application via ``parentRefs``. We assert that the
+    route created for the second ingress is attached to the *second* Gateway (not the
+    first) and routes the dashboard path to the dashboard backend.
+    """
+    namespace = ops_test.model_name
+
+    expected_route_name = (
+        f"{CHARM_NAME}-{INGRESS_ROUTE_NAME}-httproute-{HTTP_SECTION_NAME}-{SECOND_INGRESS_APP}"
+    )
+
+    # The second Gateway should exist, named after the second ingress application.
+    lightkube_client.get(GATEWAY_RESOURCE, name=SECOND_INGRESS_APP, namespace=namespace)
+
+    # Retry to give the ingress charm time to reconcile the HTTPRoute resources.
+    httproute = None
+    for attempt in RETRY_120_SECONDS:
+        with attempt:
+            httproute = lightkube_client.get(
+                HTTPROUTE_RESOURCE, name=expected_route_name, namespace=namespace
+            )
+
+    parent_refs = httproute.spec["parentRefs"]
+    assert len(parent_refs) == 1
+    # The route must be attached to the SECOND gateway, not the first.
+    assert parent_refs[0]["name"] == SECOND_INGRESS_APP
+    assert parent_refs[0]["sectionName"] == HTTP_SECTION_NAME
+
+    # And it must route the dashboard path to the dashboard backend.
+    rule = httproute.spec["rules"][0]
+    assert rule["matches"][0]["path"]["value"] == INGRESS_ROUTE_PATH
+    assert rule["backendRefs"][0]["name"] == CHARM_NAME
+
+
+@pytest.mark.abort_on_fail
+async def test_ui_is_accessible_after_second_ingress(ops_test: OpsTest):
+    """Verify that UI is still accessible through the ingress gateway after the second ingress."""
+    await assert_ui_is_accessible(ops_test)
+
+
 async def test_metrics_enpoint(ops_test: OpsTest):
     """Test metrics_endpoints are defined in relation data bag and their accessibility.
     This function gets all the metrics_endpoints from the relation data bag, checks if

tests/integration/test_charm_ambient.py

@@ -9,7 +9,12 @@
 import pytest
 import yaml
 from charmed_kubeflow_chisme.exceptions import GenericCharmRuntimeError
-from charms.istio_ingress_k8s.v0.istio_ingress_route import ProtocolType
+from charmed_kubeflow_chisme.testing import ISTIO_INGRESS_K8S_APP, ISTIO_INGRESS_ROUTE_ENDPOINT
+from charms.istio_ingress_k8s.v0.istio_ingress_route import (
+    HTTPPathMatchType,
+    IstioIngressRouteConfig,
+    ProtocolType,
+)
 from charms.kubeflow_dashboard.v0.kubeflow_dashboard_links import (
     DASHBOARD_LINKS_FIELD,
     DashboardLink,
@@ -28,6 +33,10 @@
 
 METADATA = yaml.safe_load(Path("./metadata.yaml").read_text())
 CHARM_NAME = METADATA["name"]
+# Ingress relation endpoints and the apps used to exercise them in tests.
+SIDECAR_INGRESS_ENDPOINT = "ingress"
+SIDECAR_INGRESS_APP = "istio-pilot"
+SECOND_ISTIO_INGRESS_K8S_APP = f"{ISTIO_INGRESS_K8S_APP}-2"
 RELATION_DATA = [
     {
         "app": "tensorboards-web-app",
@@ -386,9 +395,9 @@ def test_sidecar_and_ambient_relations_added(
     ):
         """Test the charm is in BlockedStatus when both sidecar and ambient relations are added."""
         # Arrange
-        harness.add_relation("ingress", "istio-pilot")
+        harness.add_relation(SIDECAR_INGRESS_ENDPOINT, SIDECAR_INGRESS_APP)
 
-        harness.add_relation("istio-ingress-route", "istio-ingress-k8s")
+        harness.add_relation(ISTIO_INGRESS_ROUTE_ENDPOINT, ISTIO_INGRESS_K8S_APP)
 
         harness.set_leader(True)
 
@@ -402,6 +411,62 @@ def test_sidecar_and_ambient_relations_added(
             BlockedStatus,
         )
 
+    @patch("charm.KubernetesServicePatch", lambda x, y: None)
+    @patch("charm.KubeflowDashboardOperator.configmap_handler")
+    @patch("charm.KubeflowDashboardOperator.k8s_resource_handler")
+    def test_multiple_ambient_relations_added(
+        self,
+        k8s_resource_handler: MagicMock,
+        configmap_handler: MagicMock,
+        harness_with_profiles: Harness,
+    ):
+        """Test the charm reconciles to active with more than one istio-ingress-route relation."""
+        # Arrange
+        harness = harness_with_profiles
+        harness.add_relation(ISTIO_INGRESS_ROUTE_ENDPOINT, ISTIO_INGRESS_K8S_APP)
+        harness.add_relation(ISTIO_INGRESS_ROUTE_ENDPOINT, SECOND_ISTIO_INGRESS_K8S_APP)
+
+        # Act
+        harness.begin_with_initial_hooks()
+        harness.container_pebble_ready(harness.charm._container_name)
+
+        # Assert
+        # More than one relation on the istio-ingress-route endpoint must not block
+        # the charm; it should reconcile all the way to active.
+        assert isinstance(harness.charm.model.unit.status, ActiveStatus)
+
+    @patch("charm.KubernetesServicePatch", lambda x, y: None)
+    @patch("charm.KubeflowDashboardOperator.k8s_resource_handler")
+    def test_each_istio_ingress_route_relation_receives_config(
+        self, k8s_resource_handler: MagicMock, harness: Harness
+    ):
+        """Test that an HTTPRoute config is submitted to every istio-ingress-route relation."""
+        # Arrange
+        harness.set_leader(True)
+        rel_id_1 = harness.add_relation(ISTIO_INGRESS_ROUTE_ENDPOINT, ISTIO_INGRESS_K8S_APP)
+        rel_id_2 = harness.add_relation(ISTIO_INGRESS_ROUTE_ENDPOINT, SECOND_ISTIO_INGRESS_K8S_APP)
+
+        # Act
+        harness.begin()
+
+        # Assert
+        # Each relation's application databag should contain a valid config that
+        # defines the kubeflow-dashboard HTTPRoute, proving the lib handles every ingress.
+        for rel_id in (rel_id_1, rel_id_2):
+            app_data = harness.get_relation_data(rel_id, harness.charm.app.name)
+            assert "config" in app_data
+
+            config = IstioIngressRouteConfig.model_validate_json(app_data["config"])
+            assert len(config.http_routes) == 1
+            http_route = config.http_routes[0]
+            assert http_route.matches[0].path.type == HTTPPathMatchType.PathPrefix
+            assert http_route.matches[0].path.value == "/"
+            assert http_route.backends[0].service == harness.charm.app.name
+            assert http_route.backends[0].port == harness.charm._port
+            # The route's parent (the Gateway listener referenced under parentRefs in
+            # the HTTPRouteSpec) should be the expected HTTP listener.
+            assert http_route.listener.name == "http-80"
+
     @pytest.mark.parametrize("tls_enabled, expected_port", [(False, 80), (True, 443)])
     @patch("charm.KubernetesServicePatch", lambda x, y: None)
     @patch("charm.IstioIngressRouteRequirer")
@@ -419,7 +484,7 @@ def test_ambient_mesh_ingress(
         mock_ingress.tls_enabled = tls_enabled
         mock_ingress_cls.return_value = mock_ingress
 
-        harness.add_relation("istio-ingress-route", "istio-ingress-k8s")
+        harness.add_relation(ISTIO_INGRESS_ROUTE_ENDPOINT, ISTIO_INGRESS_K8S_APP)
         harness.set_leader(True)
         harness.begin()