From 12572cade90d1bde7d19036f230a30c87bfa4213 Mon Sep 17 00:00:00 2001
From: Andrew Scribner <ca.scribner+1@gmail.com>
Date: Fri, 17 Nov 2023 15:56:37 -0500
Subject: [PATCH] feat: add additional service minio-service to kfp-api charm

This adds to kfp-api a service called `minio-service` which points to the related object-storage's s3 service.  This has been added to address a bug in upstream kfp, as explained [here](https://github.com/canonical/minio-operator/pull/151).  This service was originally added to the minio charm in [minio pr 151](https://github.com/canonical/minio-operator/pull/151), but has been refactored so it is added here instead as described in [minio issue 153](https://github.com/canonical/minio-operator/issues/153).
---
 charms/kfp-api/src/charm.py                   | 15 ++++--
 .../src/templates/minio-service.yaml.j2       | 13 +++++
 charms/kfp-api/tests/unit/test_operator.py    | 49 +++++++++++++++++++
 3 files changed, 74 insertions(+), 3 deletions(-)
 create mode 100644 charms/kfp-api/src/templates/minio-service.yaml.j2

diff --git a/charms/kfp-api/src/charm.py b/charms/kfp-api/src/charm.py
index 6de98fc49..0a1510c22 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."
@@ -164,6 +165,9 @@ def _context(self):
             "service": self._name,
             "grpc_port": self._grcp_port,
             "http_port": self._http_port,
+            # Must include .svc.cluster.local for DNS resolution
+            "minio_url": f"{os['service']}.{os['namespace']}.svc.cluster.local",
+            "minio_port": str(os["port"]),
         }
         return context
 
@@ -563,10 +567,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."""
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 000000000..c30b94277
--- /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 83ad8dcab..882b64f78 100644
--- a/charms/kfp-api/tests/unit/test_operator.py
+++ b/charms/kfp-api/tests/unit/test_operator.py
@@ -663,3 +663,52 @@ 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,
+        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)
+        harness.set_model_name("model")
+        harness.begin()
+
+        # 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"]