Skip to content

Allow field path env variables #935

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 23, 2024

Conversation

ndacosta
Copy link
Contributor

Summary:

Context

Currently the kubernetes scheduler translate all the env of the role into something like this.

- name: FOO
  value: bar

In some cases we may also need env variables of type:

- name: FOO
  valueFrom:
    fieldRef:
      fieldPath: bar.path

This diff adds the ability to specify env in the role that will be translated to the second option.

Changes

  • Create constant placeholder to insert in the value of env in roles if we want them to be translated to the second option
  • If placeholder is present in the value, convert to code below stripping the placeholder
V1EnvVar(
  name=key,
    value_from=V1EnvVarSource(
      field_ref=V1ObjectFieldSelector(field_path=value)
    ),
)

if not, translate to

V1EnvVar(name=key, value=value)
  • update tests

Differential Revision: D59865500

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 19, 2024
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D59865500

Copy link
Contributor

@Sanjay-Ganeshan Sanjay-Ganeshan left a comment

Choose a reason for hiding this comment

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

LGTM - let's add a comment above the constant value

ndacosta added a commit to ndacosta/torchx that referenced this pull request Jul 19, 2024
Summary:
Pull Request resolved: pytorch#935

# Context
Currently the kubernetes scheduler translate all the env of the role into something like this.
```
- name: FOO
  value: bar
```
In some cases we may also need env variables of type:
```
- name: FOO
  valueFrom:
    fieldRef:
      fieldPath: bar.path
```
This diff adds the ability to specify env in the role that will be translated to the second option.

# Changes
- Create constant placeholder to insert in the value of env in roles if we want them to be translated to the second option
- If placeholder is present in the value, convert to code below stripping the placeholder
```
V1EnvVar(
  name=key,
    value_from=V1EnvVarSource(
      field_ref=V1ObjectFieldSelector(field_path=value)
    ),
)
```
if not, translate to
```
V1EnvVar(name=key, value=value)
```
- update tests

Reviewed By: Sanjay-Ganeshan

Differential Revision: D59865500
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D59865500

ndacosta added a commit to ndacosta/torchx that referenced this pull request Jul 19, 2024
Summary:
Pull Request resolved: pytorch#935

# Context
Currently the kubernetes scheduler translate all the env of the role into something like this.
```
- name: FOO
  value: bar
```
In some cases we may also need env variables of type:
```
- name: FOO
  valueFrom:
    fieldRef:
      fieldPath: bar.path
```
This diff adds the ability to specify env in the role that will be translated to the second option.

# Changes
- Create constant placeholder to insert in the value of env in roles if we want them to be translated to the second option
- If placeholder is present in the value, convert to code below stripping the placeholder
```
V1EnvVar(
  name=key,
    value_from=V1EnvVarSource(
      field_ref=V1ObjectFieldSelector(field_path=value)
    ),
)
```
if not, translate to
```
V1EnvVar(name=key, value=value)
```
- update tests

Reviewed By: Sanjay-Ganeshan

Differential Revision: D59865500
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D59865500

Summary:
Pull Request resolved: pytorch#935

# Context
Currently the kubernetes scheduler translate all the env of the role into something like this.
```
- name: FOO
  value: bar
```
In some cases we may also need env variables of type:
```
- name: FOO
  valueFrom:
    fieldRef:
      fieldPath: bar.path
```
This diff adds the ability to specify env in the role that will be translated to the second option.

# Changes
- Create constant placeholder to insert in the value of env in roles if we want them to be translated to the second option
- If placeholder is present in the value, convert to code below stripping the placeholder
```
V1EnvVar(
  name=key,
    value_from=V1EnvVarSource(
      field_ref=V1ObjectFieldSelector(field_path=value)
    ),
)
```
if not, translate to
```
V1EnvVar(name=key, value=value)
```
- update tests

Reviewed By: Sanjay-Ganeshan

Differential Revision: D59865500
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D59865500

@facebook-github-bot facebook-github-bot merged commit 4081bcc into pytorch:main Jul 23, 2024
23 of 24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants