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

set annotation automatically when EnableGangScheduling is set to true #1032

Merged
merged 1 commit into from
Jun 24, 2019
Merged

Conversation

ChanYiLin
Copy link
Member

@ChanYiLin ChanYiLin commented Jun 21, 2019

To allow the gang-scheduling with using kube-batch, we need to create podGroup and set the annotation scheduling.k8s.io/group-name: $PODGROUP_NAME to each pod.

When EnableGangScheduling is set to true tf-operator create the podGroup automatically but doesn't set the annotation.
This PR fixes the problem by setting the annotation when create the pods.

fix #1031


This change is Reviewable

@ChanYiLin
Copy link
Member Author

/assign @gaocegege @richardsliu
thanks!

@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.744% when pulling c05002d on ChanYiLin:master into fd76dee on kubeflow:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 76.744% when pulling c05002d on ChanYiLin:master into fd76dee on kubeflow:master.

@johnugeorge
Copy link
Member

Currently, we don't have tests for gang scheduling feature. How are these annotations used when feature is enabled? Is it a must fix?

@gaocegege
Copy link
Member

/cc @k82cn

@k8s-ci-robot k8s-ci-robot requested a review from k82cn June 22, 2019 06:37
@ChanYiLin
Copy link
Member Author

I think it's necessary.
According to gang-scheduling,

kube-batch will watch PodGroup, and the annotation scheduling.k8s.io/group-name identify which group the pod belongs to

Should I open a issue for adding the tests for gang-scheduling?

@johnugeorge
Copy link
Member

I think it's necessary.
According to gang-scheduling,

kube-batch will watch PodGroup, and the annotation scheduling.k8s.io/group-name identify which group the pod belongs to

Should I open a issue for adding the tests for gang-scheduling?

Yes. This means that this feature is broken now ? Can you add tests ?

@wackxu
Copy link
Contributor

wackxu commented Jun 24, 2019

Great, we need this, for now when we enable gang scheduling, we need set the annotation manually and it is not friendly to users.

@richardsliu
Copy link
Contributor

/lgtm
Thanks for working on this. Can you open an issue for adding tests? It can be done in a separate PR.

@johnugeorge
Copy link
Member

@wackxu Thanks for the update.

/approve

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge

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

The pull request process is described 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gang schedule annotation
7 participants