Add a clusterEviction plugin #3469

Poor12 · 2023-04-26T07:14:07Z

What type of PR is this?
/kind feature

What this PR does / why we need it:
Add a clusterEviction plugin to filter cluster which has been evicted.

Which issue(s) this PR fixes:
Part of #3169

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

`karmada-scheduler`: Introduced `clusterEviction` plugin to skip the clusters that are in the process of eviction.

Poor12 · 2023-04-26T07:15:16Z

/assign @jwcesign

codecov-commenter · 2023-04-26T07:31:53Z

Codecov Report

Merging #3469 (80eab7b) into master (0f84dd7) will decrease coverage by 0.14%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #3469      +/-   ##
==========================================
- Coverage   53.76%   53.62%   -0.14%     
==========================================
  Files         210      210              
  Lines       19123    19182      +59     
==========================================
+ Hits        10281    10287       +6     
- Misses       8290     8343      +53     
  Partials      552      552

Flag	Coverage Δ
unittests	`53.62% <100.00%> (-0.14%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
pkg/util/helper/cluster.go	`100.00% <100.00%> (ø)`

... and 3 files with indirect coverage changes

jwcesign

I think it would be beneficial to include end-to-end testing for this pull request.

jwcesign · 2023-04-26T07:29:10Z

pkg/scheduler/framework/plugins/clusterevicted/cluster_evicted.go

@@ -0,0 +1,42 @@
+package clusterevicted


I suggest that we follow the naming convention of clusteraffinity and name it clustereviction.

jwcesign · 2023-04-26T08:28:17Z

cc @XiShanYongYe-Chang for checking

jwcesign

other lgtm

jwcesign · 2023-04-27T02:00:55Z

test/e2e/failover_test.go

+		var taint corev1.Taint
+		var maxGroups, minGroups, numOfFailedClusters int
+		var policy *policyv1alpha1.PropagationPolicy
+		maxGroups = 1


maxGroups, minGroups, numOfFailedClusters := 1,1,1

/cc @XiShanYongYe-Chang. I follow the former code style. To be honest, I do not know which style is better.

It's just a code style, any writing is OK. Keep the overall consistency will be nice.

chaunceyjiang · 2023-04-27T02:27:19Z

pkg/scheduler/framework/plugins/clustereviction/cluster_eviction.go

+}
+
+// Filter checks if the target cluster is in the GracefulEvictionTasks which means it has been evicted.
+func (p *ClusterEviction) Filter(_ context.Context, bindingSpec *workv1alpha2.ResourceBindingSpec, _ *workv1alpha2.ResourceBindingStatus, cluster *clusterv1alpha1.Cluster) *framework.Result {


Does this plugin only take effect when the Application Failover feature is enabled?

Not only application failover, but also the current cluster failover. See comments: #3456 (comment).

Does this conflict with the TaintToleration plugin?

I don't think there will be any conflict. The plugins will execute sequentially, ensuring that the final result does not include the cluster in eviction tasks, as intended.

Poor12 · 2023-04-27T06:42:24Z

/assign @RainbowMango

RainbowMango · 2023-04-27T12:49:20Z

pkg/scheduler/framework/plugins/clustereviction/cluster_eviction.go

+	Name = "ClusterEviction"
+)
+
+// ClusterEviction is a plugin that checks if the target cluster is in the GracefulEvictionTasks which means it has been evicted.


A cluster in eviction tasks doesn't mean it has been evicted, it means it is in the process of eviction.

RainbowMango · 2023-04-27T12:49:35Z

pkg/scheduler/framework/plugins/clustereviction/cluster_eviction.go

+
+var _ framework.FilterPlugin = &ClusterEviction{}
+
+// New instantiates the APIEnablement plugin.


APIEnablement

Signed-off-by: Poor12 <shentiecheng@huawei.com>

RainbowMango

@jwcesign @chaunceyjiang Do you have any other comments? Can we move forward now?

karmada-bot · 2023-04-28T02:27:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RainbowMango

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

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

jwcesign · 2023-04-28T02:29:22Z

/lgtm

karmada-bot added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 26, 2023

karmada-bot requested review from mrlihanbo and XiShanYongYe-Chang April 26, 2023 07:14

karmada-bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 26, 2023

karmada-bot assigned jwcesign Apr 26, 2023

jwcesign reviewed Apr 26, 2023

View reviewed changes

Poor12 force-pushed the add-filter branch 2 times, most recently from ebd61de to 6c33e76 Compare April 27, 2023 01:38

jwcesign reviewed Apr 27, 2023

View reviewed changes

chaunceyjiang reviewed Apr 27, 2023

View reviewed changes

Poor12 mentioned this pull request Apr 27, 2023

[Umbrella] Implement application failover #3169

Open

11 tasks

Poor12 changed the title ~~Add a clusterEvicted plugin~~ Add a clusterEviction plugin Apr 27, 2023

Poor12 force-pushed the add-filter branch from 6c33e76 to 920c25a Compare April 27, 2023 06:38

karmada-bot assigned RainbowMango Apr 27, 2023

RainbowMango reviewed Apr 27, 2023

View reviewed changes

add a clusterEvicted plugin

80eab7b

Signed-off-by: Poor12 <shentiecheng@huawei.com>

Poor12 force-pushed the add-filter branch from 920c25a to 80eab7b Compare April 28, 2023 01:22

RainbowMango approved these changes Apr 28, 2023

View reviewed changes

karmada-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 28, 2023

karmada-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 28, 2023

karmada-bot merged commit 185c750 into karmada-io:master Apr 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a clusterEviction plugin #3469

Add a clusterEviction plugin #3469

Poor12 commented Apr 26, 2023 •

edited by RainbowMango

Loading

Poor12 commented Apr 26, 2023

codecov-commenter commented Apr 26, 2023 •

edited

Loading

jwcesign left a comment

jwcesign Apr 26, 2023

jwcesign commented Apr 26, 2023

jwcesign left a comment

jwcesign Apr 27, 2023

Poor12 Apr 27, 2023

XiShanYongYe-Chang Apr 27, 2023

chaunceyjiang Apr 27, 2023

Poor12 Apr 27, 2023

chaunceyjiang Apr 27, 2023

jwcesign Apr 27, 2023

chaunceyjiang Apr 27, 2023

Poor12 commented Apr 27, 2023

RainbowMango Apr 27, 2023

RainbowMango Apr 27, 2023

Poor12 Apr 28, 2023

RainbowMango left a comment

karmada-bot commented Apr 28, 2023

jwcesign commented Apr 28, 2023


		var _ framework.FilterPlugin = &ClusterEviction{}

		// New instantiates the APIEnablement plugin.

Add a clusterEviction plugin #3469

Add a clusterEviction plugin #3469

Conversation

Poor12 commented Apr 26, 2023 • edited by RainbowMango Loading

Poor12 commented Apr 26, 2023

codecov-commenter commented Apr 26, 2023 • edited Loading

Codecov Report

jwcesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jwcesign commented Apr 26, 2023

jwcesign left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Poor12 commented Apr 27, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RainbowMango left a comment

Choose a reason for hiding this comment

karmada-bot commented Apr 28, 2023

jwcesign commented Apr 28, 2023

Poor12 commented Apr 26, 2023 •

edited by RainbowMango

Loading

codecov-commenter commented Apr 26, 2023 •

edited

Loading