feat: Implement preferredDuringSchedulingIgnoredDuringExecution for RemovePodsViolatingNodeAffinity #1210

jordipiqueselles · 2023-07-30T18:47:37Z

Now, the descheduler can detect and evict pods that are not optimally allocated according to the "preferred..." node affinity. It only evicts a pod if it can be scheduled on a node that scores higher in terms of preferred node affinity than the current one.

This can be activated by enabling the RemovePodsViolatingNodeAffinity plugin and passing "preferredDuringSchedulingIgnoredDuringExecution" in the args.

For example, imagine we have a pod that prefers nodes with label "key1: value1" with a weight of 10. If this pod is scheduled on a node that doesn't have "key1: value1" as label but there's another node that has this label and where this pod can potentially run, then the descheduler will evict the pod.

Another effect of this commit is that the
RemovePodsViolatingNodeAffinity plugin will not remove pods that don't fit in the current node but for other reasons than violating the node affinity. Before that, enabling this plugin could cause evictions on pods that were running on tainted nodes without the necessary tolerations.

This commit also fixes the wording of some tests from node_affinity_test.go and some parameters and expectations of these tests, which were wrong.

…emovePodsViolatingNodeAffinity Now, the descheduler can detect and evict pods that are not optimally allocated according to the "preferred..." node affinity. It only evicts a pod if it can be scheduled on a node that scores higher in terms of preferred node affinity than the current one. This can be activated by enabling the RemovePodsViolatingNodeAffinity plugin and passing "preferredDuringSchedulingIgnoredDuringExecution" in the args. For example, imagine we have a pod that prefers nodes with label "key1: value1" with a weight of 10. If this pod is scheduled on a node that doesn't have "key1: value1" as label but there's another node that has this label and where this pod can potentially run, then the descheduler will evict the pod. Another effect of this commit is that the RemovePodsViolatingNodeAffinity plugin will not remove pods that don't fit in the current node but for other reasons than violating the node affinity. Before that, enabling this plugin could cause evictions on pods that were running on tainted nodes without the necessary tolerations. This commit also fixes the wording of some tests from node_affinity_test.go and some parameters and expectations of these tests, which were wrong.

linux-foundation-easycla · 2023-07-30T18:47:40Z

The committers listed above are authorized under a signed CLA.

✅ login: jordipiqueselles / name: Jordi Piqué Sellés (cd4ab57, edcb323)

k8s-ci-robot · 2023-07-30T18:47:45Z

Hi @jordipiqueselles. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

jordipiqueselles · 2023-08-01T19:24:41Z

@a7i @ingvagabund Could you review this PR? Thanks!

a7i · 2023-08-03T02:03:53Z

/ok-to-test

pkg/framework/plugins/removepodsviolatingnodeaffinity/node_affinity.go

pkg/utils/predicates.go

a7i · 2023-08-03T19:59:28Z

/cc @knelasevero

Before checking if a pod can be evicted or if it can be scheduled somewhere else, we first check if it has the corresponding nodeAffinity field defined. Otherwise, the pod is automatically discarded as a candidate. Apart from that, the method that calculates the weight that a pod gives to a node based on its preferred node affinity has been renamed to better reflect what it does.

a7i · 2023-08-03T20:55:18Z

Thank you @jordipiqueselles this is a great addition!

/lgtm

a7i · 2023-08-03T20:56:05Z

/label tide/merge-method-squash

knelasevero · 2023-08-04T10:06:36Z

Thanks for this!

/approve

k8s-ci-robot · 2023-08-04T10:06:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: knelasevero

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~~ [knelasevero]

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

