diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 84f7a39f..66566cde 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -14,6 +14,7 @@ jobs: matrix: charm: - kfp-api + - kfp-metadata-writer - kfp-persistence - kfp-profile-controller - kfp-schedwf @@ -33,6 +34,7 @@ jobs: matrix: charm: - kfp-api + - kfp-metadata-writer - kfp-persistence - kfp-profile-controller - kfp-schedwf @@ -52,6 +54,7 @@ jobs: matrix: charm: - kfp-api + - kfp-metadata-writer - kfp-persistence - kfp-profile-controller - kfp-schedwf @@ -106,7 +109,11 @@ jobs: test-bundle: name: Test the bundle runs-on: ubuntu-20.04 - + strategy: + matrix: + sdk: + - v1 + - v2 steps: # This is a workaround for https://github.com/canonical/kfp-operators/issues/250 # Ideally we'd use self-hosted runners, but this effort is still not stable @@ -142,7 +149,7 @@ jobs: # Run integration tests against the 1.7 generic install bundle definition # Using destructive mode because of https://github.com/canonical/charmcraft/issues/1132 # and https://github.com/canonical/charmcraft/issues/1138 - sg snap_microk8s -c "tox -e bundle-integration -- --model kubeflow --bundle=./tests/integration/bundles/kfp_1.7_stable_install.yaml.j2 --destructive-mode" + sg snap_microk8s -c "tox -e bundle-integration-${{ matrix.sdk }} -- --model kubeflow --bundle=./tests/integration/bundles/kfp_latest_edge.yaml.j2 --destructive-mode" - name: Get all run: kubectl get all -A diff --git a/.gitignore b/.gitignore index e895effe..fc37ceca 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,4 @@ __pycache__/ .coverage .idea .vscode +venv diff --git a/charms/kfp-api/config.yaml b/charms/kfp-api/config.yaml index 17b5e095..bce24b57 100644 --- a/charms/kfp-api/config.yaml +++ b/charms/kfp-api/config.yaml @@ -41,7 +41,7 @@ options: Used if service account is left unspecified when creating a run init-connection-timeout: type: string - default: '5s' + default: '6m' description: | Connection timeout used when initializing clients for related services. The format used can be anything accepted by `time.ParseDuration`. diff --git a/charms/kfp-api/metadata.yaml b/charms/kfp-api/metadata.yaml index c64bb853..f00b7359 100755 --- a/charms/kfp-api/metadata.yaml +++ b/charms/kfp-api/metadata.yaml @@ -9,13 +9,13 @@ website: https://charmhub.io/kfp-api source: https://github.com/canonical/kfp-operators issues: https://github.com/canonical/kfp-operators/issues containers: - ml-pipeline-api-server: + apiserver: resource: oci-image resources: oci-image: type: oci-image description: Backing OCI image - upstream-source: charmedkubeflow/api-server:2.0.0-alpha.7_20.04_1 + upstream-source: gcr.io/ml-pipeline/api-server:2.0.3 requires: mysql: interface: mysql diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py index 75219d9c..15da2061 100755 --- a/charms/kfp-api/src/charm.py +++ b/charms/kfp-api/src/charm.py @@ -7,7 +7,6 @@ https://github.com/canonical/kfp-operators/ """ -import json import logging from pathlib import Path @@ -24,14 +23,7 @@ from lightkube.models.core_v1 import ServicePort from ops.charm import CharmBase from ops.main import main -from ops.model import ( - ActiveStatus, - BlockedStatus, - Container, - MaintenanceStatus, - ModelError, - WaitingStatus, -) +from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, ModelError, WaitingStatus from ops.pebble import CheckStatus, Layer from serialized_data_interface import ( NoCompatibleVersions, @@ -77,7 +69,7 @@ def __init__(self, *args): f"--sampleconfig={SAMPLE_CONFIG} " "-logtostderr=true " ) - self._container_name = "ml-pipeline-api-server" + self._container_name = "apiserver" self._database_name = "mlpipeline" self._container = self.unit.get_container(self._container_name) @@ -101,7 +93,7 @@ def __init__(self, *args): # setup events to be handled by main event handler self.framework.observe(self.on.leader_elected, self._on_event) self.framework.observe(self.on.config_changed, self._on_event) - self.framework.observe(self.on.ml_pipeline_api_server_pebble_ready, self._on_event) + self.framework.observe(self.on.apiserver_pebble_ready, self._on_event) change_events = [ self.on["object-storage"].relation_changed, self.on["kfp-viz"].relation_changed, @@ -175,9 +167,7 @@ def k8s_resource_handler(self, handler: KubernetesResourceHandler): @property def service_environment(self): """Return environment variables based on model configuration.""" - ret_env_vars = {"POD_NAMESPACE": self.model.name} - - return ret_env_vars + return self._generate_environment() @property def _kfp_api_layer(self) -> Layer: @@ -210,76 +200,71 @@ def _kfp_api_layer(self) -> Layer: return Layer(layer_config) - def _generate_config(self, interfaces): - """Generate configuration based on supplied data. + def _generate_environment(self) -> dict: + """Generate environment based on supplied data. Configuration is generated based on: - Supplied interfaces. - Database data: from MySQL relation data or from data platform library. - Model configuration. + + Return: + env_vars(dict): a dictionary of environment variables for the api server. """ - config = self.model.config try: + interfaces = self._get_interfaces() db_data = self._get_db_data() - os = self._get_object_storage(interfaces) - viz = self._get_viz(interfaces) + object_storage = self._get_object_storage(interfaces) + viz_data = self._get_viz(interfaces) except ErrorWithStatus as error: self.logger.error("Failed to generate container configuration.") raise error - # at this point all data is correctly populated and proper config can be generated - config_json = { - "DBConfig": { - "ConMaxLifeTime": "120s", - "DBName": db_data["db_name"], - "DriverName": "mysql", - "GroupConcatMaxLen": "4194304", - "Host": db_data["db_host"], - "Password": db_data["db_password"], - "Port": db_data["db_port"], - "User": db_data["db_username"], - }, - "ObjectStoreConfig": { - "AccessKey": os["access-key"], - "BucketName": config["object-store-bucket-name"], - "Host": f"{os['service']}.{os['namespace']}", - "Multipart": {"Disable": "true"}, - "PipelinePath": "pipelines", - "Port": str(os["port"]), - "Region": "", - "SecretAccessKey": os["secret-key"], - "Secure": str(os["secure"]).lower(), - }, - "ARCHIVE_CONFIG_LOG_FILE_NAME": config["log-archive-filename"], - "ARCHIVE_CONFIG_LOG_PATH_PREFIX": config["log-archive-prefix"], - "AUTO_UPDATE_PIPELINE_DEFAULT_VERSION": str( - config["auto-update-default-version"] - ).lower(), - "CACHE_IMAGE": config["cache-image"], - "CACHE_NODE_RESTRICTIONS": "false", - "CacheEnabled": str(config["cache-enabled"]).lower(), - "DefaultPipelineRunnerServiceAccount": config["runner-sa"], - "InitConnectionTimeout": config["init-connection-timeout"], + env_vars = { + # Configurations that are also defined in the upstream manifests + "AUTO_UPDATE_PIPELINE_DEFAULT_VERSION": self.model.config[ + "auto-update-default-version" + ], + "KFP_API_SERVICE_NAME": KFP_API_SERVICE_NAME, "KUBEFLOW_USERID_HEADER": "kubeflow-userid", "KUBEFLOW_USERID_PREFIX": "", + "POD_NAMESPACE": self.model.name, + "OBJECTSTORECONFIG_SECURE": "false", + "OBJECTSTORECONFIG_BUCKETNAME": self.model.config["object-store-bucket-name"], + "DBCONFIG_CONMAXLIFETIME": "120s", + "DB_DRIVER_NAME": "mysql", + "DBCONFIG_MYSQLCONFIG_USER": db_data["db_username"], + "DBCONFIG_MYSQLCONFIG_PASSWORD": db_data["db_password"], + "DBCONFIG_MYSQLCONFIG_DBNAME": db_data["db_name"], + "DBCONFIG_MYSQLCONFIG_HOST": db_data["db_host"], + "DBCONFIG_MYSQLCONFIG_PORT": db_data["db_port"], + "OBJECTSTORECONFIG_ACCESSKEY": object_storage["access-key"], + "OBJECTSTORECONFIG_SECRETACCESSKEY": object_storage["secret-key"], + "DEFAULTPIPELINERUNNERSERVICEACCOUNT": "default-editor", "MULTIUSER": "true", - "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_HOST": viz["service-name"], - "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_PORT": viz["service-port"], + "VISUALIZATIONSERVICE_NAME": viz_data["service-name"], + "VISUALIZATIONSERVICE_PORT": viz_data["service-port"], + "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_HOST": viz_data["service-name"], + "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_PORT": viz_data["service-port"], + "CACHE_IMAGE": self.model.config["cache-image"], + # Configurations charmed-kubeflow adds to those of upstream + "ARCHIVE_CONFIG_LOG_FILE_NAME": self.model.config["log-archive-filename"], + "ARCHIVE_CONFIG_LOG_PATH_PREFIX": self.model.config["log-archive-prefix"], + # OBJECTSTORECONFIG_HOST and _PORT set the object storage configurations, + # taking precedence over configuration in the config.json or + # MINIO_SERVICE_SERVICE_* environment variables. + # NOTE: While OBJECTSTORECONFIG_HOST and _PORT control the object store + # that the apiserver connects to, other parts of kfp currently cannot use + # object stores with arbitrary names. See + # https://github.com/kubeflow/pipelines/issues/9689 and + # https://github.com/canonical/minio-operator/pull/151 for more details. + "OBJECTSTORECONFIG_HOST": f"{object_storage['service']}.{object_storage['namespace']}", + "OBJECTSTORECONFIG_PORT": str(object_storage["port"]), + "OBJECTSTORECONFIG_REGION": "", } - return config_json - - def _check_container_connection(self, container: Container) -> None: - """Check if connection can be made with container. - - Args: - container: the named container in a unit to check. - Raises: - ErrorWithStatus if the connection cannot be made. - """ - if not container.can_connect(): - raise ErrorWithStatus("Pod startup is not complete", MaintenanceStatus) + return env_vars def _check_status(self): """Check status of workload and set status accordingly.""" @@ -301,28 +286,6 @@ def _check_status(self): raise ErrorWithStatus("Workload failed health check", MaintenanceStatus) self.model.unit.status = ActiveStatus() - def _upload_files_to_container(self, config_json): - """Upload required files to container.""" - try: - self._check_container_connection(self.container) - except ErrorWithStatus as error: - self.model.unit.status = error.status - raise error - try: - with open("src/sample_config.json", "r") as sample_config: - file_content = sample_config.read() - self.container.push(SAMPLE_CONFIG, file_content, make_dirs=True) - except ErrorWithStatus as error: - self.logger.error("Failed to upload sample config to container.") - raise error - try: - file_content = json.dumps(config_json) - config = CONFIG_DIR / "config.json" - self.container.push(config, file_content, make_dirs=True) - except ErrorWithStatus as error: - self.logger.error("Failed to upload config to container.") - raise error - def _send_info(self, interfaces): if interfaces["kfp-api"]: interfaces["kfp-api"].send_data( @@ -680,12 +643,9 @@ def _on_event(self, event, force_conflicts: bool = False) -> None: # Set up all relations/fetch required data try: self._check_leader() - interfaces = self._get_interfaces() - config_json = self._generate_config(interfaces) - self._upload_files_to_container(config_json) self._apply_k8s_resources(force_conflicts=force_conflicts) update_layer(self._container_name, self._container, self._kfp_api_layer, self.logger) - self._send_info(interfaces) + self._send_info(self._get_interfaces()) except ErrorWithStatus as err: self.model.unit.status = err.status self.logger.error(f"Failed to handle {event} with error: {err}") diff --git a/charms/kfp-api/src/sample_config.json b/charms/kfp-api/src/sample_config.json deleted file mode 100644 index a4f4145c..00000000 --- a/charms/kfp-api/src/sample_config.json +++ /dev/null @@ -1,12 +0,0 @@ -[ - { - "name": "[Tutorial] Data passing in python components", - "description": "[source code](https://github.com/kubeflow/pipelines/tree/master/samples/tutorials/Data%20passing%20in%20python%20components) Shows how to pass data between python components.", - "file": "/samples/tutorials/Data passing in python components/Data passing in python components - Files.py.yaml" - }, - { - "name": "[Tutorial] DSL - Control structures", - "description": "[source code](https://github.com/kubeflow/pipelines/tree/master/samples/tutorials/DSL%20-%20Control%20structures) Shows how to use conditional execution and exit handlers. This pipeline will randomly fail to demonstrate that the exit handler gets executed even in case of failure.", - "file": "/samples/tutorials/DSL - Control structures/DSL - Control structures.py.yaml" - } -] diff --git a/charms/kfp-api/src/templates/auth_manifests.yaml.j2 b/charms/kfp-api/src/templates/auth_manifests.yaml.j2 index 02ca6bdc..8a43e530 100644 --- a/charms/kfp-api/src/templates/auth_manifests.yaml.j2 +++ b/charms/kfp-api/src/templates/auth_manifests.yaml.j2 @@ -1,7 +1,12 @@ +# Source manifests/apps/pipeline/upstream/base/installs/multi-user/api-service/cluster-role**.yaml +# These manifest files have been modified to suit the needs of the charm; the app label, metadata name, +# and namespace fields will be rendered with information from the application and the model. apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: {{ app_name }} + labels: + app: {{ app_name }} + name: {{ app_name }}-role rules: - apiGroups: - '' @@ -51,14 +56,24 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: - name: {{ app_name }} + labels: + app: {{ app_name }} + name: {{ app_name }}-binding roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: {{ app_name }} + name: {{ app_name }}-role subjects: - kind: ServiceAccount - name: {{ app_name }} + name: {{ app_name }}-sa + namespace: {{ namespace }} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + labels: + app: {{ app_name }} + name: {{ app_name }}-sa namespace: {{ namespace }} --- # manifests/apps/pipeline/upstream/base/installs/multi-user/view-edit-cluster-roles.yaml @@ -66,6 +81,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: labels: + app: {{ app_name }} rbac.authorization.kubeflow.org/aggregate-to-kubeflow-edit: "true" name: kubeflow-pipelines-edit aggregationRule: @@ -81,6 +97,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: labels: + app: {{ app_name }} rbac.authorization.kubeflow.org/aggregate-to-kubeflow-pipelines-edit: "true" rbac.authorization.kubeflow.org/aggregate-to-kubeflow-view: "true" name: kubeflow-pipelines-view @@ -97,6 +114,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: labels: + app: {{ app_name }} app.kubernetes.io/component: ml-pipeline app.kubernetes.io/name: kubeflow-pipelines application-crd-id: kubeflow-pipelines @@ -165,6 +183,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: labels: + app: {{ app_name }} app.kubernetes.io/component: ml-pipeline app.kubernetes.io/name: kubeflow-pipelines application-crd-id: kubeflow-pipelines @@ -208,6 +227,7 @@ apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: labels: + app: {{ app_name }} application-crd-id: kubeflow-pipelines rbac.authorization.k8s.io/aggregate-to-admin: "true" name: argo-aggregate-to-admin diff --git a/charms/kfp-api/src/templates/ml-pipeline-service.yaml.j2 b/charms/kfp-api/src/templates/ml-pipeline-service.yaml.j2 index 2011404d..54cec762 100644 --- a/charms/kfp-api/src/templates/ml-pipeline-service.yaml.j2 +++ b/charms/kfp-api/src/templates/ml-pipeline-service.yaml.j2 @@ -14,4 +14,7 @@ spec: protocol: TCP targetPort: 8888 selector: - app.kubernetes.io/name: {{ service }} + # This selector ensures this Service identifies + # the kfp-api Pod correctly as ti will have + # the same tag + app.kubernetes.io/name: {{ app_name }} diff --git a/charms/kfp-api/tests/integration/test_charm.py b/charms/kfp-api/tests/integration/test_charm.py index 9c19595a..1364d25f 100644 --- a/charms/kfp-api/tests/integration/test_charm.py +++ b/charms/kfp-api/tests/integration/test_charm.py @@ -79,7 +79,7 @@ async def test_relational_db_relation_with_mysql_relation(self, ops_test: OpsTes # deploy mysql-k8s charm await ops_test.model.deploy( "mysql-k8s", - channel="8.0/edge", + channel="8.0/stable", config={"profile": "testing"}, trust=True, ) diff --git a/charms/kfp-api/tests/unit/test_operator.py b/charms/kfp-api/tests/unit/test_operator.py index 86769821..d5b691f4 100644 --- a/charms/kfp-api/tests/unit/test_operator.py +++ b/charms/kfp-api/tests/unit/test_operator.py @@ -11,7 +11,7 @@ from charm import KFP_API_SERVICE_NAME, ErrorWithStatus, KfpApiOperator -KFP_API_CONTAINER_NAME = "ml-pipeline-api-server" +KFP_API_CONTAINER_NAME = "apiserver" @pytest.fixture() @@ -108,6 +108,7 @@ def test_mysql_relation( expected_returned_data, expected_raises, expected_status, + mocked_lightkube_client, harness: Harness, ): harness.set_leader(True) @@ -132,7 +133,7 @@ def test_mysql_relation( harness.charm._get_db_data() @patch("charm.KubernetesServicePatch", lambda x, y: None) - def test_mysql_relation_too_many_relations(self, harness: Harness): + def test_mysql_relation_too_many_relations(self, mocked_lightkube_client, harness: Harness): harness.set_leader(True) harness.begin() harness.container_pebble_ready(KFP_API_CONTAINER_NAME) @@ -150,7 +151,7 @@ def test_mysql_relation_too_many_relations(self, harness: Harness): ) @patch("charm.KubernetesServicePatch", lambda x, y: None) - def test_kfp_viz_relation_missing(self, harness: Harness): + def test_kfp_viz_relation_missing(self, mocked_lightkube_client, harness: Harness): harness.set_leader(True) harness.begin() harness.container_pebble_ready(KFP_API_CONTAINER_NAME) @@ -309,6 +310,7 @@ def test_relations_that_provide_data( expected_returned_data, expected_raises, expected_status, + mocked_lightkube_client, harness: Harness, ): harness.set_leader(True) @@ -360,31 +362,33 @@ def test_install_with_all_inputs_and_pebble( harness.update_relation_data(mysql_rel_id, "mysql-provider/0", mysql_data) # object storage relation - os_data = { + objectstorage_data = { + "access-key": "access-key", + "namespace": "namespace", + "port": 1234, + "secret-key": "secret-key", + "secure": True, + "service": "service", + } + objectstorage_data_dict = { "_supported_versions": "- v1", - "data": yaml.dump( - { - "access-key": "access-key", - "namespace": "namespace", - "port": 1234, - "secret-key": "secret-key", - "secure": True, - "service": "service", - } - ), + "data": yaml.dump(objectstorage_data), } - os_rel_id = harness.add_relation("object-storage", "storage-provider") - harness.add_relation_unit(os_rel_id, "storage-provider/0") - harness.update_relation_data(os_rel_id, "storage-provider", os_data) + objectstorage_rel_id = harness.add_relation("object-storage", "storage-provider") + harness.add_relation_unit(objectstorage_rel_id, "storage-provider/0") + harness.update_relation_data( + objectstorage_rel_id, "storage-provider", objectstorage_data_dict + ) # kfp-viz relation kfp_viz_data = { - "_supported_versions": "- v1", - "data": yaml.dump({"service-name": "unset", "service-port": "1234"}), + "service-name": "viz-service", + "service-port": "1234", } + kfp_viz_data_dict = {"_supported_versions": "- v1", "data": yaml.dump(kfp_viz_data)} kfp_viz_id = harness.add_relation("kfp-viz", "kfp-viz") harness.add_relation_unit(kfp_viz_id, "kfp-viz/0") - harness.update_relation_data(kfp_viz_id, "kfp-viz", kfp_viz_data) + harness.update_relation_data(kfp_viz_id, "kfp-viz", kfp_viz_data_dict) # example kfp-api provider relation kfpapi_data = { @@ -431,22 +435,59 @@ def test_install_with_all_inputs_and_pebble( "-logtostderr=true " ) assert pebble_exec_command == f"bash -c '{exec_command}'" + + expected_env = { + "AUTO_UPDATE_PIPELINE_DEFAULT_VERSION": harness.charm.config[ + "auto-update-default-version" + ], + "KFP_API_SERVICE_NAME": KFP_API_SERVICE_NAME, + "KUBEFLOW_USERID_HEADER": "kubeflow-userid", + "KUBEFLOW_USERID_PREFIX": "", + "POD_NAMESPACE": harness.charm.model.name, + "OBJECTSTORECONFIG_SECURE": "false", + "OBJECTSTORECONFIG_BUCKETNAME": harness.charm.config["object-store-bucket-name"], + "DBCONFIG_CONMAXLIFETIME": "120s", + "DB_DRIVER_NAME": "mysql", + "DBCONFIG_MYSQLCONFIG_USER": "root", + "DBCONFIG_MYSQLCONFIG_PASSWORD": mysql_data["root_password"], + "DBCONFIG_MYSQLCONFIG_DBNAME": mysql_data["database"], + "DBCONFIG_MYSQLCONFIG_HOST": mysql_data["host"], + "DBCONFIG_MYSQLCONFIG_PORT": mysql_data["port"], + "OBJECTSTORECONFIG_ACCESSKEY": objectstorage_data["access-key"], + "OBJECTSTORECONFIG_SECRETACCESSKEY": objectstorage_data["secret-key"], + "DEFAULTPIPELINERUNNERSERVICEACCOUNT": "default-editor", + "MULTIUSER": "true", + "VISUALIZATIONSERVICE_NAME": kfp_viz_data["service-name"], + "VISUALIZATIONSERVICE_PORT": kfp_viz_data["service-port"], + "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_HOST": kfp_viz_data["service-name"], + "ML_PIPELINE_VISUALIZATIONSERVER_SERVICE_PORT": kfp_viz_data["service-port"], + "CACHE_IMAGE": harness.charm.config["cache-image"], + "ARCHIVE_CONFIG_LOG_FILE_NAME": harness.charm.config["log-archive-filename"], + "ARCHIVE_CONFIG_LOG_PATH_PREFIX": harness.charm.config["log-archive-prefix"], + # OBJECTSTORECONFIG_HOST and _PORT currently have no effect due to + # https://github.com/kubeflow/pipelines/issues/9689, described more in + # https://github.com/canonical/minio-operator/pull/151 + # They're included here so that when the upstream issue is fixed we don't break + "OBJECTSTORECONFIG_HOST": ( + f"{objectstorage_data['service']}.{objectstorage_data['namespace']}" + ), + "OBJECTSTORECONFIG_PORT": str(objectstorage_data["port"]), + "OBJECTSTORECONFIG_REGION": "", + } test_env = pebble_plan_info["services"][KFP_API_SERVICE_NAME]["environment"] - # there should be 1 environment variable - assert 1 == len(test_env) + + assert test_env == expected_env assert "test_model" == test_env["POD_NAMESPACE"] @patch("charm.KubernetesServicePatch", lambda x, y: None) @patch("charm.KfpApiOperator._apply_k8s_resources") @patch("charm.KfpApiOperator._check_status") - @patch("charm.KfpApiOperator._generate_config") - @patch("charm.KfpApiOperator._upload_files_to_container") + @patch("charm.KfpApiOperator._generate_environment") def test_update_status( self, _apply_k8s_resources: MagicMock, _check_status: MagicMock, - _generate_config: MagicMock, - _upload_files_to_conainer: MagicMock, + _generate_environment: MagicMock, harness: Harness, ): """Test update status handler.""" @@ -456,11 +497,9 @@ def test_update_status( # test successful update status _apply_k8s_resources.reset_mock() - _upload_files_to_conainer.reset_mock() harness.charm.on.update_status.emit() # this will enforce the design in which main event handler is executed in update-status _apply_k8s_resources.assert_called() - _upload_files_to_conainer.assert_called() # check status should be called _check_status.assert_called() diff --git a/charms/kfp-metadata-writer/CONTRIBUTING.md b/charms/kfp-metadata-writer/CONTRIBUTING.md new file mode 100644 index 00000000..8f7fbe77 --- /dev/null +++ b/charms/kfp-metadata-writer/CONTRIBUTING.md @@ -0,0 +1,65 @@ +# Contributing + +## Overview + +This document outlines the processes and practices recommended for contributing enhancements to `kfp-metadata-writer`. + +## Talk to us First + +Before developing enhancements to this charm, you should [open an issue](/../../issues) explaining your use case. If you would like to chat with us about your use-cases or proposed implementation, you can reach us at [MLOps Mattermost public channel](https://chat.charmhub.io/charmhub/channels/mlops-documentation) or on [Discourse](https://discourse.charmhub.io/). + +## Pull Requests + +Please help us out in ensuring easy to review branches by rebasing your pull request branch onto the `main` branch. This also avoids merge commits and creates a linear Git commit history. + +All pull requests require review before being merged. Code review typically examines: + - code quality + - test coverage + - user experience for Juju administrators of this charm. + +## Recommended Knowledge + +Familiarising yourself with the [Charmed Operator Framework](https://juju.is/docs/sdk) library will help you a lot when working on new features or bug fixes. + +## Build Charm + +To build `kfp-metadata-writer` run: + +```shell +charmcraft pack +``` + +## Developing + +You can use the environments created by `tox` for development. For example, to load the `unit` environment into your shell, run: + +```shell +tox --notest -e unit +source .tox/unit/bin/activate +``` + +### Testing + +Use tox for testing. For example to test the `integration` environment, run: + +```shell +tox -e integration +``` + +See `tox.ini` for all available environments. + +### Deploy + +```bash +# Create a model +juju add-model dev +# Enable DEBUG logging +juju model-config logging-config="=INFO;unit=DEBUG" +# Deploy the charm +juju deploy ./kfp-metadata-writer_ubuntu-20.04-amd64.charm \ + --resource oci-image=$(yq '.resources."oci-image"."upstream-source"' metadata.yaml) +``` + +## Canonical Contributor Agreement + +Canonical welcomes contributions to this charm. Please check out our [contributor agreement](https://ubuntu.com/legal/contributors) if you're interested in contributing. diff --git a/charms/kfp-metadata-writer/README.md b/charms/kfp-metadata-writer/README.md new file mode 100644 index 00000000..1287d9cd --- /dev/null +++ b/charms/kfp-metadata-writer/README.md @@ -0,0 +1,13 @@ +## Kubeflow Pipelines Metadata Writer Operator + +### Overview +This charm encompasses the Kubernetes Python operator for Kubeflow Pipelines +Metadata Writer (see [CharmHub](https://charmhub.io/?q=kfp-metadata-writer)). + +## Install + +To install Kubeflow Pipelines Metadata Writer, run: + + juju deploy kfp-metadata-writer --trust + +For more information, see https://juju.is/docs diff --git a/charms/kfp-metadata-writer/charmcraft.yaml b/charms/kfp-metadata-writer/charmcraft.yaml new file mode 100644 index 00000000..24d0fec5 --- /dev/null +++ b/charms/kfp-metadata-writer/charmcraft.yaml @@ -0,0 +1,13 @@ +# Learn more about charmcraft.yaml configuration at: +# https://juju.is/docs/sdk/charmcraft-config +type: "charm" +bases: + - build-on: + - name: "ubuntu" + channel: "20.04" + run-on: + - name: "ubuntu" + channel: "20.04" +parts: + charm: + charm-python-packages: [setuptools, pip] # Fixes install of some packages diff --git a/charms/kfp-metadata-writer/config.yaml b/charms/kfp-metadata-writer/config.yaml new file mode 100644 index 00000000..a985d462 --- /dev/null +++ b/charms/kfp-metadata-writer/config.yaml @@ -0,0 +1 @@ +options: {} diff --git a/charms/kfp-metadata-writer/icon.svg b/charms/kfp-metadata-writer/icon.svg new file mode 100644 index 00000000..1b2dee32 --- /dev/null +++ b/charms/kfp-metadata-writer/icon.svg @@ -0,0 +1,90 @@ + + + + + + image/svg+xml + + + + + + + + + + + + + + + + + + diff --git a/charms/kfp-metadata-writer/metadata.yaml b/charms/kfp-metadata-writer/metadata.yaml new file mode 100755 index 00000000..3b8c0b25 --- /dev/null +++ b/charms/kfp-metadata-writer/metadata.yaml @@ -0,0 +1,33 @@ +name: kfp-metadata-writer +summary: Reusable end-to-end ML workflows built using the Kubeflow Pipelines SDK +description: | + Machine learning (ML) toolkit that is dedicated to making deployments + of ML workflows on Kubernetes simple, portable, and scalable. +docs: https://discourse.charmhub.io/t/12108 +website: https://charmhub.io/kfp-metadata-writer +source: https://github.com/canonical/kfp-operators +containers: + kfp-metadata-writer: + resource: oci-image +resources: + oci-image: + type: oci-image + description: OCI image for KFP Metadata Writer + upstream-source: gcr.io/ml-pipeline/metadata-writer:2.0.3 +requires: + grpc: + interface: grpc + schema: + v1: + provides: + type: object + properties: + service: + type: string + port: + type: string + required: + - service + - port + versions: [v1] + __schema_source: https://raw.githubusercontent.com/canonical/operator-schemas/master/grpc.yaml diff --git a/charms/kfp-metadata-writer/pyproject.toml b/charms/kfp-metadata-writer/pyproject.toml new file mode 100644 index 00000000..de8987a1 --- /dev/null +++ b/charms/kfp-metadata-writer/pyproject.toml @@ -0,0 +1,40 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +# Testing tools configuration +[tool.coverage.run] +branch = true + +[tool.coverage.report] +show_missing = true + +[tool.pytest.ini_options] +minversion = "6.0" +log_cli_level = "INFO" + +# Formatting tools configuration +[tool.black] +line-length = 99 +target-version = ["py38"] + +[tool.isort] +line_length = 99 +profile = "black" + +# Linting tools configuration +[tool.flake8] +max-line-length = 99 +max-doc-length = 99 +max-complexity = 10 +exclude = [".git", "__pycache__", ".tox", "build", "dist", "*.egg_info", "venv"] +select = ["E", "W", "F", "C", "N", "R", "D", "H"] +# Ignore W503, E501 because using black creates errors with this +# Ignore D107 Missing docstring in __init__ +ignore = ["W503", "E501", "D107"] +# D100, D101, D102, D103: Ignore missing docstrings in tests +per-file-ignores = ["tests/*:D100,D101,D102,D103,D104"] +docstring-convention = "google" +# Check for properly formatted copyright header in each file +copyright-check = "True" +copyright-author = "Canonical Ltd." +copyright-regexp = "Copyright\\s\\d{4}([-,]\\d{4})*\\s+%(author)s" diff --git a/charms/kfp-metadata-writer/requirements-fmt.in b/charms/kfp-metadata-writer/requirements-fmt.in new file mode 100644 index 00000000..7559a405 --- /dev/null +++ b/charms/kfp-metadata-writer/requirements-fmt.in @@ -0,0 +1,2 @@ +black +isort diff --git a/charms/kfp-metadata-writer/requirements-fmt.txt b/charms/kfp-metadata-writer/requirements-fmt.txt new file mode 100644 index 00000000..4e025c98 --- /dev/null +++ b/charms/kfp-metadata-writer/requirements-fmt.txt @@ -0,0 +1,24 @@ +# +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: +# +# pip-compile requirements-fmt.in +# +black==23.9.1 + # via -r requirements-fmt.in +click==8.1.7 + # via black +isort==5.12.0 + # via -r requirements-fmt.in +mypy-extensions==1.0.0 + # via black +packaging==23.2 + # via black +pathspec==0.11.2 + # via black +platformdirs==3.11.0 + # via black +tomli==2.0.1 + # via black +typing-extensions==4.8.0 + # via black diff --git a/charms/kfp-metadata-writer/requirements-lint.in b/charms/kfp-metadata-writer/requirements-lint.in new file mode 100644 index 00000000..07a4a51c --- /dev/null +++ b/charms/kfp-metadata-writer/requirements-lint.in @@ -0,0 +1,8 @@ +black +codespell +flake8 +flake8-builtins +flake8-copyright +isort +pep8-naming +pyproject-flake8 diff --git a/charms/kfp-metadata-writer/requirements-lint.txt b/charms/kfp-metadata-writer/requirements-lint.txt new file mode 100644 index 00000000..bd72cdab --- /dev/null +++ b/charms/kfp-metadata-writer/requirements-lint.txt @@ -0,0 +1,51 @@ +# +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: +# +# pip-compile requirements-lint.in +# +black==23.9.1 + # via -r requirements-lint.in +click==8.1.7 + # via black +codespell==2.2.6 + # via -r requirements-lint.in +flake8==6.1.0 + # via + # -r requirements-lint.in + # flake8-builtins + # pep8-naming + # pyproject-flake8 +flake8-builtins==2.1.0 + # via -r requirements-lint.in +flake8-copyright==0.2.4 + # via -r requirements-lint.in +isort==5.12.0 + # via -r requirements-lint.in +mccabe==0.7.0 + # via flake8 +mypy-extensions==1.0.0 + # via black +packaging==23.2 + # via black +pathspec==0.11.2 + # via black +pep8-naming==0.13.3 + # via -r requirements-lint.in +platformdirs==3.11.0 + # via black +pycodestyle==2.11.0 + # via flake8 +pyflakes==3.1.0 + # via flake8 +pyproject-flake8==6.1.0 + # via -r requirements-lint.in +tomli==2.0.1 + # via + # black + # pyproject-flake8 +typing-extensions==4.8.0 + # via black + +# The following packages are considered to be unsafe in a requirements file: +# setuptools diff --git a/charms/kfp-metadata-writer/requirements-unit.in b/charms/kfp-metadata-writer/requirements-unit.in new file mode 100644 index 00000000..82a8e551 --- /dev/null +++ b/charms/kfp-metadata-writer/requirements-unit.in @@ -0,0 +1,21 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +# Please note this file introduces dependencies from the charm's requirements.in, +# special attention must be taken when updating this or the other .in file to try +# to avoid incompatibilities. +# Rules for editing this file: +# * Removing a dependency that is no longer used in the unit test file(s) +# is allowed, and should not represent any risk. +# * Adding a dependency in this file means the dependency is directly used +# in the unit test files(s). +# * ALL python packages/libs used directly in the unit test file(s) must be +# listed here even if requirements.in is already adding them. This will +# add clarity to the dependency list. +# * Pinning a version of a python package/lib shared with requirements.in +# must not introduce any incompatibilities. +coverage +ops +pytest +pytest-mock +pyyaml +-r requirements.in diff --git a/charms/kfp-metadata-writer/requirements-unit.txt b/charms/kfp-metadata-writer/requirements-unit.txt new file mode 100644 index 00000000..7fd4bc63 --- /dev/null +++ b/charms/kfp-metadata-writer/requirements-unit.txt @@ -0,0 +1,107 @@ +# +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: +# +# pip-compile requirements-unit.in +# +anyio==4.0.0 + # via httpcore +attrs==23.1.0 + # via jsonschema +certifi==2023.7.22 + # via + # httpcore + # httpx + # requests +charmed-kubeflow-chisme==0.2.0 + # via -r requirements.in +charset-normalizer==3.3.0 + # via requests +coverage==7.3.2 + # via -r requirements-unit.in +deepdiff==6.2.1 + # via charmed-kubeflow-chisme +exceptiongroup==1.1.3 + # via + # anyio + # pytest +h11==0.14.0 + # via httpcore +httpcore==0.18.0 + # via httpx +httpx==0.25.0 + # via lightkube +idna==3.4 + # via + # anyio + # httpx + # requests +importlib-resources==6.1.0 + # via jsonschema +iniconfig==2.0.0 + # via pytest +jinja2==3.1.2 + # via charmed-kubeflow-chisme +jsonschema==4.17.3 + # via serialized-data-interface +lightkube==0.14.0 + # via + # -r requirements.in + # charmed-kubeflow-chisme +lightkube-models==1.28.1.4 + # via lightkube +markupsafe==2.1.3 + # via jinja2 +ops==2.7.0 + # via + # -r requirements-unit.in + # -r requirements.in + # charmed-kubeflow-chisme + # serialized-data-interface +ordered-set==4.1.0 + # via deepdiff +packaging==23.2 + # via pytest +pkgutil-resolve-name==1.3.10 + # via jsonschema +pluggy==1.3.0 + # via pytest +pyrsistent==0.19.3 + # via jsonschema +pytest==7.4.2 + # via + # -r requirements-unit.in + # pytest-mock +pytest-mock==3.11.1 + # via -r requirements-unit.in +pyyaml==6.0.1 + # via + # -r requirements-unit.in + # lightkube + # ops + # serialized-data-interface +requests==2.31.0 + # via serialized-data-interface +ruamel-yaml==0.17.35 + # via charmed-kubeflow-chisme +ruamel-yaml-clib==0.2.8 + # via ruamel-yaml +serialized-data-interface==0.7.0 + # via + # -r requirements.in + # charmed-kubeflow-chisme +sniffio==1.3.0 + # via + # anyio + # httpcore + # httpx +tenacity==8.2.3 + # via charmed-kubeflow-chisme +tomli==2.0.1 + # via pytest +urllib3==2.0.6 + # via requests +websocket-client==1.6.4 + # via ops +zipp==3.17.0 + # via importlib-resources diff --git a/charms/kfp-metadata-writer/requirements.in b/charms/kfp-metadata-writer/requirements.in new file mode 100644 index 00000000..5d329801 --- /dev/null +++ b/charms/kfp-metadata-writer/requirements.in @@ -0,0 +1,7 @@ +# Pinning charmed-kubeflow-chisme will avoid pip-compile to resolve +# conflicts by using an older version of this package. +# Version >=0.2.0 contains the base charm code that's needed. +charmed-kubeflow-chisme >= 0.2.0 +lightkube +ops +serialized-data-interface diff --git a/charms/kfp-metadata-writer/requirements.txt b/charms/kfp-metadata-writer/requirements.txt new file mode 100644 index 00000000..de6c5187 --- /dev/null +++ b/charms/kfp-metadata-writer/requirements.txt @@ -0,0 +1,87 @@ +# +# This file is autogenerated by pip-compile with Python 3.8 +# by the following command: +# +# pip-compile requirements.in +# +anyio==4.0.0 + # via httpcore +attrs==23.1.0 + # via jsonschema +certifi==2023.7.22 + # via + # httpcore + # httpx + # requests +charmed-kubeflow-chisme==0.2.0 + # via -r requirements.in +charset-normalizer==3.3.0 + # via requests +deepdiff==6.2.1 + # via charmed-kubeflow-chisme +exceptiongroup==1.1.3 + # via anyio +h11==0.14.0 + # via httpcore +httpcore==0.18.0 + # via httpx +httpx==0.25.0 + # via lightkube +idna==3.4 + # via + # anyio + # httpx + # requests +importlib-resources==6.1.0 + # via jsonschema +jinja2==3.1.2 + # via charmed-kubeflow-chisme +jsonschema==4.17.3 + # via serialized-data-interface +lightkube==0.14.0 + # via + # -r requirements.in + # charmed-kubeflow-chisme +lightkube-models==1.28.1.4 + # via lightkube +markupsafe==2.1.3 + # via jinja2 +ops==2.7.0 + # via + # -r requirements.in + # charmed-kubeflow-chisme + # serialized-data-interface +ordered-set==4.1.0 + # via deepdiff +pkgutil-resolve-name==1.3.10 + # via jsonschema +pyrsistent==0.19.3 + # via jsonschema +pyyaml==6.0.1 + # via + # lightkube + # ops + # serialized-data-interface +requests==2.31.0 + # via serialized-data-interface +ruamel-yaml==0.17.35 + # via charmed-kubeflow-chisme +ruamel-yaml-clib==0.2.8 + # via ruamel-yaml +serialized-data-interface==0.7.0 + # via + # -r requirements.in + # charmed-kubeflow-chisme +sniffio==1.3.0 + # via + # anyio + # httpcore + # httpx +tenacity==8.2.3 + # via charmed-kubeflow-chisme +urllib3==2.0.6 + # via requests +websocket-client==1.6.4 + # via ops +zipp==3.17.0 + # via importlib-resources diff --git a/charms/kfp-metadata-writer/src/charm.py b/charms/kfp-metadata-writer/src/charm.py new file mode 100755 index 00000000..a5617132 --- /dev/null +++ b/charms/kfp-metadata-writer/src/charm.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Charm for the Kubeflow Pipelines Metadata Writer. + +https://github.com/canonical/kfp-operators +""" + +import logging + +import lightkube +from charmed_kubeflow_chisme.components.charm_reconciler import CharmReconciler +from charmed_kubeflow_chisme.components.kubernetes_component import KubernetesComponent +from charmed_kubeflow_chisme.components.leadership_gate_component import LeadershipGateComponent +from charmed_kubeflow_chisme.components.serialised_data_interface_components import ( + SdiRelationDataReceiverComponent, +) +from charmed_kubeflow_chisme.kubernetes import create_charm_default_labels +from lightkube.resources.core_v1 import ServiceAccount +from lightkube.resources.rbac_authorization_v1 import ClusterRole, ClusterRoleBinding +from ops.charm import CharmBase +from ops.main import main + +from components.pebble_component import KfpMetadataWriterInputs, KfpMetadataWriterPebbleService + +logger = logging.getLogger(__name__) + +K8S_RESOURCE_FILES = ["src/templates/auth_manifests.yaml.j2"] + + +class KfpMetadataWriter(CharmBase): + def __init__(self, *args): + """Charm for the Kubeflow Pipelines Metadata Writer.""" + super().__init__(*args) + + self.charm_reconciler = CharmReconciler(self) + self._namespace = self.model.name + + self.leadership_gate = self.charm_reconciler.add( + component=LeadershipGateComponent( + charm=self, + name="leadership-gate", + ), + depends_on=[], + ) + + self.grpc_relation = self.charm_reconciler.add( + component=SdiRelationDataReceiverComponent( + charm=self, name="relation:grpc", relation_name="grpc" + ), + depends_on=[self.leadership_gate], + ) + + self.kubernetes_resources = self.charm_reconciler.add( + component=KubernetesComponent( + charm=self, + name="kubernetes:auth", + resource_templates=K8S_RESOURCE_FILES, + krh_resource_types={ClusterRole, ClusterRoleBinding, ServiceAccount}, + krh_labels=create_charm_default_labels( + self.app.name, self.model.name, scope="auth" + ), + context_callable=lambda: {"app_name": self.app.name, "namespace": self._namespace}, + lightkube_client=lightkube.Client(), + ), + depends_on=[self.leadership_gate], + ) + + self.pebble_service_container = self.charm_reconciler.add( + component=KfpMetadataWriterPebbleService( + charm=self, + name="kfp-metadata-writer-pebble-service", + container_name="kfp-metadata-writer", + service_name="kfp-metadata-writer", + namespace_to_watch="", + inputs_getter=lambda: KfpMetadataWriterInputs( + METADATA_GRPC_SERVICE_SERVICE_HOST=self.grpc_relation.component.get_data()[ + "service" + ], + METADATA_GRPC_SERVICE_SERVICE_PORT=self.grpc_relation.component.get_data()[ + "port" + ], + ), + ), + depends_on=[self.kubernetes_resources, self.grpc_relation], + ) + + self.charm_reconciler.install_default_event_handlers() + + +if __name__ == "__main__": + main(KfpMetadataWriter) diff --git a/charms/kfp-metadata-writer/src/components/pebble_component.py b/charms/kfp-metadata-writer/src/components/pebble_component.py new file mode 100644 index 00000000..0e25e223 --- /dev/null +++ b/charms/kfp-metadata-writer/src/components/pebble_component.py @@ -0,0 +1,68 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +import dataclasses +import logging + +from charmed_kubeflow_chisme.components.pebble_component import PebbleServiceComponent +from ops.pebble import Layer + +logger = logging.getLogger(__name__) + + +@dataclasses.dataclass +class KfpMetadataWriterInputs: + """Defines the required inputs for KfpMetadataWriterInputs.""" + + METADATA_GRPC_SERVICE_SERVICE_HOST: str + METADATA_GRPC_SERVICE_SERVICE_PORT: str + + +class KfpMetadataWriterPebbleService(PebbleServiceComponent): + def __init__( + self, + *args, + namespace_to_watch: str = "", + **kwargs, + ): + """Pebble service container component in order to configure Pebble layer + + Args: + namespace_to_watch (str): The namespace in which the metadata-writer workload + will look for pods. Matching upstream manifests, this defaults to "" in order + for the component to look for pods in all namespaces. + """ + self.namespace_to_watch = namespace_to_watch + super().__init__(*args, **kwargs) + + def get_layer(self) -> Layer: + """Defines and returns Pebble layer configuration + + This method is required for subclassing PebbleServiceContainer + """ + logger.info("PebbleServiceComponent.get_layer executing") + + try: + inputs: KfpMetadataWriterInputs = self._inputs_getter() + except Exception as err: + raise ValueError("Failed to get inputs for Pebble container.") from err + + environment = { + "NAMESPACE_TO_WATCH": self.namespace_to_watch, + "METADATA_GRPC_SERVICE_SERVICE_HOST": inputs.METADATA_GRPC_SERVICE_SERVICE_HOST, + "METADATA_GRPC_SERVICE_SERVICE_PORT": inputs.METADATA_GRPC_SERVICE_SERVICE_PORT, + } + return Layer( + { + "summary": "kfp-metadata-writer layer", + "description": "Pebble config layer for kfp-metadata-writer", + "services": { + self.service_name: { + "override": "replace", + "summary": "Entry point for kfp-metadata-writer oci-image", + "command": "python3 -u /kfp/metadata_writer/metadata_writer.py", + "startup": "enabled", + "environment": environment, + } + }, + } + ) diff --git a/charms/kfp-metadata-writer/src/templates/auth_manifests.yaml.j2 b/charms/kfp-metadata-writer/src/templates/auth_manifests.yaml.j2 new file mode 100644 index 00000000..66e9fe86 --- /dev/null +++ b/charms/kfp-metadata-writer/src/templates/auth_manifests.yaml.j2 @@ -0,0 +1,53 @@ +# Source kustomize build apps/pipeline/upstream/base/installs/multi-user +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRole +metadata: + labels: + app: {{ app_name }} + name: {{ app_name }}-cluster-role +rules: +- apiGroups: + - "" + resources: + - pods + verbs: + - get + - list + - watch + - update + - patch +- apiGroups: + - "" + resources: + - configmaps + verbs: + - get +- apiGroups: + - argoproj.io + resources: + - workflows + verbs: + - get + - list + - watch + - update + - patch +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: ClusterRoleBinding +metadata: + name: {{ app_name }}-binding +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: ClusterRole + name: {{ app_name }}-role +subjects: +- kind: ServiceAccount + name: {{ app_name }}-sa + namespace: {{ namespace }} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + name: {{ app_name }}-sa + namespace: {{ namespace }} diff --git a/charms/kfp-metadata-writer/tests/unit/test_operator.py b/charms/kfp-metadata-writer/tests/unit/test_operator.py new file mode 100644 index 00000000..7229deec --- /dev/null +++ b/charms/kfp-metadata-writer/tests/unit/test_operator.py @@ -0,0 +1,151 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +from unittest.mock import MagicMock + +import pytest +from charmed_kubeflow_chisme.testing import add_sdi_relation_to_harness +from ops.model import ActiveStatus, BlockedStatus, WaitingStatus +from ops.testing import Harness + +from charm import KfpMetadataWriter + +MOCK_GRPC_DATA = {"service": "service-name", "port": "1234"} + + +def test_not_leader( + harness, + mocked_lightkube_client, +): + """Test that charm waits for leadership.""" + harness.begin_with_initial_hooks() + assert harness.charm.model.unit.status == WaitingStatus( + "[leadership-gate] Waiting for leadership" + ) + + +def test_kubernetes_component_created(harness, mocked_lightkube_client): + """Test that Kubernetes component is created when we have leadership.""" + # Needed because the kubernetes component will only apply to k8s if we are the leader + harness.set_leader(True) + harness.begin() + + # Need to mock the leadership-gate to be active, and the kubernetes auth component so that it + # sees the expected resources when calling _get_missing_kubernetes_resources + kubernetes_resources = harness.charm.kubernetes_resources + kubernetes_resources.component._get_missing_kubernetes_resources = MagicMock(return_value=[]) + + harness.charm.on.install.emit() + + assert isinstance(harness.charm.kubernetes_resources.status, ActiveStatus) + + # Assert that expected amount of apply calls were made + # This simulates the Kubernetes resources being created + assert mocked_lightkube_client.apply.call_count == 3 + + +def test_grpc_relation_with_data(harness, mocked_lightkube_client): + """Test that if Leadership is Active, the grpc relation operates as expected.""" + # Arrange + harness.begin() + + # Mock: + # * leadership_gate to be active and executed + harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) + + # Add relation with data. This should trigger a charm reconciliation due to relation-changed. + add_sdi_relation_to_harness(harness, "grpc", data=MOCK_GRPC_DATA) + + # Assert + assert isinstance(harness.charm.grpc_relation.status, ActiveStatus) + + +def test_grpc_relation_without_data(harness, mocked_lightkube_client): + """Test that the grpc relation goes Blocked if no data is available.""" + # Arrange + harness.begin() + + # Mock: + # * leadership_gate to be active and executed + harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) + + # Add relation with data. This should trigger a charm reconciliation due to relation-changed. + add_sdi_relation_to_harness(harness, "grpc", data={}) + + # Assert + assert isinstance(harness.charm.grpc_relation.status, BlockedStatus) + + +def test_grpc_relation_without_relation(harness, mocked_lightkube_client): + """Test that the grpc relation goes Blocked if no relation is established.""" + # Arrange + harness.begin() + + # Mock: + # * leadership_gate to be active and executed + harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) + + # Act + harness.charm.on.install.emit() + + # Assert + assert isinstance(harness.charm.grpc_relation.status, BlockedStatus) + + +def test_pebble_service_container_running(harness, mocked_lightkube_client): + """Test that the pebble service of the charm's kfp-metadata-writer container is running.""" + harness.set_leader(True) + harness.begin() + harness.set_can_connect("kfp-metadata-writer", True) + + harness.charm.kubernetes_resources.get_status = MagicMock(return_value=ActiveStatus()) + add_sdi_relation_to_harness(harness, "grpc", data=MOCK_GRPC_DATA) + + harness.charm.on.install.emit() + + assert isinstance(harness.charm.unit.status, ActiveStatus) + + container = harness.charm.unit.get_container("kfp-metadata-writer") + # Assert that sidecar container is up and its service is running + assert container.get_service("kfp-metadata-writer").is_running() + + # Assert the environment variables that are set from defaults in the service plan + environment = container.get_plan().services["kfp-metadata-writer"].environment + assert environment["NAMESPACE_TO_WATCH"] == "" + assert ( + environment["METADATA_GRPC_SERVICE_SERVICE_HOST"] + == harness.charm.grpc_relation.component.get_data()["service"] + ) + assert ( + environment["METADATA_GRPC_SERVICE_SERVICE_PORT"] + == harness.charm.grpc_relation.component.get_data()["port"] + ) + + +def test_install_before_pebble_service_container(harness, mocked_lightkube_client): + """Test that charm waits when install event happens before pebble-service-container is ready.""" + harness.set_leader(True) + harness.begin() + + harness.charm.kubernetes_resources.get_status = MagicMock(return_value=ActiveStatus()) + add_sdi_relation_to_harness(harness, "grpc", data=MOCK_GRPC_DATA) + + harness.charm.on.install.emit() + + # Assert charm is waiting on PebbleComponent + assert harness.charm.model.unit.status == WaitingStatus( + "[kfp-metadata-writer-pebble-service] Waiting for Pebble to be ready." + ) + + +@pytest.fixture +def harness(): + return Harness(KfpMetadataWriter) + + +@pytest.fixture() +def mocked_lightkube_client(mocker): + """Mocks the Lightkube Client in charm.py, returning a mock instead.""" + mocked_lightkube_client = MagicMock() + mocker.patch("charm.lightkube.Client", return_value=mocked_lightkube_client) + yield mocked_lightkube_client diff --git a/charms/kfp-metadata-writer/tox.ini b/charms/kfp-metadata-writer/tox.ini new file mode 100644 index 00000000..1fc6f86c --- /dev/null +++ b/charms/kfp-metadata-writer/tox.ini @@ -0,0 +1,74 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +[flake8] +max-line-length = 100 + +[tox] +skipsdist = True +skip_missing_interpreters = True +envlist = fmt, lint, unit, integration + +[vars] +all_path = {[vars]src_path} {[vars]tst_path} +src_path = {toxinidir}/src/ +tst_path = {toxinidir}/tests/ + +[testenv] +passenv = + PYTHONPATH + CHARM_BUILD_DIR + MODEL_SETTINGS + KUBECONFIG +setenv = + PYTHONPATH = {toxinidir}:{toxinidir}/lib:{[vars]src_path} + PYTHONBREAKPOINT=ipdb.set_trace + PY_COLORS=1 + +[testenv:update-requirements] +allowlist_externals = + bash + find + pip-compile + xargs +commands = + ; we must preserve the order of compilation, since each *.in file depends on some *.txt file. + ; For example, requirements-unit.in depends on requirements.txt and we must compile first + ; requirements.txt to ensure that requirements-unit.txt get the same dependency as the requirements.txt + bash -c 'for pattern in "requirements.in" "requirements-fmt.in" "requirements*.in"; do find . -type f -name "$pattern" -exec bash -c "cd \$(dirname "{}") && pip-compile --resolver=backtracking \$(basename "{}")" \;; done' +deps = + pip-tools +description = Update requirements files by executing pip-compile on all requirements*.in files, including those in subdirs. + +[testenv:fmt] +commands = + isort {[vars]all_path} + black {[vars]all_path} +deps = + -r requirements-fmt.txt +description = Apply coding style standards to code + +[testenv:lint] +commands = + # uncomment the following line if this charm owns a lib + # codespell {[vars]lib_path} + codespell {toxinidir}/. --skip {toxinidir}/./.git --skip {toxinidir}/./.tox \ + --skip {toxinidir}/./build --skip {toxinidir}/./lib --skip {toxinidir}/./venv \ + --skip {toxinidir}/.mypy_cache \ + --skip {toxinidir}/icon.svg --skip *.json.tmpl + # pflake8 wrapper supports config from pyproject.toml + pflake8 {[vars]all_path} + isort --check-only --diff {[vars]all_path} + black --check --diff {[vars]all_path} +deps = + -r requirements-lint.txt +description = Check code against coding style standards + +[testenv:unit] +commands = + coverage run --source={[vars]src_path} \ + -m pytest --ignore={[vars]tst_path}integration -vv --tb native {posargs} + coverage report +deps = + -r requirements-unit.txt +description = Run unit tests diff --git a/charms/kfp-persistence/metadata.yaml b/charms/kfp-persistence/metadata.yaml index 6df0971a..97da8ef7 100755 --- a/charms/kfp-persistence/metadata.yaml +++ b/charms/kfp-persistence/metadata.yaml @@ -11,7 +11,7 @@ resources: oci-image: type: oci-image description: Backing OCI image - upstream-source: charmedkubeflow/persistenceagent:2.0.0-alpha.7_22.04_1 + upstream-source: gcr.io/ml-pipeline/persistenceagent:2.0.3 requires: kfp-api: interface: k8s-service diff --git a/charms/kfp-persistence/requirements-unit.txt b/charms/kfp-persistence/requirements-unit.txt index 73ba63c0..3e2b1966 100644 --- a/charms/kfp-persistence/requirements-unit.txt +++ b/charms/kfp-persistence/requirements-unit.txt @@ -8,10 +8,13 @@ anyio==3.7.1 # via httpcore attrs==23.1.0 # via jsonschema +cachetools==5.3.1 + # via google-auth certifi==2023.7.22 # via # httpcore # httpx + # kubernetes # requests charmed-kubeflow-chisme==0.2.0 # via -r requirements.in @@ -25,6 +28,8 @@ exceptiongroup==1.1.2 # via # anyio # pytest +google-auth==2.23.3 + # via kubernetes h11==0.14.0 # via httpcore httpcore==0.17.3 @@ -44,6 +49,8 @@ jinja2==3.1.2 # via charmed-kubeflow-chisme jsonschema==4.17.3 # via serialized-data-interface +kubernetes==27.2.0 + # via -r requirements.in lightkube==0.14.0 # via # -r requirements.in @@ -52,6 +59,10 @@ lightkube-models==1.27.1.4 # via lightkube markupsafe==2.1.3 # via jinja2 +oauthlib==3.2.2 + # via + # kubernetes + # requests-oauthlib ops==2.5.0 # via # -r requirements-unit.in @@ -66,6 +77,12 @@ pkgutil-resolve-name==1.3.10 # via jsonschema pluggy==1.2.0 # via pytest +pyasn1==0.5.0 + # via + # pyasn1-modules + # rsa +pyasn1-modules==0.3.0 + # via google-auth pyrsistent==0.19.3 # via jsonschema pytest==7.4.0 @@ -77,14 +94,24 @@ pytest-lazy-fixture==0.6.3 # via -r requirements-unit.in pytest-mock==3.11.1 # via -r requirements-unit.in +python-dateutil==2.8.2 + # via kubernetes pyyaml==6.0.1 # via # -r requirements-unit.in + # kubernetes # lightkube # ops # serialized-data-interface requests==2.31.0 - # via serialized-data-interface + # via + # kubernetes + # requests-oauthlib + # serialized-data-interface +requests-oauthlib==1.3.1 + # via kubernetes +rsa==4.9 + # via google-auth ruamel-yaml==0.17.32 # via charmed-kubeflow-chisme ruamel-yaml-clib==0.2.7 @@ -93,6 +120,10 @@ serialized-data-interface==0.7.0 # via # -r requirements.in # charmed-kubeflow-chisme +six==1.16.0 + # via + # kubernetes + # python-dateutil sniffio==1.3.0 # via # anyio @@ -103,8 +134,12 @@ tenacity==8.2.2 tomli==2.0.1 # via pytest urllib3==2.0.4 - # via requests + # via + # kubernetes + # requests websocket-client==1.6.1 - # via ops + # via + # kubernetes + # ops zipp==3.16.2 # via importlib-resources diff --git a/charms/kfp-persistence/requirements.in b/charms/kfp-persistence/requirements.in index 6a4175d6..182d6aab 100644 --- a/charms/kfp-persistence/requirements.in +++ b/charms/kfp-persistence/requirements.in @@ -1,4 +1,5 @@ charmed-kubeflow-chisme +kubernetes lightkube ops serialized-data-interface diff --git a/charms/kfp-persistence/requirements.txt b/charms/kfp-persistence/requirements.txt index 668e277c..e9a2a70f 100644 --- a/charms/kfp-persistence/requirements.txt +++ b/charms/kfp-persistence/requirements.txt @@ -8,10 +8,13 @@ anyio==3.7.1 # via httpcore attrs==23.1.0 # via jsonschema +cachetools==5.3.1 + # via google-auth certifi==2023.7.22 # via # httpcore # httpx + # kubernetes # requests charmed-kubeflow-chisme==0.2.0 # via -r requirements.in @@ -21,6 +24,8 @@ deepdiff==6.2.1 # via charmed-kubeflow-chisme exceptiongroup==1.1.2 # via anyio +google-auth==2.23.3 + # via kubernetes h11==0.14.0 # via httpcore httpcore==0.17.3 @@ -38,6 +43,8 @@ jinja2==3.1.2 # via charmed-kubeflow-chisme jsonschema==4.17.3 # via serialized-data-interface +kubernetes==27.2.0 + # via -r requirements.in lightkube==0.14.0 # via # -r requirements.in @@ -46,6 +53,10 @@ lightkube-models==1.27.1.4 # via lightkube markupsafe==2.1.3 # via jinja2 +oauthlib==3.2.2 + # via + # kubernetes + # requests-oauthlib ops==2.5.0 # via # -r requirements.in @@ -55,15 +66,31 @@ ordered-set==4.1.0 # via deepdiff pkgutil-resolve-name==1.3.10 # via jsonschema +pyasn1==0.5.0 + # via + # pyasn1-modules + # rsa +pyasn1-modules==0.3.0 + # via google-auth pyrsistent==0.19.3 # via jsonschema +python-dateutil==2.8.2 + # via kubernetes pyyaml==6.0.1 # via + # kubernetes # lightkube # ops # serialized-data-interface requests==2.31.0 - # via serialized-data-interface + # via + # kubernetes + # requests-oauthlib + # serialized-data-interface +requests-oauthlib==1.3.1 + # via kubernetes +rsa==4.9 + # via google-auth ruamel-yaml==0.17.32 # via charmed-kubeflow-chisme ruamel-yaml-clib==0.2.7 @@ -72,6 +99,10 @@ serialized-data-interface==0.7.0 # via # -r requirements.in # charmed-kubeflow-chisme +six==1.16.0 + # via + # kubernetes + # python-dateutil sniffio==1.3.0 # via # anyio @@ -80,8 +111,12 @@ sniffio==1.3.0 tenacity==8.2.2 # via charmed-kubeflow-chisme urllib3==2.0.4 - # via requests + # via + # kubernetes + # requests websocket-client==1.6.1 - # via ops + # via + # kubernetes + # ops zipp==3.16.2 # via importlib-resources diff --git a/charms/kfp-persistence/src/charm.py b/charms/kfp-persistence/src/charm.py index a72b898f..408b8628 100755 --- a/charms/kfp-persistence/src/charm.py +++ b/charms/kfp-persistence/src/charm.py @@ -8,11 +8,13 @@ """ import logging +from pathlib import Path import lightkube from charmed_kubeflow_chisme.components.charm_reconciler import CharmReconciler from charmed_kubeflow_chisme.components.kubernetes_component import KubernetesComponent from charmed_kubeflow_chisme.components.leadership_gate_component import LeadershipGateComponent +from charmed_kubeflow_chisme.components.pebble_component import ContainerFileTemplate from charmed_kubeflow_chisme.components.serialised_data_interface_components import ( SdiRelationDataReceiverComponent, ) @@ -25,10 +27,16 @@ PersistenceAgentPebbleService, PesistenceAgentServiceConfig, ) +from components.sa_token_component import SaTokenComponent log = logging.getLogger() K8S_RESOURCE_FILES = ["src/templates/auth_manifests.yaml.j2"] +SA_NAME = "ml-pipeline-persistenceagent" +SA_TOKEN_PATH = "src/" +SA_TOKEN_FILENAME = "persistenceagent-sa-token" +SA_TOKEN_FULL_PATH = str(Path(SA_TOKEN_PATH, SA_TOKEN_FILENAME)) +SA_TOKEN_DESTINATION_PATH = f"/var/run/secrets/kubeflow/tokens/{SA_TOKEN_FILENAME}" class KfpPersistenceOperator(CharmBase): @@ -62,24 +70,49 @@ def __init__(self, *args, **kwargs): krh_labels=create_charm_default_labels( self.app.name, self.model.name, scope="auth" ), - context_callable=lambda: {"app_name": self.app.name, "namespace": self.model.name}, + context_callable=lambda: { + "app_name": self.app.name, + "namespace": self.model.name, + "sa_name": SA_NAME, + }, lightkube_client=lightkube.Client(), ), depends_on=[self.leadership_gate], ) + self.sa_token = self.charm_reconciler.add( + component=SaTokenComponent( + charm=self, + name="sa-token:persistenceagent", + audiences=["pipelines.kubeflow.org"], + sa_name=SA_NAME, + sa_namespace=self.model.name, + filename=SA_TOKEN_FILENAME, + path=SA_TOKEN_PATH, + expiration=4294967296, + ), + depends_on=[self.leadership_gate, self.kubernetes_resources], + ) self.persistenceagent_container = self.charm_reconciler.add( component=PersistenceAgentPebbleService( charm=self, name="container:persistenceagent", container_name="persistenceagent", service_name="persistenceagent", - files_to_push=[], + files_to_push=[ + ContainerFileTemplate( + source_template_path=SA_TOKEN_FULL_PATH, + destination_path=SA_TOKEN_DESTINATION_PATH, + ) + ], environment={ "KUBEFLOW_USERID_HEADER": "kubeflow-userid", "KUBEFLOW_USERID_PREFIX": "", # Upstream defines this in the configmap persistenceagent-config-* "MULTIUSER": "true", + "NAMESPACE": "", + "TTL_SECONDS_AFTER_WORKFLOW_FINISH": "86400", + "NUM_WORKERS": "2", }, # provide function to pebble with which it can get service configuration from # relation @@ -89,7 +122,12 @@ def __init__(self, *args, **kwargs): ], ), ), - depends_on=[self.leadership_gate, self.kfp_api_relation, self.kubernetes_resources], + depends_on=[ + self.leadership_gate, + self.kfp_api_relation, + self.kubernetes_resources, + self.sa_token, + ], ) self.charm_reconciler.install_default_event_handlers() diff --git a/charms/kfp-persistence/src/components/sa_token_component.py b/charms/kfp-persistence/src/components/sa_token_component.py new file mode 100644 index 00000000..50c92093 --- /dev/null +++ b/charms/kfp-persistence/src/components/sa_token_component.py @@ -0,0 +1,118 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Component for generating the kfp-persistence sa token.""" + +import logging +from pathlib import Path +from typing import List + +import kubernetes +from charmed_kubeflow_chisme.components.component import Component +from charmed_kubeflow_chisme.exceptions import GenericCharmRuntimeError +from kubernetes.client import AuthenticationV1TokenRequest, CoreV1Api, V1TokenRequestSpec +from kubernetes.client.rest import ApiException +from kubernetes.config import ConfigException +from ops import ActiveStatus, StatusBase + +logger = logging.getLogger(__name__) + + +class SaTokenComponent(Component): + """Create a token of a ServiceAccount for the persistence controller.""" + + def __init__( + self, + *args, + audiences: List[str], + sa_name: str, + sa_namespace: str, + path: str, + filename: str, + expiration: int, + **kwargs, + ): + """Instantiate the SaTokenComponent. + + Args: + audiences (List[str]): list of audiences for the SA token + expiration (int): token expiration time in seconds + filename (str): filename to save a token file + path (str): path to save a token file + sa_name (str): ServiceAccount name + sa_namespace (str): ServiceAccount namespace + """ + super().__init__(*args, **kwargs) + self._audiences = audiences + self._expiration = expiration + self._filename = filename + self._sa_name = sa_name + self._sa_namespace = sa_namespace + self._path = path + + @property + def kubernetes_client(self) -> CoreV1Api: + """Load cluster configuration and return a CoreV1 Kubernetes client.""" + try: + kubernetes.config.load_incluster_config() + except ConfigException: + kubernetes.config.load_kube_config() + + api_client = kubernetes.client.ApiClient() + core_v1_api = kubernetes.client.CoreV1Api(api_client) + return core_v1_api + + def _create_sa_token(self) -> AuthenticationV1TokenRequest: + """Return a TokenRequest.""" + # The TokenRequest should always have the audience pointing to pipelines.kubeflow.org + # and an large expiration time to avoid having to re-generate the token and push it + # again to the workload container. + spec = V1TokenRequestSpec(audiences=self._audiences, expiration_seconds=self._expiration) + body = kubernetes.client.AuthenticationV1TokenRequest(spec=spec) + try: + api_response = self.kubernetes_client.create_namespaced_service_account_token( + name=self._sa_name, namespace=self._sa_namespace, body=body + ) + except ApiException as e: + logger.error("Error creating the sa token.") + raise e + return api_response + + def _generate_and_save_token(self, path: str, filename: str) -> None: + """Save the sa token in path in the charm container. + + Args: + path (str): a path to store the token file + filename (str): a filename for the token file + """ + if not Path(path).is_dir(): + logger.error("Path does not exist, cannot proceed saving the sa token file.") + raise RuntimeError("Path does not exist, cannot proceed saving the sa token file.") + if Path(path, filename).is_file(): + logger.info("Token file already exists, nothing else to do.") + api_response = self._create_sa_token() + token = api_response.status.token + with open(Path(path, filename), "w") as token_file: + token_file.write(token) + + def _configure_app_leader(self, event) -> None: + """Generate and save the SA token file. + + Raises: + GenericCharmRuntimeError if the file could not be created. + """ + try: + self._generate_and_save_token(self._path, self._filename) + except (RuntimeError, ApiException) as e: + raise GenericCharmRuntimeError("Failed to create and save sa token") from e + + def get_status(self) -> StatusBase: + """Return ActiveStatus if the SA token file is present. + + Raises: + GenericCharmRuntimeError if the file is not present in the charm. + """ + if not Path(self._path, self._filename).is_file(): + raise GenericCharmRuntimeError("SA token file is not present in charm") + return ActiveStatus() diff --git a/charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 b/charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 index 1a19c933..4b34a9c4 100644 --- a/charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 +++ b/charms/kfp-persistence/src/templates/auth_manifests.yaml.j2 @@ -1,4 +1,4 @@ -# Source manifests/apps/pipeline/upstream/base/installs/multi-user/persistence-agent/ +# Source manifests/apps/pipeline/upstream/base/installs/multi-user/persistence-agent/cluster-role**.yaml # These manifest files have been modified to suit the needs of the charm; the app label, metadata name, # and namespace fields will be rendered with information from the application and the model. apiVersion: rbac.authorization.k8s.io/v1 @@ -32,15 +32,18 @@ rules: verbs: - report - apiGroups: - - '' + - pipelines.kubeflow.org resources: - - namespaces + - runs verbs: - - get + - reportMetrics + - readArtifact --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: + labels: + app: {{ app_name }} name: {{ app_name }}-binding roleRef: apiGroup: rbac.authorization.k8s.io @@ -48,11 +51,13 @@ roleRef: name: {{ app_name }}-role subjects: - kind: ServiceAccount - name: {{ app_name }}-sa + name: {{ sa_name }} namespace: {{ namespace }} --- apiVersion: v1 kind: ServiceAccount metadata: - name: {{ app_name }}-sa + labels: + app: {{ app_name }} + name: {{ sa_name }} namespace: {{ namespace }} diff --git a/charms/kfp-persistence/tests/unit/data/persistenceagent-sa-token b/charms/kfp-persistence/tests/unit/data/persistenceagent-sa-token new file mode 100644 index 00000000..3e2c70ba --- /dev/null +++ b/charms/kfp-persistence/tests/unit/data/persistenceagent-sa-token @@ -0,0 +1 @@ +dummy-sa-token-file diff --git a/charms/kfp-persistence/tests/unit/test_operator.py b/charms/kfp-persistence/tests/unit/test_operator.py index 00adacff..8e790562 100644 --- a/charms/kfp-persistence/tests/unit/test_operator.py +++ b/charms/kfp-persistence/tests/unit/test_operator.py @@ -1,9 +1,10 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest +from charmed_kubeflow_chisme.exceptions import GenericCharmRuntimeError from charmed_kubeflow_chisme.testing import add_sdi_relation_to_harness from ops.model import ActiveStatus, BlockedStatus, WaitingStatus from ops.testing import Harness @@ -29,6 +30,14 @@ def mocked_lightkube_client(mocker): yield mocked_lightkube_client +@pytest.fixture() +def mocked_kubernetes_client(mocker): + """Mocks the kubernetes client in sa token component.""" + mocked_kubernetes_client = MagicMock() + mocker.patch("charm.SaTokenComponent.kubernetes_client") + yield mocked_kubernetes_client + + def test_not_leader(harness, mocked_lightkube_client): """Test not a leader scenario.""" harness.begin_with_initial_hooks() @@ -89,6 +98,33 @@ def test_kfp_api_relation_without_relation(harness, mocked_lightkube_client): assert isinstance(harness.charm.kfp_api_relation.status, BlockedStatus) +def test_no_sa_token_file(harness, mocked_kubernetes_client, mocked_lightkube_client): + """Test the unit status when the SA token file is missing.""" + harness.begin() + harness.set_can_connect("persistenceagent", True) + + harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) + harness.charm.kubernetes_resources.component.get_status = MagicMock( + return_value=ActiveStatus() + ) + add_sdi_relation_to_harness(harness, "kfp-api", data=MOCK_KFP_API_DATA) + harness.charm.kfp_api_relation.component.get_data = MagicMock(return_value=MOCK_KFP_API_DATA) + + with pytest.raises(GenericCharmRuntimeError) as err: + harness.charm.sa_token.get_status() + + assert err.value.msg == "SA token file is not present in charm" + # The base charm arbitrarily sets the unit status to BlockedStatus + # We should fix this in charmed-kubeflow-chisme as it doesn't really + # show the actual error and can be misleading + assert isinstance(harness.charm.unit.status, BlockedStatus) + assert ( + harness.charm.unit.status.message + == "[sa-token:persistenceagent] Failed to compute status. See logs for details." + ) + + +@patch("charm.SA_TOKEN_FULL_PATH", "tests/unit/data/persistenceagent-sa-token") def test_pebble_services_running(harness, mocked_lightkube_client): """Test that if the Kubernetes Component is Active, the pebble services successfully start.""" # Arrange @@ -101,6 +137,7 @@ def test_pebble_services_running(harness, mocked_lightkube_client): # * object_storage_relation_component to be active and executed, and have data that can be # returned harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) + harness.charm.sa_token.get_status = MagicMock(return_value=ActiveStatus()) harness.charm.kfp_api_relation.component.get_data = MagicMock(return_value=MOCK_KFP_API_DATA) # Act @@ -113,4 +150,9 @@ def test_pebble_services_running(harness, mocked_lightkube_client): # Assert the environment variables that are set from inputs are correctly applied environment = container.get_plan().services["persistenceagent"].environment + assert environment["NAMESPACE"] == "" + assert environment["MULTIUSER"] == "true" assert environment["KUBEFLOW_USERID_HEADER"] == "kubeflow-userid" + assert environment["KUBEFLOW_USERID_PREFIX"] == "" + assert environment["TTL_SECONDS_AFTER_WORKFLOW_FINISH"] == "86400" + assert environment["NUM_WORKERS"] == "2" diff --git a/charms/kfp-profile-controller/files/upstream/sync.py b/charms/kfp-profile-controller/files/upstream/sync.py index 730930a5..ada66327 100644 --- a/charms/kfp-profile-controller/files/upstream/sync.py +++ b/charms/kfp-profile-controller/files/upstream/sync.py @@ -1,4 +1,8 @@ # Source: manifests/apps/pipeline/upstream/base/installs/multi-user/pipelines-profile-controller/sync.py +# Note for Charmed Kubeflow developers: Because this file is a lightly modified version of an +# upstream file, we only make changes to it that are functionally required. As much as possible, +# we keep the file comparable to upstream so it can easily be diffed. In particular, keep the +# formatting as is even though it does not meet our own style configurations. # Copyright 2020-2021 The Kubeflow Authors # # Licensed under the Apache License, Version 2.0 (the "License"); @@ -42,10 +46,10 @@ def emit_settings_to_logs(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, + 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, - kfp_default_pipeline_root=None, metadata_grpc_service_host=None, + metadata_grpc_service_host=None, metadata_grpc_service_port=None): """ Returns a dict of settings from environment variables relevant to the controller @@ -352,16 +356,24 @@ def sync(self, parent, attachments): {'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'} - } - }, + { + "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": { @@ -453,6 +465,29 @@ def sync(self, parent, attachments): } } }, + # This AuthorizationPolicy was added from https://github.com/canonical/kfp-operators/pull/356 + # to fix https://github.com/canonical/notebook-operators/issues/311 + # and https://github.com/canonical/kfp-operators/issues/355. + # Remove when istio sidecars are implemented. + { + "apiVersion": "security.istio.io/v1beta1", + "kind": "AuthorizationPolicy", + "metadata": {"name": "ns-owner-access-istio-charmed", "namespace": namespace}, + "spec": { + "rules": [ + { + "when": [ + {"key": "request.headers[kubeflow-userid]", "values": ["*"]} + ] + }, + { + "to": [ + {"operation": {"methods": ["GET"], "paths": ["*/api/kernels"]}} + ] + }, + ] + }, + }, ] 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)) diff --git a/charms/kfp-profile-controller/src/charm.py b/charms/kfp-profile-controller/src/charm.py index e1e5c835..3fe498d6 100755 --- a/charms/kfp-profile-controller/src/charm.py +++ b/charms/kfp-profile-controller/src/charm.py @@ -45,8 +45,11 @@ "src/templates/secrets.yaml.j2", ] KFP_DEFAULT_PIPELINE_ROOT = "" -KFP_IMAGES_VERSION = "2.0.0-alpha.7" -METADATA_GRPC_SERVICE_HOST = "mlmd.kubeflow" +KFP_IMAGES_VERSION = "2.0.3" +# This service name must be the Service from the mlmd-operator +# FIXME: leaving it hardcoded now, but we should share this +# host and port through relation data +METADATA_GRPC_SERVICE_HOST = "metadata-grpc-service.kubeflow" METADATA_GRPC_SERVICE_PORT = "8080" NAMESPACE_LABEL = "pipelines.kubeflow.org/enabled" SYNC_CODE_FILE = Path("files/upstream/sync.py") diff --git a/charms/kfp-profile-controller/src/components/pebble_components.py b/charms/kfp-profile-controller/src/components/pebble_components.py index a1b9d6d2..c022135e 100644 --- a/charms/kfp-profile-controller/src/components/pebble_components.py +++ b/charms/kfp-profile-controller/src/components/pebble_components.py @@ -50,7 +50,6 @@ def get_layer(self) -> Layer: "command": "python /hooks/sync.py", # Must be a string "startup": "enabled", "environment": { - "minio-secret": inputs.MINIO_SECRET, "MINIO_HOST": inputs.MINIO_HOST, "MINIO_PORT": inputs.MINIO_PORT, "MINIO_NAMESPACE": inputs.MINIO_NAMESPACE, diff --git a/charms/kfp-profile-controller/src/templates/crd_manifests.yaml.j2 b/charms/kfp-profile-controller/src/templates/crd_manifests.yaml.j2 index fc28f06b..325e1cc4 100644 --- a/charms/kfp-profile-controller/src/templates/crd_manifests.yaml.j2 +++ b/charms/kfp-profile-controller/src/templates/crd_manifests.yaml.j2 @@ -1,3 +1,6 @@ +# Source manifests/apps/pipeline/upstream/base/installs/multi-user/pipelines-profile-controller/composite-controller.yaml +# Change resyncPeriodSeconds to 1 hour from insane 20 seconds +# Only sync namespaces with pipelines.kubeflow.org/enabled = "true" apiVersion: metacontroller.k8s.io/v1alpha1 kind: DecoratorController metadata: @@ -7,7 +10,7 @@ spec: - apiVersion: v1 resource: secrets updateStrategy: - method: InPlace + method: OnDelete - apiVersion: v1 resource: configmaps updateStrategy: @@ -20,10 +23,16 @@ spec: resource: services updateStrategy: method: InPlace - - apiVersion: kubeflow.org/v1alpha1 - resource: poddefaults - updateStrategy: - method: InPlace + # istio objects are intentionally omitted from Charmed Kubeflow. + # kfp-profile-controller currently does not use them, see in the sync.py. + # - apiVersion: networking.istio.io/v1alpha3 + # resource: destinationrules + # updateStrategy: + # method: InPlace + # - apiVersion: security.istio.io/v1beta1 + # resource: authorizationpolicies + # updateStrategy: + # method: InPlace hooks: sync: webhook: diff --git a/charms/kfp-profile-controller/src/templates/secrets.yaml.j2 b/charms/kfp-profile-controller/src/templates/secrets.yaml.j2 index e1bfa143..b11f3613 100644 --- a/charms/kfp-profile-controller/src/templates/secrets.yaml.j2 +++ b/charms/kfp-profile-controller/src/templates/secrets.yaml.j2 @@ -4,5 +4,7 @@ data: MINIO_SECRET_KEY: {{ secret_key }} kind: Secret metadata: + labels: + app: {{ app_name }} name: {{ minio_secret_name }} namespace: {{ namespace }} diff --git a/charms/kfp-profile-controller/tests/unit/test_operator.py b/charms/kfp-profile-controller/tests/unit/test_operator.py index ec3e29a2..c1cbb316 100644 --- a/charms/kfp-profile-controller/tests/unit/test_operator.py +++ b/charms/kfp-profile-controller/tests/unit/test_operator.py @@ -8,7 +8,15 @@ from ops.model import ActiveStatus, BlockedStatus from ops.testing import Harness -from charm import KfpProfileControllerOperator +from charm import ( + CONTROLLER_PORT, + DISABLE_ISTIO_SIDECAR, + KFP_DEFAULT_PIPELINE_ROOT, + KFP_IMAGES_VERSION, + METADATA_GRPC_SERVICE_HOST, + METADATA_GRPC_SERVICE_PORT, + KfpProfileControllerOperator, +) MOCK_OBJECT_STORAGE_DATA = { "access-key": "access-key", @@ -20,18 +28,17 @@ } EXPECTED_ENVIRONMENT = { - "CONTROLLER_PORT": 80, - "DISABLE_ISTIO_SIDECAR": "false", - "KFP_DEFAULT_PIPELINE_ROOT": "", - "KFP_VERSION": "2.0.0-alpha.7", - "METADATA_GRPC_SERVICE_HOST": "mlmd.kubeflow", - "METADATA_GRPC_SERVICE_PORT": "8080", - "MINIO_ACCESS_KEY": "access-key", - "MINIO_HOST": "service", - "MINIO_NAMESPACE": "namespace", - "MINIO_PORT": 1234, - "MINIO_SECRET_KEY": "secret-key", - "minio-secret": '{"secret": {"name": ' '"kfp-profile-controller-minio-credentials"}}', + "CONTROLLER_PORT": CONTROLLER_PORT, + "DISABLE_ISTIO_SIDECAR": DISABLE_ISTIO_SIDECAR, + "KFP_DEFAULT_PIPELINE_ROOT": KFP_DEFAULT_PIPELINE_ROOT, + "KFP_VERSION": KFP_IMAGES_VERSION, + "METADATA_GRPC_SERVICE_HOST": METADATA_GRPC_SERVICE_HOST, + "METADATA_GRPC_SERVICE_PORT": METADATA_GRPC_SERVICE_PORT, + "MINIO_ACCESS_KEY": MOCK_OBJECT_STORAGE_DATA["access-key"], + "MINIO_HOST": MOCK_OBJECT_STORAGE_DATA["service"], + "MINIO_NAMESPACE": MOCK_OBJECT_STORAGE_DATA["namespace"], + "MINIO_PORT": MOCK_OBJECT_STORAGE_DATA["port"], + "MINIO_SECRET_KEY": MOCK_OBJECT_STORAGE_DATA["secret-key"], } diff --git a/charms/kfp-schedwf/metadata.yaml b/charms/kfp-schedwf/metadata.yaml index 514480ff..60a70750 100755 --- a/charms/kfp-schedwf/metadata.yaml +++ b/charms/kfp-schedwf/metadata.yaml @@ -11,4 +11,4 @@ resources: oci-image: type: oci-image description: Backing OCI image - upstream-source: charmedkubeflow/scheduledworkflow:2.0.0-alpha.7_22.04_1 + upstream-source: gcr.io/ml-pipeline/scheduledworkflow:2.0.3 diff --git a/charms/kfp-schedwf/src/charm.py b/charms/kfp-schedwf/src/charm.py index 965107b7..6b11ad9c 100755 --- a/charms/kfp-schedwf/src/charm.py +++ b/charms/kfp-schedwf/src/charm.py @@ -16,7 +16,7 @@ from charmed_kubeflow_chisme.kubernetes import create_charm_default_labels from lightkube.resources.apiextensions_v1 import CustomResourceDefinition from lightkube.resources.core_v1 import ServiceAccount -from lightkube.resources.rbac_authorization_v1 import Role, RoleBinding +from lightkube.resources.rbac_authorization_v1 import ClusterRole, ClusterRoleBinding from ops.charm import CharmBase from ops.main import main @@ -48,7 +48,12 @@ def __init__(self, *args): charm=self, name="kubernetes:auth-and-crds", resource_templates=K8S_RESOURCE_FILES, - krh_resource_types={CustomResourceDefinition, Role, RoleBinding, ServiceAccount}, + krh_resource_types={ + ClusterRole, + ClusterRoleBinding, + CustomResourceDefinition, + ServiceAccount, + }, krh_labels=create_charm_default_labels( self.app.name, self.model.name, scope="auth-and-crds" ), diff --git a/charms/kfp-schedwf/src/templates/auth_manifests.yaml.j2 b/charms/kfp-schedwf/src/templates/auth_manifests.yaml.j2 index 55cfa381..9b997994 100644 --- a/charms/kfp-schedwf/src/templates/auth_manifests.yaml.j2 +++ b/charms/kfp-schedwf/src/templates/auth_manifests.yaml.j2 @@ -1,13 +1,12 @@ -# Source manifests/apps/pipeline/upstream/base/pipeline/ml-pipeline-scheduledworkflow-**.yaml +# Source manifests/apps/pipeline/upstream/base/installs/multi-user/scheduled-workflow/cluster-role**.yaml # These manifest files have been modified to suit the needs of the charm; the app label, metadata name, # and namespace fields will be rendered with information from the application and the model. apiVersion: rbac.authorization.k8s.io/v1 -kind: Role +kind: ClusterRole metadata: labels: app: {{ app_name }} name: {{ app_name }}-role - namespace: {{ namespace }} rules: - apiGroups: - argoproj.io @@ -43,20 +42,24 @@ rules: - patch --- apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding +kind: ClusterRoleBinding metadata: + labels: + app: {{ app_name }} name: {{ app_name }}-binding - namespace: {{ namespace }} roleRef: apiGroup: rbac.authorization.k8s.io - kind: Role + kind: ClusterRole name: {{ app_name }}-role subjects: - kind: ServiceAccount name: {{ app_name }}-sa + namespace: {{ namespace }} --- apiVersion: v1 kind: ServiceAccount metadata: + labels: + app: {{ app_name }} name: {{ app_name }}-sa namespace: {{ namespace }} diff --git a/charms/kfp-ui/metadata.yaml b/charms/kfp-ui/metadata.yaml index 5a2b50f3..b9ed0358 100755 --- a/charms/kfp-ui/metadata.yaml +++ b/charms/kfp-ui/metadata.yaml @@ -11,7 +11,7 @@ resources: ml-pipeline-ui: type: oci-image description: OCI image for ml-pipeline-ui - upstream-source: gcr.io/ml-pipeline/frontend:2.0.0-alpha.7 + upstream-source: gcr.io/ml-pipeline/frontend:2.0.3 requires: object-storage: interface: object-storage diff --git a/charms/kfp-ui/src/charm.py b/charms/kfp-ui/src/charm.py index 400425e5..5b6c62f0 100755 --- a/charms/kfp-ui/src/charm.py +++ b/charms/kfp-ui/src/charm.py @@ -26,6 +26,7 @@ ) from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch from lightkube.models.core_v1 import ServicePort +from lightkube.resources.core_v1 import ServiceAccount from lightkube.resources.rbac_authorization_v1 import ClusterRole, ClusterRoleBinding from ops import CharmBase, main @@ -151,9 +152,9 @@ def __init__(self, *args): charm=self, name="kubernetes:auth", resource_templates=K8S_RESOURCE_FILES, - krh_resource_types={ClusterRole, ClusterRoleBinding}, + krh_resource_types={ClusterRole, ClusterRoleBinding, ServiceAccount}, krh_labels=create_charm_default_labels( - self.app.name, self.model.name, scope="auth-and-crds" + self.app.name, self.model.name, scope="auth" ), context_callable=lambda: {"app_name": self.app.name, "namespace": self.model.name}, lightkube_client=lightkube.Client(), # TODO: Make this easier to test on @@ -198,6 +199,12 @@ def __init__(self, *args): inputs_getter=lambda: MlPipelineUiInputs( ALLOW_CUSTOM_VISUALIZATIONS=self.model.config["allow-custom-visualizations"], HIDE_SIDENAV=self.model.config["hide-sidenav"], + MINIO_ACCESS_KEY=self.object_storage_relation.component.get_data()[ + "access-key" + ], + MINIO_SECRET_KEY=self.object_storage_relation.component.get_data()[ + "secret-key" + ], MINIO_HOST=self.object_storage_relation.component.get_data()["service"], MINIO_NAMESPACE=self.object_storage_relation.component.get_data()["namespace"], MINIO_PORT=self.object_storage_relation.component.get_data()["port"], diff --git a/charms/kfp-ui/src/components/pebble_components.py b/charms/kfp-ui/src/components/pebble_components.py index d953e82f..26861c2f 100644 --- a/charms/kfp-ui/src/components/pebble_components.py +++ b/charms/kfp-ui/src/components/pebble_components.py @@ -13,7 +13,8 @@ class MlPipelineUiInputs: ALLOW_CUSTOM_VISUALIZATIONS: bool HIDE_SIDENAV: bool - # minio_secret: Dict[str, Dict[str, str]] # TODO: Is this required? + MINIO_ACCESS_KEY: str + MINIO_SECRET_KEY: str MINIO_HOST: str MINIO_NAMESPACE: str MINIO_PORT: str @@ -65,7 +66,8 @@ def get_layer(self) -> Layer: "KUBEFLOW_USERID_PREFIX": "", "METADATA_ENVOY_SERVICE_SERVICE_HOST": "localhost", "METADATA_ENVOY_SERVICE_SERVICE_PORT": "9090", - # "minio-secret": inputs.minio_secret, # TODO: Is this required? + "MINIO_ACCESS_KEY": f"{inputs.MINIO_ACCESS_KEY}", + "MINIO_SECRET_KEY": inputs.MINIO_SECRET_KEY, "MINIO_HOST": inputs.MINIO_HOST, "MINIO_NAMESPACE": inputs.MINIO_NAMESPACE, "MINIO_PORT": inputs.MINIO_PORT, diff --git a/charms/kfp-ui/src/templates/auth_manifests.yaml.j2 b/charms/kfp-ui/src/templates/auth_manifests.yaml.j2 index 20cbaa6c..2886941e 100644 --- a/charms/kfp-ui/src/templates/auth_manifests.yaml.j2 +++ b/charms/kfp-ui/src/templates/auth_manifests.yaml.j2 @@ -1,7 +1,12 @@ +# Source manifests/apps/pipeline/upstream/base/installs/multi-user/pipelines-ui/cluster-role**.yaml +# These manifest files have been modified to suit the needs of the charm; the app label, metadata name, +# and namespace fields will be rendered with information from the application and the model. apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: - name: {{ app_name }} + labels: + app: {{ app_name }} + name: {{ app_name }}-role rules: - apiGroups: - "" @@ -44,12 +49,22 @@ rules: apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding metadata: - name: {{ app_name }} + labels: + app: {{ app_name }} + name: {{ app_name }}-binding roleRef: apiGroup: rbac.authorization.k8s.io kind: ClusterRole - name: {{ app_name }} + name: {{ app_name }}-role subjects: - kind: ServiceAccount - name: {{ app_name }} + name: {{ app_name }}-sa + namespace: {{ namespace }} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + labels: + app: {{ app_name }} + name: {{ app_name }}-sa namespace: {{ namespace }} diff --git a/charms/kfp-ui/tests/unit/test_operator.py b/charms/kfp-ui/tests/unit/test_operator.py index 44ae8d45..2a8eaaaf 100644 --- a/charms/kfp-ui/tests/unit/test_operator.py +++ b/charms/kfp-ui/tests/unit/test_operator.py @@ -51,7 +51,7 @@ def test_kubernetes_created_method( harness.charm.on.install.emit() # Assert - assert mocked_lightkube_client.apply.call_count == 2 + assert mocked_lightkube_client.apply.call_count == 3 assert isinstance(harness.charm.kubernetes_resources.status, ActiveStatus) diff --git a/charms/kfp-viewer/metadata.yaml b/charms/kfp-viewer/metadata.yaml index 50edf98d..4245445b 100755 --- a/charms/kfp-viewer/metadata.yaml +++ b/charms/kfp-viewer/metadata.yaml @@ -12,4 +12,4 @@ resources: kfp-viewer-image: type: oci-image description: OCI image for KFP Viewer - upstream-source: charmedkubeflow/viewer-crd-controller:2.0.0-alpha.7_22.04_1 + upstream-source: gcr.io/ml-pipeline/viewer-crd-controller:2.0.3 diff --git a/charms/kfp-viewer/src/charm.py b/charms/kfp-viewer/src/charm.py index 7715ca5f..336c498f 100755 --- a/charms/kfp-viewer/src/charm.py +++ b/charms/kfp-viewer/src/charm.py @@ -17,7 +17,7 @@ from charmed_kubeflow_chisme.kubernetes import create_charm_default_labels from lightkube.resources.apiextensions_v1 import CustomResourceDefinition from lightkube.resources.core_v1 import ServiceAccount -from lightkube.resources.rbac_authorization_v1 import Role, RoleBinding +from lightkube.resources.rbac_authorization_v1 import ClusterRole, ClusterRoleBinding from ops.charm import CharmBase from ops.main import main @@ -56,7 +56,12 @@ def __init__(self, *args): charm=self, name="kubernetes:auth-and-crds", resource_templates=K8S_RESOURCE_FILES, - krh_resource_types={CustomResourceDefinition, Role, RoleBinding, ServiceAccount}, + krh_resource_types={ + ClusterRole, + ClusterRoleBinding, + CustomResourceDefinition, + ServiceAccount, + }, krh_labels=create_charm_default_labels( self.app.name, self.model.name, scope="auth-and-crds" ), @@ -73,7 +78,6 @@ def __init__(self, *args): container_name="kfp-viewer", service_name="controller", max_num_viewers=self.model.config["max-num-viewers"], - minio_namespace=self._namespace, ), depends_on=[self.kubernetes_resources], ) diff --git a/charms/kfp-viewer/src/components/pebble_component.py b/charms/kfp-viewer/src/components/pebble_component.py index e6673f6b..6660e7de 100644 --- a/charms/kfp-viewer/src/components/pebble_component.py +++ b/charms/kfp-viewer/src/components/pebble_component.py @@ -13,16 +13,15 @@ def __init__( self, *args, max_num_viewers: str, - minio_namespace: str, **kwargs, ): """Pebble service container component in order to configure Pebble layer""" super().__init__(*args, **kwargs) self.max_num_viewers = max_num_viewers - self.minio_namespace = minio_namespace + self.namespace = "" self.environment = { "MAX_NUM_VIEWERS": max_num_viewers, - "MINIO_NAMESPACE": minio_namespace, + "NAMESPACE": self.namespace, } def get_layer(self) -> Layer: @@ -43,7 +42,7 @@ def get_layer(self) -> Layer: "/bin/controller" " -logtostderr=true" f" -max_num_viewers={self.max_num_viewers}" - f" --namespace={self.minio_namespace}" + f" --namespace={self.namespace}" ), "startup": "enabled", "environment": self.environment, diff --git a/charms/kfp-viewer/src/templates/auth_manifests.yaml.j2 b/charms/kfp-viewer/src/templates/auth_manifests.yaml.j2 index c69cc250..94dc6773 100644 --- a/charms/kfp-viewer/src/templates/auth_manifests.yaml.j2 +++ b/charms/kfp-viewer/src/templates/auth_manifests.yaml.j2 @@ -1,16 +1,12 @@ -# Source manifests/apps/pipeline/upstream/base/pipeline/ml-pipeline-viewer-crd-**.yaml ---- -apiVersion: v1 -kind: ServiceAccount -metadata: - name: {{ app_name }} - namespace: {{ namespace }} ---- +# Source manifests/apps/pipeline/upstream/base/installs/multi-user/viewer-controller/cluster-role**.yaml +# These manifest files have been modified to suit the needs of the charm; the app label, metadata name, +# and namespace fields will be rendered with information from the application and the model. apiVersion: rbac.authorization.k8s.io/v1 -kind: Role +kind: ClusterRole metadata: - name: {{ app_name }} - namespace: {{ namespace }} + labels: + app: {{ app_name }} + name: {{ app_name }}-role rules: - apiGroups: - '*' @@ -40,14 +36,24 @@ rules: - delete --- apiVersion: rbac.authorization.k8s.io/v1 -kind: RoleBinding +kind: ClusterRoleBinding metadata: - name: {{ app_name }} - namespace: {{ namespace }} + labels: + app: {{ app_name }} + name: {{ app_name }}-binding roleRef: apiGroup: rbac.authorization.k8s.io - kind: Role - name: {{ app_name }} + kind: ClusterRole + name: {{ app_name }}-role subjects: - kind: ServiceAccount - name: {{ app_name }} + name: {{ app_name }}-sa + namespace: {{ namespace }} +--- +apiVersion: v1 +kind: ServiceAccount +metadata: + labels: + app: {{ app_name }} + name: {{ app_name }}-sa + namespace: {{ namespace }} diff --git a/charms/kfp-viewer/tests/unit/test_operator.py b/charms/kfp-viewer/tests/unit/test_operator.py index 13c8a41e..70a9061f 100644 --- a/charms/kfp-viewer/tests/unit/test_operator.py +++ b/charms/kfp-viewer/tests/unit/test_operator.py @@ -72,7 +72,7 @@ def test_pebble_service_container_running(harness, mocked_lightkube_client): # Assert the environment variables that are set from inputs are correctly applied environment = container.get_plan().services["controller"].environment assert environment["MAX_NUM_VIEWERS"] == harness.charm.config.get("max-num-viewers") - assert environment["MINIO_NAMESPACE"] == str(harness.charm._namespace).lower() + assert environment["NAMESPACE"] == "" def test_install_before_pebble_service_container(harness, mocked_lightkube_client): diff --git a/charms/kfp-viz/metadata.yaml b/charms/kfp-viz/metadata.yaml index bea609a0..2fe61842 100755 --- a/charms/kfp-viz/metadata.yaml +++ b/charms/kfp-viz/metadata.yaml @@ -11,7 +11,7 @@ resources: oci-image: type: oci-image description: OCI image for ml-pipeline-visualizationserver - upstream-source: gcr.io/ml-pipeline/visualization-server:2.0.0-alpha.7 + upstream-source: gcr.io/ml-pipeline/visualization-server:2.0.3 provides: kfp-viz: interface: k8s-service diff --git a/charms/kfp-viz/src/components/pebble_components.py b/charms/kfp-viz/src/components/pebble_components.py index 4f61e3e5..cb43bf06 100644 --- a/charms/kfp-viz/src/components/pebble_components.py +++ b/charms/kfp-viz/src/components/pebble_components.py @@ -18,7 +18,7 @@ def get_layer(self) -> Layer: self.service_name: { "override": "replace", "summary": "entry point for ml-pipeline-visualizationserver", - "command": "python3.6 server.py", # Must be a string + "command": "python3 server.py", # Must be a string "startup": "enabled", "on-check-failure": {"kfp-viz-up": "restart"}, } diff --git a/requirements-integration.in b/requirements-integration.in index 2e8d9484..0da56c83 100644 --- a/requirements-integration.in +++ b/requirements-integration.in @@ -4,8 +4,6 @@ aiohttp jsonschema<4.18 # Pinning to <4.0 due to compatibility with the 3.1 controller version juju<4.0 -# Need this version of kfp to work with kfp 2.0.0-alpha.7 -kfp==1.8.22 lightkube pytest pytest-operator diff --git a/requirements-integration.txt b/requirements-integration.txt index fcf50256..4ea9d935 100644 --- a/requirements-integration.txt +++ b/requirements-integration.txt @@ -4,8 +4,6 @@ # # pip-compile requirements-integration.in # -absl-py==1.4.0 - # via kfp aiohttp==3.8.5 # via -r requirements-integration.in aiosignal==1.3.1 @@ -30,7 +28,6 @@ certifi==2023.7.22 # via # httpcore # httpx - # kfp-server-api # kubernetes # requests cffi==1.15.1 @@ -41,71 +38,28 @@ charset-normalizer==3.2.0 # via # aiohttp # requests -click==8.1.7 - # via - # kfp - # typer -cloudpickle==2.2.1 - # via kfp cryptography==41.0.3 # via paramiko decorator==5.1.1 # via # ipdb # ipython -deprecated==1.2.14 - # via kfp -docstring-parser==0.15 - # via kfp exceptiongroup==1.1.3 # via # anyio # pytest executing==1.2.0 # via stack-data -fire==0.5.0 - # via kfp frozenlist==1.4.0 # via # aiohttp # aiosignal -google-api-core==2.11.1 - # via - # google-api-python-client - # google-cloud-core - # google-cloud-storage - # kfp -google-api-python-client==1.12.11 - # via kfp google-auth==2.22.0 - # via - # google-api-core - # google-api-python-client - # google-auth-httplib2 - # google-cloud-core - # google-cloud-storage - # kfp - # kubernetes -google-auth-httplib2==0.1.0 - # via google-api-python-client -google-cloud-core==2.3.3 - # via google-cloud-storage -google-cloud-storage==2.10.0 - # via kfp -google-crc32c==1.5.0 - # via google-resumable-media -google-resumable-media==2.6.0 - # via google-cloud-storage -googleapis-common-protos==1.60.0 - # via google-api-core + # via kubernetes h11==0.14.0 # via httpcore httpcore==0.17.3 # via httpx -httplib2==0.22.0 - # via - # google-api-python-client - # google-auth-httplib2 httpx==0.24.1 # via lightkube hvac==1.2.0 @@ -129,23 +83,13 @@ jedi==0.19.0 jinja2==3.1.2 # via pytest-operator jsonschema==4.17.3 - # via - # -r requirements-integration.in - # kfp + # via -r requirements-integration.in juju==3.2.2 # via # -r requirements-integration.in # pytest-operator -kfp==1.8.22 - # via -r requirements-integration.in -kfp-pipeline-spec==0.1.16 - # via kfp -kfp-server-api==1.8.5 - # via kfp kubernetes==25.3.0 - # via - # juju - # kfp + # via juju lightkube==0.14.0 # via -r requirements-integration.in lightkube-models==1.28.1.4 @@ -181,12 +125,7 @@ pluggy==1.3.0 prompt-toolkit==3.0.39 # via ipython protobuf==3.20.3 - # via - # google-api-core - # googleapis-common-protos - # kfp - # kfp-pipeline-spec - # macaroonbakery + # via macaroonbakery ptyprocess==0.7.0 # via pexpect pure-eval==0.2.2 @@ -200,8 +139,6 @@ pyasn1-modules==0.3.0 # via google-auth pycparser==2.21 # via cffi -pydantic==1.10.12 - # via kfp pygments==2.16.1 # via ipython pyhcl==0.4.5 @@ -213,8 +150,6 @@ pynacl==1.5.0 # macaroonbakery # paramiko # pymacaroons -pyparsing==3.1.1 - # via httplib2 pyrfc3339==1.1 # via # juju @@ -231,42 +166,30 @@ pytest-asyncio==0.21.1 pytest-operator==0.29.0 # via -r requirements-integration.in python-dateutil==2.8.2 - # via - # kfp-server-api - # kubernetes + # via kubernetes pytz==2023.3.post1 # via pyrfc3339 pyyaml==6.0.1 # via # -r requirements-integration.in # juju - # kfp # kubernetes # lightkube # pytest-operator requests==2.31.0 # via - # google-api-core - # google-cloud-storage # hvac # kubernetes # macaroonbakery # requests-oauthlib - # requests-toolbelt requests-oauthlib==1.3.1 # via kubernetes -requests-toolbelt==0.10.1 - # via kfp rsa==4.9 # via google-auth six==1.16.0 # via # asttokens - # fire - # google-api-python-client # google-auth - # google-auth-httplib2 - # kfp-server-api # kubernetes # macaroonbakery # paramiko @@ -279,14 +202,8 @@ sniffio==1.3.0 # httpx stack-data==0.6.2 # via ipython -strip-hints==0.1.10 - # via kfp -tabulate==0.9.0 - # via kfp tenacity==8.2.3 # via -r requirements-integration.in -termcolor==2.3.0 - # via fire tomli==2.0.1 # via # ipdb @@ -297,26 +214,15 @@ traitlets==5.9.0 # via # ipython # matplotlib-inline -typer==0.9.0 - # via kfp typing-extensions==4.7.1 # via # ipython - # kfp - # pydantic - # typer # typing-inspect typing-inspect==0.9.0 # via juju -uritemplate==3.0.1 - # via - # google-api-python-client - # kfp urllib3==1.26.16 # via # google-auth - # kfp - # kfp-server-api # kubernetes # requests wcwidth==0.2.6 @@ -325,10 +231,6 @@ websocket-client==1.6.2 # via kubernetes websockets==8.1 # via juju -wheel==0.41.2 - # via strip-hints -wrapt==1.15.0 - # via deprecated yarl==1.9.2 # via aiohttp zipp==3.16.2 diff --git a/tests/README.md b/tests/README.md index 110be336..865b66ca 100644 --- a/tests/README.md +++ b/tests/README.md @@ -19,9 +19,8 @@ This directory has the following structure: │   ├── k8s_resources.py │   └── localize_bundle.py ├── kfp_globals.py - ├── pipelines - │   ├── sample_pipeline.yaml - │   └── sample_pipeline_execution_order.py + ├── pipelines/ + │   └── ... # Sample pipelines ├── profile │   └── profile.yaml ├── test_kfp_functional.py diff --git a/tests/integration/bundles/kfp_1.7_stable_install.yaml.j2 b/tests/integration/bundles/kfp_1.7_stable_install.yaml.j2 index 40bafba5..d6c3201d 100644 --- a/tests/integration/bundles/kfp_1.7_stable_install.yaml.j2 +++ b/tests/integration/bundles/kfp_1.7_stable_install.yaml.j2 @@ -24,7 +24,7 @@ applications: trust: true kfp-db: charm: mysql-k8s - channel: 8.0/edge + channel: 8.0/stable scale: 1 options: profile: testing @@ -46,34 +46,33 @@ applications: {%- if local_build == false %} kfp-api: charm: kfp-api - channel: 2.0/stable + channel: 2.0-alpha.7/stable scale: 1 trust: true kfp-persistence: charm: kfp-persistence - channel: 2.0/stable + channel: 2.0-alpha.7/stable + scale: 1 + kfp-profile-controller: + charm: kfp-profile-controller + channel: 2.0-alpha.7/stable scale: 1 - trust: true kfp-schedwf: charm: kfp-schedwf - channel: 2.0/stable + channel: 2.0-alpha.7/stable scale: 1 - trust: true kfp-ui: charm: kfp-ui - channel: 2.0/stable + channel: 2.0-alpha.7/stable scale: 1 - trust: true kfp-viewer: charm: kfp-viewer - channel: 2.0/stable + channel: 2.0-alpha.7/stable scale: 1 - trust: true kfp-viz: charm: kfp-viz - channel: 2.0/stable + channel: 2.0-alpha.7/stable scale: 1 - trust: true {% else %} kfp-api: charm: {{ kfp_api }} @@ -84,27 +83,26 @@ applications: charm: {{ kfp_persistence }} resources: {{ kfp_persistence_resources }} scale: 1 - trust: true + kfp-profile-controller: + charm: {{ kfp_profile_controller }} + resources: {{ kfp_profile_controller_resources }} + scale: 1 kfp-schedwf: charm: {{ kfp_schedwf }} resources: {{ kfp_schedwf_resources }} scale: 1 - trust: true kfp-ui: charm: {{ kfp_ui }} resources: {{ kfp_ui_resources }} scale: 1 - trust: true kfp-viewer: charm: {{ kfp_viewer }} resources: {{ kfp_viewer_resources }} scale: 1 - trust: true kfp-viz: charm: {{ kfp_viz }} resources: {{ kfp_viz_resources }} scale: 1 - trust: true {%- endif %} relations: - - kfp-api:kfp-api @@ -121,6 +119,8 @@ relations: - kfp-ui:ingress - - kfp-api:kfp-api - kfp-ui:kfp-api +- - kfp-profile-controller:object-storage + - minio:object-storage - - kfp-ui:object-storage - minio:object-storage - - argo-controller:object-storage diff --git a/tests/integration/bundles/kfp_latest_edge.yaml.j2 b/tests/integration/bundles/kfp_latest_edge.yaml.j2 index 28d572d1..6e42dc73 100644 --- a/tests/integration/bundles/kfp_latest_edge.yaml.j2 +++ b/tests/integration/bundles/kfp_latest_edge.yaml.j2 @@ -1,14 +1,37 @@ bundle: kubernetes name: kubeflow-pipelines applications: - argo-controller: { charm: ch:argo-controller, channel: latest/edge, scale: 1 } + argo-controller: { charm: ch:argo-controller, channel: latest/edge, scale: 1, trust: true } metacontroller-operator: { charm: ch:metacontroller-operator, channel: latest/edge, scale: 1, trust: true } minio: { charm: ch:minio, channel: latest/edge, scale: 1 } - kfp-db: { charm: ch:mysql-k8s, channel: 8.0/stable, scale: 1, constraints: mem=2G} + kfp-db: { charm: ch:mysql-k8s, channel: 8.0/stable, scale: 1, constraints: mem=2G, trust: true } + mlmd: { charm: ch:mlmd, channel: latest/edge, scale: 1 } + envoy: { charm: ch:envoy, channel: latest/edge, scale: 1 } + kubeflow-profiles: { charm: ch:kubeflow-profiles, channel: latest/edge, scale: 1, trust: true } + istio-ingressgateway: + charm: istio-gateway + channel: latest/edge + scale: 1 + options: + kind: ingress + trust: true + istio-pilot: + charm: istio-pilot + channel: latest/edge + scale: 1 + options: + default-gateway: kubeflow-gateway + trust: true + kubeflow-roles: + charm: kubeflow-roles + channel: latest/edge + scale: 1 + trust: true {%- if local_build == false %} kfp-api: { charm: ch:kfp-api, channel: latest/edge, scale: 1, trust: true} - kfp-profile-controller: { charm: ch:kfp-profile-controller, channel: latest/edge, scale: 1, trust: true } + kfp-metadata-writer: { charm: ch:kfp-metadata-writer, channel: latest/edge, scale: 1, trust: true} kfp-persistence: { charm: ch:kfp-persistence, channel: latest/edge, scale: 1, trust: true } + kfp-profile-controller: { charm: ch:kfp-profile-controller, channel: latest/edge, scale: 1, trust: true } kfp-schedwf: { charm: ch:kfp-schedwf, channel: latest/edge, scale: 1, trust: true} kfp-ui: { charm: ch:kfp-ui, channel: latest/edge, trust: true, scale: 1 } kfp-viewer: { charm: ch:kfp-viewer, channel: latest/edge, trust: true scale: 1 } @@ -19,6 +42,11 @@ applications: resources: {{ kfp_api_resources }} scale: 1 trust: true + kfp-metadata-writer: + charm: {{ kfp_metadata_writer }} + resources: {{ kfp_metadata_writer_resources }} + scale: 1 + trust: true kfp-persistence: charm: {{ kfp_persistence }} resources: {{ kfp_persistence_resources }} @@ -28,6 +56,7 @@ applications: charm: {{ kfp_profile_controller }} resources: {{ kfp_profile_controller_resources }} scale: 1 + trust: true kfp-schedwf: charm: {{ kfp_schedwf }} resources: {{ kfp_schedwf_resources }} @@ -56,5 +85,9 @@ relations: - [kfp-api:kfp-api, kfp-ui:kfp-api] - [kfp-api:kfp-viz, kfp-viz:kfp-viz] - [kfp-api:object-storage, minio:object-storage] -- [kfp-profile-controller:object-storage, minio:object-storage] - [kfp-ui:object-storage, minio:object-storage] +- [envoy:grpc, mlmd:grpc] +- [kfp-metadata-writer:grpc, mlmd:grpc] +- [envoy:ingress, istio-pilot:ingress] +- [istio-ingressgateway:istio-pilot, istio-pilot:istio-pilot] +- [kfp-profile-controller:object-storage, minio:object-storage] diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py index c5e498cc..9b983f6b 100644 --- a/tests/integration/conftest.py +++ b/tests/integration/conftest.py @@ -30,7 +30,7 @@ KUBEFLOW_USER_NAME = PROFILE_FILE["spec"]["owner"]["name"] -@pytest.fixture(scope="module") +@pytest.fixture(scope="session") def forward_kfp_ui(): """Port forward the kfp-ui service.""" kfp_ui_process = subprocess.Popen( @@ -46,7 +46,7 @@ def forward_kfp_ui(): kfp_ui_process.terminate() -@pytest.fixture(scope="module") +@pytest.fixture(scope="session") def apply_profile(lightkube_client): """Apply a Profile simulating a user.""" # Create a Viewer namespaced resource @@ -59,7 +59,7 @@ def apply_profile(lightkube_client): yield - # Remove namespace + # Remove profile read_yaml = Path(PROFILE_FILE_PATH).read_text() yaml_loaded = codecs.load_all_yaml(read_yaml) for obj in yaml_loaded: @@ -73,7 +73,7 @@ def apply_profile(lightkube_client): raise api_error -@pytest.fixture(scope="module") +@pytest.fixture(scope="session") def kfp_client(apply_profile, forward_kfp_ui) -> kfp.Client: """Returns a KFP Client that can talk to the KFP API Server.""" # Instantiate the KFP Client @@ -82,37 +82,13 @@ def kfp_client(apply_profile, forward_kfp_ui) -> kfp.Client: return client -@pytest.fixture(scope="module") +@pytest.fixture(scope="session") def lightkube_client() -> lightkube.Client: """Returns a lightkube Client that can talk to the K8s API.""" client = lightkube.Client(field_manager="kfp-operators") return client -@pytest.fixture(scope="function") -def upload_and_clean_pipeline(kfp_client: kfp.Client): - """Upload an arbitrary pipeline and remove after test case execution.""" - pipeline_upload_response = kfp_client.pipeline_uploads.upload_pipeline( - uploadfile=SAMPLE_PIPELINE, name=SAMPLE_PIPELINE_NAME - ) - - yield pipeline_upload_response - - kfp_client.delete_pipeline(pipeline_id=pipeline_upload_response.id) - - -@pytest.fixture(scope="function") -def create_and_clean_experiment(kfp_client: kfp.Client): - """Create an experiment and remove after test case execution.""" - experiment_response = kfp_client.create_experiment( - name="test-experiment", namespace=KUBEFLOW_PROFILE_NAMESPACE - ) - - yield experiment_response - - kfp_client.delete_experiment(experiment_id=experiment_response.id) - - def pytest_addoption(parser: Parser): parser.addoption( "--bundle", diff --git a/tests/integration/kfp_globals.py b/tests/integration/kfp_globals.py index cb8dc007..9f74dc5a 100644 --- a/tests/integration/kfp_globals.py +++ b/tests/integration/kfp_globals.py @@ -10,7 +10,9 @@ # All charms in the kfp-operators repository, except kfp-profile-controller KFP_CHARMS = [ "kfp-api", + "kfp-metadata-writer", "kfp-persistence", + "kfp-profile-controller", "kfp-schedwf", "kfp-ui", "kfp-viewer", @@ -18,8 +20,13 @@ ] # Variables for uploading/creating pipelines/experiments/runs -SAMPLE_PIPELINE = f"{basedir}/tests/integration/pipelines/sample_pipeline.yaml" -SAMPLE_PIPELINE_NAME = "sample-pipeline-2" +SAMPLE_PIPELINES_PATH = f"{basedir}/tests/integration/pipelines" +SAMPLE_PIPELINE = { + "v1": f"{SAMPLE_PIPELINES_PATH}/sample_pipeline.yaml", + "v2": f"{SAMPLE_PIPELINES_PATH}/pipeline_container_no_input.yaml", +} + +SAMPLE_PIPELINE_NAME = "sample-pipeline" # Variables for creating a viewer SAMPLE_VIEWER = f"{basedir}/tests/integration/viewer/mnist.yaml" diff --git a/tests/integration/pipelines/pipeline_container_no_input.py b/tests/integration/pipelines/pipeline_container_no_input.py new file mode 100644 index 00000000..ca430c68 --- /dev/null +++ b/tests/integration/pipelines/pipeline_container_no_input.py @@ -0,0 +1,38 @@ +# Copyright 2022 The Kubeflow Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +import os + +from kfp import compiler +from kfp import dsl + + +@dsl.container_component +def container_no_input(): + return dsl.ContainerSpec( + image="python:3.7", + command=["echo", "hello world"], + args=[], + ) + + +@dsl.pipeline(name="v2-container-component-no-input") +def pipeline_container_no_input(): + container_no_input() + + +if __name__ == "__main__": + # execute only if run as a script + compiler.Compiler().compile( + pipeline_func=pipeline_container_no_input, package_path="pipeline_container_no_input.yaml" + ) diff --git a/tests/integration/pipelines/pipeline_container_no_input.yaml b/tests/integration/pipelines/pipeline_container_no_input.yaml new file mode 100644 index 00000000..5bdd4279 --- /dev/null +++ b/tests/integration/pipelines/pipeline_container_no_input.yaml @@ -0,0 +1,29 @@ +# PIPELINE DEFINITION +# Name: v2-container-component-no-input +# This pipeline was generated using the sample_pipeline_execution_order.py in this directory +# Please do not edit this file directly +components: + comp-container-no-input: + executorLabel: exec-container-no-input +deploymentSpec: + executors: + exec-container-no-input: + container: + command: + - echo + - hello world + image: python:3.7 +pipelineInfo: + name: v2-container-component-no-input +root: + dag: + tasks: + container-no-input: + cachingOptions: + enableCache: true + componentRef: + name: comp-container-no-input + taskInfo: + name: container-no-input +schemaVersion: 2.1.0 +sdkVersion: kfp-2.0.0 diff --git a/tests/integration/test_kfp_functional.py b/tests/integration/test_kfp_functional_v1.py similarity index 73% rename from tests/integration/test_kfp_functional.py rename to tests/integration/test_kfp_functional_v1.py index f5d73f75..fa555270 100644 --- a/tests/integration/test_kfp_functional.py +++ b/tests/integration/test_kfp_functional_v1.py @@ -1,7 +1,7 @@ #!/usr/bin/env python3 # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -"""Functional tests for kfp-operators.""" +"""Functional tests for kfp-operators with the KFP SDK v1.""" import logging import time from pathlib import Path @@ -14,9 +14,11 @@ KFP_CHARMS, KUBEFLOW_PROFILE_NAMESPACE, SAMPLE_PIPELINE, + SAMPLE_PIPELINE_NAME, SAMPLE_VIEWER, ) +import kfp import lightkube import pytest import tenacity @@ -26,9 +28,42 @@ from pytest_operator.plugin import OpsTest +KFP_SDK_VERSION = "v1" log = logging.getLogger(__name__) +# ---- KFP SDK V1 fixtures +@pytest.fixture(scope="function") +def upload_and_clean_pipeline_v1(kfp_client: kfp.Client): + """Upload an arbitrary pipeline and remove after test case execution.""" + pipeline_upload_response = kfp_client.pipeline_uploads.upload_pipeline( + uploadfile=SAMPLE_PIPELINE[KFP_SDK_VERSION], name=SAMPLE_PIPELINE_NAME + ) + # The newer pipelines backend requires the deletion of the pipelines versions + # before we can actually remove the pipeline. This variable extracts the pipeline + # version id that can be used to remove it later in the test exectution. + pipeline_version_id = ( + kfp_client.list_pipeline_versions(pipeline_upload_response.id).versions[0].id + ) + + yield pipeline_upload_response + + kfp_client.delete_pipeline_version(pipeline_version_id) + kfp_client.delete_pipeline(pipeline_id=pipeline_upload_response.id) + + +@pytest.fixture(scope="function") +def create_and_clean_experiment_v1(kfp_client: kfp.Client): + """Create an experiment and remove after test case execution.""" + experiment_response = kfp_client.create_experiment( + name="test-experiment", namespace=KUBEFLOW_PROFILE_NAMESPACE + ) + + yield experiment_response + + kfp_client.delete_experiment(experiment_id=experiment_response.id) + + @pytest.mark.abort_on_fail async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client): """Build and deploy kfp-operators charms.""" @@ -74,35 +109,39 @@ async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client): raise_on_blocked=False, # These apps block while waiting for each other to deploy/relate raise_on_error=True, timeout=3600, - idle_period=120, + idle_period=30, ) + # ---- KFP API Server focused test cases async def test_upload_pipeline(kfp_client): """Upload a pipeline from a YAML file and assert its presence.""" # Upload a pipeline and get the server response + pipeline_name = f"test-pipeline-sdk-{KFP_SDK_VERSION}" pipeline_upload_response = kfp_client.pipeline_uploads.upload_pipeline( - uploadfile=SAMPLE_PIPELINE, name="test-upload-pipeline" + uploadfile=SAMPLE_PIPELINE[KFP_SDK_VERSION], + name=pipeline_name, ) # Upload a pipeline and get its ID uploaded_pipeline_id = pipeline_upload_response.id # Get pipeline id by name, default='sample-pipeline' - server_pipeline_id = kfp_client.get_pipeline_id(name="test-upload-pipeline") + server_pipeline_id = kfp_client.get_pipeline_id(name=pipeline_name) assert uploaded_pipeline_id == server_pipeline_id -async def test_create_and_monitor_run(kfp_client, create_and_clean_experiment): + +async def test_create_and_monitor_run(kfp_client, create_and_clean_experiment_v1): """Create a run and monitor it to completion.""" # Create a run, save response in variable for easy manipulation # Create an experiment for this run - experiment_response = create_and_clean_experiment + experiment_response = create_and_clean_experiment_v1 # Create a run from a pipeline file (SAMPLE_PIPELINE) and an experiment (create_experiment). # This call uses the 'default' kubeflow service account to be able to edit Workflows create_run_response = kfp_client.create_run_from_pipeline_package( - pipeline_file=SAMPLE_PIPELINE, + pipeline_file=SAMPLE_PIPELINE[KFP_SDK_VERSION], arguments={}, - run_name="test-run-1", + run_name=f"test-run-sdk-{KFP_SDK_VERSION}", experiment_name=experiment_response.name, namespace=KUBEFLOW_PROFILE_NAMESPACE, ) @@ -111,33 +150,34 @@ async def test_create_and_monitor_run(kfp_client, create_and_clean_experiment): # Related issue: https://github.com/canonical/kfp-operators/issues/244 # Monitor the run to completion, the pipeline should not be executed in # more than 300 seconds as it is a very simple operation - #monitor_response = kfp_client.wait_for_run_completion(create_run_response.run_id, timeout=600) + # monitor_response = kfp_client.wait_for_run_completion(create_run_response.run_id, timeout=600) - #assert monitor_response.run.status == "Succeeded" + # assert monitor_response.run.status == "Succeeded" # At least get the run and extract some data while the previous check # works properly on the GitHub runners test_run = kfp_client.get_run(create_run_response.run_id).run assert test_run is not None + # ---- ScheduledWorfklows and Argo focused test case async def test_create_and_monitor_recurring_run( - kfp_client, upload_and_clean_pipeline, create_and_clean_experiment + kfp_client, upload_and_clean_pipeline_v1, create_and_clean_experiment_v1 ): """Create a recurring run and monitor it to completion.""" # Upload a pipeline from file - pipeline_response = upload_and_clean_pipeline + pipeline_response = upload_and_clean_pipeline_v1 # Create an experiment for this run - experiment_response = create_and_clean_experiment + experiment_response = create_and_clean_experiment_v1 # Create a recurring run from a pipeline (upload_pipeline_from_file) and an experiment (create_experiment) # This call uses the 'default' kubeflow service account to be able to edit ScheduledWorkflows # This ScheduledWorkflow (Recurring Run) will run once every two seconds create_recurring_run_response = kfp_client.create_recurring_run( experiment_id=experiment_response.id, - job_name="recurring-job-1", + job_name=f"recurring-job-{KFP_SDK_VERSION}", pipeline_id=pipeline_response.id, enabled=True, cron_expression="*/2 * * * * *", @@ -163,33 +203,3 @@ async def test_create_and_monitor_recurring_run( # Assert the job is disabled # assert recurring_job.enabled is False - - -# ---- KFP Viewer and Visualization focused test cases -async def test_apply_sample_viewer(lightkube_client): - """Test a Viewer can be applied and its presence is verified.""" - # Create a Viewer namespaced resource - viewer_class_resource = create_namespaced_resource( - group="kubeflow.org", version="v1beta1", kind="Viewer", plural="viewers" - ) - - # Apply viewer - viewer_object = apply_manifests(lightkube_client, yaml_file_path=SAMPLE_VIEWER) - - viewer = lightkube_client.get( - res=viewer_class_resource, - name=viewer_object.metadata.name, - namespace=viewer_object.metadata.namespace, - ) - assert viewer is not None - - -async def test_viz_server_healthcheck(ops_test: OpsTest): - """Run a healthcheck on the server endpoint.""" - status = await ops_test.model.get_status() - units = status["applications"]["kfp-viz"]["units"] - url = units["kfp-viz/0"]["address"] - headers = {"kubeflow-userid": "user"} - result_status, result_text = await fetch_response(url=f"http://{url}:8888", headers=headers) - - assert result_status == 200 diff --git a/tests/integration/test_kfp_functional_v2.py b/tests/integration/test_kfp_functional_v2.py new file mode 100644 index 00000000..1fb15274 --- /dev/null +++ b/tests/integration/test_kfp_functional_v2.py @@ -0,0 +1,232 @@ +#!/usr/bin/env python3 +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. +"""Functional tests for kfp-operators with the KFP SDK v2.""" +import logging +import time +from pathlib import Path + +from helpers.bundle_mgmt import render_bundle, deploy_bundle +from helpers.k8s_resources import apply_manifests, fetch_response +from helpers.localize_bundle import get_resources_from_charm_file +from kfp_globals import ( + CHARM_PATH_TEMPLATE, + KFP_CHARMS, + KUBEFLOW_PROFILE_NAMESPACE, + SAMPLE_PIPELINE, + SAMPLE_PIPELINE_NAME, + SAMPLE_VIEWER, +) + +import kfp +import lightkube +import pytest +import tenacity +from lightkube import codecs +from lightkube.generic_resource import create_namespaced_resource +from lightkube.resources.apps_v1 import Deployment +from pytest_operator.plugin import OpsTest + +KFP_SDK_VERSION = "v2" +log = logging.getLogger(__name__) + + +# ---- KFP SDK V2 fixtures +@pytest.fixture(scope="function") +def upload_and_clean_pipeline_v2(kfp_client: kfp.Client): + """Upload an arbitrary v2 pipeline and remove after test case execution.""" + pipeline_upload_response = kfp_client.pipeline_uploads.upload_pipeline( + uploadfile=SAMPLE_PIPELINE[KFP_SDK_VERSION], name=SAMPLE_PIPELINE_NAME + ) + # The newer pipelines backend requires the deletion of the pipelines versions + # before we can actually remove the pipeline. This variable extracts the pipeline + # version id that can be used to remove it later in the test exectution. + pipeline_version_id = ( + kfp_client.list_pipeline_versions(pipeline_upload_response.pipeline_id) + .pipeline_versions[0] + .pipeline_version_id + ) + + yield pipeline_upload_response, pipeline_version_id + + kfp_client.delete_pipeline_version(pipeline_upload_response.pipeline_id, pipeline_version_id) + kfp_client.delete_pipeline(pipeline_upload_response.pipeline_id) + + +@pytest.fixture(scope="function") +def create_and_clean_experiment_v2(kfp_client: kfp.Client): + """Create an experiment and remove after test case execution.""" + experiment_response = kfp_client.create_experiment( + name="test-experiment", namespace=KUBEFLOW_PROFILE_NAMESPACE + ) + + yield experiment_response + + kfp_client.delete_experiment(experiment_id=experiment_response.experiment_id) + + +@pytest.mark.abort_on_fail +async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client): + """Build and deploy kfp-operators charms.""" + no_build = request.config.getoption("no_build") + + # Immediately raise an error if the model name is not kubeflow + if ops_test.model_name != "kubeflow": + raise ValueError("kfp must be deployed to namespace kubeflow") + + # Get/load template bundle from command line args + bundlefile_path = Path(request.config.getoption("bundle")) + basedir = Path("./").absolute() + + # Build the charms we need to build only if --no-build is not set + context = {} + if not no_build: + charms_to_build = { + charm: Path(CHARM_PATH_TEMPLATE.format(basedir=str(basedir), charm=charm)) + for charm in KFP_CHARMS + } + log.info(f"Building charms for: {charms_to_build}") + built_charms = await ops_test.build_charms(*charms_to_build.values()) + log.info(f"Built charms: {built_charms}") + + for charm, charm_file in built_charms.items(): + charm_resources = get_resources_from_charm_file(charm_file) + context.update([(f"{charm.replace('-', '_')}_resources", charm_resources)]) + context.update([(f"{charm.replace('-', '_')}", charm_file)]) + + # Render kfp-operators bundle file with locally built charms and their resources + rendered_bundle = render_bundle( + ops_test, bundle_path=bundlefile_path, context=context, no_build=no_build + ) + + # Deploy the kfp-operators bundle from the rendered bundle file + await deploy_bundle(ops_test, bundle_path=rendered_bundle, trust=True) + + # Wait for everything to be up. Note, at time of writing these charms would naturally go + # into blocked during deploy while waiting for each other to satisfy relations, so we don't + # raise_on_blocked. + await ops_test.model.wait_for_idle( + status="active", + raise_on_blocked=False, # These apps block while waiting for each other to deploy/relate + raise_on_error=True, + timeout=3600, + idle_period=30, + ) + + +# ---- KFP API Server focused test cases +async def test_upload_pipeline(kfp_client): + """Upload a pipeline from a YAML file and assert its presence.""" + # Upload a pipeline and get the server response + pipeline_name = f"test-pipeline-sdk-{KFP_SDK_VERSION}" + pipeline_upload_response = kfp_client.pipeline_uploads.upload_pipeline( + uploadfile=SAMPLE_PIPELINE[KFP_SDK_VERSION], + name=pipeline_name, + ) + # Upload a pipeline and get its ID + uploaded_pipeline_id = pipeline_upload_response.pipeline_id + + # Get pipeline id by name, default='sample-pipeline' + server_pipeline_id = kfp_client.get_pipeline_id(name=pipeline_name) + assert uploaded_pipeline_id == server_pipeline_id + + +async def test_create_and_monitor_run(kfp_client, create_and_clean_experiment_v2): + """Create a run and monitor it to completion.""" + # Create a run, save response in variable for easy manipulation + # Create an experiment for this run + experiment_response = create_and_clean_experiment_v2 + + # Create a run from a pipeline file (SAMPLE_PIPELINE) and an experiment (create_experiment). + # This call uses the 'default' kubeflow service account to be able to edit Workflows + create_run_response = kfp_client.create_run_from_pipeline_package( + pipeline_file=SAMPLE_PIPELINE[KFP_SDK_VERSION], + arguments={}, + run_name=f"test-run-sdk-{KFP_SDK_VERSION}", + experiment_name=experiment_response.display_name, + namespace=KUBEFLOW_PROFILE_NAMESPACE, + ) + + # Monitor the run to completion, the pipeline should not be executed in + # more than 300 seconds as it is a very simple operation + monitor_response = kfp_client.wait_for_run_completion(create_run_response.run_id, timeout=600) + + assert monitor_response.state == "SUCCEEDED" + + +# ---- ScheduledWorfklows and Argo focused test case +async def test_create_and_monitor_recurring_run( + kfp_client, upload_and_clean_pipeline_v2, create_and_clean_experiment_v2 +): + """Create a recurring run and monitor it to completion.""" + + # Upload a pipeline from file + pipeline_response, pipeline_version_id = upload_and_clean_pipeline_v2 + + # Create an experiment for this run + experiment_response = create_and_clean_experiment_v2 + + # Create a recurring run from a pipeline (upload_pipeline_from_file) and an experiment (create_experiment) + # This call uses the 'default' kubeflow service account to be able to edit ScheduledWorkflows + # This ScheduledWorkflow (Recurring Run) will run once every two seconds + create_recurring_run_response = kfp_client.create_recurring_run( + experiment_id=experiment_response.experiment_id, + job_name=f"recurring-job-{KFP_SDK_VERSION}", + pipeline_id=pipeline_response.pipeline_id, + version_id=pipeline_version_id, + enabled=True, + cron_expression="*/2 * * * * *", + max_concurrency=1, + ) + + recurring_job = create_recurring_run_response + + # Assert the job is enabled + assert recurring_job.status == "ENABLED" + + # Assert the job executes once every two seconds + assert recurring_job.trigger.cron_schedule.cron == "*/2 * * * * *" + + # Wait for the recurring job to schedule some runs + time.sleep(6) + + # FIXME: disabling the job does not work at the moment, it seems like + # the status of the recurring run is never updated and is causing the + # following assertion to fail + # Related issue: https://github.com/canonical/kfp-operators/issues/244 + # Disable the job after few runs + + kfp_client.disable_recurring_run(recurring_job.recurring_run_id) + + # Assert the job is disabled + # assert recurring_job.status is "DISABLED" + + +# ---- KFP Viewer and Visualization focused test cases +async def test_apply_sample_viewer(lightkube_client): + """Test a Viewer can be applied and its presence is verified.""" + # Create a Viewer namespaced resource + viewer_class_resource = create_namespaced_resource( + group="kubeflow.org", version="v1beta1", kind="Viewer", plural="viewers" + ) + + # Apply viewer + viewer_object = apply_manifests(lightkube_client, yaml_file_path=SAMPLE_VIEWER) + + viewer = lightkube_client.get( + res=viewer_class_resource, + name=viewer_object.metadata.name, + namespace=viewer_object.metadata.namespace, + ) + assert viewer is not None + + +async def test_viz_server_healthcheck(ops_test: OpsTest): + """Run a healthcheck on the server endpoint.""" + status = await ops_test.model.get_status() + units = status["applications"]["kfp-viz"]["units"] + url = units["kfp-viz/0"]["address"] + headers = {"kubeflow-userid": "user"} + result_status, result_text = await fetch_response(url=f"http://{url}:8888", headers=headers) + + assert result_status == 200 diff --git a/tox.ini b/tox.ini index a3ff54ce..8fb7d2e4 100644 --- a/tox.ini +++ b/tox.ini @@ -3,7 +3,7 @@ [tox] skipsdist=True skip_missing_interpreters = True -envlist = {kfp-api,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{lint,unit,integration},bundle-integration +envlist = {kfp-api,kfp-metadata-writer,kfp-persistence,kfp-profile-controller,kfp-schedwf,kfp-ui,kfp-viewer,kfp-viz}-{lint,unit,integration},bundle-integration-{v1, v2} [vars] tst_path = {toxinidir}/tests/ @@ -12,6 +12,7 @@ tst_path = {toxinidir}/tests/ allowlist_externals = tox setenv = api: CHARM = api + metadata-writer: CHARM = metadata-writer persistence: CHARM = persistence profile-controller: CHARM = profile-controller schedwf: CHARM = schedwf @@ -40,7 +41,14 @@ deps = pip-tools description = Update requirements files by executing pip-compile on all requirements*.in files, including those in subdirs. -[testenv:bundle-integration] -commands = pytest -vv --tb=native -s {posargs} {[vars]tst_path}/integration +[testenv:bundle-integration-v1] +commands = pytest -vv --tb=native -s {posargs} {[vars]tst_path}integration/test_kfp_functional_v1.py deps = -r requirements-integration.txt + kfp>=1.8,<2.0 + +[testenv:bundle-integration-v2] +commands = pytest -vv --tb=native -s {posargs} {[vars]tst_path}integration/test_kfp_functional_v2.py +deps = + -r requirements-integration.txt + kfp>=2.4,<3.0