Skip to content

add envFrom #192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 11, 2025
Merged

add envFrom #192

merged 1 commit into from
Jul 11, 2025

Conversation

garrett361
Copy link
Contributor

Issue link

What changes have been made

Adds an optional envFrom field, making it easier to read in environment variables from secret files.

Example:

  1. Create a secrets.yaml:
apiVersion: v1
kind: Secret
metadata:
  name: my-secrets
  namespace: my-namespace
type: Opaque
stringData:
  SECRET_ENV_0: my_api_key
  SECRET_ENV_1: another_api_key
  1. oc create -f secrets.yaml to create the open shift secrets
  2. Add an envFrom section to your job yaml:
jobName: my-job

[...]

envFrom:
 - secretRef:
    name: my-secrets
  1. Launch a job: both SECRET_ENV_{0,1} will be set in all pods.

Verification steps

Tested by running through the above workflow one one- and two-node jobs, verifying that the env vars are set in all containers.

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • Testing is not required for this change

@garrett361
Copy link
Contributor Author

I don't really understand why pre-commit run --all-files wants to entirely remove my description of envFrom from the README. I've iterated a few times, but it is always removed. Could use some help there.

@dgrove-oss
Copy link
Collaborator

Thanks for the addition. I agree this is a useful shortcut if you want to load all the entries in a Secret or ConfigMap into the Pod's environment (the current support via enviornmentVariables forces you to list each entry in the Secret/ConfigMap).

The README.md file gets generated from the comments in values.yaml. So you'll need to add something similar to how environmentVariables is documented.

We've had enough problems with users defining both topologyFileConfigMap and a NCCL_TOPO_FILE environment variable that we added an explicit check for that here. We can't do that check if we're bulk loading an entire ConfigMap or Secret. This is fine, but it might be nice to include in the documentation that the user has to be sure they aren't making this mistake if they use envFrom.

Signed-off-by: Garrett Goon <goon@ibm.com>
@garrett361
Copy link
Contributor Author

Updated @dgrove-oss

Copy link
Collaborator

@dgrove-oss dgrove-oss left a comment

Choose a reason for hiding this comment

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

lgtm; thanks!

@dgrove-oss dgrove-oss merged commit dc49b0b into project-codeflare:main Jul 11, 2025
1 check passed
@garrett361 garrett361 mentioned this pull request Jul 16, 2025
4 tasks
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.

2 participants