Adopting coschduling plugin #1724

tenzen-y · 2023-01-14T22:57:40Z

What this PR does / why we need it:
Adopts updated kubeflow/common with PodGroupControl designed for more generic gang-schedulers.

Follow up on #1526.

Reviewers can check the coscheduling feature with the scheduler-plugins in the following steps:

$ kind create cluster --image kindest/node:v1.24.7
$ git clone [email protected]:kubernetes-sigs/scheduler-plugins.git
$ cd scheduler-plugins/manifests/install/charts
$ helm install scheduler-plugins as-a-second-scheduler/
$ kubectl apply -k "github.com/tenzen-y/training-operator/manifests/overlays/standalone?ref=coscheduling"
$ kubectl patch -n kubeflow deployments training-operator -p '{"spec":{"template":{"spec":{"containers":[{"command":["/manager","--gang-scheduler-name=scheduler-plugins"],"image":"ytenzen/training-operator:test","name":"training-operator", "imagePullPolicy": "Always"}]}}}}'
$ cat <<EOF | kubectl apply -f -
apiVersion: "kubeflow.org/v1"
kind: TFJob
metadata:
  name: tfjob-simple
  namespace: kubeflow
spec:
  runPolicy:
    schedulingPolicy:
      minAvailable: 10
  tfReplicaSpecs:
    Worker:
      replicas: 2
      restartPolicy: OnFailure
      template:
        spec:
          schedulerName: scheduler-plugins-scheduler
          containers:
          - name: tensorflow
            image: gcr.io/kubeflow-ci/tf-mnist-with-summaries:1.0
            command:
            - "python"
            - "/var/tf_mnist/mnist_with_summaries.py"
EOF
$ kubectl get pods -n kubeflow
NAME                                 READY   STATUS    RESTARTS   AGE
tfjob-simple-worker-0                0/1     Pending   0          7m37s
tfjob-simple-worker-1                0/1     Pending   0          7m37s
training-operator-67cc5b6584-t7qtl   1/1     Running   0          8m39s
$ kubectl describe pods -n kubeflow tfjob-simple-worker-0 | grep -A10  Events
Events:
  Type     Reason            Age    From                         Message
  ----     ------            ----   ----                         -------
  Warning  FailedScheduling  8m26s  scheduler-plugins-scheduler  0/1 nodes are available: 1 pre-filter pod tfjob-simple-worker-0 cannot find enough sibling pods, current pods number: 1, minMember of group: 10. preemption: 0/1 nodes are available: 1 Preemption is not helpful for scheduling., PodGroup kubeflow/tfjob-simple gets rejected due to Pod tfjob-simple-worker-0 is unschedulable even after PostFilter, not eligible due to failed to read from cycleState
  Warning  FailedScheduling  2m57s  scheduler-plugins-scheduler  0/1 nodes are available: 1 pre-filter pod tfjob-simple-worker-0 cannot find enough sibling pods, current pods number: 2, minMember of group: 10. preemption: 0/1 nodes are available: 1 Preemption is not helpful for scheduling., PodGroup kubeflow/tfjob-simple gets rejected due to Pod tfjob-simple-worker-0 is unschedulable even after PostFilter, not eligible due to failed to read from cycleState

Also, this PR includes HPA changes and upgrading PriorityClass apiVersion.

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

Checklist:

Docs included if any changes are user facing

coveralls · 2023-01-14T23:00:56Z

Pull Request Test Coverage Report for Build 3920653750

16 of 49 (32.65%) changed or added relevant lines in 7 files are covered.
6 unchanged lines in 3 files lost coverage.
Overall coverage increased (+0.2%) to 39.383%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controller.v1/mxnet/mxjob_controller.go	0	3	0.0%
pkg/controller.v1/xgboost/xgboostjob_controller.go	0	3	0.0%
pkg/controller.v1/tensorflow/tfjob_controller.go	4	9	44.44%
pkg/controller.v1/mpi/mpijob_controller.go	6	16	37.5%
pkg/controller.v1/register_controller.go	0	12	0.0%

Files with Coverage Reduction	New Missed Lines	%
pkg/controller.v1/register_controller.go	1	50.0%
pkg/controller.v1/mpi/mpijob_controller.go	2	77.47%
pkg/common/util/scheduler.go	3	0%

Totals
Change from base Build 3894456879:	0.2%
Covered Lines:	2669
Relevant Lines:	6777

