Skip to content

feat: add helm chart for training-operator - #3

Open
vicoooo26 wants to merge 10 commits into
AliyunContainerService:developfrom
vicoooo26:feat/helm-chart
Open

feat: add helm chart for training-operator#3
vicoooo26 wants to merge 10 commits into
AliyunContainerService:developfrom
vicoooo26:feat/helm-chart

Conversation

@vicoooo26

@vicoooo26 vicoooo26 commented Jul 17, 2026

Copy link
Copy Markdown
Collaborator

What this PR does / why we need it:

Add a Helm chart for deploying the training-operator on Kubernetes. The chart includes all necessary CRDs (TFJob, PyTorchJob, JAXJob, MPIJob, PaddleJob, XGBoostJob), RBAC resources, webhook configuration, and configurable values for production deployments.

Which issue(s) this PR fixes (optional, in Fixes #<issue number>, #<issue number>, ... format, will close the issue(s) when PR gets merged):

Checklist:

  • Docs included if any changes are user facing

@github-actions

Copy link
Copy Markdown

🎉 Welcome to the Kubeflow Trainer! 🎉

Thanks for opening your first PR! We're happy to have you as part of our community 🚀

Here's what happens next:

  • If you haven't already, please check out our Contributing Guide for repo-specific guidelines and the Kubeflow Contributor Guide for general community standards.
  • Our team will review your PR soon! cc @kubeflow/kubeflow-trainer-team

Join the community:

Feel free to ask questions in the comments if you need any help or clarification!
Thanks again for contributing to Kubeflow! 🙏

@vicoooo26
vicoooo26 force-pushed the feat/helm-chart branch 2 times, most recently from 62e1608 to d307513 Compare July 20, 2026 11:26

@cheyang cheyang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Early feedback (still a draft). The templates are in good shape — the fail guard on replicas>1 without leader election, the probes, the security context, and values.schema.json (incl. the min/maxUnavailable mutual-exclusion) are all nice; helm lint is clean and helm template renders. A few things stood out, mostly from a first-run user’s perspective:

  • The PR description says “cert-manager integration”, but the chart doesn’t use cert-manager (no Certificate/Issuer, no caBundle / inject annotation). It relies on the operator’s built-in self-signed cert written into an empty Secret and patching the webhook caBundle at runtime (hence the validatingwebhookconfigurations: update grant). Worth correcting the description.
  • The README install commands reference the aliyun-incubator/training-operator repo, but there’s no helm repo add aliyun-incubator <url> step — the documented install won’t work as written.
  • Minor: CRDs under crds/ aren’t upgraded by Helm; a note in the README about applying CRD updates manually would help.

Comment thread charts/training-operator/values.yaml Outdated
Comment thread charts/training-operator/templates/tests/test-connection.yaml Outdated
Comment thread charts/training-operator/Chart.yaml Outdated
@vicoooo26
vicoooo26 changed the base branch from release-1.9 to develop July 24, 2026 05:39
@vicoooo26
vicoooo26 force-pushed the feat/helm-chart branch 3 times, most recently from 44fb34b to 8b06a7e Compare July 27, 2026 03:08
@cheyang
cheyang requested a review from Copilot July 28, 2026 11:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a Helm chart to deploy the Kubeflow Training Operator on Kubernetes (including RBAC, webhook wiring, deployment/service, and chart docs), and updates CI/test configs to keep Python tests stable with newer kubernetes client changes.

Changes:

  • Add a new charts/training-operator Helm chart with templates, values, schema, and README.
  • Adjust Python/CI test configurations (e.g., pin kubernetes<36, tweak JAXJob e2e resource limits).
  • Add Helm chart packaging/support files (e.g., .helmignore, logo, chart metadata).

Reviewed changes

Copilot reviewed 25 out of 32 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
sdk/python/test/e2e/test_e2e_jaxjob.py Increases JAXJob e2e container memory limit.
charts/training-operator/Chart.yaml Adds Helm chart metadata for training-operator.
charts/training-operator/values.yaml Provides default, configurable chart values.
charts/training-operator/values.schema.json Adds JSON schema validation for values.
charts/training-operator/README.md Documents installation, configuration, and values.
charts/training-operator/logo.svg Adds chart icon asset.
charts/training-operator/.helmignore Adds Helm ignore patterns for packaging.
charts/training-operator/templates/_helpers.tpl Adds naming/label helper templates.
charts/training-operator/templates/deployment.yaml Defines operator Deployment, probes, args, volumes, HA behavior.
charts/training-operator/templates/service.yaml Exposes metrics and webhook ports via Service.
charts/training-operator/templates/serviceaccount.yaml Creates ServiceAccount (optionally) for the operator.
charts/training-operator/templates/clusterrole.yaml Defines cluster-scoped RBAC for the operator.
charts/training-operator/templates/clusterrolebinding.yaml Binds ServiceAccount to ClusterRole.
charts/training-operator/templates/role.yaml Defines namespace-scoped RBAC (leases/secrets).
charts/training-operator/templates/rolebinding.yaml Binds ServiceAccount to Role in the release namespace.
charts/training-operator/templates/validatingwebhookconfiguration.yaml Creates ValidatingWebhookConfiguration for Training Operator CRDs.
charts/training-operator/templates/secret-webhook-cert.yaml Creates webhook cert Secret placeholder for cert rotator.
charts/training-operator/templates/configmap.yaml Optionally provides custom PyTorch init container template ConfigMap.
charts/training-operator/templates/pdb.yaml Adds optional PodDisruptionBudget for HA deployments.
charts/training-operator/templates/NOTES.txt Adds post-install guidance notes.
charts/training-operator/templates/tests/test-connection.yaml Adds Helm test pod to check metrics endpoint connectivity.
.pre-commit-config.yaml Excludes Helm templates from generic YAML checks.
.github/workflows/test-python.yaml Pins Python kubernetes<36 for unit-test compatibility.
.github/workflows/test-example-notebooks.yaml Pins kubernetes<36 and fastjsonschema<2.22 for notebook runs.
.github/workflows/integration-tests.yaml Pins kubernetes<36 for integration tests.
.github/workflows/e2e-test-train-api.yaml Pins kubernetes<36 for e2e train API tests.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread charts/training-operator/templates/_helpers.tpl
Comment thread charts/training-operator/templates/tests/test-connection.yaml
Comment thread charts/training-operator/README.md Outdated
Comment thread charts/training-operator/README.md Outdated
@vicoooo26
vicoooo26 marked this pull request as ready for review July 28, 2026 11:32
kubernetes 36.0.0 regenerated the sync client with Pydantic models,
breaking the FakeResponse deserialization hack used in SDK unit tests.
Pin kubernetes<36 in all CI workflows that install the Python SDK
(test-python, integration-tests, e2e-test-train-api, test-example-notebooks)
to avoid impacting SDK release. Add a TODO for future Pydantic work.

Signed-off-by: Vico Chu <vico24826@gmail.com>
fastjsonschema 2.22.0 introduced PEP 604 union type syntax (dict | bool)
in type annotations, which requires Python 3.10+. This breaks papermill
imports on Python 3.9 in the notebook test workflow. Pin to <2.22 until
Python 3.9 support is dropped from the test matrix.

Signed-off-by: Vico Chu <vico24826@gmail.com>
Signed-off-by: Vico Chu <vico24826@gmail.com>
Signed-off-by: Vico Chu <vico24826@gmail.com>
…and update chart meta info

Signed-off-by: Vico Chu <vico24826@gmail.com>
Helm template files contain Go template syntax ({{ }}) which is not
valid YAML. Add charts/training-operator/templates/.* to the pre-commit
exclude pattern so check-yaml skips these files.

Signed-off-by: Vico Chu <vico24826@gmail.com>
Signed-off-by: Vico Chu <vico24826@gmail.com>
Signed-off-by: Vico Chu <vico24826@gmail.com>
@vicoooo26
vicoooo26 force-pushed the feat/helm-chart branch 4 times, most recently from 7ae2021 to 3368de9 Compare July 29, 2026 09:05
…yaml

Signed-off-by: Vico Chu <vico24826@gmail.com>
Signed-off-by: Vico Chu <vico24826@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants