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

@@ -248,15 +248,14 @@ func startClusterController(ctx controllerscontext.Context) (enabled bool, err e
 	opts := ctx.Opts
 
 	clusterController := &cluster.Controller{
-		Client:                             mgr.GetClient(),
-		EventRecorder:                      mgr.GetEventRecorderFor(cluster.ControllerName),
-		ClusterMonitorPeriod:               opts.ClusterMonitorPeriod.Duration,
-		ClusterMonitorGracePeriod:          opts.ClusterMonitorGracePeriod.Duration,
-		ClusterStartupGracePeriod:          opts.ClusterStartupGracePeriod.Duration,
-		EnableTaintManager:                 ctx.Opts.EnableTaintManager,
-		ClusterTaintEvictionRetryFrequency: 10 * time.Second,
-		ExecutionSpaceRetryFrequency:       10 * time.Second,
-		RateLimiterOptions:                 ctx.Opts.RateLimiterOptions,
+		Client:                                    mgr.GetClient(),
+		EventRecorder:                             mgr.GetEventRecorderFor(cluster.ControllerName),
+		ClusterMonitorPeriod:                      opts.ClusterMonitorPeriod.Duration,
+		ClusterMonitorGracePeriod:                 opts.ClusterMonitorGracePeriod.Duration,
+		ClusterStartupGracePeriod:                 opts.ClusterStartupGracePeriod.Duration,
+		ResourceCleanupWaitingRetryFrequency:      10 * time.Second,
+		ExecutionSpaceRemoveWaitingRetryFrequency: 10 * time.Second,
+		RateLimiterOptions:                        ctx.Opts.RateLimiterOptions,
 	}
 	if err := clusterController.SetupWithManager(mgr); err != nil {
 		return false, err

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

@@ -75,20 +75,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.
@@ -101,8 +93,8 @@ type Controller struct {
 	// When cluster is just created, e.g. agent bootstrap or cluster join, we give a longer grace period.
 	ClusterStartupGracePeriod time.Duration
 
-	ClusterTaintEvictionRetryFrequency time.Duration
-	ExecutionSpaceRetryFrequency       time.Duration
+	ResourceCleanupWaitingRetryFrequency      time.Duration
+	ExecutionSpaceRemoveWaitingRetryFrequency time.Duration
 
 	// Per Cluster map stores last observed health together with a local time when it was observed.
 	clusterHealthMap   *clusterHealthMap
@@ -225,12 +217,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())
@@ -244,21 +230,19 @@ func (c *Controller) removeCluster(ctx context.Context, cluster *clusterv1alpha1
 		return controllerruntime.Result{}, err
 	} else if exist {
 		klog.Infof("Requeuing operation until the cluster(%s) execution space deleted", cluster.Name)
-		return controllerruntime.Result{RequeueAfter: c.ExecutionSpaceRetryFrequency}, nil
+		return controllerruntime.Result{RequeueAfter: c.ExecutionSpaceRemoveWaitingRetryFrequency}, nil
 	}
 
-	// delete the health data from the map explicitly after we removing the cluster.
+	// delete the health data from the map explicitly after we're 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
-		}
+	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.ResourceCleanupWaitingRetryFrequency}, nil
 	}
 
 	return c.removeFinalizer(ctx, cluster)

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/core/generic_scheduler.go

@@ -132,6 +132,13 @@ func (g *genericScheduler) findClustersThatFit(
 	// DO NOT filter unhealthy cluster, let users make decisions by using ClusterTolerations of Placement.
 	clusters := clusterInfo.GetClusters()
 	for _, c := range clusters {
+		// When cluster is deleting, we will clean up the scheduled results in the cluster.
+		// So we should not schedule resource to the deleting cluster.
+		if !c.Cluster().DeletionTimestamp.IsZero() {
+			klog.V(4).Infof("Cluster %q is deleting, skip it", c.Cluster().Name)
+			continue
+		}
+
 		if result := g.scheduleFramework.RunFilterPlugins(ctx, bindingSpec, bindingStatus, c.Cluster()); !result.IsSuccess() {
 			klog.V(4).Infof("Cluster %q is not fit, reason: %v", c.Cluster().Name, result.AsError())
 			diagnosis.ClusterToResultMap[c.Cluster().Name] = result

pkg/scheduler/event_handler.go

@@ -268,10 +268,12 @@ func (s *Scheduler) updateCluster(oldObj, newObj interface{}) {
 	}
 
 	switch {
+	case oldCluster.DeletionTimestamp.IsZero() && !newCluster.DeletionTimestamp.IsZero():
+		s.clusterReconcileWorker.Add(newCluster)
 	case !equality.Semantic.DeepEqual(oldCluster.Labels, newCluster.Labels):
 		fallthrough
 	case oldCluster.Generation != newCluster.Generation:
-		// To distinguish the obd and new cluster objects, we need to add the entire object
+		// To distinguish the old and new cluster objects, we need to add the entire object
 		// to the worker. Therefore, call Add func instead of Enqueue func.
 		s.clusterReconcileWorker.Add(oldCluster)
 		s.clusterReconcileWorker.Add(newCluster)

pkg/scheduler/scheduler.go

@@ -25,6 +25,8 @@ import (
 	"strings"
 	"time"
 
+	"k8s.io/apimachinery/pkg/labels"
+
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
@@ -429,6 +431,12 @@ func (s *Scheduler) doScheduleBinding(namespace, name string) (err error) {
 		return err
 	}
 	// TODO(dddddai): reschedule bindings on cluster change
+	if s.someScheduledClusterDeleted(rb) {
+		klog.Infof("Reschedule ResourceBinding(%s/%s) as some scheduled clusters are deleted", namespace, name)
+		err = s.scheduleResourceBinding(rb)
+		metrics.BindingSchedule(string(ReconcileSchedule), utilmetrics.DurationInSeconds(start), err)
+		return err
+	}
 	klog.V(3).Infof("Don't need to schedule ResourceBinding(%s/%s)", rb.Namespace, rb.Name)
 
 	// If no scheduling is required, we need to ensure that binding.Generation is equal to
@@ -512,6 +520,25 @@ func (s *Scheduler) doScheduleClusterBinding(name string) (err error) {
 	return nil
 }
 
+func (s *Scheduler) someScheduledClusterDeleted(rb *workv1alpha2.ResourceBinding) bool {
+	clusters, err := s.clusterLister.List(labels.Everything())
+	if err != nil {
+		klog.Errorf("Failed to list clusters: %v", err)
+		return false
+	}
+
+	for _, cluster := range clusters {
+		if cluster.DeletionTimestamp.IsZero() {
+			continue
+		}
+
+		if rb.Spec.TargetContains(cluster.Name) {
+			return true
+		}
+	}
+	return false
+}
+
 func (s *Scheduler) scheduleResourceBinding(rb *workv1alpha2.ResourceBinding) (err error) {
 	defer func() {
 		condition, ignoreErr := getConditionByError(err)