This PR contains the following updates: | Package | Update | Change | |---|---|---| | [descheduler](https://github.com/kubernetes-sigs/descheduler) | minor | `0.27.1` -> `0.28.0` | --- ### Release Notes <details> <summary>kubernetes-sigs/descheduler (descheduler)</summary> ### [`v0.28.0`](https://github.com/kubernetes-sigs/descheduler/releases/tag/v0.28.0): Descheduler v0.28.0 [Compare Source](kubernetes-sigs/descheduler@v0.27.1...v0.28.0) #### What's Changed - add unit tests for version compatibility check by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1096 - fix plugin arg conversion when using multiple profiles with same plugin by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1143 - Use dl.k8s.io instead of kubernetes-release bucket by [@ratnopamc](https://github.com/ratnopamc) in kubernetes-sigs/descheduler#1145 - update deprecated sets.String to generic sets by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1146 - helm: ability to override command and args. set args inline by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1151 - fix: imagepullsecrets indentation for kind: Deployment by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1150 - docs: supplement missing link by [@mikutas](https://github.com/mikutas) in kubernetes-sigs/descheduler#1159 - bump chart to v0.27.1 by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1160 - Custom labels for ServiceMonitor resource by [@nlamirault](https://github.com/nlamirault) in kubernetes-sigs/descheduler#1147 - pod anti-affinity check among nodes by [@10hin](https://github.com/10hin) in kubernetes-sigs/descheduler#1033 - PodLifeTime: support CrashLoopBackOff container state by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1164 - update pause image from 'kubernetes/pause' to 'registry.k8s.io/pause' by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1166 - TooManyRestart: state filter for CrashLoopBackOff by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1165 - Replace deprecated command with environment file by [@jongwooo](https://github.com/jongwooo) in kubernetes-sigs/descheduler#1173 - use pod informers for listing pods in removepodsviolatingtopologyspreadconstraint and removepodsviolatinginterpodantiaffinity by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1163 - removepodsviolatingtopologyspreadconstraint: implement explicit constraints by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1148 - FakeClientset: bump watch channel size by [@ingvagabund](https://github.com/ingvagabund) in kubernetes-sigs/descheduler#1174 - deschedule/balance order (continuation) by [@ingvagabund](https://github.com/ingvagabund) in kubernetes-sigs/descheduler#1177 - bump log level for processing info by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1141 - fix priority threshold by name alone by [@knelasevero](https://github.com/knelasevero) in kubernetes-sigs/descheduler#1186 - feat: Enable open telemetry tracing by [@harshanarayana](https://github.com/harshanarayana) in kubernetes-sigs/descheduler#1189 - bump to k8s 1.28 beta.0 by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1201 - nodefit: aggregate errors by [@lucming](https://github.com/lucming) in kubernetes-sigs/descheduler#1203 - fix: base configmap missing plugin RemoveDuplicates by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1207 - feat: Implement preferredDuringSchedulingIgnoredDuringExecution for RemovePodsViolatingNodeAffinity by [@jordipiqueselles](https://github.com/jordipiqueselles) in kubernetes-sigs/descheduler#1210 - fix: descheduler_loop_duration_seconds has wrong value by [@Abirdcfly](https://github.com/Abirdcfly) in kubernetes-sigs/descheduler#1215 - .gitattribute to not pollute PRs or stats by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1202 - Bump Kubernetes dependencies to v1.28.0 by [@JaneLiuL](https://github.com/JaneLiuL) in kubernetes-sigs/descheduler#1216 - profile: fix span attribute typo by [@antoinedeschenes](https://github.com/antoinedeschenes) in kubernetes-sigs/descheduler#1221 - k8s 1.28: update docs and go-version by [@a7i](https://github.com/a7i) in kubernetes-sigs/descheduler#1224 #### New Contributors - [@ratnopamc](https://github.com/ratnopamc) made their first contribution in kubernetes-sigs/descheduler#1145 - [@mikutas](https://github.com/mikutas) made their first contribution in kubernetes-sigs/descheduler#1159 - [@nlamirault](https://github.com/nlamirault) made their first contribution in kubernetes-sigs/descheduler#1147 - [@10hin](https://github.com/10hin) made their first contribution in kubernetes-sigs/descheduler#1033 - [@jongwooo](https://github.com/jongwooo) made their first contribution in kubernetes-sigs/descheduler#1173 - [@jordipiqueselles](https://github.com/jordipiqueselles) made their first contribution in kubernetes-sigs/descheduler#1210 - [@Abirdcfly](https://github.com/Abirdcfly) made their first contribution in kubernetes-sigs/descheduler#1215 - [@antoinedeschenes](https://github.com/antoinedeschenes) made their first contribution in kubernetes-sigs/descheduler#1221 **Full Changelog**: kubernetes-sigs/descheduler@v0.27.0...v0.28.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://git.devmem.ru/projects/infra/pulls/949 Co-authored-by: Renovate Bot <renovate@devmem.ru> Co-committed-by: Renovate Bot <renovate@devmem.ru>

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 30, 2023

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 30, 2023

k8s-ci-robot requested review from a7i and ingvagabund July 30, 2023 18:48

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Jul 30, 2023

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 3, 2023

a7i reviewed Aug 3, 2023

View reviewed changes

k8s-ci-robot requested a review from knelasevero August 3, 2023 19:59

jordipiqueselles requested a review from a7i August 3, 2023 20:54

k8s-ci-robot assigned a7i Aug 3, 2023

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 3, 2023

k8s-ci-robot added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Aug 3, 2023

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 4, 2023

k8s-ci-robot merged commit 3170404 into kubernetes-sigs:master Aug 4, 2023

miqm mentioned this pull request Sep 4, 2023

Prefered scheduling #211

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Implement preferredDuringSchedulingIgnoredDuringExecution for RemovePodsViolatingNodeAffinity #1210

feat: Implement preferredDuringSchedulingIgnoredDuringExecution for RemovePodsViolatingNodeAffinity #1210

jordipiqueselles commented Jul 30, 2023

linux-foundation-easycla bot commented Jul 30, 2023 •

edited

Loading

k8s-ci-robot commented Jul 30, 2023

jordipiqueselles commented Aug 1, 2023

a7i commented Aug 3, 2023

a7i commented Aug 3, 2023

a7i commented Aug 3, 2023

a7i commented Aug 3, 2023

knelasevero commented Aug 4, 2023

k8s-ci-robot commented Aug 4, 2023

feat: Implement preferredDuringSchedulingIgnoredDuringExecution for RemovePodsViolatingNodeAffinity #1210

feat: Implement preferredDuringSchedulingIgnoredDuringExecution for RemovePodsViolatingNodeAffinity #1210

Conversation

jordipiqueselles commented Jul 30, 2023

linux-foundation-easycla bot commented Jul 30, 2023 • edited Loading

k8s-ci-robot commented Jul 30, 2023

jordipiqueselles commented Aug 1, 2023

a7i commented Aug 3, 2023

a7i commented Aug 3, 2023

a7i commented Aug 3, 2023

a7i commented Aug 3, 2023

knelasevero commented Aug 4, 2023

k8s-ci-robot commented Aug 4, 2023

linux-foundation-easycla bot commented Jul 30, 2023 •

edited

Loading