no longer ad terminting NoExecute taint for the Deleting Cluster

Signed-off-by: changzhen <[email protected]>

Author: XiShanYongYe-Chang

cmd/controller-manager/app/controllermanager.go

@@ -254,7 +254,6 @@ func startClusterController(ctx controllerscontext.Context) (enabled bool, err e
 		ClusterMonitorGracePeriod:          opts.ClusterMonitorGracePeriod.Duration,
 		ClusterStartupGracePeriod:          opts.ClusterStartupGracePeriod.Duration,
 		FailoverEvictionTimeout:            opts.FailoverEvictionTimeout.Duration,
-		EnableTaintManager:                 ctx.Opts.EnableTaintManager,
 		ClusterTaintEvictionRetryFrequency: 10 * time.Second,
 		ExecutionSpaceRetryFrequency:       10 * time.Second,
 		RateLimiterOptions:                 ctx.Opts.RateLimiterOptions,

pkg/apis/cluster/v1alpha1/well_known_constants.go

@@ -27,8 +27,6 @@ const (
 	// (corresponding to ClusterConditionReady status ConditionUnknown)
 	// and removed when cluster becomes reachable (ClusterConditionReady status ConditionTrue).
 	TaintClusterUnreachable = "cluster.karmada.io/unreachable"
-	// TaintClusterTerminating will be added when cluster is terminating.
-	TaintClusterTerminating = "cluster.karmada.io/terminating"
 
 	// CacheSourceAnnotationKey is the annotation that added to a resource to
 	// represent which cluster it cached from.

pkg/controllers/cluster/cluster_controller.go

@@ -29,7 +29,6 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"k8s.io/apimachinery/pkg/fields"
 	"k8s.io/apimachinery/pkg/types"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
@@ -42,13 +41,11 @@ import (
 
 	clusterv1alpha1 "github.com/karmada-io/karmada/pkg/apis/cluster/v1alpha1"
 	workv1alpha1 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha1"
-	workv1alpha2 "github.com/karmada-io/karmada/pkg/apis/work/v1alpha2"
 	"github.com/karmada-io/karmada/pkg/events"
 	"github.com/karmada-io/karmada/pkg/features"
 	"github.com/karmada-io/karmada/pkg/sharedcli/ratelimiterflag"
 	"github.com/karmada-io/karmada/pkg/util"
 	utilhelper "github.com/karmada-io/karmada/pkg/util/helper"
-	"github.com/karmada-io/karmada/pkg/util/indexregistry"
 	"github.com/karmada-io/karmada/pkg/util/names"
 )
 
@@ -90,20 +87,12 @@ var (
 		Key:    clusterv1alpha1.TaintClusterNotReady,
 		Effect: corev1.TaintEffectNoSchedule,
 	}
-
-	// TerminatingTaintTemplate is the taint for when a cluster is terminating executing resources.
-	// Used for taint based eviction.
-	TerminatingTaintTemplate = &corev1.Taint{
-		Key:    clusterv1alpha1.TaintClusterTerminating,
-		Effect: corev1.TaintEffectNoExecute,
-	}
 )
 
 // Controller is to sync Cluster.
 type Controller struct {
-	client.Client      // used to operate Cluster resources.
-	EventRecorder      record.EventRecorder
-	EnableTaintManager bool
+	client.Client // used to operate Cluster resources.
+	EventRecorder record.EventRecorder
 
 	// ClusterMonitorPeriod represents cluster-controller monitoring period, i.e. how often does
 	// cluster-controller check cluster health signal posted from cluster-status-controller.
@@ -243,12 +232,6 @@ func (c *Controller) syncCluster(ctx context.Context, cluster *clusterv1alpha1.C
 }
 
 func (c *Controller) removeCluster(ctx context.Context, cluster *clusterv1alpha1.Cluster) (controllerruntime.Result, error) {
-	// add terminating taint before cluster is deleted
-	if err := c.updateClusterTaints(ctx, []*corev1.Taint{TerminatingTaintTemplate}, nil, cluster); err != nil {
-		klog.ErrorS(err, "Failed to update terminating taint", "cluster", cluster.Name)
-		return controllerruntime.Result{}, err
-	}
-
 	if err := c.removeExecutionSpace(ctx, cluster); err != nil {
 		klog.Errorf("Failed to remove execution space %s: %v", cluster.Name, err)
 		c.EventRecorder.Event(cluster, corev1.EventTypeWarning, events.EventReasonRemoveExecutionSpaceFailed, err.Error())
@@ -268,46 +251,9 @@ func (c *Controller) removeCluster(ctx context.Context, cluster *clusterv1alpha1
 	// delete the health data from the map explicitly after we removing the cluster.
 	c.clusterHealthMap.delete(cluster.Name)
 
-	// check if target cluster is removed from all bindings.
-	if c.EnableTaintManager {
-		if done, err := c.isTargetClusterRemoved(ctx, cluster); err != nil {
-			klog.ErrorS(err, "Failed to check whether target cluster is removed from bindings", "cluster", cluster.Name)
-			return controllerruntime.Result{}, err
-		} else if !done {
-			klog.InfoS("Terminating taint eviction process has not finished yet, will try again later", "cluster", cluster.Name)
-			return controllerruntime.Result{RequeueAfter: c.ClusterTaintEvictionRetryFrequency}, nil
-		}
-	}
-
 	return c.removeFinalizer(ctx, cluster)
 }
 
-func (c *Controller) isTargetClusterRemoved(ctx context.Context, cluster *clusterv1alpha1.Cluster) (bool, error) {
-	// List all ResourceBindings which are assigned to this cluster.
-	rbList := &workv1alpha2.ResourceBindingList{}
-	if err := c.List(ctx, rbList, client.MatchingFieldsSelector{
-		Selector: fields.OneTermEqualSelector(indexregistry.ResourceBindingIndexByFieldCluster, cluster.Name),
-	}); err != nil {
-		klog.ErrorS(err, "Failed to list ResourceBindings", "cluster", cluster.Name)
-		return false, err
-	}
-	if len(rbList.Items) != 0 {
-		return false, nil
-	}
-	// List all ClusterResourceBindings which are assigned to this cluster.
-	crbList := &workv1alpha2.ClusterResourceBindingList{}
-	if err := c.List(ctx, crbList, client.MatchingFieldsSelector{
-		Selector: fields.OneTermEqualSelector(indexregistry.ClusterResourceBindingIndexByFieldCluster, cluster.Name),
-	}); err != nil {
-		klog.ErrorS(err, "Failed to list ClusterResourceBindings", "cluster", cluster.Name)
-		return false, err
-	}
-	if len(crbList.Items) != 0 {
-		return false, nil
-	}
-	return true, nil
-}
-
 // removeExecutionSpace deletes the given execution space
 func (c *Controller) removeExecutionSpace(ctx context.Context, cluster *clusterv1alpha1.Cluster) error {
 	executionSpaceName := names.GenerateExecutionSpaceName(cluster.Name)

pkg/controllers/cluster/cluster_controller_test.go

@@ -38,7 +38,6 @@ import (
 	"github.com/karmada-io/karmada/pkg/util"
 	"github.com/karmada-io/karmada/pkg/util/gclient"
 	"github.com/karmada-io/karmada/pkg/util/indexregistry"
-	"github.com/karmada-io/karmada/pkg/util/names"
 )
 
 func newClusterController() *Controller {
@@ -65,7 +64,6 @@ func newClusterController() *Controller {
 		Client:                    client,
 		EventRecorder:             record.NewFakeRecorder(1024),
 		clusterHealthMap:          newClusterHealthMap(),
-		EnableTaintManager:        true,
 		ClusterMonitorGracePeriod: 40 * time.Second,
 	}
 }
@@ -249,41 +247,6 @@ func TestController_Reconcile(t *testing.T) {
 			want:    controllerruntime.Result{},
 			wantErr: false,
 		},
-		{
-			name: "remove cluster failed",
-			cluster: &clusterv1alpha1.Cluster{
-				ObjectMeta: controllerruntime.ObjectMeta{
-					Name:       "test-cluster",
-					Finalizers: []string{util.ClusterControllerFinalizer},
-				},
-				Spec: clusterv1alpha1.ClusterSpec{
-					SyncMode: clusterv1alpha1.Pull,
-				},
-				Status: clusterv1alpha1.ClusterStatus{
-					Conditions: []metav1.Condition{
-						{
-							Type:   clusterv1alpha1.ClusterConditionReady,
-							Status: metav1.ConditionFalse,
-						},
-					},
-				},
-			},
-			ns: &corev1.Namespace{
-				ObjectMeta: metav1.ObjectMeta{
-					Name: names.GenerateExecutionSpaceName("test-cluster"),
-				},
-			},
-			work: &workv1alpha1.Work{
-				ObjectMeta: metav1.ObjectMeta{
-					Name:       "test-work",
-					Namespace:  names.GenerateExecutionSpaceName("test-cluster"),
-					Finalizers: []string{util.ExecutionControllerFinalizer},
-				},
-			},
-			del:     true,
-			want:    controllerruntime.Result{},
-			wantErr: true,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

pkg/scheduler/event_handler.go

@@ -300,6 +300,8 @@ func (s *Scheduler) deleteCluster(obj interface{}) {
 	if s.enableSchedulerEstimator {
 		s.schedulerEstimatorWorker.Add(cluster.Name)
 	}
+
+	s.clusterReconcileWorker.Add(cluster)
 }
 
 func schedulerNameFilter(schedulerNameFromOptions, schedulerName string) bool {

pkg/scheduler/event_handler_test.go

@@ -310,20 +310,23 @@ func TestDeleteCluster(t *testing.T) {
 		obj                      interface{}
 		expectedAdded            bool
 		expectedClusterName      string
+		expectedReconcileAdded   int
 	}{
 		{
 			name:                     "valid cluster object with estimator enabled",
 			enableSchedulerEstimator: true,
 			obj:                      createCluster("test-cluster", 0, nil),
 			expectedAdded:            true,
 			expectedClusterName:      "test-cluster",
+			expectedReconcileAdded:   1,
 		},
 		{
 			name:                     "valid cluster object with estimator disabled",
 			enableSchedulerEstimator: false,
 			obj:                      createCluster("test-cluster", 0, nil),
 			expectedAdded:            false,
 			expectedClusterName:      "",
+			expectedReconcileAdded:   1,
 		},
 		{
 			name:                     "deleted final state unknown with valid cluster",
@@ -332,8 +335,9 @@ func TestDeleteCluster(t *testing.T) {
 				Key: "test-cluster",
 				Obj: createCluster("test-cluster", 0, nil),
 			},
-			expectedAdded:       true,
-			expectedClusterName: "test-cluster",
+			expectedAdded:          true,
+			expectedClusterName:    "test-cluster",
+			expectedReconcileAdded: 1,
 		},
 		{
 			name:                     "deleted final state unknown with invalid object",
@@ -342,35 +346,42 @@ func TestDeleteCluster(t *testing.T) {
 				Key: "test-pod",
 				Obj: &corev1.Pod{},
 			},
-			expectedAdded:       false,
-			expectedClusterName: "",
+			expectedAdded:          false,
+			expectedClusterName:    "",
+			expectedReconcileAdded: 0,
 		},
 		{
 			name:                     "invalid object type",
 			enableSchedulerEstimator: true,
 			obj:                      &corev1.Pod{},
 			expectedAdded:            false,
 			expectedClusterName:      "",
+			expectedReconcileAdded:   0,
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			worker := &mockAsyncWorker{}
+			estimatorWorker := &mockAsyncWorker{}
+			reconcileWorker := &mockAsyncWorker{}
 			s := &Scheduler{
 				enableSchedulerEstimator: tt.enableSchedulerEstimator,
-				schedulerEstimatorWorker: worker,
+				schedulerEstimatorWorker: estimatorWorker,
+				clusterReconcileWorker:   reconcileWorker,
 			}
 
 			s.deleteCluster(tt.obj)
 
 			if tt.expectedAdded {
-				assert.Equal(t, 1, worker.addCount, "Worker Add should have been called once")
-				assert.Equal(t, tt.expectedClusterName, worker.lastAdded, "Incorrect cluster name added to worker")
+				assert.Equal(t, 1, estimatorWorker.addCount, "Worker Add should have been called once")
+				assert.Equal(t, tt.expectedClusterName, estimatorWorker.lastAdded, "Incorrect cluster name added to worker")
 			} else {
-				assert.Equal(t, 0, worker.addCount, "Worker Add should not have been called")
-				assert.Nil(t, worker.lastAdded, "No cluster name should have been added")
+				assert.Equal(t, 0, estimatorWorker.addCount, "Worker Add should not have been called")
+				assert.Nil(t, estimatorWorker.lastAdded, "No cluster name should have been added")
 			}
+
+			// Check clusterReconcileWorker
+			assert.Equal(t, tt.expectedReconcileAdded, reconcileWorker.addCount, "Reconcile worker Add called unexpected number of times")
 		})
 	}
 }