Skip to content
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

add a label key: TaskIndexKey = volcano.sh/task-index #97

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

shuaiyy
Copy link

@shuaiyy shuaiyy commented Jan 18, 2023

the task pod has a label of task spec by TaskSpecKey = "volcano.sh/task-spec",
while task index missed, it's useful for pod lbael selectors to select a task pod by it's spec and index

the issue is here: add label key volcano.sh/task-index #98

@volcano-sh-bot
Copy link
Collaborator

Welcome @shuaiyy!

It looks like this is your first PR to volcano-sh/apis 馃帀.

Thank you, and welcome to Volcano. 😃

@volcano-sh-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign william-wang
You can assign the PR to them by writing /assign @william-wang in a comment when ready.

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

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

Copy link
Member

@hwdef hwdef left a comment

Choose a reason for hiding this comment

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

I don't quite understand this requirement. Do you have a more detailed explanation? Is there any place where this field is used?

@shuaiyy
Copy link
Author

shuaiyy commented Jan 18, 2023

I don't quite understand this requirement. Do you have a more detailed explanation? Is there any place where this field is used?

I have updated the description.

task-index info will be set to task pod's label, it has been set to container env key VC_TASK_INDEX by env plugin, but missed in pod labels.

@Thor-wl
Copy link
Contributor

Thor-wl commented Jan 19, 2023

task-index info will be set to task pod's label, it has been set to container env key VC_TASK_INDEX by env plugin, but missed in pod labels.

Can you provide the original requirement? Yes, this key has been added into the environment variables.

@shuaiyy
Copy link
Author

shuaiyy commented Jan 19, 2023

task-index info will be set to task pod's label, it has been set to container env key VC_TASK_INDEX by env plugin, but missed in pod labels.

Can you provide the original requirement? Yes, this key has been added into the environment variables.

Okay. I'm trying to use volcano to replace kubeflow training-operator in my ml-platform; the training-operator provide job-name|task-spec|task-index label in task pod, while volcano missed task-index.

our ml-platform need these label:

  1. mount env key TASK_INDEX, the key is used by users and it's hard to let them change to VC_TASK_INDEX
  2. create a service with pod selector for a spectific task pod, and then create an ingress; if users need to debug their task by http tools;
  3. watch task pod's change and sync some info to persist db.

in our scenes, the use of task-index label may not common to other volcano users, but I do think the pod label info task-index is common, should be provided no matter how users will use it

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.

4 participants