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

Descheduler strategies as plugins #557

Closed
ingvagabund opened this issue Apr 28, 2021 · 16 comments
Closed

Descheduler strategies as plugins #557

ingvagabund opened this issue Apr 28, 2021 · 16 comments
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.

Comments

@ingvagabund
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The number of strategies is increasing and we still have code of all the strategies under the same directory. Which increases risk of identifier name collisions. It’s also currently impractical to add custom strategies into the Descheduler (in case someone decides to fork the code and integrate e.g. proprietary strategies.).

Evicting pods outside the strategies will free us from passing pod evictor into each strategy. It will also allow us to run the Descheduler as a simulator more easily. E.g. by checking which strategies are evicting the same pods, how one strategy impacts other, etc.

Describe the solution you'd like

Create a factory like initialization of plugins and a new strategy interface with Deschedule method

Describe alternatives you've considered
NONE

What version of descheduler are you using?

Any version

Additional context

Google doc for open discussion: https://docs.google.com/document/d/1VuqbwakL69H4sbsGP8J6C10x1NfEpeAQY_cHibrZY0M/edit#

@ingvagabund ingvagabund added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 28, 2021
@damemi
Copy link
Contributor

damemi commented Apr 28, 2021

+1, standardizing an interface for each strategy and moving them into their own packages is at the very least a big improvement for our code organization. I would go even further to say this is more than a feature, but a considerable redesign (and one well worth having).

I'll leave some comments on the doc, thanks for starting this @ingvagabund !
/kind design

@k8s-ci-robot k8s-ci-robot added the kind/design Categorizes issue or PR as related to design. label Apr 28, 2021
@bytetwin
Copy link
Contributor

This is a great refractor. I would be interested to pick this up as it will give some good understanding of the code base.
Is this in a shape to start? I can take a shot at it and raise doubts here/design doc or via PR as I encounter them

There is another recommendation of converting strategy params to an interface.
Can I attempt both in the same change ?

@damemi
Copy link
Contributor

damemi commented Jun 16, 2021

Can I attempt both in the same change ?

Let's keep different changes to their own PRs. Especially with refactorings like this, if something goes wrong that makes it easier to track down and revert

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 14, 2021
@seanmalloy
Copy link
Member

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 1, 2021
@MadhavJivrajani
Copy link

/remove-kind design
/kind feature
kind/design is migrated to kind/feature, see kubernetes/community#6144 for more details

@k8s-ci-robot k8s-ci-robot removed the kind/design Categorizes issue or PR as related to design. label Oct 11, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 9, 2022
@ingvagabund
Copy link
Contributor Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 10, 2022
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 10, 2022
@ingvagabund
Copy link
Contributor Author

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 10, 2022
@ingvagabund
Copy link
Contributor Author

PoC in #781

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 9, 2022
@damemi
Copy link
Contributor

damemi commented Jul 11, 2022

/remove-lifecycle stale
/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jul 11, 2022
@knelasevero
Copy link
Contributor

@ingvagabund @damemi @a7i should we close this one? Considering the title, strategies are moved to plugins now

@damemi
Copy link
Contributor

damemi commented Aug 18, 2022

I think it's worth keeping open until we've gotten off of the migration wrapper

@ingvagabund
Copy link
Contributor Author

The migration wrapper removed through #1006.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness.
Projects
None yet
Development

No branches or pull requests

8 participants