[KFP] Connect KFP to use MLRun CE SeaweedFS Solution#261
Open
shay79il wants to merge 1 commit intomlrun:developmentfrom
Open
[KFP] Connect KFP to use MLRun CE SeaweedFS Solution#261shay79il wants to merge 1 commit intomlrun:developmentfrom
shay79il wants to merge 1 commit intomlrun:developmentfrom
Conversation
shay79il
force-pushed
the
CEML-622-kfp
branch
3 times, most recently
from
0f3d1c2 to
541cd3d
Compare
There was a problem hiding this comment.
Pull request overview
This PR configures Kubeflow Pipelines (KFP) to use the SeaweedFS S3-compatible storage gateway for artifact storage, with KFP 2.x compatibility updates and security hardening for pipeline components.
Changes:
- Configured KFP launcher with SeaweedFS artifact storage using the
minio://URI scheme (KFP's standard S3-compatible protocol) - Added KFP 2.x environment variables for artifact retention and proxy configuration with optional flags
- Hardened metadata-envoy deployment with security context restrictions
- Externalized LOG_LEVEL configuration to ConfigMap for better configurability
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| ct.yaml | New configuration file to disable version increment checks for chart-testing tool |
| charts/mlrun-ce/Chart.yaml | Bumped chart version from 0.11.0-rc9 to 0.11.0-rc10 |
| Makefile | Updated helm-lint target to use ct.yaml configuration |
| charts/mlrun-ce/templates/pipelines/configmaps/pipeline-install-config.yaml | Added LOG_LEVEL configuration with INFO as default |
| charts/mlrun-ce/templates/pipelines/configmaps/kfp-launcher.yaml | Configured defaultPipelineRoot with minio:// scheme pointing to SeaweedFS S3 gateway, includes comprehensive documentation |
| charts/mlrun-ce/templates/pipelines/deployments/ml-pipeline.yaml | Moved LOG_LEVEL to ConfigMap reference, added optional KFP 2.x environment variables (ARTIFACT_RETENTION_DAYS, ARTIFACTS_PROXY_ENABLED), and improved code organization with section comments |
| charts/mlrun-ce/templates/pipelines/deployments/metadata-envoy-deployment.yaml | Added container security context with privilege escalation prevention and capability restrictions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
charts/mlrun-ce/templates/pipelines/deployments/ml-pipeline.yaml
Outdated
Show resolved
Hide resolved
charts/mlrun-ce/templates/pipelines/deployments/metadata-envoy-deployment.yaml
Outdated
Show resolved
Hide resolved
charts/mlrun-ce/templates/pipelines/configmaps/pipeline-install-config.yaml
Outdated
Show resolved
Hide resolved
charts/mlrun-ce/templates/pipelines/deployments/ml-pipeline.yaml
Outdated
Show resolved
Hide resolved
shay79il
force-pushed
the
CEML-622-kfp
branch
4 times, most recently
from
f6c862e to
d3f9cc3
Compare
Configure Kubeflow Pipelines to use SeaweedFS S3 gateway for artifact storage, with KFP 2.x compatibility updates [CEML-622](https://iguazio.atlassian.net/browse/CEML-622)
shay79il
force-pushed
the
CEML-622-kfp
branch
from
d3f9cc3 to
47dcc7f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # It does NOT refer to the MinIO product - it works with ANY S3-compliant storage including SeaweedFS. | ||
| # This is the official KFP convention as per: https://www.kubeflow.org/docs/components/pipelines/operator-guides/configure-object-store/ | ||
| # Format: minio://<bucket>/<path> | ||
| defaultPipelineRoot: "minio://{{ .Values.s3.bucket }}/v2/artifacts" |
There was a problem hiding this comment.
defaultPipelineRoot currently uses .Values.s3.bucket directly, while other pipelines templates use the shared helper {{ include "mlrun-ce.s3.bucket" . }} (e.g., pipeline-install-config.yaml uses it for bucketName). Using the helper here too would keep bucket resolution consistent and avoid future drift if the helper logic changes.
Suggested change
| defaultPipelineRoot: "minio://{{ .Values.s3.bucket }}/v2/artifacts" | |
| defaultPipelineRoot: "minio://{{ include "mlrun-ce.s3.bucket" . }}/v2/artifacts" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Configure Kubeflow Pipelines to use SeaweedFS S3 gateway for artifact storage,
with KFP 2.x compatibility updates and security hardening.
CEML-622