Skip to content

feat(trainer): add envFrom option for kubernetes backend#477

Open
Roxxas96 wants to merge 2 commits into
kubeflow:mainfrom
Roxxas96:feat/trainer-k8s-options-env-from
Open

feat(trainer): add envFrom option for kubernetes backend#477
Roxxas96 wants to merge 2 commits into
kubeflow:mainfrom
Roxxas96:feat/trainer-k8s-options-env-from

Conversation

@Roxxas96
Copy link
Copy Markdown

What this PR does:

This PR adds a TrainerEnfFrom option for the kubernetes backend that allows users to specify environment variables sourced from ConfigMaps or Secrets. The added variables will be concatenated with the ones added in trainer.env.

Why we need it

As discussed in the slack thread, we need this feature whenever we have to deal with secrets inside the node container.

One example could be using model_registry s3 upload to log the model after a train, you would need the access key id & secret key available as env var inside the training pod's node container.

Opinionated changes

Compared to the proposition in the slack thread, I did some opinionated changes:

  1. I renamed TrainerEnv option class to TrainerEnvFrom as it is only dealing with valueFrom environment variables, classic value/name variables are set by the trainer.env property (therefore also renamed from_secret() -> secret() & from_config_map() -> config_map())
  2. I decided to concat the values defined in trainer.env with the env defined by the options instead of just overriding it as I think both should coexist.

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

No Issue was created as we agreed that it was not needed.

Checklist:

  • Docs included if any changes are user facing

Copilot AI review requested due to automatic review settings April 30, 2026 18:22
@google-oss-prow
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign electronic-waste for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions
Copy link
Copy Markdown
Contributor

🎉 Welcome to the Kubeflow SDK! 🎉

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-sdk-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! 🙏

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a new Kubernetes-specific trainer option to inject environment variables sourced from Secrets/ConfigMaps into the trainer container, and updates the Kubernetes backend to merge these env overrides with trainer.env.

Changes:

  • Introduce TrainerEnvFrom option (with secret() / config_map() helpers) to append valueFrom env vars to .spec.trainer.env.
  • Update Kubernetes backend spec generation to append option-provided env vars to those produced from trainer.env.
  • Add unit/integration test coverage and expose the option in public imports + docs.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
kubeflow/trainer/options/kubernetes.py Adds TrainerEnvFrom option implementation and validation.
kubeflow/trainer/options/init.py Exposes TrainerEnvFrom via the public options import surface.
kubeflow/trainer/options/kubernetes_test.py Extends option validation/application unit tests for TrainerEnvFrom.
kubeflow/trainer/backends/kubernetes/backend.py Appends env overrides to trainer.env when building the TrainJob spec.
kubeflow/trainer/backends/kubernetes/backend_test.py Adds end-to-end backend tests for env override merging behavior.
docs/source/train/options.rst Documents TrainerEnvFrom in the options reference.

Comment thread kubeflow/trainer/options/kubernetes.py
Comment thread kubeflow/trainer/options/kubernetes.py
This commit adds a TrainerEnfFrom option for the kubernetes backend that allows users to specify environment variables sourced from ConfigMaps or Secrets.
The added variables will be concatenated with the ones added in trainer.env.

Signed-off-by: Alexandre Gomez <gomez.a.corneille@gmail.com>
@Roxxas96 Roxxas96 force-pushed the feat/trainer-k8s-options-env-from branch from e43f893 to 59ccc58 Compare April 30, 2026 18:35
@Roxxas96
Copy link
Copy Markdown
Author

I still have an open question concerning that PR: for now i limit the usage to CustomTrainer and CustomTrainerContainer types but actually this option could be 100% used with the BuiltinTrainer as well.

The thing is, BuiltinTrainer doesn't have the env property which the other trainers have. It feels wrong to enable k8s valueFrom env vars without adding a env property to the BuiltinTrainer.

remove @DataClass from TrainerEnvFrom class

Signed-off-by: Alexandre Gomez <gomez.a.corneille@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants