Remove redundant checks for taints and nodeSelectors from strategies.

[RyanDevlin]

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

Currently many features implement their own checks to determine if a pod will end up on the same node after eviction. Some strategies check for taints, others check for nodeSelectors, others check both, and some don't check for this edge case. With the addition of the nodeFit feature, checks for taints, nodeSelectors, and whether a node is unschedulable can be turned on and off for almost all strategies. The nodeFit feature allows for a consistent approach to this optimization so that all strategies are in line with one another.

Describe the solution you'd like

Ideally, every strategy should explicitly not check for taints and/or nodeSelectors. If the consideration of these parameters is desired, one could enable the nodeFit strategy parameter and achieve the same optimization.

Describe alternatives you've considered

None.

What version of descheduler are you using?
Any version.

Additional context

See the discussion in #559 for more detailed information regarding exactly where edits should be made to the codebase.

[RyanDevlin]

@damemi @ingvagabund I remember we had a conversation about waiting for 1-2 releases before enabling Node Fit filtering by default and removing the redundant checks in the strategies. Is this something we can now implement, or should we wait for a few more releases?

[damemi]

I think we should put a release between when we first added it and when we enable it by default, to give enough soak time for users. Does that sound good?

[RyanDevlin]

Okay I think that's fair. So essentially once the next release occurs, work can begin on this feature?

[damemi]

Okay I think that's fair. So essentially once the next release occurs, work can begin on this feature?

Yup, that sounds good to me

[RyanDevlin]

@damemi I'm going to open up a PR to fix #604 by the end of this week. In the work that was done to implement #604, I ended up consolidating some of the pod filtering logic. Now that it's consolidated, I see a clear path to fixing the issue in this thread as well.

Do you think I should do these in two separate PRs, or should I follow through with aligning all the strategies in my current work?

[damemi]

@RyanDevlin I think 2 separate PRs is fine. Refactoring work can always be a follow up

[RyanDevlin]

@damemi I didn't want to go against what you said here, but I ended up putting it in one PR. After reviewing the strategies I realized that my work for #604 completed everything but the test cases needed for this issue. Thus, in order to make them two PR's I would have had to undo a bunch of work, and then redo it for the second PR.

So instead I just added the missing test cases and submitted them as one. I hope that's okay, let me know if you'd still like me to break this into two PRs.

[k8s-triage-robot]

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-triage-robot]

The Kubernetes project currently lacks enough active 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 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 rotten

[ingvagabund]

/remove-lifecycle rotten

[k8s-triage-robot]

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

[ingvagabund]

/remove-lifecycle stale

[ingvagabund]

Fixed in #790
/close

[k8s-ci-robot]

@ingvagabund: Closing this issue.

In response to this:

Fixed in #790
/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.