From dcbf8d2b963dcf4ffd0fd05f8937d74ba8412667 Mon Sep 17 00:00:00 2001
From: RainbowMango <qdurenhongcai@gmail.com>
Date: Wed, 27 Jul 2022 17:21:41 +0800
Subject: [PATCH] Introduce RemoveCluster for ResourceBinding and
 ClusterResourceBinding

Signed-off-by: RainbowMango <qdurenhongcai@gmail.com>
---
 .../work/v1alpha2/binding_types_helper.go     | 19 +++++++
 .../v1alpha2/binding_types_helper_test.go     | 55 ++++++++++++++++++-
 pkg/controllers/cluster/taint_manager.go      |  4 +-
 pkg/util/binding.go                           | 11 ----
 4 files changed, 75 insertions(+), 14 deletions(-)

diff --git a/pkg/apis/work/v1alpha2/binding_types_helper.go b/pkg/apis/work/v1alpha2/binding_types_helper.go
index 2064b78ef..716c88989 100644
--- a/pkg/apis/work/v1alpha2/binding_types_helper.go
+++ b/pkg/apis/work/v1alpha2/binding_types_helper.go
@@ -21,3 +21,22 @@ func (s *ResourceBindingSpec) AssignedReplicasForCluster(targetCluster string) i
 
 	return 0
 }
+
+// RemoveCluster removes specific cluster from the target list.
+// This function no-opts if cluster not exist.
+func (s *ResourceBindingSpec) RemoveCluster(name string) {
+	var i int
+
+	for i = 0; i < len(s.Clusters); i++ {
+		if s.Clusters[i].Name == name {
+			break
+		}
+	}
+
+	// not found, do nothing
+	if i >= len(s.Clusters) {
+		return
+	}
+
+	s.Clusters = append(s.Clusters[:i], s.Clusters[i+1:]...)
+}
diff --git a/pkg/apis/work/v1alpha2/binding_types_helper_test.go b/pkg/apis/work/v1alpha2/binding_types_helper_test.go
index 9514b342d..6474f64ea 100644
--- a/pkg/apis/work/v1alpha2/binding_types_helper_test.go
+++ b/pkg/apis/work/v1alpha2/binding_types_helper_test.go
@@ -1,6 +1,9 @@
 package v1alpha2
 
-import "testing"
+import (
+	"reflect"
+	"testing"
+)
 
 func TestResourceBindingSpec_TargetContains(t *testing.T) {
 	tests := []struct {
@@ -76,3 +79,53 @@ func TestResourceBindingSpec_AssignedReplicasForCluster(t *testing.T) {
 		})
 	}
 }
+
+func TestResourceBindingSpec_RemoveCluster(t *testing.T) {
+	tests := []struct {
+		Name        string
+		InputSpec   ResourceBindingSpec
+		ClusterName string
+		ExpectSpec  ResourceBindingSpec
+	}{
+		{
+			Name:        "cluster not exist should do nothing",
+			InputSpec:   ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m1"}, {Name: "m2"}, {Name: "m3"}}},
+			ClusterName: "no-exist",
+			ExpectSpec:  ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m1"}, {Name: "m2"}, {Name: "m3"}}},
+		},
+		{
+			Name:        "remove cluster from head",
+			InputSpec:   ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m1"}, {Name: "m2"}, {Name: "m3"}}},
+			ClusterName: "m1",
+			ExpectSpec:  ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m2"}, {Name: "m3"}}},
+		},
+		{
+			Name:        "remove cluster from middle",
+			InputSpec:   ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m1"}, {Name: "m2"}, {Name: "m3"}}},
+			ClusterName: "m2",
+			ExpectSpec:  ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m1"}, {Name: "m3"}}},
+		},
+		{
+			Name:        "remove cluster from tail",
+			InputSpec:   ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m1"}, {Name: "m2"}, {Name: "m3"}}},
+			ClusterName: "m3",
+			ExpectSpec:  ResourceBindingSpec{Clusters: []TargetCluster{{Name: "m1"}, {Name: "m2"}}},
+		},
+		{
+			Name:        "remove cluster from empty list",
+			InputSpec:   ResourceBindingSpec{Clusters: []TargetCluster{}},
+			ClusterName: "na",
+			ExpectSpec:  ResourceBindingSpec{Clusters: []TargetCluster{}},
+		},
+	}
+
+	for _, test := range tests {
+		tc := test
+		t.Run(tc.Name, func(t *testing.T) {
+			tc.InputSpec.RemoveCluster(tc.ClusterName)
+			if !reflect.DeepEqual(tc.InputSpec.Clusters, tc.ExpectSpec.Clusters) {
+				t.Fatalf("expect: %v, but got: %v", tc.ExpectSpec.Clusters, tc.InputSpec.Clusters)
+			}
+		})
+	}
+}
diff --git a/pkg/controllers/cluster/taint_manager.go b/pkg/controllers/cluster/taint_manager.go
index dd09ea04f..151a9e7bb 100644
--- a/pkg/controllers/cluster/taint_manager.go
+++ b/pkg/controllers/cluster/taint_manager.go
@@ -153,7 +153,7 @@ func (tc *NoExecuteTaintManager) syncBindingEviction(key util.QueueKey) error {
 	// Case 3: Tolerate forever, we do nothing.
 	if needEviction || tolerationTime == 0 {
 		// update final result to evict the target cluster
-		binding.Spec.Clusters = util.RemoveTargetCluster(binding.Spec.Clusters, cluster)
+		binding.Spec.RemoveCluster(cluster)
 		if err = tc.Update(context.TODO(), binding); err != nil {
 			klog.ErrorS(err, "Failed to update binding", "binding", klog.KObj(binding))
 			return err
@@ -198,7 +198,7 @@ func (tc *NoExecuteTaintManager) syncClusterBindingEviction(key util.QueueKey) e
 	// Case 3: Tolerate forever, we do nothing.
 	if needEviction || tolerationTime == 0 {
 		// update final result to evict the target cluster
-		binding.Spec.Clusters = util.RemoveTargetCluster(binding.Spec.Clusters, cluster)
+		binding.Spec.RemoveCluster(cluster)
 		if err = tc.Update(context.TODO(), binding); err != nil {
 			klog.ErrorS(err, "Failed to update cluster binding", "binding", binding.Name)
 			return err
diff --git a/pkg/util/binding.go b/pkg/util/binding.go
index e6764bc1d..460715c12 100644
--- a/pkg/util/binding.go
+++ b/pkg/util/binding.go
@@ -111,14 +111,3 @@ func MergeTargetClusters(old, new []workv1alpha2.TargetCluster) []workv1alpha2.T
 	}
 	return new
 }
-
-// RemoveTargetCluster will delete a target cluster from cluster list.
-func RemoveTargetCluster(clusters []workv1alpha2.TargetCluster, target string) []workv1alpha2.TargetCluster {
-	res := make([]workv1alpha2.TargetCluster, 0, len(clusters)-1)
-	for _, cluster := range clusters {
-		if cluster.Name != target {
-			res = append(res, cluster)
-		}
-	}
-	return res
-}