search

kubernetes-sigs / descheduler

Descheduler for Kubernetes
https://sigs.k8s.io/descheduler
Apache License 2.0
4.47k stars 668 forks source link

deschedule/balance order #1162

Closed knelasevero closed 1 year ago

knelasevero commented 1 year ago

Also generalized RunDeschedulerLoop and RunProfiles for when/if we start working on the descheduler/kube-scheduler simulator.

fix #979

knelasevero commented 1 year ago

@a7i @damemi @ingvagabund

knelasevero commented 1 year ago

Both RunDeschedulerLoop and RunProfiles can be turned into methods with an PoC new data type definition for the framework. At the end a user is expected to invoke NewFramework or a similar function that will follow with fmwrk.DescheduleOnce(...) or similar.

I think before that we would need to properly transform Descheduler into a kubernetes controller to stop using wait.NonSlidingUntil and wire the other things around. (In short - I think it should come with another PR/PRs)

ingvagabund commented 1 year ago

I think before that we would need to properly transform Descheduler into a kubernetes controller to stop using wait.NonSlidingUntil and wire the other things around. (In short - I think it should come with another PR/PRs)

Ok. Let's focus on:

knelasevero commented 1 year ago

I think before that we would need to properly transform Descheduler into a kubernetes controller to stop using wait.NonSlidingUntil and wire the other things around. (In short - I think it should come with another PR/PRs)

Ok. Let's focus on:

  • making the new functions internal so we can revisit the parameters later
  • reducing the cost of re-creating a profile for each extension point so it can be later extended with other batch of EPs

Just adding a comment to say I decided to go back and turn the functions into methods with an PoC new data type. I am calling it Descheduler, and we have NewDescheduler to get an instance of it populated with all needed stuff, and RunDeschedulerLoop and RunProfiles as methods under it.

knelasevero commented 1 year ago

/assign @ingvagabund

/cc @damemi @a7i

k8s-ci-robot commented 1 year ago

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Once this PR has been reviewed and has the lgtm label, please ask for approval from ingvagabund. For more information see the Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files: - **[OWNERS](https://github.com/kubernetes-sigs/descheduler/blob/master/OWNERS)** Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment
knelasevero commented 1 year ago

/label tide/merge-method-squash

ingvagabund commented 1 year ago

@knelasevero feel free to cherry-pick from https://github.com/kubernetes-sigs/descheduler/pull/1177 so we can merge the PR. The rest looks good to me.

ingvagabund commented 1 year ago

/test all

k8s-ci-robot commented 1 year ago

@knelasevero: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-descheduler-verify-master cf2d43d19d7459be822108750386a0aad6451927 link true /test pull-descheduler-verify-master

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
ingvagabund commented 1 year ago

Closed in favor of https://github.com/kubernetes-sigs/descheduler/pull/1177. @knelasevero thank you for all the help and work here.