Skip to content

Commit

Permalink
Merge pull request #5026 from RainbowMango/automated-cherry-pick-of-#…
Browse files Browse the repository at this point in the history
…5012-upstream-release-1.9

Automated cherry pick of #5012: Account for unschedulable plugin result during maxreplica
  • Loading branch information
karmada-bot authored Jun 12, 2024
2 parents e0f15bc + 65ce95f commit 8ef8d37
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 6 deletions.
6 changes: 6 additions & 0 deletions pkg/estimator/server/estimate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()))
}
Expand Down
7 changes: 6 additions & 1 deletion pkg/estimator/server/framework/interface.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/estimator/server/framework/runtime/framework.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions pkg/estimator/server/framework/runtime/framework_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
},
},
Expand All @@ -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"),
},
},
Expand Down
7 changes: 5 additions & 2 deletions pkg/scheduler/core/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 8ef8d37

Please sign in to comment.