adjust the Priority of General and Accurate Estimator of Karmada Scheduler

[chaosi-zju]

What type of PR is this?

/kind feature

What this PR does / why we need it:

If the accurate estimator is enabled and karmada-scheduler-estimator components also are enabled, the accurate estimator should be prior to ResourceModeling rather than using the minumal value of them.

Which issue(s) this PR fixes:

Fixes #3333

Special notes for your reviewer:

CC @lonelyCZ @whitewindmills

Does this PR introduce a user-facing change?:

introduced priority to schedule estimator so that higher-priority estimator can override the result given by lower-priority.

[codecov-commenter]

Codecov Report

Attention: 48 lines in your changes are missing coverage. Please review.

Comparison is base (697170b) 51.82% compared to head (2a208c7) 51.84%. Report is 5 commits behind head on master.

Files	Patch %	Lines
pkg/descheduler/core/helper.go	0.00%	34 Missing :warning:
pkg/scheduler/core/util.go	65.71%	7 Missing and 5 partials :warning:
pkg/descheduler/descheduler.go	0.00%	2 Missing :warning:

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4398      +/-   ##
==========================================
+ Coverage   51.82%   51.84%   +0.02%     
==========================================
  Files         244      246       +2     
  Lines       24238    24378     +140     
==========================================
+ Hits        12561    12639      +78     
- Misses      10995    11049      +54     
- Partials      682      690       +8     

Flag	Coverage Δ
unittests	51.84% <34.24%> (+0.02%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[chaunceyjiang]

Just a question, will this modification require compatibility? Will it force users to upgrade the Estimator component?

[chaosi-zju]

will this modification require compatibility

I don't think it matters. What do you think?

Will it force users to upgrade the Estimator component

I think it's only about the scheduler, and doesn't require upgrading estimator

[jwcesign]

/lgtm

[zhzhuang-zju]

/lgtm

[chaosi-zju]

CC @RainbowMango may be ready to merge, any further comments?

[chaosi-zju]

@whitewindmills sorry for delayed a while, I have updated the code, do you have time checking once more?

[whitewindmills]

thanks for your hard work. /lgtm

[chaosi-zju]

CC @RainbowMango

[Garrybest]

/assign

[chaosi-zju]

@whitewindmills @RainbowMango @Garrybest

I have improved the logic as:

• priority is not declared by estimator itself, but by the function caller, just like:

// NewGeneralEstimator builds a new GeneralEstimator.
func NewGeneralEstimator(priority EstimatorPriority) *GeneralEstimator {
	return &GeneralEstimator{priority: priority}
}

// GetPriority return the priority of this estimator
func (ge *GeneralEstimator) GetPriority() EstimatorPriority {
	return ge.priority
}

• If higher-priority estimators haven't given the result for certain member clusters, lower-priority estimator only continue to estimate for such clusters haven't got a result. Just like for the cluster which already estimated by its karmada-estimator no need to back off to general estimator.

Sorry for delayed, and thanks for review again~

[whitewindmills]

/lgtm

[karmada-bot]

New changes are detected. LGTM label has been removed.

[karmada-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: To complete the pull request process, please ask for approval from garrybest after the PR has been reviewed.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment Approvers can cancel approval by writing /approve cancel in a comment