topologySpreadConstraints: handle `nodeTaintsPolicy` and `nodeAffinityPolicy` constraints

[nitindagar0]

In this PR Incorporated Marc's Changes for node-affinity as well as fixed failing test cases. Below is more information on existing PR

https://github.com/kubernetes-sigs/descheduler/pull/1205

[linux-foundation-easycla[bot]]

The committers listed above are authorized under a signed CLA.

• :white_check_mark: login: MarcPow / name: Marc Power (abd7e887a014f01e6c750723189adbe58901db24, 7ba113a914dfefa2f76eb08ef90c8638483c1e73)
• :white_check_mark: login: nitindagar0 (ba4b275b18ee6a50f6a9d53e71beb73fe9b531c6, 7cfd52b739885ec6aaf6afb44d169f706f90be14, ffcaf878a864ab8a72f3da8f1a0077b997b14110, 2b9f6d3dff8f4e77342ec29d84f05848138f75e4)

[k8s-ci-robot]

Welcome @nitindagar0!

It looks like this is your first PR to kubernetes-sigs/descheduler 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/descheduler has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. :smiley:

[k8s-ci-robot]

Hi @nitindagar0. 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](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.

[k8s-ci-robot]

[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 assign a7i for approval. 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

[nitindagar0]

Hi @a7i & @damemi, Please review this PR. The test cases have been fixed which were failing in existing PR https://github.com/kubernetes-sigs/descheduler/pull/1205.

[nitindagar0]

Also we are not able to access/fork this repo using our microsoft account and Hence we are using our personal account. Please allow our "nitindagar_microsoft" account to get the access.

[damemi]

@nitindagar0 thanks, should we close #1205 if this replaces it? Also please see the comment from that PR about if the new plans for NodeFit spec will fit with this: https://github.com/kubernetes-sigs/descheduler/pull/1205#issuecomment-1648842029

I would also still like to see some test cases that clearly demonstrate the use case for this and shows that the new feature addresses that use case.

Regarding not being able to fork the repo, this is a public repo that anyone should be able to access. We don't grant individual accounts access and can't do that anyway as the repo's access is controlled by kubernetes-sig org policies. I tried to see your nitindagar_microsoft account and it looks like I need Microsoft SSO, so maybe you have some github policies from there preventing that account from forking publicly.

[damemi]

/ok-to-test

[nitindagar0]

/retest

[nitindagar0]

@damemi we have gone through NodeFit spec this looks like a good approach for finding out eligble nodes but right now we wanted to quickly fix the issue and once nodefit spec PR is merged we will plan to use that.

for the other PR, Lets wait for Marc to close that as he is on leave and will be back in couple of days.

we are working on test cases and will add some more tests for this new behaviour.

[nitindagar0]

@damemi , I have added few more test cases to test the new behaviour. Please review

[a7i]

/retest-required

[a7i]

/label tide/merge-method-squash

[a7i]

/retitle topologySpreadConstraints: handle nodeTaintsPolicy and nodeAffinityPolicy constraints

[a7i]

/cc @knelasevero @ingvagabund

[a7i]

@MarcPow @nitindagar0 Would you please add an e2e test for this here? Let me know if you run into any issues with that

[nitindagar0]

@MarcPow @nitindagar0 Would you please add an e2e test for this here? Let me know if you run into any issues with that

Wanted to understand more, What exact scenarios we wanted to add in e2e test cases, we already added scenario specific to nodeaffinity and node selector in tsc_test file

[a7i]

@MarcPow @nitindagar0 Would you please add an e2e test for this here? Let me know if you run into any issues with that

Wanted to understand more, What exact scenarios we wanted to add in e2e test cases, we already added scenario specific to nodeaffinity and node selector in tsc_test file

@nitindagar0 end-to-end tests run against a local Kubernetes cluster and test against real Kubernetes resources. It has several benefits such as:

• ensure that scheduling is fine given input constraints, and descheduler evicts accordingly
• parses policy/profile to ensure that things are validated end-to-end

If you can squash your commits, I can help with adding an extra commit with a simple end-to-end test.

[nitindagar0]

@a7i I was able to squash last four commits only and pushed the changes

[nitindagar0]

Hi @a7i, any update for E2E test case, We squashed the commits last week as you suggested

[a7i]

hey @nitindagar0 I do not have permission to push. Can you enable maintainers to edit?

[nitindagar0]

@a7i Thankyou for adding e2e test case in the PR. Also I have checked "allow maintainers to edit" on this PR. I have also added you as collaborator in the forked repo "deschedulerUpdate"

[nitindagar0]

Since you have created a separate PR. Should this PR be closed ?

[a7i]

You can either cherry-pick my commit or we can close this PR

[nitindagar0]

Closing this PR as required changes are available in following PR 1218