From 72e6d04ab49fb08c22eaa04914cc70a1b154d6d2 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 11:20:55 +0300 Subject: [PATCH 01/14] Introduce ingress relation Introduce ingress relation so that there is a virtualService pointing to the workload's service and UI components can access it via gRPC-Web protocol. --- metadata.yaml | 40 ++++++++++++++++++++++++++++++++++++++++ src/charm.py | 15 +++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/metadata.yaml b/metadata.yaml index 67e53ee..6fb9dce 100755 --- a/metadata.yaml +++ b/metadata.yaml @@ -25,3 +25,43 @@ requires: interface: grpc schema: https://raw.githubusercontent.com/canonical/operator-schemas/master/grpc.yaml versions: [v1] + ingress: + interface: ingress + schema: + v2: + requires: + type: object + properties: + service: + type: string + port: + type: integer + namespace: + type: string + prefix: + type: string + rewrite: + type: string + required: + - service + - port + - namespace + - prefix + v1: + requires: + type: object + properties: + service: + type: string + port: + type: integer + prefix: + type: string + rewrite: + type: string + required: + - service + - port + - prefix + versions: [v1] + __schema_source: https://raw.githubusercontent.com/canonical/operator-schemas/master/ingress.yaml diff --git a/src/charm.py b/src/charm.py index 20adee0..b35cdd8 100755 --- a/src/charm.py +++ b/src/charm.py @@ -152,6 +152,7 @@ def __init__(self, *args): self.on.leader_elected, self.on["grpc"].relation_changed, self.on["grpc-web"].relation_changed, + self.on["ingress"].relation_changed, ]: self.framework.observe(event, self.set_pod_spec) @@ -166,6 +167,9 @@ def set_pod_spec(self, event): upstreams = self._check_grpc(interfaces) self._send_info(interfaces) + + self._configure_mesh(interfaces) + except CheckFailed as check_failed: self.model.unit.status = check_failed.status return @@ -276,6 +280,17 @@ def _check_grpc(self, interfaces): raise CheckFailed("Waiting for upstream gRPC connection information.", WaitingStatus) return upstreams + def _configure_mesh(self, interfaces): + if interfaces["ingress"]: + interfaces["ingress"].send_data( + { + "prefix": "/ml_metadata", + "rewrite": "/ml_metadata", + "service": self.model.app.name, + "port": int(self.model.config["http-port"]), + } + ) + if __name__ == "__main__": main(Operator) From d457ae02f697d5ee71e0445b9048bf576b4daeb3 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 11:25:19 +0300 Subject: [PATCH 02/14] tests(integration): Add test_virtual_service - Assert that virtual service exists - Assert that the right endpoint is served --- requirements-integration.in | 2 + requirements-integration.txt | 43 ++++++++++++++++- tests/integration/test_charm.py | 82 +++++++++++++++++++++++++++++++++ 3 files changed, 125 insertions(+), 2 deletions(-) diff --git a/requirements-integration.in b/requirements-integration.in index 586ff18..b7b626c 100644 --- a/requirements-integration.in +++ b/requirements-integration.in @@ -2,4 +2,6 @@ juju pytest-operator selenium selenium-wire +lightkube +aiohttp -r requirements.txt diff --git a/requirements-integration.txt b/requirements-integration.txt index 9429c3b..08c5b0a 100644 --- a/requirements-integration.txt +++ b/requirements-integration.txt @@ -4,11 +4,20 @@ # # pip-compile requirements-integration.in # +aiohttp==3.8.6 + # via -r requirements-integration.in +aiosignal==1.3.1 + # via aiohttp +anyio==4.0.0 + # via httpcore asttokens==2.4.0 # via stack-data +async-timeout==4.0.3 + # via aiohttp attrs==23.1.0 # via # -r requirements.txt + # aiohttp # jsonschema # outcome # trio @@ -29,6 +38,8 @@ cachetools==5.3.1 certifi==2023.7.22 # via # -r requirements.txt + # httpcore + # httpx # kubernetes # requests # selenium @@ -40,6 +51,7 @@ cffi==1.16.0 charset-normalizer==3.3.0 # via # -r requirements.txt + # aiohttp # requests cryptography==41.0.4 # via @@ -53,11 +65,16 @@ envoy-data-plane==0.2.5 # via -r requirements.txt exceptiongroup==1.1.3 # via + # anyio # pytest # trio # trio-websocket executing==2.0.0 # via stack-data +frozenlist==1.4.0 + # via + # aiohttp + # aiosignal google-auth==2.23.3 # via kubernetes grpclib==0.4.6 @@ -65,7 +82,9 @@ grpclib==0.4.6 # -r requirements.txt # betterproto h11==0.14.0 - # via wsproto + # via + # httpcore + # wsproto h2==4.1.0 # via # -r requirements.txt @@ -75,6 +94,10 @@ hpack==4.0.0 # via # -r requirements.txt # h2 +httpcore==0.18.0 + # via httpx +httpx==0.25.0 + # via lightkube hvac==1.2.1 # via juju hyperframe==6.0.1 @@ -85,8 +108,11 @@ hyperframe==6.0.1 idna==3.4 # via # -r requirements.txt + # anyio + # httpx # requests # trio + # yarl importlib-resources==6.1.0 # via # -r requirements.txt @@ -113,6 +139,10 @@ kaitaistruct==0.10 # via selenium-wire kubernetes==27.2.0 # via juju +lightkube==0.14.0 + # via -r requirements-integration.in +lightkube-models==1.28.1.4 + # via lightkube macaroonbakery==1.3.1 # via juju markupsafe==2.1.3 @@ -122,7 +152,9 @@ matplotlib-inline==0.1.6 multidict==6.0.4 # via # -r requirements.txt + # aiohttp # grpclib + # yarl mypy-extensions==1.0.0 # via typing-inspect oauthlib==3.2.2 @@ -218,6 +250,7 @@ pyyaml==6.0.1 # -r requirements.txt # juju # kubernetes + # lightkube # ops # pytest-operator # serialized-data-interface @@ -251,7 +284,11 @@ six==1.16.0 # pymacaroons # python-dateutil sniffio==1.3.0 - # via trio + # via + # anyio + # httpcore + # httpx + # trio sortedcontainers==2.4.0 # via trio stack-data==0.6.3 @@ -299,6 +336,8 @@ wsproto==1.2.0 # via # selenium-wire # trio-websocket +yarl==1.9.2 + # via aiohttp zipp==3.17.0 # via # -r requirements.txt diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 986015e..05cec92 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -5,9 +5,13 @@ import logging from pathlib import Path +import aiohttp import pytest import requests import yaml +from lightkube import Client +from lightkube.generic_resource import create_namespaced_resource +from lightkube.resources.core_v1 import Service log = logging.getLogger(__name__) @@ -20,6 +24,14 @@ GRAFANA = "grafana-k8s" PROMETHEUS_SCRAPE = "prometheus-scrape-config-k8s" MLMD = "mlmd" +ISTIO_PILOT = "istio-pilot" +ISTIO_GW = "istio-ingressgateway" + + +@pytest.fixture(scope="session") +def lightkube_client() -> Client: + client = Client(field_manager=APP_NAME) + return client @pytest.mark.abort_on_fail @@ -37,6 +49,40 @@ async def test_build_and_deploy(ops_test): assert all([endpoint.name == "grpc" for endpoint in relation.endpoints]) +@pytest.mark.abort_on_fail +async def test_virtual_service(ops_test, lightkube_client): + await ops_test.model.deploy( + ISTIO_PILOT, + channel="latest/edge", + config={"default-gateway": "kubeflow-gateway"}, + trust=True, + ) + + await ops_test.model.deploy( + "istio-gateway", + application_name="istio-ingressgateway", + channel="latest/edge", + config={"kind": "ingress"}, + trust=True, + ) + await ops_test.model.add_relation(f"{ISTIO_PILOT}:{ISTIO_PILOT}", f"{ISTIO_GW}:{ISTIO_PILOT}") + await ops_test.model.add_relation(ISTIO_PILOT, APP_NAME) + + await ops_test.model.wait_for_idle( + raise_on_blocked=False, + raise_on_error=True, + timeout=300, + ) + + # Verify that virtualService is as expected + assert_virtualservice_exists(name=APP_NAME, namespace=ops_test.model.name) + + # Verify `/ml_metadata` endpoint is served + regular_ingress_gateway_ip = await get_gateway_ip(namespace=ops_test.model.name) + res_status, res_text = await fetch_response(f"http://{regular_ingress_gateway_ip}/ml_metadata") + assert res_status != 404 + + @pytest.mark.abort_on_fail async def test_deploy_with_prometheus_and_grafana(ops_test): scrape_config = {"scrape_interval": "30s"} @@ -74,3 +120,39 @@ async def test_correct_observability_setup(ops_test): assert response_metric["juju_charm"] == APP_NAME assert response_metric["juju_model"] == ops_test.model_name assert response_metric["juju_unit"] == f"{APP_NAME}/0" + + +def assert_virtualservice_exists(name: str, namespace: str): + """Will raise a ApiError(404) if the virtualservice does not exist.""" + log.info(f"Asserting that VirtualService '{name}' exists.") + lightkube_client = Client() + virtual_service_lightkube_resource = create_namespaced_resource( + group="networking.istio.io", + version="v1alpha3", + kind="VirtualService", + plural="virtualservices", + ) + lightkube_client.get(virtual_service_lightkube_resource, name, namespace=namespace) + log.info(f"VirtualService '{name}' exists.") + + +async def fetch_response(url, headers=None): + """Fetch provided URL and return pair - status and text (int, string).""" + result_status = 0 + result_text = "" + async with aiohttp.ClientSession() as session: + async with session.get(url, headers=headers) as response: + result_status = response.status + result_text = await response.text() + return result_status, str(result_text) + + +async def get_gateway_ip( + namespace: str, + service_name: str = "istio-ingressgateway-workload", +): + log.info(f"Getting {service_name} ingress ip") + lightkube_client = Client() + service = lightkube_client.get(Service, service_name, namespace=namespace) + gateway_ip = service.status.loadBalancer.ingress[0].ip + return gateway_ip From ecd057f3120524ea22beec31fb0db9b4cb0e70d6 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 11:30:07 +0300 Subject: [PATCH 03/14] tests(unit): refactor --- tests/unit/test_charm.py | 61 ++++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 33 deletions(-) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 5391520..e0f52e3 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -29,14 +29,8 @@ def test_missing_image(harness): def test_no_relation(harness): harness.set_leader(True) - harness.add_oci_resource( - "oci-image", - { - "registrypath": "ci-test", - "username": "", - "password": "", - }, - ) + add_oci_image(harness) + harness.begin_with_initial_hooks() assert harness.charm.model.unit.status == BlockedStatus("No upstream gRPC services.") @@ -44,6 +38,24 @@ def test_no_relation(harness): def test_many_relations(harness): harness.set_leader(True) + add_oci_image(harness) + + setup_grpc_relation(harness, "grpc-one", "8080") + setup_grpc_relation(harness, "grpc-two", "9090") + harness.begin_with_initial_hooks() + + pod_spec, _ = harness.get_pod_spec() + + expected = yaml.safe_load(open("tests/unit/many_relations.yaml")) + + c = pod_spec["containers"][0]["volumeConfig"][0]["files"][0]["content"] + assert json.loads(c) == expected + + assert harness.charm.model.unit.status == ActiveStatus("") + + +# Helper functions +def add_oci_image(harness: Harness): harness.add_oci_resource( "oci-image", { @@ -52,34 +64,17 @@ def test_many_relations(harness): "password": "", }, ) - rel_id1 = harness.add_relation("grpc", "grpc-one") - harness.add_relation_unit(rel_id1, "grpc-one/0") - harness.update_relation_data( - rel_id1, - "grpc-one", - { - "_supported_versions": "- v1", - "data": yaml.dump({"service": "grpc-one", "port": "8080"}), - }, - ) - rel_id2 = harness.add_relation("grpc", "grpc-two") - harness.add_relation_unit(rel_id2, "grpc-two/0") + +def setup_grpc_relation(harness: Harness, name: str, port: str): + rel_id = harness.add_relation("grpc", name) + harness.add_relation_unit(rel_id, f"{name}/0") harness.update_relation_data( - rel_id2, - "grpc-two", + rel_id, + name, { "_supported_versions": "- v1", - "data": yaml.dump({"service": "grpc-two", "port": "9090"}), + "data": yaml.dump({"service": name, "port": port}), }, ) - harness.begin_with_initial_hooks() - - pod_spec, _ = harness.get_pod_spec() - - expected = yaml.safe_load(open("tests/unit/many_relations.yaml")) - - c = pod_spec["containers"][0]["volumeConfig"][0]["files"][0]["content"] - assert json.loads(c) == expected - - assert harness.charm.model.unit.status == ActiveStatus("") + return rel_id From a6151102cd6214f67690d97c9a10419b2a7b9cf3 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 11:30:21 +0300 Subject: [PATCH 04/14] tests(unit): Add ingress relation test --- tests/unit/test_charm.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index e0f52e3..d36fa09 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -54,6 +54,20 @@ def test_many_relations(harness): assert harness.charm.model.unit.status == ActiveStatus("") +def test_with_ingress_relation(harness): + harness.set_leader(True) + add_oci_image(harness) + + # Set required grpc relation + setup_grpc_relation(harness, "grpc-one", "8080") + + setup_ingress_relation(harness) + + harness.begin_with_initial_hooks() + + assert isinstance(harness.charm.model.unit.status, ActiveStatus) + + # Helper functions def add_oci_image(harness: Harness): harness.add_oci_resource( @@ -66,6 +80,17 @@ def add_oci_image(harness: Harness): ) +def setup_ingress_relation(harness: Harness): + rel_id = harness.add_relation("ingress", "istio-pilot") + harness.add_relation_unit(rel_id, "istio-pilot/0") + harness.update_relation_data( + rel_id, + "istio-pilot", + {"_supported_versions": "- v1"}, + ) + return rel_id + + def setup_grpc_relation(harness: Harness, name: str, port: str): rel_id = harness.add_relation("grpc", name) harness.add_relation_unit(rel_id, f"{name}/0") From b675d4430ad72ef158a123dff8c0cd26ab42ff1c Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 12:32:32 +0300 Subject: [PATCH 05/14] ci: Update logs namespace --- .github/workflows/integrate.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 49516e5..edff189 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -71,19 +71,19 @@ jobs: if: failure() - name: Get envoy workload logs - run: kubectl logs --tail 100 -nci-test -ljuju-app=envoy + run: kubectl logs --tail 100 -ntesting -ljuju-app=envoy if: failure() - name: Get envoy operator logs - run: kubectl logs --tail 100 -nci-test -ljuju-operator=envoy + run: kubectl logs --tail 100 -ntesting -ljuju-operator=envoy if: failure() - name: Get mlmd workload logs - run: kubectl logs --tail 100 -nci-test -ljuju-app=mlmd + run: kubectl logs --tail 100 -ntesting -ljuju-app=mlmd if: failure() - name: Get mlmd operator logs - run: kubectl logs --tail 100 -nci-test -ljuju-operator=mlmd + run: kubectl logs --tail 100 -ntesting -ljuju-operator=mlmd if: failure() - name: Collect charm debug artifacts From 40ccc1a426054059e65d5d240049ed1aa896515a Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 12:37:51 +0300 Subject: [PATCH 06/14] ci: Add istio-pilot logs --- .github/workflows/integrate.yaml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index edff189..ce68ea1 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -86,6 +86,14 @@ jobs: run: kubectl logs --tail 100 -ntesting -ljuju-operator=mlmd if: failure() + - name: Get istio-pilot workload logs + run: kubectl logs --tail 100 -ntesting -ljuju-app=istio-pilot + if: failure() + + - name: Get istio-pilot operator logs + run: kubectl logs --tail 100 -ntesting -ljuju-operator=istio-pilot + if: failure() + - name: Collect charm debug artifacts uses: canonical/kubeflow-ci/actions/dump-charm-debug-artifacts@main if: always() From c4682c27d629bc5326823f6d83899bf35ebc1336 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 12:38:12 +0300 Subject: [PATCH 07/14] tests(integration): Refactor --- tests/integration/test_charm.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 05cec92..8732c5a 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -60,13 +60,12 @@ async def test_virtual_service(ops_test, lightkube_client): await ops_test.model.deploy( "istio-gateway", - application_name="istio-ingressgateway", + application_name=ISTIO_GW, channel="latest/edge", config={"kind": "ingress"}, trust=True, ) await ops_test.model.add_relation(f"{ISTIO_PILOT}:{ISTIO_PILOT}", f"{ISTIO_GW}:{ISTIO_PILOT}") - await ops_test.model.add_relation(ISTIO_PILOT, APP_NAME) await ops_test.model.wait_for_idle( raise_on_blocked=False, @@ -74,6 +73,8 @@ async def test_virtual_service(ops_test, lightkube_client): timeout=300, ) + await ops_test.model.add_relation(ISTIO_PILOT, APP_NAME) + # Verify that virtualService is as expected assert_virtualservice_exists(name=APP_NAME, namespace=ops_test.model.name) From a1b1f7695347a9da6363d23386910d24184c7736 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 14:32:11 +0300 Subject: [PATCH 08/14] tests(integration): update --- tests/integration/test_charm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 8732c5a..1625d66 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -66,15 +66,15 @@ async def test_virtual_service(ops_test, lightkube_client): trust=True, ) await ops_test.model.add_relation(f"{ISTIO_PILOT}:{ISTIO_PILOT}", f"{ISTIO_GW}:{ISTIO_PILOT}") + await ops_test.model.add_relation(ISTIO_PILOT, APP_NAME) await ops_test.model.wait_for_idle( + status="active", raise_on_blocked=False, raise_on_error=True, timeout=300, ) - await ops_test.model.add_relation(ISTIO_PILOT, APP_NAME) - # Verify that virtualService is as expected assert_virtualservice_exists(name=APP_NAME, namespace=ops_test.model.name) From 8b58bed696561ed334d34a2bc81ec504f2fdc606 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Fri, 20 Oct 2023 14:38:48 +0300 Subject: [PATCH 09/14] ci: Enable metallb addon in microk8s --- .github/workflows/integrate.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index ce68ea1..8e153ab 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -57,7 +57,7 @@ jobs: provider: microk8s channel: 1.25-strict/stable juju-channel: 3.1/stable - microk8s-addons: "dns hostpath-storage rbac" + microk8s-addons: "dns hostpath-storage rbac metallb:10.64.140.43-10.64.140.49" - name: Test run: sg snap_microk8s -c "tox -vve integration -- --model testing" From 9dc0ff067b0397cf409f1d06d89febc85407c51f Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Mon, 23 Oct 2023 14:12:41 +0300 Subject: [PATCH 10/14] review: Rename configure_mesh and add docstring --- src/charm.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/charm.py b/src/charm.py index b35cdd8..6055ed7 100755 --- a/src/charm.py +++ b/src/charm.py @@ -168,7 +168,7 @@ def set_pod_spec(self, event): self._send_info(interfaces) - self._configure_mesh(interfaces) + self._send_data_to_ingress_provider(interfaces) except CheckFailed as check_failed: self.model.unit.status = check_failed.status @@ -280,7 +280,10 @@ def _check_grpc(self, interfaces): raise CheckFailed("Waiting for upstream gRPC connection information.", WaitingStatus) return upstreams - def _configure_mesh(self, interfaces): + def _send_data_to_ingress_provider(self, interfaces): + """Send data to the ingress relation data bag so the VirtualServices provider configures + a VirtualService routing traffic from `/ml_metadata` path to envoy service. + """ if interfaces["ingress"]: interfaces["ingress"].send_data( { From ef28effbf1e0bed616abd1c8448b509387077c5e Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Mon, 23 Oct 2023 14:42:29 +0300 Subject: [PATCH 11/14] review: Block the charm if no ingress relation available --- src/charm.py | 7 +++++++ tests/integration/test_charm.py | 7 ++++++- tests/unit/test_charm.py | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 6055ed7..109daff 100755 --- a/src/charm.py +++ b/src/charm.py @@ -283,6 +283,8 @@ def _check_grpc(self, interfaces): def _send_data_to_ingress_provider(self, interfaces): """Send data to the ingress relation data bag so the VirtualServices provider configures a VirtualService routing traffic from `/ml_metadata` path to envoy service. + + Raises an exception and sets the charm to Blocked if there is no ingress relation available """ if interfaces["ingress"]: interfaces["ingress"].send_data( @@ -293,6 +295,11 @@ def _send_data_to_ingress_provider(self, interfaces): "port": int(self.model.config["http-port"]), } ) + else: + raise CheckFailed( + "No 'ingress' relation available, please relate to a VirtualServices provider e.g. istio-pilot.", # noqa E501 + BlockedStatus, + ) if __name__ == "__main__": diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 1625d66..32a91b7 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -42,7 +42,12 @@ async def test_build_and_deploy(ops_test): resources = {"oci-image": image_path} await ops_test.model.deploy(charm, resources=resources) await ops_test.model.add_relation(APP_NAME, MLMD) - await ops_test.model.wait_for_idle(status="active", raise_on_blocked=True, idle_period=30) + await ops_test.model.wait_for_idle( + apps=[MLMD], status="active", raise_on_blocked=False, idle_period=30 + ) + await ops_test.model.wait_for_idle( + apps=[APP_NAME], status="blocked", raise_on_blocked=False, idle_period=30 + ) relation = ops_test.model.relations[0] assert [app.entity_id for app in relation.applications] == [APP_NAME, MLMD] diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index d36fa09..b0a5f17 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -42,6 +42,8 @@ def test_many_relations(harness): setup_grpc_relation(harness, "grpc-one", "8080") setup_grpc_relation(harness, "grpc-two", "9090") + # In order to avoid the charm going to Blocked + setup_ingress_relation(harness) harness.begin_with_initial_hooks() pod_spec, _ = harness.get_pod_spec() From f29a8d9b173e11ded47ef804007f471b2f0533c4 Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 24 Oct 2023 13:16:18 +0300 Subject: [PATCH 12/14] review(integration-tests): Use lightkube fixture and tenacity --- requirements-integration.in | 1 + requirements-integration.txt | 2 ++ tests/integration/test_charm.py | 28 +++++++++++++++++++++------- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/requirements-integration.in b/requirements-integration.in index b7b626c..884b346 100644 --- a/requirements-integration.in +++ b/requirements-integration.in @@ -4,4 +4,5 @@ selenium selenium-wire lightkube aiohttp +tenacity -r requirements.txt diff --git a/requirements-integration.txt b/requirements-integration.txt index 08c5b0a..9b208eb 100644 --- a/requirements-integration.txt +++ b/requirements-integration.txt @@ -295,6 +295,8 @@ stack-data==0.6.3 # via ipython stringcase==1.2.0 # via -r requirements.txt +tenacity==8.2.3 + # via -r requirements-integration.in tomli==2.0.1 # via # ipdb diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 32a91b7..014b163 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -8,6 +8,7 @@ import aiohttp import pytest import requests +import tenacity import yaml from lightkube import Client from lightkube.generic_resource import create_namespaced_resource @@ -81,11 +82,13 @@ async def test_virtual_service(ops_test, lightkube_client): ) # Verify that virtualService is as expected - assert_virtualservice_exists(name=APP_NAME, namespace=ops_test.model.name) + assert_virtualservice_exists( + name=APP_NAME, namespace=ops_test.model.name, lightkube_client=lightkube_client + ) # Verify `/ml_metadata` endpoint is served - regular_ingress_gateway_ip = await get_gateway_ip(namespace=ops_test.model.name) - res_status, res_text = await fetch_response(f"http://{regular_ingress_gateway_ip}/ml_metadata") + await assert_metadata_endpoint_is_served(ops_test, lightkube_client=lightkube_client) + assert res_status != 404 @@ -128,10 +131,9 @@ async def test_correct_observability_setup(ops_test): assert response_metric["juju_unit"] == f"{APP_NAME}/0" -def assert_virtualservice_exists(name: str, namespace: str): +def assert_virtualservice_exists(name: str, namespace: str, lightkube_client): """Will raise a ApiError(404) if the virtualservice does not exist.""" log.info(f"Asserting that VirtualService '{name}' exists.") - lightkube_client = Client() virtual_service_lightkube_resource = create_namespaced_resource( group="networking.istio.io", version="v1alpha3", @@ -142,6 +144,19 @@ def assert_virtualservice_exists(name: str, namespace: str): log.info(f"VirtualService '{name}' exists.") +@tenacity.retry( + stop=tenacity.stop_after_delay(10), + wait=tenacity.wait_fixed(2), + reraise=True, +) +async def assert_metadata_endpoint_is_served(ops_test, lightkube_client): + regular_ingress_gateway_ip = await get_gateway_ip( + namespace=ops_test.model.name, lightkube_client=lightkube_client + ) + res_status, res_text = await fetch_response(f"http://{regular_ingress_gateway_ip}/ml_metadata") + assert res_status != 404 + log.info("Endpoint /ml_metadata is reachable.") + async def fetch_response(url, headers=None): """Fetch provided URL and return pair - status and text (int, string).""" result_status = 0 @@ -154,8 +169,7 @@ async def fetch_response(url, headers=None): async def get_gateway_ip( - namespace: str, - service_name: str = "istio-ingressgateway-workload", + namespace: str, lightkube_client, service_name: str = "istio-ingressgateway-workload" ): log.info(f"Getting {service_name} ingress ip") lightkube_client = Client() From 6c1821e97388b2cca3349c68ce3a22363f8a818b Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Tue, 24 Oct 2023 13:17:53 +0300 Subject: [PATCH 13/14] tests(integration): Assert grpc-web protocol is working --- tests/integration/test_charm.py | 30 +++++++++++++++++++++++++----- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 014b163..3c8a3d5 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -89,7 +89,7 @@ async def test_virtual_service(ops_test, lightkube_client): # Verify `/ml_metadata` endpoint is served await assert_metadata_endpoint_is_served(ops_test, lightkube_client=lightkube_client) - assert res_status != 404 + await assert_grpc_web_protocol_responds(ops_test, lightkube_client=lightkube_client) @pytest.mark.abort_on_fail @@ -157,22 +157,42 @@ async def assert_metadata_endpoint_is_served(ops_test, lightkube_client): assert res_status != 404 log.info("Endpoint /ml_metadata is reachable.") + +@tenacity.retry( + stop=tenacity.stop_after_delay(10), + wait=tenacity.wait_fixed(2), + reraise=True, +) +async def assert_grpc_web_protocol_responds(ops_test, lightkube_client): + regular_ingress_gateway_ip = await get_gateway_ip( + namespace=ops_test.model.name, lightkube_client=lightkube_client + ) + headers = {"Content-Type": "application/grpc-web-text"} + res_status, res_headers = await fetch_response( + f"http://{regular_ingress_gateway_ip}/ml_metadata", headers + ) + assert res_status == 200 + log.info("Endpoint /ml_metadata serves grpc-web protocol.") + + async def fetch_response(url, headers=None): """Fetch provided URL and return pair - status and text (int, string).""" - result_status = 0 - result_text = "" + async with aiohttp.ClientSession() as session: async with session.get(url, headers=headers) as response: result_status = response.status result_text = await response.text() - return result_status, str(result_text) + result_headers = response.headers + if headers is None: + return result_status, str(result_text) + else: + return result_status, result_headers async def get_gateway_ip( namespace: str, lightkube_client, service_name: str = "istio-ingressgateway-workload" ): log.info(f"Getting {service_name} ingress ip") - lightkube_client = Client() service = lightkube_client.get(Service, service_name, namespace=namespace) gateway_ip = service.status.loadBalancer.ingress[0].ip return gateway_ip From 994173cf5066e5811f25610e9e483d8c3db2316e Mon Sep 17 00:00:00 2001 From: Orfeas Kourkakis Date: Wed, 25 Oct 2023 13:31:15 +0300 Subject: [PATCH 14/14] review: Update Blocked message --- src/charm.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/charm.py b/src/charm.py index 109daff..2249987 100755 --- a/src/charm.py +++ b/src/charm.py @@ -297,7 +297,7 @@ def _send_data_to_ingress_provider(self, interfaces): ) else: raise CheckFailed( - "No 'ingress' relation available, please relate to a VirtualServices provider e.g. istio-pilot.", # noqa E501 + "Please relate to istio-pilot, no ingress relation available.", BlockedStatus, )