diff --git a/.github/workflows/integrate.yaml b/.github/workflows/integrate.yaml index 66566cde..599be728 100644 --- a/.github/workflows/integrate.yaml +++ b/.github/workflows/integrate.yaml @@ -99,8 +99,12 @@ jobs: juju-channel: 3.1/stable charmcraft-channel: latest/candidate - - run: | - sg snap_microk8s -c "tox -e ${{ matrix.charm }}-integration" + - name: Integration tests + run: | + # Requires the model to be called kubeflow due to + # https://github.com/canonical/kfp-operators/issues/389 + juju add-model kubeflow + sg snap_microk8s -c "tox -e ${{ matrix.charm }}-integration -- --model kubeflow" - name: Collect charm debug artifacts uses: canonical/kubeflow-ci/actions/dump-charm-debug-artifacts@main @@ -110,6 +114,7 @@ jobs: name: Test the bundle runs-on: ubuntu-20.04 strategy: + fail-fast: false matrix: sdk: - v1 diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py index 3d8002d8..9d2ec7f6 100755 --- a/charms/kfp-api/src/charm.py +++ b/charms/kfp-api/src/charm.py @@ -45,6 +45,7 @@ K8S_RESOURCE_FILES = [ "src/templates/auth_manifests.yaml.j2", "src/templates/ml-pipeline-service.yaml.j2", + "src/templates/minio-service.yaml.j2", ] MYSQL_WARNING = "Relation mysql is deprecated." UNBLOCK_MESSAGE = "Remove deprecated mysql relation to unblock." @@ -77,14 +78,6 @@ def __init__(self, *args): self._database_name = "mlpipeline" self._container = self.unit.get_container(self._container_name) - # setup context to be used for updating K8S resources - self._context = { - "app_name": self._name, - "namespace": self._namespace, - "service": self._name, - "grpc_port": self._grcp_port, - "http_port": self._http_port, - } self._k8s_resource_handler = None grpc_port = ServicePort(int(self._grcp_port), name="grpc-port") @@ -160,6 +153,26 @@ def container(self): """Return container.""" return self._container + @property + def _context(self): + """Return the context used for generating kubernetes resources.""" + interfaces = self._get_interfaces() + object_storage = self._get_object_storage(interfaces) + + minio_url = f"{object_storage['service']}.{object_storage['namespace']}.svc.cluster.local" + + context = { + "app_name": self._name, + "namespace": self._namespace, + "service": self._name, + "grpc_port": self._grcp_port, + "http_port": self._http_port, + # Must include .svc.cluster.local for DNS resolution + "minio_url": minio_url, + "minio_port": str(object_storage["port"]), + } + return context + @property def k8s_resource_handler(self): """Update K8S with K8S resources.""" @@ -281,6 +294,16 @@ def _generate_environment(self) -> dict: return env_vars + def _check_model_name(self): + if self.model.name != "kubeflow": + # Remove when this bug is resolved: + # https://github.com/canonical/kfp-operators/issues/389 + raise ErrorWithStatus( + "kfp-api must be deployed to model named `kubeflow` due to" + " https://github.com/canonical/kfp-operators/issues/389", + BlockedStatus, + ) + def _check_status(self): """Check status of workload and set status accordingly.""" self._check_leader() @@ -556,10 +579,15 @@ def _apply_k8s_resources(self, force_conflicts: bool = False) -> None: raise GenericCharmRuntimeError("K8S resources creation failed") from error self.model.unit.status = MaintenanceStatus("K8S resources created") - def _on_install(self, _): + def _on_install(self, event): """Installation only tasks.""" - # deploy K8S resources to speed up deployment - self._apply_k8s_resources() + try: + # deploy K8S resources early to speed up deployment + self._apply_k8s_resources() + except ErrorWithStatus as err: + self.model.unit.status = err.status + self.logger.error(f"Failed to handle {event} with error: {err}") + return def _on_upgrade(self, _): """Perform upgrade steps.""" @@ -663,6 +691,7 @@ def _on_relational_db_relation_remove(self, event): def _on_event(self, event, force_conflicts: bool = False) -> None: # Set up all relations/fetch required data try: + self._check_model_name() self._check_leader() self._apply_k8s_resources(force_conflicts=force_conflicts) update_layer(self._container_name, self._container, self._kfp_api_layer, self.logger) diff --git a/charms/kfp-api/src/templates/minio-service.yaml.j2 b/charms/kfp-api/src/templates/minio-service.yaml.j2 new file mode 100644 index 00000000..c30b9427 --- /dev/null +++ b/charms/kfp-api/src/templates/minio-service.yaml.j2 @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: Service +metadata: + name: minio-service + namespace: '{{ namespace }}' +spec: + type: ExternalName + externalName: {{ minio_url }} + ports: + - name: minio + protocol: TCP + port: 9000 + targetPort: {{ minio_port }} diff --git a/charms/kfp-api/tests/unit/test_operator.py b/charms/kfp-api/tests/unit/test_operator.py index d5b691f4..4f11df06 100644 --- a/charms/kfp-api/tests/unit/test_operator.py +++ b/charms/kfp-api/tests/unit/test_operator.py @@ -6,6 +6,7 @@ import pytest import yaml +from charmed_kubeflow_chisme.kubernetes import KubernetesResourceHandler from ops.model import ActiveStatus, BlockedStatus, MaintenanceStatus, WaitingStatus from ops.testing import Harness @@ -16,18 +17,16 @@ @pytest.fixture() def mocked_resource_handler(mocker): - """Yields a mocked resource handler.""" + """Yields a mocked resource handler with a mocked Lightkube Client.""" mocked_resource_handler = MagicMock() mocked_resource_handler_factory = mocker.patch("charm.KubernetesResourceHandler") - mocked_resource_handler_factory.return_value = mocked_resource_handler - yield mocked_resource_handler + def return_krh_with_mocked_lightkube(*args, **kwargs): + kwargs["lightkube_client"] = MagicMock() + return KubernetesResourceHandler(*args, **kwargs) -@pytest.fixture() -def mocked_lightkube_client(mocker, mocked_resource_handler): - """Prevents lightkube clients from being created, returning a mock instead.""" - mocked_resource_handler.lightkube_client = MagicMock() - yield mocked_resource_handler.lightkube_client + mocked_resource_handler_factory.side_effect = return_krh_with_mocked_lightkube + yield mocked_resource_handler @pytest.fixture() @@ -45,6 +44,9 @@ def harness() -> Harness: # setup container networking simulation harness.set_can_connect(KFP_API_CONTAINER_NAME, True) + # Set required model name + harness.set_model_name("kubeflow") + return harness @@ -58,6 +60,15 @@ def test_not_leader(self, k8s_resource_handler: MagicMock, harness: Harness): harness.container_pebble_ready(KFP_API_CONTAINER_NAME) assert harness.charm.model.unit.status == WaitingStatus("Waiting for leadership") + @patch("charm.KubernetesServicePatch", lambda x, y: None) + @patch("charm.KfpApiOperator.k8s_resource_handler") + def test_check_model_name_failure(self, k8s_resource_handler: MagicMock, harness: Harness): + """Tests that the charm blocks if model name is not 'kubeflow'.""" + harness.set_model_name("not-kubeflow") + harness.begin_with_initial_hooks() + assert isinstance(harness.charm.model.unit.status, BlockedStatus) + assert harness.charm.model.unit.status.message.startswith("kfp-api must be deployed to") + @pytest.mark.parametrize( "relation_data,expected_returned_data,expected_raises,expected_status", ( @@ -108,7 +119,6 @@ def test_mysql_relation( expected_returned_data, expected_raises, expected_status, - mocked_lightkube_client, harness: Harness, ): harness.set_leader(True) @@ -133,7 +143,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, mocked_lightkube_client, harness: Harness): + def test_mysql_relation_too_many_relations(self, harness: Harness): harness.set_leader(True) harness.begin() harness.container_pebble_ready(KFP_API_CONTAINER_NAME) @@ -151,7 +161,7 @@ def test_mysql_relation_too_many_relations(self, mocked_lightkube_client, harnes ) @patch("charm.KubernetesServicePatch", lambda x, y: None) - def test_kfp_viz_relation_missing(self, mocked_lightkube_client, harness: Harness): + def test_kfp_viz_relation_missing(self, harness: Harness): harness.set_leader(True) harness.begin() harness.container_pebble_ready(KFP_API_CONTAINER_NAME) @@ -310,8 +320,8 @@ def test_relations_that_provide_data( expected_returned_data, expected_raises, expected_status, - mocked_lightkube_client, harness: Harness, + mocked_resource_handler, ): harness.set_leader(True) harness.begin() @@ -343,7 +353,7 @@ def test_install_with_all_inputs_and_pebble( """Test complete installation with all required relations and verify pebble layer.""" harness.set_leader(True) kfpapi_relation_name = "kfp-api" - model_name = "test_model" + model_name = "kubeflow" service_port = "8888" harness.set_model_name(model_name) harness.update_config({"http-port": service_port}) @@ -477,7 +487,7 @@ def test_install_with_all_inputs_and_pebble( test_env = pebble_plan_info["services"][KFP_API_SERVICE_NAME]["environment"] assert test_env == expected_env - assert "test_model" == test_env["POD_NAMESPACE"] + assert model_name == test_env["POD_NAMESPACE"] @patch("charm.KubernetesServicePatch", lambda x, y: None) @patch("charm.KfpApiOperator._apply_k8s_resources") @@ -513,7 +523,6 @@ def _get_relation_db_only_side_effect_func(self, relation): def test_relational_db_relation_no_data( self, mocked_resource_handler, - mocked_lightkube_client, mocked_kubernetes_service_patcher, harness: Harness, ): @@ -536,7 +545,6 @@ def test_relational_db_relation_no_data( def test_relational_db_relation_missing_attributes( self, mocked_resource_handler, - mocked_lightkube_client, mocked_kubernetes_service_patcher, harness: Harness, ): @@ -559,7 +567,6 @@ def test_relational_db_relation_missing_attributes( def test_relational_db_relation_bad_data( self, mocked_resource_handler, - mocked_lightkube_client, mocked_kubernetes_service_patcher, harness: Harness, ): @@ -582,7 +589,6 @@ def test_relational_db_relation_bad_data( def test_relational_db_relation_with_data( self, mocked_resource_handler, - mocked_lightkube_client, mocked_kubernetes_service_patcher, harness: Harness, ): @@ -615,11 +621,11 @@ def test_relational_db_relation_with_data( def test_relational_db_relation_broken( self, mocked_resource_handler, - mocked_lightkube_client, mocked_kubernetes_service_patcher, harness: Harness, ): """Test that a relation broken event is properly handled.""" + # Arrange database = MagicMock() fetch_relation_data = MagicMock(side_effect=KeyError()) database.fetch_relation_data = fetch_relation_data @@ -631,7 +637,22 @@ def test_relational_db_relation_broken( rel_id = harness.add_relation(rel_name, "relational-db-provider") harness.begin() + + # Mock the object storage data access to keep it from blocking the charm + # Cannot mock by adding a relation to the harness because harness.model.get_relation is + # mocked above to be specific to the db relation. This test's mocks could use a refactor. + objectstorage_data = { + "access-key": "access-key", + "namespace": "namespace", + "port": 1234, + "secret-key": "secret-key", + "secure": True, + "service": "service", + } + harness.charm._get_object_storage = MagicMock(return_value=objectstorage_data) harness.set_leader(True) + + # Act and Assert harness.container_pebble_ready(KFP_API_CONTAINER_NAME) assert harness.model.unit.status == WaitingStatus("Waiting for relational-db data") @@ -647,3 +668,63 @@ def test_relational_db_relation_broken( harness.charm.on.remove.emit() assert harness.model.unit.status == MaintenanceStatus("K8S resources removed") + + def test_minio_service_rendered_as_expected( + self, + mocker, + mocked_kubernetes_service_patcher, + harness: Harness, + ): + # Arrange + + # object storage relation + 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(objectstorage_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 + ) + + harness.set_leader(True) + model_name = "kubeflow" + harness.set_model_name(model_name) + harness.begin() + + # Mock the KubernetesResourceHandler to always have a mocked Lightkube Client + mocked_resource_handler_factory = mocker.patch("charm.KubernetesResourceHandler") + + def return_krh_with_mocked_lightkube(*args, **kwargs): + kwargs["lightkube_client"] = MagicMock() + return KubernetesResourceHandler(*args, **kwargs) + + mocked_resource_handler_factory.side_effect = return_krh_with_mocked_lightkube + + # Act + krh = harness.charm.k8s_resource_handler + manifests = krh.render_manifests() + + # Assert that manifests include a Service(name='minio-service'), and that it has the + # expected configuration data from object-storage + minio_service = next( + (m for m in manifests if m.kind == "Service" and m.metadata.name == "minio-service"), + None, + ) + assert minio_service.metadata.namespace == harness.charm.model.name + assert ( + minio_service.spec.externalName + == f"{objectstorage_data['service']}.{objectstorage_data['namespace']}" + f".svc.cluster.local" + ) + assert len(minio_service.spec.ports) == 1 + assert minio_service.spec.ports[0].targetPort == objectstorage_data["port"]