💛 - Coveralls

coveralls · 2023-01-14T23:00:56Z

Pull Request Test Coverage Report for Build 3975832997

38 of 164 (23.17%) changed or added relevant lines in 8 files are covered.
6 unchanged lines in 4 files lost coverage.
Overall coverage decreased (-0.09%) to 39.145%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/controller.v1/paddlepaddle/paddlepaddle_controller.go	9	21	42.86%
pkg/controller.v1/pytorch/pytorchjob_controller.go	9	21	42.86%
pkg/controller.v1/register_controller.go	0	12	0.0%
pkg/apis/kubeflow.org/v1/openapi_generated.go	0	15	0.0%
pkg/controller.v1/tensorflow/tfjob_controller.go	9	26	34.62%
pkg/controller.v1/mpi/mpijob_controller.go	11	29	37.93%
pkg/controller.v1/mxnet/mxjob_controller.go	0	20	0.0%
pkg/controller.v1/xgboost/xgboostjob_controller.go	0	20	0.0%

Files with Coverage Reduction	New Missed Lines	%
pkg/controller.v1/mxnet/mxjob_controller.go	1	0%
pkg/controller.v1/register_controller.go	1	50.0%
pkg/controller.v1/xgboost/xgboostjob_controller.go	1	0%
pkg/common/util/scheduler.go	3	0%

Totals
Change from base Build 3975791653:	-0.09%
Covered Lines:	2692
Relevant Lines:	6877

💛 - Coveralls

tenzen-y · 2023-01-15T20:02:08Z

/hold for kubeflow/common#203

tenzen-y · 2023-01-15T20:10:56Z

This PR is ready for review. PTAL.
/assign @johnugeorge @zw0610 @terrytangyuan @gaocegege

go.mod

tenzen-y · 2023-01-19T08:33:39Z

@zw0610 @johnugeorge @terrytangyuan Please take a look. I would like to include PodGroup with the scheduler-plugin feature in this release.

johnugeorge · 2023-01-19T17:42:14Z

@tenzen-y What is the best way to add e2e test for this?

Also, can you update docs in a separate PR? (In kubeflow/website?)

tenzen-y · 2023-01-19T18:38:43Z

What is the best way to add e2e test for this?

Maybe, we could run e2e test by updating integration-tests.yaml like the following:

...
jobs:
  integration-test:
    runs-on: ubuntu-latest
    strategy:
      fail-fast: false
      matrix:
        kubernetes-version: ["v1.23.12", "v1.24.6", "v1.25.2"]
+       gang-scheduler-name: ["none", "scheduler-plugins"]
    steps:
      - name: Checkout
...
      - name: Deploy training operator
        run: |
          ./scripts/gha/setup-training-operator.sh
        env:
          KIND_CLUSTER: training-operator-cluster
          TRAINING_CI_IMAGE: kubeflowtraining/training-operator:test
+         GANG_SCHEDULER_NAME: ${{ matrix.gang-scheduler-name }}
...

Also, can you update docs in a separate PR? (In kubeflow/website?)

Yes, sure.

@johnugeorge

johnugeorge · 2023-01-20T18:25:02Z

@tenzen-y You can use this release https://github.com/kubeflow/common/releases/tag/v0.4.6

Note: You need to take changes for HPA fixes from #1732 as the release contains HPA selector field change as well.

tenzen-y · 2023-01-20T18:37:53Z

@tenzen-y You can use this release https://github.com/kubeflow/common/releases/tag/v0.4.6

Note: You need to take changes for HPA fixes from #1732 as the release contains HPA selector field change as well.

@johnugeorge Thanks for letting me know.
I will upgrade kubeflow/common version after #1732 is merged.

tenzen-y · 2023-01-20T20:27:34Z

@johnugeorge Completed.

johnugeorge · 2023-01-21T10:02:59Z

What is the best way to add e2e test for this?

Maybe, we could run e2e test by updating integration-tests.yaml like the following:

...
jobs:
  integration-test:
    runs-on: ubuntu-latest
    strategy:
      fail-fast: false
      matrix:
        kubernetes-version: ["v1.23.12", "v1.24.6", "v1.25.2"]
+       gang-scheduler-name: ["none", "scheduler-plugins"]
    steps:
      - name: Checkout
