kube-scheduler-simulator icon indicating copy to clipboard operation
kube-scheduler-simulator copied to clipboard

Published 20 hours ago • verified publisher icon kubernetes-sigs

Changed the design to allow Scenario to be used with more kind of controllers

Open sanposhiho opened this issue 2 years ago • 7 comments

What type of PR is this?

/kind documentation

What this PR does / why we need it:

Propose the way to stop scheduling via ScenarioStatus.SchedulerStatus

Add SchedulerStatus on Status which indicates the scheduler's current status. Both scheduler and the scenario controller will change this field, see more detail in the change of this PR.

By going this way, we don't need to create the scenario controller in simulator; we can create it as an isolated binary. That means the scenario controller can be used without simulator. (Like in minikube, kind or anything else) and Scenario can be used in the outside scheduler as well. (https://github.com/kubernetes-sigs/kube-scheduler-simulator/issues/182)

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

/assign @196Ikuchil /cc @everpeace

/label tide/merge-method-squash

sanposhiho avatar Jul 07 '22 02:07 sanposhiho

shall we add a note to ScenarioPodScheduleResult that AllCandidateNodes,AllFilteredNodes,PluginResult will be all blank if the scheduler is NOT scheduling framework based one?

👍 will do it later.

sanposhiho avatar Jul 11 '22 04:07 sanposhiho

  • add appropriate jsontag, +optional, patchStrategy.
  • add the comment for https://github.com/kubernetes-sigs/kube-scheduler-simulator/pull/194#pullrequestreview-1033752942

sanposhiho avatar Jul 11 '22 10:07 sanposhiho

@196Ikuchil @everpeace Updated in https://github.com/kubernetes-sigs/kube-scheduler-simulator/pull/194/commits/87017e9b121326fbf41b4d22fc73f3c2da3a9e9d.

Please take a look again when have a chance.

  • create StepPhase and use it instead of schedulerStatus.
    • The names are changed, but the purpose behind the field is the same.
  • judge it can no longer schedule any more Pods in scheduler's NextPod.
  • specify the supported scheduler

sanposhiho avatar Jul 16 '22 05:07 sanposhiho

/retitle elaborate more on how to/when stop scheduling

sanposhiho avatar Jul 16 '22 07:07 sanposhiho

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: sanposhiho

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

k8s-ci-robot avatar Jul 19 '22 03:07 k8s-ci-robot

/retitle Changed the design to allow Scenario to be used with more kind of controllers

sanposhiho avatar Jul 31 '22 18:07 sanposhiho

@everpeace @196Ikuchil

Applied a big update.... 🙇 🙏 Please take a look again.

sanposhiho avatar Jul 31 '22 18:07 sanposhiho

@everpeace @196Ikuchil Sorry for toooooo late updating. I think I addressed/replied all your reviews. Please retake a look 🙏

sanposhiho avatar Sep 11 '22 04:09 sanposhiho

@sanposhiho Thanks! LGTM

@everpeace What do you think? I think it is time to start implementing this. And we would probably notice some new things as we begin it.

196Ikuchil avatar Sep 22 '22 10:09 196Ikuchil

@sanposhiho LGTM! Thanks for your very hard work!

What do you think? I think it is time to start implementing this.

@196Ikuchil Yes. I think this PR is now time to merge. Let's proceed. Thanks for your review either!!

everpeace avatar Sep 28 '22 01:09 everpeace

@sanposhiho Thanks for your hard work! /lgtm

196Ikuchil avatar Sep 28 '22 04:09 196Ikuchil