diff --git a/concierge.yaml b/concierge.yaml index 7704c69..516ebdf 100644 --- a/concierge.yaml +++ b/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 diff --git a/src/components/istio_ambient_requirer_component.py b/src/components/istio_ambient_requirer_component.py index c66f2dd..b52cda8 100644 --- a/src/components/istio_ambient_requirer_component.py +++ b/src/components/istio_ambient_requirer_component.py @@ -52,6 +52,8 @@ def get_status(self) -> StatusBase: def _configure_app_leader(self, event): if self.ingress.is_ready(): try: + # submit_config publishes this same config to every istio-ingress-route + # relation, so all related ingress providers are (re)configured at once. self.ingress.submit_config(self._istio_ingress_route_config) except Exception as e: raise GenericCharmRuntimeError(f"Failed to submit ingress config: {e}") diff --git a/src/components/istio_relations_conflict_detector.py b/src/components/istio_relations_conflict_detector.py index f15ea0c..7018ac5 100644 --- a/src/components/istio_relations_conflict_detector.py +++ b/src/components/istio_relations_conflict_detector.py @@ -16,10 +16,10 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) def get_status(self) -> StatusBase: - sdi_relation = self._charm.model.get_relation(SDI_RELATION) - istio_relation = self._charm.model.get_relation(ISTIO_RELATION) + sdi_relations = self._charm.model.relations[SDI_RELATION] + istio_relations = self._charm.model.relations[ISTIO_RELATION] - if sdi_relation and istio_relation: + if sdi_relations and istio_relations: logger.warn( f"Both SDI relation '{SDI_RELATION}` and Istio relation '{ISTIO_RELATION}' found." f"Please only relate to one of the relations, and not both at the same time" diff --git a/tests/integration/test_charm_ambient.py b/tests/integration/test_charm_ambient.py index b9f0ee4..4f4c08f 100644 --- a/tests/integration/test_charm_ambient.py +++ b/tests/integration/test_charm_ambient.py @@ -6,9 +6,12 @@ import lightkube import pytest +import tenacity import yaml from charmed_kubeflow_chisme.testing import ( GRAFANA_AGENT_APP, + ISTIO_INGRESS_K8S_APP, + ISTIO_INGRESS_ROUTE_ENDPOINT, assert_alert_rules, assert_logging, assert_metrics_endpoint, @@ -21,6 +24,7 @@ ) from charms_dependencies import MLMD from lightkube import Client +from lightkube.generic_resource import create_namespaced_resource from lightkube.resources.core_v1 import Service from pyease_grpc import RpcSession, RpcUri from pytest_operator.plugin import OpsTest @@ -34,6 +38,28 @@ INGRESS_K8S_SERVICE = "istio-ingress-k8s-istio" MLMD_PROTO_PATH = Path("tests/integration/data/metadata_store_service.proto") +# 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 envoy (see AmbientMeshRequirerComponent). +INGRESS_ROUTE_NAME = "http-ingress" +# Gateway listener section for cleartext HTTP on port 80. +HTTP_SECTION_NAME = "http-80" +# Path matched by the envoy HTTPRoute. +INGRESS_ROUTE_PATH = "/ml_metadata.MetadataStoreService/" +# 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__) web_grpc_session = RpcSession.from_file(MLMD_PROTO_PATH.as_posix()) @@ -98,8 +124,8 @@ async def test_logging(ops_test: OpsTest): await assert_logging(app) -async def test_web_grpc_mlmd(ops_test: OpsTest, lightkube_client: lightkube.Client): - """Test the web-grpc envoy connection to mlmd.""" +async def assert_web_grpc_mlmd(ops_test: OpsTest, lightkube_client: lightkube.Client): + """Verify the web-grpc envoy connection to mlmd through the ingress gateway.""" # Get the gateway IP service = lightkube_client.get(Service, INGRESS_K8S_SERVICE, namespace=ops_test.model.name) gateway_ip = service.status.loadBalancer.ingress[0].ip @@ -115,6 +141,12 @@ async def test_web_grpc_mlmd(ops_test: OpsTest, lightkube_client: lightkube.Clie assert response.response.status_code == 200 +@pytest.mark.abort_on_fail +async def test_web_grpc_mlmd(ops_test: OpsTest, lightkube_client: lightkube.Client): + """Test the web-grpc envoy connection to mlmd before the second ingress.""" + await assert_web_grpc_mlmd(ops_test, lightkube_client) + + @pytest.mark.parametrize("container_name", list(CONTAINERS_SECURITY_CONTEXT_MAP.keys())) async def test_container_security_context( ops_test: OpsTest, @@ -134,3 +166,86 @@ async def test_container_security_context( CONTAINERS_SECURITY_CONTEXT_MAP, ops_test.model.name, ) + + +@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 envoy. + + envoy 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"{ENVOY_APP_NAME}:{ISTIO_INGRESS_ROUTE_ENDPOINT}", + ) + await ops_test.model.wait_for_idle( + [ENVOY_APP_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[ENVOY_APP_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 envoy path to the envoy backend. + """ + namespace = ops_test.model.name + + expected_route_name = ( + f"{ENVOY_APP_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 envoy path to the envoy backend. + rule = httproute.spec["rules"][0] + assert rule["matches"][0]["path"]["value"] == INGRESS_ROUTE_PATH + assert rule["backendRefs"][0]["name"] == ENVOY_APP_NAME + + +@pytest.mark.abort_on_fail +async def test_web_grpc_mlmd_after_second_ingress( + ops_test: OpsTest, lightkube_client: lightkube.Client +): + """Test the web-grpc envoy connection to mlmd after the second ingress.""" + await assert_web_grpc_mlmd(ops_test, lightkube_client) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 401603a..3d8be86 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -3,7 +3,11 @@ from unittest.mock import MagicMock, patch import pytest -from charms.istio_ingress_k8s.v0.istio_ingress_route import ProtocolType +from charms.istio_ingress_k8s.v0.istio_ingress_route import ( + HTTPPathMatchType, + IstioIngressRouteConfig, + ProtocolType, +) from ops import BlockedStatus from ops.model import ActiveStatus, TooManyRelatedAppsError, WaitingStatus from ops.testing import Harness @@ -81,6 +85,46 @@ def test_multiple_ingress_relations(self, harness: Harness): ) assert isinstance(harness.charm.model.unit.status, BlockedStatus) + def test_multiple_istio_ingress_route_relations(self, harness: Harness): + """Test that multiple istio-ingress-route relations do not block the charm.""" + harness.add_relation("istio-ingress-route", "istio-ingress-k8s") + harness.add_relation("istio-ingress-route", "istio-ingress-k8s-2") + harness.begin() + + # Inspecting the full list of relations per endpoint must not raise even + # though there is more than one relation on the istio-ingress-route endpoint. + status = harness.charm.istio_relations_conflict_detector.component.get_status() + assert isinstance(status, ActiveStatus) + + def test_each_istio_ingress_route_relation_receives_config(self, harness: Harness): + """Test that an HTTPRoute config is submitted to every istio-ingress-route relation.""" + harness.begin() + + rel_id_1 = harness.add_relation("istio-ingress-route", "istio-ingress-k8s") + rel_id_2 = harness.add_relation("istio-ingress-route", "istio-ingress-k8s-2") + + # Use the real submit_config so we can inspect the databags; only force readiness. + ingress = harness.charm.ambient_ingress.component.ingress + ingress.is_ready = MagicMock(return_value=True) + + # Reconcile the charm so the component submits its config. + harness.charm.on.install.emit() + + # Each relation's application databag should contain a valid config that + # defines the envoy 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 == "/ml_metadata.MetadataStoreService/" + assert http_route.backends[0].service == harness.charm.app.name + assert http_route.backends[0].port == int(harness.charm.model.config["http-port"]) + assert http_route.listener.name == "http-80" + def test_many_relations(self, harness: Harness): """Test the grpc component and charm are not active when >1 grpc relation is present."""