...
      - name: Deploy training operator
        run: |
          ./scripts/gha/setup-training-operator.sh
        env:
          KIND_CLUSTER: training-operator-cluster
          TRAINING_CI_IMAGE: kubeflowtraining/training-operator:test
+         GANG_SCHEDULER_NAME: ${{ matrix.gang-scheduler-name }}
...

Also, can you update docs in a separate PR? (In kubeflow/website?)

Yes, sure.

@johnugeorge

Do you plan to add CI tests in this PR or separate one ?

pkg/controller.v1/pytorch/pytorchjob_controller.go

tenzen-y · 2023-01-21T10:57:34Z

What is the best way to add e2e test for this?

Maybe, we could run e2e test by updating integration-tests.yaml like the following:

...
jobs:
  integration-test:
    runs-on: ubuntu-latest
    strategy:
      fail-fast: false
      matrix:
        kubernetes-version: ["v1.23.12", "v1.24.6", "v1.25.2"]
+       gang-scheduler-name: ["none", "scheduler-plugins"]
    steps:
      - name: Checkout
...
      - name: Deploy training operator
        run: |
          ./scripts/gha/setup-training-operator.sh
        env:
          KIND_CLUSTER: training-operator-cluster
          TRAINING_CI_IMAGE: kubeflowtraining/training-operator:test
+         GANG_SCHEDULER_NAME: ${{ matrix.gang-scheduler-name }}
...

Also, can you update docs in a separate PR? (In kubeflow/website?)

Yes, sure.

@johnugeorge

@johnugeorge I will work on E2E on another PR.

Signed-off-by: Yuki Iwai <[email protected]> Co-authored-by: Wang Zhang <[email protected]> Signed-off-by: Yuki Iwai <[email protected]>

Signed-off-by: Yuki Iwai <[email protected]>

…1beta1 PodGroup Signed-off-by: Yuki Iwai <[email protected]>

Signed-off-by: Yuki Iwai <[email protected]>

tenzen-y · 2023-01-21T17:24:10Z

@johnugeorge Rebased.

johnugeorge · 2023-01-21T17:33:19Z

Great work @tenzen-y Thank you for adding this feature

/lgtm
/approve

google-oss-prow · 2023-01-21T17:33:25Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: johnugeorge, tenzen-y

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:

~~OWNERS~~ [johnugeorge]

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

tenzen-y · 2023-01-21T17:36:14Z

Great work @tenzen-y Thank you for adding this feature

/lgtm /approve

@johnugeorge
Thanks for the quick and detailed review!

I'll create a PR to add E2E test for scheduler-plugins by the end of Monday.

tenzen-y · 2023-01-21T17:37:19Z

/hold cancel

google-oss-prow bot added do-not-merge/work-in-progress size/L labels Jan 14, 2023

google-oss-prow bot requested review from jinchihe and kuizhiqing January 14, 2023 22:57

tenzen-y changed the title ~~[WIP] Add scheduler plugin for coscheduling~~ [WIP] Adopting coschduling plugins Jan 14, 2023

tenzen-y mentioned this pull request Jan 14, 2023

Refactor PodGroup control with more generic design kubeflow/common#203

Merged

tenzen-y force-pushed the coscheduling branch 3 times, most recently from 4ae57d6 to f9fa8ac Compare January 15, 2023 19:24

tenzen-y changed the title ~~[WIP] Adopting coschduling plugins~~ Adopting coschduling plugins Jan 15, 2023

google-oss-prow bot removed the do-not-merge/work-in-progress label Jan 15, 2023

tenzen-y force-pushed the coscheduling branch from e5d38bc to 2ae0b6a Compare January 15, 2023 20:01

google-oss-prow bot added the do-not-merge/hold label Jan 15, 2023

google-oss-prow bot assigned gaocegege, johnugeorge, terrytangyuan and zw0610 Jan 15, 2023

google-oss-prow bot added size/XL and removed size/L labels Jan 15, 2023

tenzen-y commented Jan 16, 2023

View reviewed changes

go.mod Outdated Show resolved Hide resolved

tenzen-y force-pushed the coscheduling branch from 57d1b55 to f760c7e Compare January 18, 2023 18:34

tenzen-y force-pushed the coscheduling branch from a4b8b31 to a2a8dc6 Compare January 20, 2023 20:12

johnugeorge mentioned this pull request Jan 21, 2023

