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

Add Namespace filtering to RemoveDuplicates strategy #406

Merged

Conversation

damemi
Copy link
Contributor

@damemi damemi commented Sep 18, 2020

Fixes #405

This also adds a parameters table to each strategy for a quicker reference at what the params are available

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Sep 18, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: damemi

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 18, 2020
@damemi damemi force-pushed the duplicates-namespace-filtering branch from 1200380 to be0cc56 Compare September 18, 2020 16:07
@damemi damemi force-pushed the duplicates-namespace-filtering branch from be0cc56 to b9d73ec Compare September 18, 2020 16:17
Copy link
Contributor

@ingvagabund ingvagabund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for parameter tables

README.md Outdated
@@ -188,8 +206,17 @@ This strategy makes sure that pods violating interpod anti-affinity are removed
if there is podA on a node and podB and podC (running on the same node) have anti-affinity rules which prohibit
them to run on the same node, then podA will be evicted from the node so that podB and podC could run. This
issue could happen, when the anti-affinity rules for podB and podC are created when they are already running on
node. Currently, there are no parameters associated with this strategy. To disable this strategy, the
policy should look like:
node. Currently, there are no parameters associated with this strategy.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, there are no parameters associated with this strategy. not necessary here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To disable this strategy, the policy should look like still applies here unless the example's enabled field is set to true

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

@damemi damemi force-pushed the duplicates-namespace-filtering branch from b9d73ec to 1f02d87 Compare September 21, 2020 15:08
@ingvagabund
Copy link
Contributor

/lgtm
/hold

Holding for others to LGTM. Feel free to unhold.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 21, 2020
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 21, 2020
Copy link
Contributor

@lixiang233 lixiang233 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just one nit in README, otherwise LGTM

README.md Outdated Show resolved Hide resolved
@damemi damemi force-pushed the duplicates-namespace-filtering branch from 1f02d87 to 11b9829 Compare September 22, 2020 14:28
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 22, 2020
@ingvagabund
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 22, 2020
@lixiang233
Copy link
Contributor

lixiang233 commented Sep 23, 2020

/lgtm
/hold cancel

@lixiang233
Copy link
Contributor

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 23, 2020
@k8s-ci-robot k8s-ci-robot merged commit 8c3a80f into kubernetes-sigs:master Sep 23, 2020
@seanmalloy
Copy link
Member

/kind feature

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Sep 26, 2020
briend pushed a commit to briend/descheduler that referenced this pull request Feb 11, 2022
…ce-filtering

Add Namespace filtering to RemoveDuplicates strategy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add namespace filtering to RemoveDuplicates strategy
5 participants