Merge pull request #5012 from mszacillo/unschedulable-plugin-fix

Account for unschedulable plugin result during maxreplica estimation

Author: karmada-bot

pkg/estimator/server/estimate.go

@@ -76,6 +76,12 @@ func (es *AccurateSchedulerEstimatorServer) estimateReplicas(
 
 	var res int32
 	replicas, ret := es.estimateFramework.RunEstimateReplicasPlugins(ctx, snapshot, &requirements)
+
+	// No replicas can be scheduled on the cluster, skip further checks and return 0
+	if ret.IsUnschedulable() {
+		return 0, nil
+	}
+
 	if !ret.IsSuccess() && !ret.IsNoOperation() {
 		return replicas, fmt.Errorf(fmt.Sprintf("estimate replice plugins fails with %s", ret.Reasons()))
 	}

pkg/estimator/server/framework/interface.go

@@ -155,7 +155,12 @@ func (s *Result) IsSuccess() bool {
 	return s == nil || s.code == Success
 }
 
-// IsNoOperation return true if "Result" is not nil and Code is "Nooperation"
+// IsUnschedulable returns true if "Result" is not nil and Code is "Unschedulable".
+func (s *Result) IsUnschedulable() bool {
+	return s != nil && s.code == Unschedulable
+}
+
+// IsNoOperation returns true if "Result" is not nil and Code is "Nooperation"
 // ToDo (wengyao04): we can remove it once we include node resource estimation as the default plugin in the future
 func (s *Result) IsNoOperation() bool {
 	return s != nil && s.code == Noopperation

pkg/estimator/server/framework/runtime/framework.go

@@ -125,7 +125,7 @@ func (frw *frameworkImpl) RunEstimateReplicasPlugins(ctx context.Context, snapsh
 	results := make(framework.PluginToResult)
 	for _, pl := range frw.estimateReplicasPlugins {
 		plReplica, ret := frw.runEstimateReplicasPlugins(ctx, pl, snapshot, replicaRequirements)
-		if ret.IsSuccess() && plReplica < replica {
+		if (ret.IsSuccess() || ret.IsUnschedulable()) && plReplica < replica {
 			replica = plReplica
 		}
 		results[pl.Name()] = ret

pkg/estimator/server/framework/runtime/framework_test.go

@@ -117,7 +117,7 @@ func Test_frameworkImpl_RunEstimateReplicasPlugins(t *testing.T) {
 				},
 			},
 			expected: estimateReplicaResult{
-				replica: 1,
+				replica: 0,
 				ret:     framework.NewResult(framework.Unschedulable, "plugin 2 is unschedulable"),
 			},
 		},
@@ -144,7 +144,7 @@ func Test_frameworkImpl_RunEstimateReplicasPlugins(t *testing.T) {
 				},
 			},
 			expected: estimateReplicaResult{
-				replica: math.MaxInt32,
+				replica: 0,
 				ret:     framework.NewResult(framework.Unschedulable, "plugin 1 is unschedulable", "plugin 2 is no operation"),
 			},
 		},

pkg/scheduler/core/util.go

@@ -62,22 +62,25 @@ func calAvailableReplicas(clusters []*clusterv1alpha1.Cluster, spec *workv1alpha
 
 	// For non-workload, like ServiceAccount, ConfigMap, Secret and etc, it's unnecessary to calculate available replicas in member clusters.
 	// See issue: https://github.com/karmada-io/karmada/issues/3743.
+	namespacedKey := names.NamespacedKey(spec.Resource.Namespace, spec.Resource.Name)
 	if spec.Replicas == 0 {
 		klog.V(4).Infof("Do not calculate available replicas for non-workload(%s, kind=%s, %s).", spec.Resource.APIVersion,
-			spec.Resource.Kind, names.NamespacedKey(spec.Resource.Namespace, spec.Resource.Name))
+			spec.Resource.Kind, namespacedKey)
 		return availableTargetClusters
 	}
 
 	// Get the minimum value of MaxAvailableReplicas in terms of all estimators.
 	estimators := estimatorclient.GetReplicaEstimators()
 	ctx := context.WithValue(context.TODO(), util.ContextKeyObject,
 		fmt.Sprintf("kind=%s, name=%s/%s", spec.Resource.Kind, spec.Resource.Namespace, spec.Resource.Name))
-	for _, estimator := range estimators {
+	for name, estimator := range estimators {
 		res, err := estimator.MaxAvailableReplicas(ctx, clusters, spec.ReplicaRequirements)
 		if err != nil {
 			klog.Errorf("Max cluster available replicas error: %v", err)
 			continue
 		}
+		klog.V(4).Infof("Invoked MaxAvailableReplicas of estimator %s for workload(%s, kind=%s, %s): %v", name,
+			spec.Resource.APIVersion, spec.Resource.Kind, namespacedKey, res)
 		for i := range res {
 			if res[i].Replicas == estimatorclient.UnauthenticReplica {
 				continue