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

HighNodeUtilization README.md is duplication of LowNodeUtilization #591

Closed
chrisjohnson00 opened this issue Jun 23, 2021 · 3 comments
Closed

Comments

@chrisjohnson00
Copy link
Contributor

Much of the HighNodeUtilization documentation in the README.md is a copy/paste from LowNodeUtilization with /s/Low/High replaced. Other than the type change, the documentation is incorrect as it's a copy of Low Node's threshold functionality.

HighNodeUtilization

This strategy finds nodes that are under utilized and evicts pods in the hope that these pods will be scheduled compactly into fewer nodes. This strategy must be used with the scheduler strategy MostRequestedPriority. The parameters of this strategy are configured under nodeResourceUtilizationThresholds.

The above example explains what I mean, where it says it

...finds nodes that are under utilized and evicts pods in the hope that these pods will be...

I don't know the inner workings of descheduler to propose this documentation update, but if other contributors are willing to review my PR with that in mind, I'd be willing to give it a crack at correcting it (LMK)

@damemi
Copy link
Contributor

damemi commented Jun 24, 2021

Hi @chrisjohnson00,
The wording for the 2 strategies is very similar, because under the hood they are essentially the same functions with an invert. However I don't think the section you shared is incorrect:

This strategy finds nodes that are under utilized and evicts pods in the hope that these pods will be scheduled compactly into fewer nodes.

For HighNodeUtilization, this means that it is looking for nodes that are under the utilization thresholds and evicts pods from those nodes to compact them onto other nodes

This strategy finds nodes that are under utilized and evicts pods, if possible, from other nodes in the hope that recreation of evicted pods will be scheduled on these underutilized nodes

This is the matching section from LowNodeUtilization, which means that while it is still looking for nodes that are under the thresholds, it is instead evicting pods from other nodes onto those under utilized nodes.

This could probably be worded better in the docs though, so if you have the time to improve them then please feel free to submit a PR 🙂

@seanmalloy
Copy link
Member

Docs have been updated in #592.

/close

@k8s-ci-robot
Copy link
Contributor

@seanmalloy: Closing this issue.

In response to this:

Docs have been updated in #592.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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

No branches or pull requests

4 participants