From a425c46b1e09dfa3f2aee5d7415d3bf0d7381af5 Mon Sep 17 00:00:00 2001 From: Daniela Plascencia Date: Tue, 17 Dec 2024 19:20:10 +0100 Subject: [PATCH] ci, tests: pass charm artefacts to deploy and test charms This commit enables the "--charm-path" option to pass .charm artefacts to be deployed and tested at an individual level. This change enables the option to pass pre-built charms to the tests to avoid building them on the same test. It also ensures compatibility with the build_charm.py reusable workflow (from canonical/data-platform-workflows). Fixes: #639 --- .github/workflows/integrate.yaml | 19 +- charms/kfp-api/tests/integration/conftest.py | 12 + .../kfp-api/tests/integration/test_charm.py | 13 +- .../tests/integration/conftest.py | 12 + .../tests/integration/test_charm.py | 18 +- .../tests/integration/conftest.py | 12 + .../tests/integration/test_charm.py | 14 +- .../files/upstream/sync.py | 434 +++++++++--------- .../tests/integration/conftest.py | 12 + .../tests/integration/test_charm.py | 24 +- .../kfp-schedwf/tests/integration/conftest.py | 12 + .../tests/integration/test_charm.py | 17 +- .../v0/kubeflow_dashboard_links.py | 7 +- charms/kfp-ui/tests/integration/conftest.py | 12 + charms/kfp-ui/tests/integration/test_charm.py | 18 +- .../kfp-viewer/tests/integration/conftest.py | 12 + .../tests/integration/test_charm.py | 18 +- charms/kfp-viz/tests/integration/conftest.py | 12 + .../kfp-viz/tests/integration/test_charm.py | 19 +- 19 files changed, 426 insertions(+), 271 deletions(-) create mode 100644 charms/kfp-api/tests/integration/conftest.py create mode 100644 charms/kfp-metadata-writer/tests/integration/conftest.py create mode 100644 charms/kfp-persistence/tests/integration/conftest.py create mode 100644 charms/kfp-profile-controller/tests/integration/conftest.py create mode 100644 charms/kfp-schedwf/tests/integration/conftest.py create mode 100644 charms/kfp-ui/tests/integration/conftest.py create mode 100644 charms/kfp-viewer/tests/integration/conftest.py create mode 100644 charms/kfp-viz/tests/integration/conftest.py diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 17f5fc26..927f5d4b 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -122,12 +122,29 @@ jobs: # Pinned to 3.x/stable due to https://github.com/canonical/charmcraft/issues/1845 charmcraft-channel: 3.x/stable + - name: Download packed charm(s) + id: download-charms + timeout-minutes: 5 + uses: actions/download-artifact@v4 + with: + pattern: packed-charm-cache-true-.-charms-${{ matrix.charm }}-* + merge-multiple: true + + - name: Debug + if: steps.download-charms.outcome == 'success' + run: | + ls -lh ${{ github.workspace }}/charms/${{ matrix.charm }}/ + - name: Integration tests + if: steps.download-charms.outcome == 'success' run: | # Requires the model to be called kubeflow due to # https://github.com/canonical/kfp-operators/issues/389 juju add-model kubeflow - sg snap_microk8s -c "tox -e ${{ matrix.charm }}-integration -- --model kubeflow" + # Pass the path where the charm artefact is downloaded to the tox command + # FIXME: Right now the complete path is half hardcoded to _ubuntu-20.04-amd64.charm + # We need to find a better way to dynamically get this value + sg snap_microk8s -c "tox -e ${{ matrix.charm }}-integration -- --model kubeflow --charm-path=${{ github.workspace }}/charms/${{ matrix.charm }}/${{ matrix.charm }}_ubuntu-20.04-amd64.charm" - name: Collect charm debug artifacts uses: canonical/kubeflow-ci/actions/dump-charm-debug-artifacts@main diff --git a/charms/kfp-api/tests/integration/conftest.py b/charms/kfp-api/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-api/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-api/tests/integration/test_charm.py b/charms/kfp-api/tests/integration/test_charm.py index 22e4374d..a89fd834 100644 --- a/charms/kfp-api/tests/integration/test_charm.py +++ b/charms/kfp-api/tests/integration/test_charm.py @@ -43,16 +43,21 @@ class TestCharm: """Integration test charm""" @pytest.mark.abort_on_fail - async def test_build_and_deploy(self, ops_test: OpsTest): + async def test_build_and_deploy(self, ops_test: OpsTest, request): """Deploy kfp-api with required charms and relations.""" - built_charm_path = await ops_test.build_charm("./") - logger.info(f"Built charm {built_charm_path}") image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) await ops_test.model.deploy( - entity_url=built_charm_path, + entity_url=entity_url, application_name=APP_NAME, resources=resources, trust=True, diff --git a/charms/kfp-metadata-writer/tests/integration/conftest.py b/charms/kfp-metadata-writer/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-metadata-writer/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-metadata-writer/tests/integration/test_charm.py b/charms/kfp-metadata-writer/tests/integration/test_charm.py index 0f0d5d77..4b4b6a13 100644 --- a/charms/kfp-metadata-writer/tests/integration/test_charm.py +++ b/charms/kfp-metadata-writer/tests/integration/test_charm.py @@ -26,16 +26,24 @@ @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") - +async def test_build_and_deploy_with_relations(ops_test: OpsTest, request): image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) await ops_test.model.deploy( - entity_url=built_charm_path, application_name=APP_NAME, resources=resources, trust=True + entity_url=entity_url, + application_name=APP_NAME, + resources=resources, + trust=True, ) + await ops_test.model.deploy(entity_url=MLMD, channel=MLMD_CHANNEL, trust=True) await ops_test.model.integrate(f"{MLMD}:grpc", f"{APP_NAME}:grpc") await ops_test.model.wait_for_idle(apps=[APP_NAME, MLMD], status="active", timeout=10 * 60) diff --git a/charms/kfp-persistence/tests/integration/conftest.py b/charms/kfp-persistence/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-persistence/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-persistence/tests/integration/test_charm.py b/charms/kfp-persistence/tests/integration/test_charm.py index bd0c3882..58ac45a5 100644 --- a/charms/kfp-persistence/tests/integration/test_charm.py +++ b/charms/kfp-persistence/tests/integration/test_charm.py @@ -39,16 +39,20 @@ class TestCharm: """Integration test charm""" @pytest.mark.abort_on_fail - async def test_build_and_deploy(self, ops_test: OpsTest): + async def test_build_and_deploy(self, ops_test: OpsTest, request): """Deploy kfp-persistence with required charms and relations.""" - built_charm_path = await ops_test.build_charm("./") - logger.info(f"Built charm {built_charm_path}") - image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) await ops_test.model.deploy( - entity_url=built_charm_path, + entity_url=entity_url, application_name=APP_NAME, resources=resources, trust=True, diff --git a/charms/kfp-profile-controller/files/upstream/sync.py b/charms/kfp-profile-controller/files/upstream/sync.py index ada66327..a6c62f88 100644 --- a/charms/kfp-profile-controller/files/upstream/sync.py +++ b/charms/kfp-profile-controller/files/upstream/sync.py @@ -44,13 +44,22 @@ def emit_settings_to_logs(settings): logger.info(f"Settings = {safe_settings}") -def get_settings_from_env(controller_port=None, - visualization_server_image=None, frontend_image=None, - visualization_server_tag=None, frontend_tag=None, disable_istio_sidecar=None, - minio_access_key=None, minio_secret_key=None, kfp_default_pipeline_root=None, - minio_host=None, minio_port=None, minio_namespace=None, - metadata_grpc_service_host=None, - metadata_grpc_service_port=None): +def get_settings_from_env( + controller_port=None, + visualization_server_image=None, + frontend_image=None, + visualization_server_tag=None, + frontend_tag=None, + disable_istio_sidecar=None, + minio_access_key=None, + minio_secret_key=None, + kfp_default_pipeline_root=None, + minio_host=None, + minio_port=None, + minio_namespace=None, + metadata_grpc_service_host=None, + metadata_grpc_service_port=None, +): """ Returns a dict of settings from environment variables relevant to the controller @@ -73,80 +82,86 @@ def get_settings_from_env(controller_port=None, metadata_grpc_service_port: 8080 """ settings = dict() - settings["controller_port"] = \ - controller_port or \ - os.environ.get("CONTROLLER_PORT", "8080") + settings["controller_port"] = controller_port or os.environ.get("CONTROLLER_PORT", "8080") - settings["visualization_server_image"] = \ - visualization_server_image or \ - os.environ.get("VISUALIZATION_SERVER_IMAGE", "gcr.io/ml-pipeline/visualization-server") + settings["visualization_server_image"] = visualization_server_image or os.environ.get( + "VISUALIZATION_SERVER_IMAGE", "gcr.io/ml-pipeline/visualization-server" + ) - settings["frontend_image"] = \ - frontend_image or \ - os.environ.get("FRONTEND_IMAGE", "gcr.io/ml-pipeline/frontend") + settings["frontend_image"] = frontend_image or os.environ.get( + "FRONTEND_IMAGE", "gcr.io/ml-pipeline/frontend" + ) # Look for specific tags for each image first, falling back to # previously used KFP_VERSION environment variable for backwards # compatibility - settings["visualization_server_tag"] = \ - visualization_server_tag or \ - os.environ.get("VISUALIZATION_SERVER_TAG") or \ - os.environ["KFP_VERSION"] + settings["visualization_server_tag"] = ( + visualization_server_tag + or os.environ.get("VISUALIZATION_SERVER_TAG") + or os.environ["KFP_VERSION"] + ) - settings["frontend_tag"] = \ - frontend_tag or \ - os.environ.get("FRONTEND_TAG") or \ - os.environ["KFP_VERSION"] + settings["frontend_tag"] = ( + frontend_tag or os.environ.get("FRONTEND_TAG") or os.environ["KFP_VERSION"] + ) - settings["disable_istio_sidecar"] = \ - disable_istio_sidecar if disable_istio_sidecar is not None \ - else os.environ.get("DISABLE_ISTIO_SIDECAR") == "true" + settings["disable_istio_sidecar"] = ( + disable_istio_sidecar + if disable_istio_sidecar is not None + else os.environ.get("DISABLE_ISTIO_SIDECAR") == "true" + ) - settings["minio_access_key"] = \ - minio_access_key or \ - base64.b64encode(bytes(os.environ.get("MINIO_ACCESS_KEY"), 'utf-8')).decode('utf-8') + settings["minio_access_key"] = minio_access_key or base64.b64encode( + bytes(os.environ.get("MINIO_ACCESS_KEY"), "utf-8") + ).decode("utf-8") - settings["minio_secret_key"] = \ - minio_secret_key or \ - base64.b64encode(bytes(os.environ.get("MINIO_SECRET_KEY"), 'utf-8')).decode('utf-8') + settings["minio_secret_key"] = minio_secret_key or base64.b64encode( + bytes(os.environ.get("MINIO_SECRET_KEY"), "utf-8") + ).decode("utf-8") - settings["minio_host"] = \ - minio_host or \ - os.environ.get("MINIO_HOST", "minio") + settings["minio_host"] = minio_host or os.environ.get("MINIO_HOST", "minio") - settings["minio_port"] = \ - minio_port or \ - os.environ.get("MINIO_PORT", "9000") + settings["minio_port"] = minio_port or os.environ.get("MINIO_PORT", "9000") - settings["minio_namespace"] = \ - minio_namespace or \ - os.environ.get("MINIO_NAMESPACE", "kubeflow") + settings["minio_namespace"] = minio_namespace or os.environ.get("MINIO_NAMESPACE", "kubeflow") # KFP_DEFAULT_PIPELINE_ROOT is optional - settings["kfp_default_pipeline_root"] = \ - kfp_default_pipeline_root or \ - os.environ.get("KFP_DEFAULT_PIPELINE_ROOT") + settings["kfp_default_pipeline_root"] = kfp_default_pipeline_root or os.environ.get( + "KFP_DEFAULT_PIPELINE_ROOT" + ) - settings["metadata_grpc_service_host"] = \ - metadata_grpc_service_host or \ - os.environ.get("METADATA_GRPC_SERVICE_HOST", "metadata-grpc-service.kubeflow") + settings["metadata_grpc_service_host"] = metadata_grpc_service_host or os.environ.get( + "METADATA_GRPC_SERVICE_HOST", "metadata-grpc-service.kubeflow" + ) - settings["metadata_grpc_service_port"] = \ - metadata_grpc_service_port or \ - os.environ.get("METADATA_GRPC_SERVICE_PORT", "8080") + settings["metadata_grpc_service_port"] = metadata_grpc_service_port or os.environ.get( + "METADATA_GRPC_SERVICE_PORT", "8080" + ) return settings -def server_factory(visualization_server_image, - visualization_server_tag, frontend_image, frontend_tag, - disable_istio_sidecar, minio_access_key, - minio_secret_key, minio_host, minio_namespace, minio_port, - metadata_grpc_service_host, metadata_grpc_service_port, - kfp_default_pipeline_root=None, url="", controller_port=8080): +def server_factory( + visualization_server_image, + visualization_server_tag, + frontend_image, + frontend_tag, + disable_istio_sidecar, + minio_access_key, + minio_secret_key, + minio_host, + minio_namespace, + minio_port, + metadata_grpc_service_host, + metadata_grpc_service_port, + kfp_default_pipeline_root=None, + url="", + controller_port=8080, +): """ Returns an HTTPServer populated with Handler with customized settings """ + class Controller(BaseHTTPRequestHandler): def sync(self, parent, attachments): logger.info("Got new request") @@ -154,41 +169,46 @@ def sync(self, parent, attachments): # parent is a namespace namespace = parent.get("metadata", {}).get("name") - pipeline_enabled = parent.get("metadata", {}).get( - "labels", {}).get("pipelines.kubeflow.org/enabled") + pipeline_enabled = ( + parent.get("metadata", {}).get("labels", {}).get("pipelines.kubeflow.org/enabled") + ) if pipeline_enabled != "true": - logger.info(f"Namespace not in scope, no action taken (metadata.labels.pipelines.kubeflow.org/enabled = {pipeline_enabled}, must be 'true')") + logger.info( + f"Namespace not in scope, no action taken (metadata.labels.pipelines.kubeflow.org/enabled = {pipeline_enabled}, must be 'true')" + ) return {"status": {}, "attachments": []} desired_configmap_count = 1 desired_resources = [] if kfp_default_pipeline_root: desired_configmap_count = 2 - desired_resources += [{ - "apiVersion": "v1", - "kind": "ConfigMap", - "metadata": { - "name": "kfp-launcher", - "namespace": namespace, - }, - "data": { - "defaultPipelineRoot": kfp_default_pipeline_root, - }, - }] - + desired_resources += [ + { + "apiVersion": "v1", + "kind": "ConfigMap", + "metadata": { + "name": "kfp-launcher", + "namespace": namespace, + }, + "data": { + "defaultPipelineRoot": kfp_default_pipeline_root, + }, + } + ] # Compute status based on observed state. desired_status = { - "kubeflow-pipelines-ready": - len(attachments["Secret.v1"]) == 1 and - len(attachments["ConfigMap.v1"]) == desired_configmap_count and - len(attachments["Deployment.apps/v1"]) == 2 and - len(attachments["Service.v1"]) == 2 and - # TODO CANONICAL: This only works if istio is available. Disabled for now - # len(attachments["DestinationRule.networking.istio.io/v1alpha3"]) == 1 and - # len(attachments["AuthorizationPolicy.security.istio.io/v1beta1"]) == 1 and - "True" or "False" + "kubeflow-pipelines-ready": len(attachments["Secret.v1"]) == 1 + and len(attachments["ConfigMap.v1"]) == desired_configmap_count + and len(attachments["Deployment.apps/v1"]) == 2 + and len(attachments["Service.v1"]) == 2 + and + # TODO CANONICAL: This only works if istio is available. Disabled for now + # len(attachments["DestinationRule.networking.istio.io/v1alpha3"]) == 1 and + # len(attachments["AuthorizationPolicy.security.istio.io/v1beta1"]) == 1 and + "True" + or "False" } # Generate the desired child object(s). @@ -210,50 +230,35 @@ def sync(self, parent, attachments): "apiVersion": "apps/v1", "kind": "Deployment", "metadata": { - "labels": { - "app": "ml-pipeline-visualizationserver" - }, + "labels": {"app": "ml-pipeline-visualizationserver"}, "name": "ml-pipeline-visualizationserver", "namespace": namespace, }, "spec": { "selector": { - "matchLabels": { - "app": "ml-pipeline-visualizationserver" - }, + "matchLabels": {"app": "ml-pipeline-visualizationserver"}, }, "template": { "metadata": { - "labels": { - "app": "ml-pipeline-visualizationserver" - }, - "annotations": disable_istio_sidecar and { - "sidecar.istio.io/inject": "false" - } or {}, + "labels": {"app": "ml-pipeline-visualizationserver"}, + "annotations": disable_istio_sidecar + and {"sidecar.istio.io/inject": "false"} + or {}, }, "spec": { - "containers": [{ - "image": f"{visualization_server_image}:{visualization_server_tag}", - "imagePullPolicy": - "IfNotPresent", - "name": - "ml-pipeline-visualizationserver", - "ports": [{ - "containerPort": 8888 - }], - "resources": { - "requests": { - "cpu": "50m", - "memory": "200Mi" - }, - "limits": { - "cpu": "500m", - "memory": "1Gi" + "containers": [ + { + "image": f"{visualization_server_image}:{visualization_server_tag}", + "imagePullPolicy": "IfNotPresent", + "name": "ml-pipeline-visualizationserver", + "ports": [{"containerPort": 8888}], + "resources": { + "requests": {"cpu": "50m", "memory": "200Mi"}, + "limits": {"cpu": "500m", "memory": "1Gi"}, }, } - }], - "serviceAccountName": - "default-editor", + ], + "serviceAccountName": "default-editor", }, }, }, @@ -305,12 +310,14 @@ def sync(self, parent, attachments): "namespace": namespace, }, "spec": { - "ports": [{ - "name": "http", - "port": 8888, - "protocol": "TCP", - "targetPort": 8888, - }], + "ports": [ + { + "name": "http", + "port": 8888, + "protocol": "TCP", + "targetPort": 8888, + } + ], "selector": { "app": "ml-pipeline-visualizationserver", }, @@ -321,76 +328,59 @@ def sync(self, parent, attachments): "apiVersion": "apps/v1", "kind": "Deployment", "metadata": { - "labels": { - "app": "ml-pipeline-ui-artifact" - }, + "labels": {"app": "ml-pipeline-ui-artifact"}, "name": "ml-pipeline-ui-artifact", "namespace": namespace, }, "spec": { - "selector": { - "matchLabels": { - "app": "ml-pipeline-ui-artifact" - } - }, + "selector": {"matchLabels": {"app": "ml-pipeline-ui-artifact"}}, "template": { "metadata": { - "labels": { - "app": "ml-pipeline-ui-artifact" - }, - "annotations": disable_istio_sidecar and { - "sidecar.istio.io/inject": "false" - } or {}, + "labels": {"app": "ml-pipeline-ui-artifact"}, + "annotations": disable_istio_sidecar + and {"sidecar.istio.io/inject": "false"} + or {}, }, "spec": { - "containers": [{ - "name": - "ml-pipeline-ui-artifact", - "image": f"{frontend_image}:{frontend_tag}", - "imagePullPolicy": - "IfNotPresent", - "ports": [{ - "containerPort": 3000 - }], - "env": [ - {'name': "MINIO_PORT", 'value': minio_port}, - {'name': "MINIO_HOST", 'value': minio_host}, - {'name': "MINIO_NAMESPACE", 'value': minio_namespace}, - { - "name": "MINIO_ACCESS_KEY", - "valueFrom": { - "secretKeyRef": { - "key": "accesskey", - "name": "mlpipeline-minio-artifact" - } - } - }, - { - "name": "MINIO_SECRET_KEY", - "valueFrom": { - "secretKeyRef": { - "key": "secretkey", - "name": "mlpipeline-minio-artifact" - } - } - } - ], - "resources": { - "requests": { - "cpu": "10m", - "memory": "70Mi" - }, - "limits": { - "cpu": "100m", - "memory": "500Mi" + "containers": [ + { + "name": "ml-pipeline-ui-artifact", + "image": f"{frontend_image}:{frontend_tag}", + "imagePullPolicy": "IfNotPresent", + "ports": [{"containerPort": 3000}], + "env": [ + {"name": "MINIO_PORT", "value": minio_port}, + {"name": "MINIO_HOST", "value": minio_host}, + {"name": "MINIO_NAMESPACE", "value": minio_namespace}, + { + "name": "MINIO_ACCESS_KEY", + "valueFrom": { + "secretKeyRef": { + "key": "accesskey", + "name": "mlpipeline-minio-artifact", + } + }, + }, + { + "name": "MINIO_SECRET_KEY", + "valueFrom": { + "secretKeyRef": { + "key": "secretkey", + "name": "mlpipeline-minio-artifact", + } + }, + }, + ], + "resources": { + "requests": {"cpu": "10m", "memory": "70Mi"}, + "limits": {"cpu": "100m", "memory": "500Mi"}, }, } - }], - "serviceAccountName": - "default-editor" - } - } - } + ], + "serviceAccountName": "default-editor", + }, + }, + }, }, # Added from https://github.com/kubeflow/pipelines/pull/6629 to fix # https://github.com/canonical/bundle-kubeflow/issues/423. This was not yet in @@ -400,17 +390,10 @@ def sync(self, parent, attachments): { "apiVersion": "kubeflow.org/v1alpha1", "kind": "PodDefault", - "metadata": { - "name": "access-ml-pipeline", - "namespace": namespace - }, + "metadata": {"name": "access-ml-pipeline", "namespace": namespace}, "spec": { "desc": "Allow access to Kubeflow Pipelines", - "selector": { - "matchLabels": { - "access-ml-pipeline": "true" - } - }, + "selector": {"matchLabels": {"access-ml-pipeline": "true"}}, "volumes": [ { "name": "volume-kf-pipeline-token", @@ -420,27 +403,27 @@ def sync(self, parent, attachments): "serviceAccountToken": { "path": "token", "expirationSeconds": 7200, - "audience": "pipelines.kubeflow.org" + "audience": "pipelines.kubeflow.org", } } ] - } + }, } ], "volumeMounts": [ { "mountPath": "/var/run/secrets/kubeflow/pipelines", "name": "volume-kf-pipeline-token", - "readOnly": True + "readOnly": True, } ], "env": [ { "name": "KF_PIPELINES_SA_TOKEN_PATH", - "value": "/var/run/secrets/kubeflow/pipelines/token" + "value": "/var/run/secrets/kubeflow/pipelines/token", } - ] - } + ], + }, }, { "apiVersion": "v1", @@ -448,22 +431,19 @@ def sync(self, parent, attachments): "metadata": { "name": "ml-pipeline-ui-artifact", "namespace": namespace, - "labels": { - "app": "ml-pipeline-ui-artifact" - } + "labels": {"app": "ml-pipeline-ui-artifact"}, }, "spec": { - "ports": [{ - "name": - "http", # name is required to let istio understand request protocol - "port": 80, - "protocol": "TCP", - "targetPort": 3000 - }], - "selector": { - "app": "ml-pipeline-ui-artifact" - } - } + "ports": [ + { + "name": "http", # name is required to let istio understand request protocol + "port": 80, + "protocol": "TCP", + "targetPort": 3000, + } + ], + "selector": {"app": "ml-pipeline-ui-artifact"}, + }, }, # This AuthorizationPolicy was added from https://github.com/canonical/kfp-operators/pull/356 # to fix https://github.com/canonical/notebook-operators/issues/311 @@ -489,35 +469,39 @@ def sync(self, parent, attachments): }, }, ] - print('Received request:\n', json.dumps(parent, indent=2, sort_keys=True)) - print('Desired resources except secrets:\n', json.dumps(desired_resources, indent=2, sort_keys=True)) + print("Received request:\n", json.dumps(parent, indent=2, sort_keys=True)) + print( + "Desired resources except secrets:\n", + json.dumps(desired_resources, indent=2, sort_keys=True), + ) # Moved after the print argument because this is sensitive data. - desired_resources.append({ - "apiVersion": "v1", - "kind": "Secret", - "metadata": { - "name": "mlpipeline-minio-artifact", - "namespace": namespace, - }, - "data": { - "accesskey": minio_access_key, - "secretkey": minio_secret_key, - }, - }) + desired_resources.append( + { + "apiVersion": "v1", + "kind": "Secret", + "metadata": { + "name": "mlpipeline-minio-artifact", + "namespace": namespace, + }, + "data": { + "accesskey": minio_access_key, + "secretkey": minio_secret_key, + }, + } + ) return {"status": desired_status, "attachments": desired_resources} def do_POST(self): # Serve the sync() function as a JSON webhook. - observed = json.loads( - self.rfile.read(int(self.headers.get("content-length")))) + observed = json.loads(self.rfile.read(int(self.headers.get("content-length")))) logger.info(f"Request is {observed}") desired = self.sync(observed["object"], observed["attachments"]) self.send_response(200) self.send_header("Content-type", "application/json") self.end_headers() - self.wfile.write(bytes(json.dumps(desired), 'utf-8')) + self.wfile.write(bytes(json.dumps(desired), "utf-8")) return HTTPServer((url, int(controller_port)), Controller) diff --git a/charms/kfp-profile-controller/tests/integration/conftest.py b/charms/kfp-profile-controller/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-profile-controller/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-profile-controller/tests/integration/test_charm.py b/charms/kfp-profile-controller/tests/integration/test_charm.py index 1160bdbf..ba42f476 100644 --- a/charms/kfp-profile-controller/tests/integration/test_charm.py +++ b/charms/kfp-profile-controller/tests/integration/test_charm.py @@ -48,19 +48,14 @@ @pytest.mark.abort_on_fail -async def test_build_and_deploy(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm("./") - logger.info(f"Built charm {built_charm_path}") - - image_path = METADATA["resources"]["oci-image"]["upstream-source"] - resources = {"oci-image": image_path} - +async def test_build_and_deploy(ops_test: OpsTest, request): # Deploy the admission webhook to apply the PodDefault CRD required by the charm workload await ops_test.model.deploy( entity_url=ADMISSION_WEBHOOK, channel=ADMISSION_WEBHOOK_CHANNEL, trust=ADMISSION_WEBHOOK_TRUST, ) + # TODO: The webhook charm must be active before the metacontroller is deployed, due to the bug # described here: https://github.com/canonical/metacontroller-operator/issues/86 # Drop this wait_for_idle once the above issue is closed @@ -72,8 +67,21 @@ async def test_build_and_deploy(ops_test: OpsTest): trust=METACONTROLLER_TRUST, ) + # Deploy the charm under test + image_path = METADATA["resources"]["oci-image"]["upstream-source"] + resources = {"oci-image": image_path} + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) + await ops_test.model.deploy( - built_charm_path, application_name=CHARM_NAME, resources=resources, trust=True + entity_url=entity_url, + resources=resources, + trust=True, ) # Deploy required relations diff --git a/charms/kfp-schedwf/tests/integration/conftest.py b/charms/kfp-schedwf/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-schedwf/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-schedwf/tests/integration/test_charm.py b/charms/kfp-schedwf/tests/integration/test_charm.py index 64d8c229..4e3563b3 100644 --- a/charms/kfp-schedwf/tests/integration/test_charm.py +++ b/charms/kfp-schedwf/tests/integration/test_charm.py @@ -21,15 +21,22 @@ @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") - +async def test_build_and_deploy_with_relations(ops_test: OpsTest, request): image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) await ops_test.model.deploy( - entity_url=built_charm_path, application_name=APP_NAME, resources=resources, trust=True + entity_url=entity_url, + application_name=APP_NAME, + resources=resources, + trust=True, ) await ops_test.model.wait_for_idle( diff --git a/charms/kfp-ui/lib/charms/kubeflow_dashboard/v0/kubeflow_dashboard_links.py b/charms/kfp-ui/lib/charms/kubeflow_dashboard/v0/kubeflow_dashboard_links.py index 9ab3e819..3481cf75 100644 --- a/charms/kfp-ui/lib/charms/kubeflow_dashboard/v0/kubeflow_dashboard_links.py +++ b/charms/kfp-ui/lib/charms/kubeflow_dashboard/v0/kubeflow_dashboard_links.py @@ -56,6 +56,7 @@ def __init__(self, *args): # ... ``` """ + import os from dataclasses import dataclass, asdict import json @@ -78,7 +79,7 @@ def __init__(self, *args): LIBPATCH = 3 -DASHBOARD_LINK_LOCATIONS = ['menu', 'external', 'quick', 'documentation'] +DASHBOARD_LINK_LOCATIONS = ["menu", "external", "quick", "documentation"] DASHBOARD_LINKS_FIELD = "dashboard_links" @@ -109,7 +110,9 @@ class DashboardLink: def __post_init__(self): """Validate that location is one of the accepted values.""" if self.location not in DASHBOARD_LINK_LOCATIONS: - raise ValueError(f"location must be one of {DASHBOARD_LINK_LOCATIONS} - got '{self.location}'.") + raise ValueError( + f"location must be one of {DASHBOARD_LINK_LOCATIONS} - got '{self.location}'." + ) class KubeflowDashboardLinksUpdatedEvent(RelationEvent): diff --git a/charms/kfp-ui/tests/integration/conftest.py b/charms/kfp-ui/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-ui/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-ui/tests/integration/test_charm.py b/charms/kfp-ui/tests/integration/test_charm.py index 02c59c4e..e0992c97 100644 --- a/charms/kfp-ui/tests/integration/test_charm.py +++ b/charms/kfp-ui/tests/integration/test_charm.py @@ -22,16 +22,24 @@ @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") - +async def test_build_and_deploy_with_relations(ops_test: OpsTest, request): image_path = METADATA["resources"]["ml-pipeline-ui"]["upstream-source"] resources = {"ml-pipeline-ui": image_path} + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) await ops_test.model.deploy( - entity_url=built_charm_path, application_name=APP_NAME, resources=resources, trust=True + entity_url=entity_url, + application_name=APP_NAME, + resources=resources, + trust=True, ) + await ops_test.model.deploy(BUNDLE, trust=True) await ops_test.model.integrate(f"{APP_NAME}:kfp-api", "kfp-api:kfp-api") await ops_test.model.integrate(f"{APP_NAME}:object-storage", "minio:object-storage") diff --git a/charms/kfp-viewer/tests/integration/conftest.py b/charms/kfp-viewer/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-viewer/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-viewer/tests/integration/test_charm.py b/charms/kfp-viewer/tests/integration/test_charm.py index c96f8794..1cd1ea96 100644 --- a/charms/kfp-viewer/tests/integration/test_charm.py +++ b/charms/kfp-viewer/tests/integration/test_charm.py @@ -21,16 +21,24 @@ @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") - +async def test_build_and_deploy_with_relations(ops_test: OpsTest, request): image_path = METADATA["resources"]["kfp-viewer-image"]["upstream-source"] resources = {"kfp-viewer-image": image_path} + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) await ops_test.model.deploy( - entity_url=built_charm_path, application_name=APP_NAME, resources=resources, trust=True + entity_url=entity_url, + application_name=APP_NAME, + resources=resources, + trust=True, ) + await ops_test.model.wait_for_idle( apps=[APP_NAME], status="active", diff --git a/charms/kfp-viz/tests/integration/conftest.py b/charms/kfp-viz/tests/integration/conftest.py new file mode 100644 index 00000000..ac7f08ff --- /dev/null +++ b/charms/kfp-viz/tests/integration/conftest.py @@ -0,0 +1,12 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +from _pytest.config.argparsing import Parser + + +def pytest_addoption(parser: Parser): + parser.addoption( + "--charm-path", + help="Path to charm file when downloaded as artefact as a result of build_charm.yaml", + ) diff --git a/charms/kfp-viz/tests/integration/test_charm.py b/charms/kfp-viz/tests/integration/test_charm.py index 7eb9b68f..b712d40c 100644 --- a/charms/kfp-viz/tests/integration/test_charm.py +++ b/charms/kfp-viz/tests/integration/test_charm.py @@ -21,16 +21,23 @@ @pytest.mark.abort_on_fail -async def test_build_and_deploy_with_relations(ops_test: OpsTest): - built_charm_path = await ops_test.build_charm(CHARM_ROOT) - log.info(f"Built charm {built_charm_path}") - +async def test_build_and_deploy_with_relations(ops_test: OpsTest, request): image_path = METADATA["resources"]["oci-image"]["upstream-source"] resources = {"oci-image": image_path} - + # Keep the option to run the integration tests locally + # by building the charm and then deploying + entity_url = ( + await ops_test.build_charm("./") + if not (entity_url := request.config.getoption("--charm-path")) + else entity_url + ) await ops_test.model.deploy( - entity_url=built_charm_path, application_name=APP_NAME, resources=resources, trust=True + entity_url=entity_url, + application_name=APP_NAME, + resources=resources, + trust=True, ) + await ops_test.model.wait_for_idle( apps=[APP_NAME], status="active",