WIP: Adding compatibility for API supporting HPA #1732

Closed

johnugeorge reviewed Jan 21, 2023

View reviewed changes

pkg/controller.v1/pytorch/pytorchjob_controller.go Show resolved Hide resolved

tenzen-y mentioned this pull request Jan 21, 2023

[Training] Add the guide on how to use Scheduler Plugins with coscheduling kubeflow/website#3432

Merged

tenzen-y force-pushed the coscheduling branch from a2a8dc6 to 8f52a72 Compare January 21, 2023 17:19

tenzen-y and others added 4 commits January 22, 2023 02:19

Add scheduler plugin for coscheduling

4d2a848

Signed-off-by: Yuki Iwai <[email protected]> Co-authored-by: Wang Zhang <[email protected]> Signed-off-by: Yuki Iwai <[email protected]>

generate codes

6fae993

Signed-off-by: Yuki Iwai <[email protected]>

Add a custom Watcher for scheduler-plugins PodGroup to controllers

ec8165c

Signed-off-by: Yuki Iwai <[email protected]>

Fix predicates of paddlepaddle-controller for scheduling.volcano.sh/v…

580dac8

…1beta1 PodGroup Signed-off-by: Yuki Iwai <[email protected]>

tenzen-y force-pushed the coscheduling branch from 8f52a72 to e379a7f Compare January 21, 2023 17:20

Upgrade kubeflow/common version to v0.4.6 with HPA changes

cc3359e

Signed-off-by: Yuki Iwai <[email protected]>

tenzen-y force-pushed the coscheduling branch from e379a7f to cc3359e Compare January 21, 2023 17:22

google-oss-prow bot added the lgtm label Jan 21, 2023

google-oss-prow bot added the approved label Jan 21, 2023

google-oss-prow bot removed the do-not-merge/hold label Jan 21, 2023

johnugeorge mentioned this pull request Jan 21, 2023

WIP: adopting coschduling plugins with updated kubeflow/common #1526

Closed

tenzen-y changed the title ~~Adopting coschduling plugins~~ Adopting coschduling plugin Jan 21, 2023

google-oss-prow bot merged commit 0126d96 into kubeflow:master Jan 21, 2023

tenzen-y mentioned this pull request Jan 21, 2023

*job API(master) cannot compatible with old job #1725

Closed

tenzen-y deleted the coscheduling branch January 21, 2023 18:44

johnugeorge mentioned this pull request Jan 21, 2023

trainning-operator may need to monitor PodGroup #1574

Closed

tenzen-y mentioned this pull request Jan 22, 2023

Add E2E test for gang-scheduling #1736

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adopting coschduling plugin #1724

Adopting coschduling plugin #1724

tenzen-y commented Jan 14, 2023 •

edited

Loading

coveralls commented Jan 14, 2023

coveralls commented Jan 14, 2023 •

edited

Loading

tenzen-y commented Jan 15, 2023

tenzen-y commented Jan 15, 2023

tenzen-y commented Jan 19, 2023

johnugeorge commented Jan 19, 2023

tenzen-y commented Jan 19, 2023

johnugeorge commented Jan 20, 2023

tenzen-y commented Jan 20, 2023

tenzen-y commented Jan 20, 2023

johnugeorge commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

johnugeorge commented Jan 21, 2023

google-oss-prow bot commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

Adopting coschduling plugin #1724

Adopting coschduling plugin #1724

Conversation

tenzen-y commented Jan 14, 2023 • edited Loading

coveralls commented Jan 14, 2023

Pull Request Test Coverage Report for Build 3920653750

💛 - Coveralls

coveralls commented Jan 14, 2023 • edited Loading

Pull Request Test Coverage Report for Build 3975832997

💛 - Coveralls

tenzen-y commented Jan 15, 2023

tenzen-y commented Jan 15, 2023

tenzen-y commented Jan 19, 2023

johnugeorge commented Jan 19, 2023

tenzen-y commented Jan 19, 2023

johnugeorge commented Jan 20, 2023

tenzen-y commented Jan 20, 2023

tenzen-y commented Jan 20, 2023

johnugeorge commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

johnugeorge commented Jan 21, 2023

google-oss-prow bot commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

tenzen-y commented Jan 21, 2023

tenzen-y commented Jan 14, 2023 •

edited

Loading

coveralls commented Jan 14, 2023 •

edited

Loading