diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index c6cee518..dce22d9e 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -70,6 +70,7 @@ jobs: fail-fast: false matrix: charm: + - kfp-persistence - kfp-profile-controller - kfp-api steps: diff --git a/charms/kfp-api/tests/integration/test_charm.py b/charms/kfp-api/tests/integration/test_charm.py index 5d330bf5..8a6d5dbd 100644 --- a/charms/kfp-api/tests/integration/test_charm.py +++ b/charms/kfp-api/tests/integration/test_charm.py @@ -87,7 +87,7 @@ async def test_relational_db_relation_with_mysql_relation(self, ops_test: OpsTes apps=["mysql-k8s"], status="active", raise_on_blocked=True, - timeout=90 * 20, + timeout=90 * 30, idle_period=20, ) diff --git a/charms/kfp-persistence/charmcraft.yaml b/charms/kfp-persistence/charmcraft.yaml index 24d0fec5..7b561513 100644 --- a/charms/kfp-persistence/charmcraft.yaml +++ b/charms/kfp-persistence/charmcraft.yaml @@ -10,4 +10,6 @@ bases: channel: "20.04" parts: charm: + build-packages: + - git # To install packages directly from github. Remove when not needed charm-python-packages: [setuptools, pip] # Fixes install of some packages diff --git a/charms/kfp-persistence/metadata.yaml b/charms/kfp-persistence/metadata.yaml index 88c2cded..c5d4387e 100755 --- a/charms/kfp-persistence/metadata.yaml +++ b/charms/kfp-persistence/metadata.yaml @@ -3,11 +3,9 @@ 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. -min-juju-version: "2.9.0" -series: [kubernetes] -deployment: - type: stateless - service: omit +containers: + persistenceagent: + resource: oci-image resources: oci-image: type: oci-image diff --git a/charms/kfp-persistence/requirements-integration.txt b/charms/kfp-persistence/requirements-integration.txt index 1c89298a..74934af4 100644 --- a/charms/kfp-persistence/requirements-integration.txt +++ b/charms/kfp-persistence/requirements-integration.txt @@ -32,7 +32,7 @@ cffi==1.15.1 # pynacl charset-normalizer==3.2.0 # via requests -cryptography==41.0.2 +cryptography==41.0.3 # via # paramiko # pyopenssl @@ -178,7 +178,7 @@ requests-oauthlib==1.3.1 # via kubernetes rsa==4.9 # via google-auth -selenium==4.10.0 +selenium==4.11.2 # via # -r ./requirements-integration.in # selenium-wire diff --git a/charms/kfp-persistence/requirements-unit.in b/charms/kfp-persistence/requirements-unit.in index 84024f88..421af666 100644 --- a/charms/kfp-persistence/requirements-unit.in +++ b/charms/kfp-persistence/requirements-unit.in @@ -14,7 +14,6 @@ # * Pinning a version of a python package/lib shared with requirements.in # must not introduce any incompatibilities. coverage -oci_image ops pytest pytest-mock diff --git a/charms/kfp-persistence/requirements-unit.txt b/charms/kfp-persistence/requirements-unit.txt index b9bcf966..b584d0a1 100644 --- a/charms/kfp-persistence/requirements-unit.txt +++ b/charms/kfp-persistence/requirements-unit.txt @@ -4,33 +4,60 @@ # # pip-compile ./requirements-unit.in # +anyio==3.7.1 + # via httpcore attrs==23.1.0 # via jsonschema certifi==2023.7.22 - # via requests + # via + # httpcore + # httpx + # requests +charmed-kubeflow-chisme==0.2.0 + # via -r ./requirements.in charset-normalizer==3.2.0 # via requests coverage==7.2.7 # via -r ./requirements-unit.in +deepdiff==6.2.1 + # via charmed-kubeflow-chisme exceptiongroup==1.1.2 - # via pytest + # via + # anyio + # pytest +h11==0.14.0 + # via httpcore +httpcore==0.17.3 + # via httpx +httpx==0.24.1 + # via lightkube idna==3.4 - # via requests + # via + # anyio + # httpx + # requests importlib-resources==6.0.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 -oci-image==1.0.0 - # via - # -r ./requirements-unit.in - # -r ./requirements.in -ops==2.4.1 +lightkube==0.14.0 + # via charmed-kubeflow-chisme +lightkube-models==1.27.1.4 + # via lightkube +markupsafe==2.1.3 + # via jinja2 +ops==2.5.0 # via # -r ./requirements-unit.in # -r ./requirements.in + # charmed-kubeflow-chisme # serialized-data-interface +ordered-set==4.1.0 + # via deepdiff packaging==23.1 # via pytest pkgutil-resolve-name==1.3.10 @@ -51,12 +78,26 @@ pytest-mock==3.11.1 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.32 + # via charmed-kubeflow-chisme +ruamel-yaml-clib==0.2.7 + # via ruamel-yaml serialized-data-interface==0.7.0 - # via -r ./requirements.in + # via + # -r ./requirements.in + # charmed-kubeflow-chisme +sniffio==1.3.0 + # via + # anyio + # httpcore + # httpx +tenacity==8.2.2 + # via charmed-kubeflow-chisme tomli==2.0.1 # via pytest urllib3==2.0.4 diff --git a/charms/kfp-persistence/requirements.in b/charms/kfp-persistence/requirements.in index 8cf0f9b1..9569fd46 100644 --- a/charms/kfp-persistence/requirements.in +++ b/charms/kfp-persistence/requirements.in @@ -1,3 +1,3 @@ +charmed-kubeflow-chisme ops -oci-image serialized-data-interface diff --git a/charms/kfp-persistence/requirements.txt b/charms/kfp-persistence/requirements.txt index be26a022..e1f2bace 100644 --- a/charms/kfp-persistence/requirements.txt +++ b/charms/kfp-persistence/requirements.txt @@ -4,36 +4,79 @@ # # pip-compile ./requirements.in # +anyio==3.7.1 + # via httpcore attrs==23.1.0 # via jsonschema certifi==2023.7.22 - # via requests + # via + # httpcore + # httpx + # requests +charmed-kubeflow-chisme==0.2.0 + # via -r ./requirements.in charset-normalizer==3.2.0 # via requests +deepdiff==6.2.1 + # via charmed-kubeflow-chisme +exceptiongroup==1.1.2 + # via anyio +h11==0.14.0 + # via httpcore +httpcore==0.17.3 + # via httpx +httpx==0.24.1 + # via lightkube idna==3.4 - # via requests + # via + # anyio + # httpx + # requests importlib-resources==6.0.0 # via jsonschema +jinja2==3.1.2 + # via charmed-kubeflow-chisme jsonschema==4.17.3 # via serialized-data-interface -oci-image==1.0.0 - # via -r ./requirements.in -ops==2.4.1 +lightkube==0.14.0 + # via charmed-kubeflow-chisme +lightkube-models==1.27.1.4 + # via lightkube +markupsafe==2.1.3 + # via jinja2 +ops==2.5.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.32 + # via charmed-kubeflow-chisme +ruamel-yaml-clib==0.2.7 + # via ruamel-yaml serialized-data-interface==0.7.0 - # via -r ./requirements.in + # via + # -r ./requirements.in + # charmed-kubeflow-chisme +sniffio==1.3.0 + # via + # anyio + # httpcore + # httpx +tenacity==8.2.2 + # via charmed-kubeflow-chisme urllib3==2.0.4 # via requests websocket-client==1.6.1 diff --git a/charms/kfp-persistence/src/charm.py b/charms/kfp-persistence/src/charm.py index b5c0e6cf..f5608ac3 100755 --- a/charms/kfp-persistence/src/charm.py +++ b/charms/kfp-persistence/src/charm.py @@ -9,191 +9,69 @@ import logging -from jsonschema import ValidationError -from oci_image import OCIImageResource, OCIImageResourceError -from ops.charm import CharmBase -from ops.main import main -from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus -from serialized_data_interface.errors import ( - NoCompatibleVersions, - NoVersionsListed, - RelationDataError, +from charmed_kubeflow_chisme.components.charm_reconciler import CharmReconciler +from charmed_kubeflow_chisme.components.leadership_gate_component import LeadershipGateComponent +from charmed_kubeflow_chisme.components.serialised_data_interface_components import ( + SdiRelationDataReceiverComponent, +) +from ops import CharmBase, main + +from components.pebble_components import ( + PersistenceAgentPebbleService, + PesistenceAgentServiceConfig, ) -from serialized_data_interface.sdi import SerializedDataInterface, get_interfaces log = logging.getLogger() class KfpPersistenceOperator(CharmBase): - """Charm for the data persistence application of Kubeflow Pipelines. - - https://github.com/canonical/kfp-operators/ - """ - - def __init__(self, *args): - super().__init__(*args) - - self.log = logging.getLogger() - self.image = OCIImageResource(self, "oci-image") - - self.framework.observe(self.on.install, self._main) - self.framework.observe(self.on.upgrade_charm, self._main) - self.framework.observe(self.on.config_changed, self._main) - self.framework.observe(self.on.leader_elected, self._main) - self.framework.observe(self.on["kfp-api"].relation_changed, self._main) - - def _main(self, event): - # Set up all relations/fetch required data - try: - self._check_leader() - interfaces = self._get_interfaces() - image_details = self.image.fetch() - kfpapi = self._get_kfpapi(interfaces) - except (CheckFailedError, OCIImageResourceError) as check_failed: - self.model.unit.status = check_failed.status - self.log.info(str(check_failed.status)) - return - - self.model.unit.status = MaintenanceStatus("Setting pod spec") - - self.model.pod.set_spec( - { - "version": 3, - "serviceAccount": { - "roles": [ - { - "global": True, - "rules": [ - { - "apiGroups": ["argoproj.io"], - "resources": ["workflows"], - "verbs": ["get", "list", "watch"], - }, - { - "apiGroups": ["kubeflow.org"], - "resources": ["scheduledworkflows"], - "verbs": ["get", "list", "watch"], - }, - { - "apiGroups": [""], - "resources": ["namespaces"], - "verbs": ["get"], - }, - ], - } - ] + """Charm for the data persistence application of Kubeflow Pipelines.""" + + def __init__(self, *args, **kwargs): + """Initialize charm and setup the container.""" + super().__init__(*args, **kwargs) + + # Charm logic + self.charm_reconciler = CharmReconciler(self) + + # Components + self.leadership_gate = self.charm_reconciler.add( + component=LeadershipGateComponent(charm=self, name="leadership-gate"), depends_on=[] + ) + + self.kfp_api_relation = self.charm_reconciler.add( + component=SdiRelationDataReceiverComponent( + charm=self, name="relation:kfp-api", relation_name="kfp-api" + ), + depends_on=[self.leadership_gate], + ) + + self.persistenceagent_container = self.charm_reconciler.add( + component=PersistenceAgentPebbleService( + charm=self, + name="container:persistenceagent", + container_name="persistenceagent", + service_name="persistenceagent", + files_to_push=[], + environment={ + "KUBEFLOW_USERID_HEADER": "kubeflow-userid", + "KUBEFLOW_USERID_PREFIX": "", + # Upstream defines this in the configmap persistenceagent-config-* + "MULTIUSER": "true", }, - "containers": [ - { - "name": "ml-pipeline-persistenceagent", - "imageDetails": image_details, - "command": [ - "persistence_agent", - "--logtostderr=true", - "--namespace=", - "--ttlSecondsAfterWorkflowFinish=86400", - "--numWorker=2", - f"--mlPipelineAPIServerName={kfpapi['service-name']}", - ], - "envConfig": { - "KUBEFLOW_USERID_HEADER": "kubeflow-userid", - "KUBEFLOW_USERID_PREFIX": "", - # Upstream defines this in the configmap persistenceagent-config-* - "MULTIUSER": "true", - }, - } - ], - }, + # provide function to pebble with which it can get service configuration from + # relation + inputs_getter=lambda: PesistenceAgentServiceConfig( + KFP_API_SERVICE_NAME=self.kfp_api_relation.component.get_data()[ + "service-name" + ], + NAMESPACE=str(self.model.name), + ), + ), + depends_on=[self.leadership_gate, self.kfp_api_relation], ) - self.model.unit.status = ActiveStatus() - - def _check_leader(self): - if not self.unit.is_leader(): - # We can't do anything useful when not the leader, so do nothing. - raise CheckFailedError("Waiting for leadership", WaitingStatus) - - def _get_interfaces(self): - # Remove this abstraction when SDI adds .status attribute to NoVersionsListed, - # NoCompatibleVersionsListed: - # https://github.com/canonical/serialized-data-interface/issues/26 - try: - interfaces = get_interfaces(self) - except NoVersionsListed as err: - raise CheckFailedError(str(err), WaitingStatus) - except NoCompatibleVersions as err: - raise CheckFailedError(str(err), BlockedStatus) - except RelationDataError as err: - raise CheckFailedError(str(err), BlockedStatus) - return interfaces - - def _get_kfpapi(self, interfaces): - relation_name = "kfp-api" - return self._validate_sdi_interface(interfaces, relation_name) - - def _validate_sdi_interface(self, interfaces: dict, relation_name: str, default_return=None): - """Validates data received from SerializedDataInterface, returning the data if valid. - - Optionally can return a default_return value when no relation is established - - Raises: - CheckFailed(..., Blocked) when no relation established (unless default_return set) - CheckFailed(..., Blocked) if interface is not using SDI - CheckFailed(..., Blocked) if data in interface fails schema check - CheckFailed(..., Waiting) if we have a relation established but no data passed - - Params: - interfaces: - - Returns: - (dict) interface data - """ - # If nothing is related to this relation, return a default value or raise an error - if relation_name not in interfaces or interfaces[relation_name] is None: - if default_return is not None: - return default_return - else: - raise CheckFailedError( - f"Missing required relation for {relation_name}", BlockedStatus - ) - - relations = interfaces[relation_name] - if not isinstance(relations, SerializedDataInterface): - raise CheckFailedError( - f"Unexpected error with {relation_name} relation data - data not as expected", - BlockedStatus, - ) - - # Get and validate data from the relation - try: - # relations is a dict of {(ops.model.Relation, ops.model.Application): data} - unpacked_relation_data = relations.get_data() - except ValidationError as val_error: - # Validation in .get_data() ensures if data is populated, it matches the schema and is - # not incomplete - self.log.exception(val_error) - raise CheckFailedError( - f"Found incomplete/incorrect relation data for {relation_name}. See logs", - BlockedStatus, - ) - - # Check if we have an established relation with no data exchanged - if len(unpacked_relation_data) == 0: - raise CheckFailedError(f"Waiting for {relation_name} relation data", WaitingStatus) - - # Unpack data (we care only about the first element) - data_dict = list(unpacked_relation_data.values())[0] - - # Catch if empty data dict is received (JSONSchema ValidationError above does not raise - # when this happens) - # Remove once addressed in: - # https://github.com/canonical/serialized-data-interface/issues/28 - if len(data_dict) == 0: - raise CheckFailedError( - f"Found incomplete/incorrect relation data for {relation_name}.", - BlockedStatus, - ) - - return data_dict + + self.charm_reconciler.install_default_event_handlers() class CheckFailedError(Exception): diff --git a/charms/kfp-persistence/src/components/pebble_components.py b/charms/kfp-persistence/src/components/pebble_components.py new file mode 100644 index 00000000..a6f9e9e1 --- /dev/null +++ b/charms/kfp-persistence/src/components/pebble_components.py @@ -0,0 +1,94 @@ +import dataclasses +import logging +from typing import Dict + +from charmed_kubeflow_chisme.components.pebble_component import PebbleServiceComponent +from ops import StatusBase, WaitingStatus +from ops.pebble import Layer + +logger = logging.getLogger(__name__) + + +@dataclasses.dataclass +class PesistenceAgentServiceConfig: + """Defines configuration for PersistenceAgent Service.""" + + KFP_API_SERVICE_NAME: str + NAMESPACE: str + + +class PersistenceAgentPebbleService(PebbleServiceComponent): + """Pebble Service for Persistence Agent Container.""" + + def __init__( + self, + *args, + environment: Dict[str, str], + **kwargs, + ): + """Initialize component.""" + super().__init__(*args, **kwargs) + self._environment = environment + + def get_layer(self) -> Layer: + """Pebble configuration layer for persistenceagent. + + This method is required for subclassing PebbleServiceComponent + """ + logger.info(f"{self.name}: create layer") + + # retrieve up-to-date service configuration as setup by charm + try: + service_config: PesistenceAgentServiceConfig = self._inputs_getter() + except Exception as err: + raise ValueError(f"{self.name}: configuration is not provided") from err + + if len(service_config.KFP_API_SERVICE_NAME) == 0 or len(service_config.NAMESPACE) == 0: + logger.info(f"{self.name}: configuration is not valid") + return None + + # setup command with parameters provided in configuration + command = ( + "persistence_agent", + "--logtostderr=true", + f"--namespace={service_config.NAMESPACE}", + "--ttlSecondsAfterWorkflowFinish=86400", + "--numWorker=2", + f"--mlPipelineAPIServerName={service_config.KFP_API_SERVICE_NAME}", + ) + + # generate and return layer + return Layer( + { + "services": { + self.service_name: { + "override": "replace", + "summary": "persistenceagent service", + "command": " ".join(command), + "startup": "enabled", + "environment": self._environment, + } + }, + "checks": { + "persistenceagent-get": { + "override": "replace", + "period": "30s", + "http": {"url": "http://localhost:8080/metrics"}, + } + }, + } + ) + + def get_status(self) -> StatusBase: + """Return status.""" + # validate configuration availability + try: + service_config: PesistenceAgentServiceConfig = self._inputs_getter() + except Exception as err: + return WaitingStatus(f"Configuration is not provided: {err}") + + # validate values + if len(service_config.KFP_API_SERVICE_NAME) == 0 or len(service_config.NAMESPACE) == 0: + return WaitingStatus("Configuration is not valid") + + return super().get_status() diff --git a/charms/kfp-persistence/tests/integration/test_charm.py b/charms/kfp-persistence/tests/integration/test_charm.py new file mode 100644 index 00000000..65cbf40a --- /dev/null +++ b/charms/kfp-persistence/tests/integration/test_charm.py @@ -0,0 +1,77 @@ +# Copyright 2022 Canonical Ltd. +# See LICENSE file for licensing details. + +import logging +from pathlib import Path + +import pytest +import yaml +from pytest_operator.plugin import OpsTest + +logger = logging.getLogger(__name__) + +APP_NAME = "kfp-persistence" +METADATA = yaml.safe_load(Path("./metadata.yaml").read_text()) + +MINIO_CONFIG = {"access-key": "minio", "secret-key": "minio-secret-key"} +KFP_DB_CONFIG = {"database": "mlpipeline"} + + +class TestCharm: + """Integration test charm""" + + @pytest.mark.abort_on_fail + async def test_build_and_deploy(self, ops_test: OpsTest): + """Deploy kfp-persistence with required charms and relations.""" + built_charm_path = await ops_test.build_charm("./") + logger.info(f"Built charm {built_charm_path}") + + image_path = METADATA["resources"]["oci-image"]["upstream-source"] + resources = {"oci-image": image_path} + + await ops_test.model.deploy( + entity_url=built_charm_path, + application_name=APP_NAME, + resources=resources, + trust=True, + ) + + await ops_test.model.deploy( + entity_url="mysql-k8s", + application_name="kfp-db", + config={"profile": "testing"}, + channel="8.0/edge", + trust=True, + ) + + await ops_test.model.deploy( + entity_url="minio", config=MINIO_CONFIG, channel="ckf-1.7/stable", trust=True + ) + await ops_test.model.deploy(entity_url="kfp-viz", channel="2.0/stable", trust=True) + + # deploy kfp-api which needs to be related to this charm + await ops_test.model.deploy(entity_url="kfp-api", channel="2.0/stable", trust=True) + + await ops_test.model.add_relation("kfp-api:relational-db", "kfp-db:database") + await ops_test.model.add_relation("kfp-api:object-storage", "minio:object-storage") + await ops_test.model.add_relation("kfp-api:kfp-viz", "kfp-viz:kfp-viz") + + await ops_test.model.wait_for_idle( + apps=["kfp-api", "kfp-db"], + status="active", + raise_on_blocked=False, + raise_on_error=False, + timeout=90 * 30, + idle_period=30, + ) + + await ops_test.model.add_relation(f"{APP_NAME}:kfp-api", "kfp-api:kfp-api") + + await ops_test.model.wait_for_idle( + apps=[APP_NAME], + status="active", + raise_on_blocked=False, + raise_on_error=False, + timeout=60 * 10, + idle_period=30, + ) diff --git a/charms/kfp-persistence/tests/unit/test_operator.py b/charms/kfp-persistence/tests/unit/test_operator.py index f3b4fed6..1052ce3c 100644 --- a/charms/kfp-persistence/tests/unit/test_operator.py +++ b/charms/kfp-persistence/tests/unit/test_operator.py @@ -1,262 +1,110 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -from contextlib import nullcontext as does_not_raise +from unittest.mock import MagicMock -import ops import pytest -import yaml -from oci_image import MissingResourceError +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 CheckFailedError, KfpPersistenceOperator +from charm import KfpPersistenceOperator -# TODO: Tests missing for dropped/reloaded relations +MOCK_KFP_API_DATA = {"service-name": "service-name", "service-port": "1234"} -ops.testing.SIMULATE_CAN_CONNECT = True - - -def test_not_leader(harness): - harness.begin_with_initial_hooks() - assert harness.charm.model.unit.status == WaitingStatus("Waiting for leadership") +@pytest.fixture +def harness(): + """Initialize Harness instance.""" + harness = Harness(KfpPersistenceOperator) + harness.set_can_connect("persistenceagent", True) + return harness -def test_image_fetch(harness, oci_resource_data): - harness.begin() - with pytest.raises(MissingResourceError): - harness.charm.image.fetch() - harness.add_oci_resource(**oci_resource_data) - with does_not_raise(): - harness.charm.image.fetch() +def test_not_leader( + harness, # noqa F811 +): + """Test not a leader scenario.""" + harness.begin_with_initial_hooks() + assert harness.charm.model.unit.status == WaitingStatus( + "[leadership-gate] Waiting for leadership" + ) RELATION_NAME = "kfp-api" OTHER_APP_NAME = "kfp-api-provider" -@pytest.mark.parametrize( - "relation_data,expected_returned_data,expected_raises,expected_status", - ( - ( - # No relation established. Raises CheckFailedError - None, - None, - pytest.raises(CheckFailedError), - BlockedStatus(f"Missing required relation for {RELATION_NAME}"), - ), - ( - # Relation exists but no versions set yet - {}, - None, - pytest.raises(CheckFailedError), - WaitingStatus( - f"List of versions not found for apps:" - f" {OTHER_APP_NAME}" - ), - ), - ( - # Relation exists with versions, but no data posted yet - {"_supported_versions": "- v1"}, - None, - pytest.raises(CheckFailedError), - WaitingStatus(f"Waiting for {RELATION_NAME} relation data"), - ), - ( - # Relation exists with versions and empty data - {"_supported_versions": "- v1", "data": yaml.dump({})}, - None, - pytest.raises(CheckFailedError), - WaitingStatus("Waiting for kfp-api relation data"), - ), - ( - # Relation exists with versions and invalid (partial) data - { - "_supported_versions": "- v1", - "data": yaml.dump({"service-name": "service-name"}), - }, - None, - pytest.raises(CheckFailedError), - BlockedStatus(f"Failed to validate data on {RELATION_NAME}:0 from {OTHER_APP_NAME}"), - ), - ( - # Relation exists with valid data - { - "_supported_versions": "- v1", - "data": yaml.dump({"service-name": "service-name", "service-port": "1928"}), - }, - {"service-name": "service-name", "service-port": "1928"}, - does_not_raise(), - None, - ), - ), -) -def test_kfp_api_relation( - harness, relation_data, expected_returned_data, expected_raises, expected_status -): - harness.set_leader() +def test_kfp_api_relation_with_data(harness): + """Test that if Leadership is Active, the kfp-api relation operates as expected.""" + # Arrange harness.begin() - relation_name = RELATION_NAME - other_app = OTHER_APP_NAME - other_unit = f"{other_app}/0" + # Mock: + # * leadership_gate_component_item to be active and executed + harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) - if relation_data is not None: - rel_id = harness.add_relation(relation_name, other_app) - harness.add_relation_unit(rel_id, other_unit) - harness.update_relation_data(rel_id, other_app, relation_data) + # Add relation with data. This should trigger a charm reconciliation due to relation-changed. + add_sdi_relation_to_harness(harness, "kfp-api", data=MOCK_KFP_API_DATA) - with expected_raises as partial_relation_data: - interfaces = harness.charm._get_interfaces() - data = harness.charm._get_kfpapi(interfaces) - if expected_status is None: - assert data == expected_returned_data - else: - assert partial_relation_data.value.status == expected_status + # Assert + assert isinstance(harness.charm.kfp_api_relation.status, ActiveStatus) -@pytest.mark.parametrize( - "relation_name,relation_data,expected_returned_data,expected_raises,expected_status", - ( - # kfp-api - ( - # No relation established. Raises CheckFailedError - "kfp-api", - None, - None, - pytest.raises(CheckFailedError), - BlockedStatus("Missing required relation for kfp-api"), - ), - ( - # Relation exists but no versions set yet - "kfp-api", - {}, - None, - pytest.raises(CheckFailedError), - WaitingStatus( - f"List of versions not found for apps:" - " other-app" - ), - ), - ( - # Relation exists with versions, but no data posted yet - "kfp-api", - {"_supported_versions": "- v1"}, - None, - pytest.raises(CheckFailedError), - WaitingStatus("Waiting for kfp-api relation data"), - ), - ( - # Relation exists with versions and empty data - "kfp-api", - {"_supported_versions": "- v1", "data": yaml.dump({})}, - None, - pytest.raises(CheckFailedError), - WaitingStatus("Waiting for kfp-api relation data"), - ), - ( - # Relation exists with versions and invalid (partial) data - "kfp-api", - { - "_supported_versions": "- v1", - "data": yaml.dump({"service-name": "service-name"}), - }, - None, - pytest.raises(CheckFailedError), - BlockedStatus("Failed to validate data on kfp-api:0 from other-app"), - ), - ( - # Relation exists with valid data - "kfp-api", - { - "_supported_versions": "- v1", - "data": yaml.dump( - { - "service-name": "service-name", - "service-port": "1234", - } - ), - }, - { - "service-name": "service-name", - "service-port": "1234", - }, - does_not_raise(), - None, - ), - ), -) -def test_relations_that_provide_data( - harness, - relation_name, - relation_data, - expected_returned_data, - expected_raises, - expected_status, -): - harness.set_leader() +def test_kfp_api_relation_without_data(harness): + """Test that the kfp-api relation goes Blocked if no data is available.""" + # Arrange harness.begin() - other_app = "other-app" - other_unit = f"{other_app}/0" + # Mock: + # * leadership_gate_component_item to be active and executed + harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) - if relation_data is not None: - rel_id = harness.add_relation(relation_name, other_app) - harness.add_relation_unit(rel_id, other_unit) - harness.update_relation_data(rel_id, other_app, relation_data) + # Add relation with data. This should trigger a charm reconciliation due to relation-changed. + add_sdi_relation_to_harness(harness, "kfp-api", data={}) - with expected_raises as partial_relation_data: - interfaces = harness.charm._get_interfaces() - data = harness.charm._validate_sdi_interface(interfaces, relation_name) - if expected_status is None: - assert data == expected_returned_data - else: - assert partial_relation_data.value.status == expected_status + # Assert + assert isinstance(harness.charm.kfp_api_relation.status, BlockedStatus) -def test_install_with_all_inputs(harness, oci_resource_data): - harness.set_leader() - model_name = "test_model" - harness.set_model_name(model_name) - - # Set up required relations - # Future: convert these to fixtures and share with the tests above - harness.add_oci_resource(**oci_resource_data) +def test_kfp_api_relation_without_relation(harness): + """Test that the kfp-api relation goes Blocked if no relation is established.""" + # Arrange + harness.begin() - # kfp-api relation data - os_data = { - "_supported_versions": "- v1", - "data": yaml.dump( - { - "service-name": "some-service.some-namespace", - "service-port": "1928", - } - ), - } - os_rel_id = harness.add_relation("kfp-api", "kfp-api-provider") - harness.add_relation_unit(os_rel_id, "kfp-api-provider/0") - harness.update_relation_data(os_rel_id, "kfp-api-provider", os_data) + # Mock: + # * leadership_gate_component_item to be active and executed + harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus()) - harness.begin_with_initial_hooks() + # Act + harness.charm.on.install.emit() - # confirm that we can serialize the pod spec and that the unit is active - yaml.safe_dump(harness.get_pod_spec()) - assert harness.charm.model.unit.status == ActiveStatus() + # Assert + assert isinstance(harness.charm.kfp_api_relation.status, BlockedStatus) -@pytest.fixture -def harness(): - return Harness(KfpPersistenceOperator) - - -@pytest.fixture -def oci_resource_data(): - return { - "resource_name": "oci-image", - "contents": { - "registrypath": "ci-test", - "username": "", - "password": "", - }, - } +def test_pebble_services_running(harness): + """Test that if the Kubernetes Component is Active, the pebble services successfully start.""" + # Arrange + harness.begin() + harness.set_can_connect("persistenceagent", True) + + # Mock: + # * leadership_gate_component_item to be active and executed + # * kubernetes_resources_component_item to be active and executed + # * 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.kfp_api_relation.component.get_data = MagicMock(return_value=MOCK_KFP_API_DATA) + + # Act + harness.charm.on.install.emit() + + # Assert + container = harness.charm.unit.get_container("persistenceagent") + service = container.get_service("persistenceagent") + assert service.is_running() + + # Assert the environment variables that are set from inputs are correctly applied + environment = container.get_plan().services["persistenceagent"].environment + assert environment["KUBEFLOW_USERID_HEADER"] == "kubeflow-userid" 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 b3284031..a7f52624 100644 --- a/tests/integration/bundles/kfp_1.7_stable_install.yaml.j2 +++ b/tests/integration/bundles/kfp_1.7_stable_install.yaml.j2 @@ -53,6 +53,7 @@ applications: charm: kfp-persistence channel: 2.0/stable scale: 1 + trust: true kfp-schedwf: charm: kfp-schedwf channel: 2.0/stable @@ -79,6 +80,7 @@ applications: charm: {{ kfp_persistence }} resources: {{ kfp_persistence_resources }} scale: 1 + trust: true kfp-schedwf: charm: {{ kfp_schedwf }} resources: {{ kfp_schedwf_resources }} diff --git a/tests/integration/bundles/kfp_latest_edge.yaml.j2 b/tests/integration/bundles/kfp_latest_edge.yaml.j2 index 1c63abae..195c53ba 100644 --- a/tests/integration/bundles/kfp_latest_edge.yaml.j2 +++ b/tests/integration/bundles/kfp_latest_edge.yaml.j2 @@ -8,7 +8,7 @@ applications: {%- 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 } - kfp-persistence: { charm: ch:kfp-persistence, channel: latest/edge, scale: 1 } + kfp-persistence: { charm: ch:kfp-persistence, channel: latest/edge, scale: 1, trust: true } kfp-schedwf: { charm: ch:kfp-schedwf, channel: latest/edge, scale: 1 } kfp-ui: { charm: ch:kfp-ui, channel: latest/edge, scale: 1 } kfp-viewer: { charm: ch:kfp-viewer, channel: latest/edge, scale: 1 } @@ -23,6 +23,7 @@ applications: charm: {{ kfp_persistence }} resources: {{ kfp_persistence_resources }} scale: 1 + trust: true kfp-profile-controller: charm: {{ kfp_profile_controller }} resources: {{ kfp_profile_controller_resources }} diff --git a/tests/integration/helpers/bundle_mgmt.py b/tests/integration/helpers/bundle_mgmt.py index d720f00d..f9785845 100644 --- a/tests/integration/helpers/bundle_mgmt.py +++ b/tests/integration/helpers/bundle_mgmt.py @@ -24,6 +24,7 @@ def render_bundle(ops_test: OpsTest, bundle_path: Path, context: dict, no_build: # Render the bundle and get its path # The pytest-operator will save it in `self.tmp_path / "bundles"` logger.debug(f"Rendering the bundle in {bundle_path} with context {context}") + logger.debug(f"Saving in {ops_test.tmp_path}") rendered_bundle_path = ops_test.render_bundle(bundle_path, context=context) logger.debug(f"Rendered bundle saved in {rendered_bundle_path}") return rendered_bundle_path diff --git a/tests/integration/test_kfp_functional.py b/tests/integration/test_kfp_functional.py index 80c68e6f..f5d73f75 100644 --- a/tests/integration/test_kfp_functional.py +++ b/tests/integration/test_kfp_functional.py @@ -73,10 +73,10 @@ async def test_build_and_deploy(ops_test: OpsTest, request, lightkube_client): status="active", raise_on_blocked=False, # These apps block while waiting for each other to deploy/relate raise_on_error=True, - timeout=1800, + timeout=3600, + idle_period=120, ) - # ---- KFP API Server focused test cases async def test_upload_pipeline(kfp_client): """Upload a pipeline from a YAML file and assert its presence.""" @@ -91,7 +91,6 @@ async def test_upload_pipeline(kfp_client): server_pipeline_id = kfp_client.get_pipeline_id(name="test-upload-pipeline") assert uploaded_pipeline_id == server_pipeline_id - async def test_create_and_monitor_run(kfp_client, create_and_clean_experiment): """Create a run and monitor it to completion.""" # Create a run, save response in variable for easy manipulation