[WIP] Add MLflow to the Kubeflow Community Distribution - #3582
[WIP] Add MLflow to the Kubeflow Community Distribution#3582kramaranya wants to merge 6 commits into
Conversation
Signed-off-by: kramaranya <kramaranya15@gmail.com>
|
Welcome to the Kubeflow Community Distribution Repository Thanks for opening your first PR. Your contribution means a lot to the Kubeflow community. Before making more PRs: Community Resources:
Thanks again for helping to improve Kubeflow. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
applications/mlflow/README.md:59
- This command names a file that does not exist; the added test is
tests/test_mlflow_manifests.py, so following these synchronization instructions fails immediately.
./tests/mlflow_manifests_test.py
tests/mlflow_install.sh:14
- This check only waits for Deployment availability; it never exercises the newly introduced multi-user authentication and Profile authorization path. The comprehensive integration workflow can therefore pass while identity headers or role mappings make MLflow unusable. Add a gateway request with a Profile-bound identity and a negative cross-Profile authorization assertion.
kubectl wait --for=condition=Available deployment/mlflow \
--namespace kubeflow \
--timeout=300s
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (3)
applications/mlflow/values-kubeflow.yaml:45
- The platform
kubeflow-gatewayaccepts every host, and the integration environment accesses it throughlocalhost:8080, but this VirtualService exposes/mlflowonly for the two example domains. The Dashboard link therefore receives an ingress 404 in the standard port-forward installation before MLflow can apply its own strict host validation. Keepmlflow.allowedHostsrestricted, but allow this route on the platform gateway like the other Kubeflow VirtualServices.
hosts:
- kubeflow.example.com
- "*.kubeflow.example.com"
.github/workflows/full_kubeflow_integration_test.yaml:73
- This integration step verifies only that the Deployment becomes available; it never requests
/mlflowor verifies workspace isolation. Profile discovery, normalized identity headers, SubjectAccessReview authorization, and cross-Profile denial can therefore all be broken while the integration job passes. Add an MLflow verification test using Profile tokens for allowed same-Profile access and denied cross-Profile access, then run it after the gateway port-forward starts.
- name: Install MLflow
run: ./tests/mlflow_install.sh
applications/mlflow/README.md:59
- This command references a file that does not exist; the added manifest test is
tests/test_mlflow_manifests.py. Following the synchronization instructions currently ends with “No such file or directory.”
./tests/mlflow_manifests_test.py
|
CI is failing because |
| and submit Kubernetes SubjectAccessReviews. Callers receive MLflow permissions | ||
| through the standard Kubeflow Profile roles. | ||
|
|
||
| ## Install |
There was a problem hiding this comment.
Install -> Installation
|
@danish9039 please take a look |
There was a problem hiding this comment.
Where is mlflow tested?
There was a problem hiding this comment.
We might need to get this upstream into the dashboard repository.
| - `overlays/kubeflow` adds Profile-aggregated view, edit, and administrator roles. | ||
|
|
||
| The deployment uses the `kubeflow` namespace, the `/mlflow` URL prefix, the | ||
| `kubeflow-gateway` Istio gateway, and the identity headers supplied by the |
There was a problem hiding this comment.
Can I also directly use the service account default-editor token without the headers from the istio request authentication?
There was a problem hiding this comment.
All of this must be tested
| port: 15090 | ||
| from: | ||
| - podSelector: {} | ||
| - namespaceSelector: {} |
There was a problem hiding this comment.
why do you add namespaceSelector: {}
Why not just part of kubeflow profile namespaces similar to the other nwtwork policies ?
| - kubeflow.example.com | ||
| - "*.kubeflow.example.com" |
There was a problem hiding this comment.
This might have to be changed once you add proper tests. Please check how pipelines and other components do it.
There was a problem hiding this comment.
@danish9039 please compare with the other synchronization scripts.
There was a problem hiding this comment.
we also need tests/mlflow_test.sh
| | KServe | applications/kserve/kserve | [v0.20.0](https://github.com/kserve/kserve/tree/v0.20.0) | 600m | 1200Mi | 0GB | | ||
| | Kubeflow Pipelines | applications/pipeline/upstream | [2.17.0](https://github.com/kubeflow/pipelines/tree/2.17.0/manifests/kustomize) | 970m | 3552Mi | 35GB | | ||
| | Kubeflow Hub | applications/hub/upstream | [v0.3.14](https://github.com/kubeflow/hub/tree/v0.3.14/manifests/kustomize) | 510m | 2112Mi | 20GB | | ||
| | MLflow | applications/mlflow/upstream | [d276153](https://github.com/kubeflow/mlflow-integration/tree/d276153b84844c076d92c74519a3c405936220de/charts/mlflow) | Not measured | Not measured | 2GB | |
There was a problem hiding this comment.
please add measurements
There was a problem hiding this comment.
There was a problem hiding this comment.
the filename prefix is wrong. it must start with mlflow_
| MLflow maps workspaces to Kubeflow Profile namespaces and authorizes requests | ||
| through Kubernetes role-based access control. Install the multi-user overlay: | ||
|
|
||
| ```sh | ||
| ./tests/mlflow_install.sh | ||
| ``` | ||
|
|
||
| The example uses SQLite metadata, file-based artifacts, and a 2 GiB persistent | ||
| volume for single-replica evaluation. Configure shared database and artifact | ||
| stores before production use. See | ||
| [`applications/mlflow/README.md`](applications/mlflow/README.md) for the storage, | ||
| authorization, and synchronization details. |
There was a problem hiding this comment.
| MLflow maps workspaces to Kubeflow Profile namespaces and authorizes requests | |
| through Kubernetes role-based access control. Install the multi-user overlay: | |
| ```sh | |
| ./tests/mlflow_install.sh | |
| ``` | |
| The example uses SQLite metadata, file-based artifacts, and a 2 GiB persistent | |
| volume for single-replica evaluation. Configure shared database and artifact | |
| stores before production use. See | |
| [`applications/mlflow/README.md`](applications/mlflow/README.md) for the storage, | |
| authorization, and synchronization details. | |
| ```sh | |
| ./tests/mlflow_install.sh |
|
And first of all thank you for the PR. |
|
Lets make sure we are following the new KDC projects policy for incuding new projects: https://github.com/kubeflow/community/blob/master/committee-distribution/PROJECTS.md |
Signed-off-by: kramaranya <kramaranya15@gmail.com>
|
Please do not merge from master it destroys the commit history. Fix it with a proper rebase to master and fix the conflicts. |
✏️ Summary of Changes
Adds MLflow to the Kubeflow Community Distribution in the
kubeflownamespace./mlflowwith ingress authorization and destination TLS policy.📦 Dependencies
d276153b84844c076d92c74519a3c405936220de.applications/mlflow/values-kubeflow.yaml.🐛 Related Issues
Not applicable.
✅ Contributor Checklist