New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix panic when process RunScorePlugins for cap out of range #121632
Conversation
Signed-off-by: kerthcet <kerthcet@gmail.com>
This issue is currently awaiting triage. If a SIG or subproject determines this is a relevant issue, they will accept it by applying the The Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kerthcet The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@@ -1385,6 +1385,22 @@ func TestRunScorePlugins(t *testing.T) { | |||
}, | |||
}, | |||
}, | |||
{ | |||
name: "skipped prescore plugin number greater than the number of score plugins", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Panic here when revert the change.
/hold |
/hold cancel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Thanks @kerthcet
LGTM label has been added. Git tree hash: 588774c1c03e1f39fc06eb24c15f14f0847121e4
|
Does this affect default scheduler? Do we need to cherry-pick? |
Please add a release note. |
If it's a regression, let's ensure we tag it as |
Yes, I'll cherry-pick this.
Done. /kind regression |
Thanks, please add the specific release (1.x.y) that regressed in the release note |
Do I have to add the patch version? This bug was introduced since 1.27, so basically 1.27.X and 1.28.X should all have this bug. Or what I need to add is simply EDIT: |
Just to clarify, this affects the default scheduler, but not the default profile, as we never have a Score plugin enabled without its PreScore counterpart. |
Yes, for default profile, |
The upgrade of scheduler-plugins also hit this regression: kubernetes-sigs/scheduler-plugins#670 (comment) |
…1632-upstream-release-1.28 Automated cherry pick of #121632: Fix panic when process RunScorePlugins for cap out of range
…1632-upstream-release-1.27 Automated cherry pick of #121632: Fix panic when process RunScorePlugins for cap out of range
What type of PR is this?
/kind bug
/sig scheduling
/kind regression
What this PR does / why we need it:
The root cause is we can randomly customize the preScore & Score plugins in the
KubeSchedulerConfiguration
so there maybe 0 score plugins but x prescore plugins, then as long as one prescore plugins returns skip, we'll panic as the cap is negative, reportingpanic: runtime error: makeslice: cap out of range
The precise number of numPlugins should be plugin in f.scorePlugins also in SkipScorePlugins, then we will have to traverse these two slices for comparison, considering the plugin number will not large, so I pick the
len(f.scorePlugins)
instead.This bug was introduced in #115652, so it's a regression.
Which issue(s) this PR fixes:
Fixes #121630
Special notes for your reviewer:
Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: