scheduler: expose Run[Pre]ScorePlugins functions to PreemptionHandle(through PluginRunner) #93534

everpeace · 2020-07-29T14:15:20Z

What type of PR is this?
/kind feature
/sig scheduling

What this PR does / why we need it:

As discussed in #90711 (comment), adding RunScorePlugins into PreemptHanlde(though PluginRunner interface) can give out-of-tree plugins more flexibility to implement their preemption plugins, without reinventing existing Score algorithms.

I think just adding RunScorePlugins definitions in PluginRunner can achieve this.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

Scheduling Framework: expose Run[Pre]ScorePlugins functions to PreemptionHandle which can be used in PostFilter extention point.

k8s-ci-robot · 2020-07-29T14:15:22Z

@everpeace: The label(s) sig/ cannot be applied, because the repository doesn't have them

In response to this:

What type of PR is this?
/kind feature
/sig scheduling

What this PR does / why we need it:

As discussed in #90711 (comment), adding RunScorePlugins into PreemptHanlde(though PluginRunner interface) can give out-of-tree plugins more flexibility to implement their preemption plugins, without reinventing existing Score algorithms.

Special notes for your reviewer:

Does this PR introduce a user-facing change?:
NONE

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.

everpeace · 2020-07-29T14:18:04Z

/assign @Huang-Wei

could you kindly review this??

everpeace · 2020-07-29T14:51:59Z

/retest

Huang-Wei

LGTM, but we cannot make it to 1.19.

BTW: could you add some tests?

everpeace · 2020-07-31T10:49:22Z

LGTM, but we cannot make it to 1.19.

don't worry. no problem.

BTW: could you add some tests?

sorry, I missed that. I added PostFilter plugin test in framework/framework_test.go. PTAL.

everpeace · 2020-07-31T12:53:18Z

/retest

everpeace · 2020-07-31T15:37:32Z

/retest

test/integration/scheduler/framework_test.go

chendave · 2020-08-01T08:22:18Z

test/integration/scheduler/framework_test.go

+	for _, nodeInfo := range nodeInfos {
+		nodes = append(nodes, nodeInfo.Node())
+	}
+	ph.RunScorePlugins(ctx, state, pod, nodes)


Not sure if we need some caveat here or in PluginsRunner interface to make it clear that this is potentially for out-of-tree plugins only.

Thanks for the comment. I think I don't need comments here and PluginsRunner because the scheduler framework is essentially for out-of-tree plugins.

okay, thanks!

Huang-Wei

The CI failure is real. See comments below.

BTW: can you also update L792 to make the scoreNum is obtained atomically?

pkg/scheduler/framework/v1alpha1/interface.go

test/integration/scheduler/framework_test.go

everpeace

sorry for submitting poor test... I believe I fixed the integration test this time.

test/integration/scheduler/framework_test.go

everpeace · 2020-08-04T13:52:16Z

test/integration/scheduler/framework_test.go

@@ -748,7 +797,7 @@ func TestScorePlugin(t *testing.T) {
 				}
 			}

-			if scorePlugin.numScoreCalled == 0 {
+			if numScoreCalled := atomic.LoadInt32(&scorePlugin.numScoreCalled); numScoreCalled == 0 {


as mentioned in #93534 (review)

BTW: can you also update L792 to make the scoreNum is obtained atomically?

Sure, I updated to read numScoreCalled with atomic in TestPostFilterPlugin, too.

everpeace · 2020-08-04T13:55:58Z

test/integration/scheduler/framework_test.go

 		expectPostFilterNumCalled int
 	}{
 		{
-			name:                      "Filter passed",
+			name:                      "Filter passed and Score success",
+			numNodes:                  3,


ref: #93534 (comment)
I introduced numNodes in test case. I set it to 3 here and others to 1

Huang-Wei

LGTM now. @ahg-g are you ok with introducing Run[Pre]ScorePlugins to PluginRunner?

ahg-g · 2020-08-08T00:17:49Z

LGTM now. @ahg-g are you ok with introducing Run[Pre]ScorePlugins to PluginRunner?

I can't think of a reason why not.

Huang-Wei · 2020-08-08T02:15:08Z

Thanks @everpeace !

/lgtm
/approve

k8s-ci-robot · 2020-08-08T02:15:39Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: everpeace, Huang-Wei

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:

~~pkg/scheduler/OWNERS~~ [Huang-Wei]
~~test/integration/scheduler/OWNERS~~ [Huang-Wei]

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

ahg-g · 2020-08-21T14:23:40Z

/priority important-longterm

everpeace · 2020-10-09T15:17:54Z

/test pull-kubernetes-e2e-kind-ipv6

fejta-bot · 2020-10-09T18:41:51Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-10-09T22:11:32Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

Huang-Wei · 2020-10-09T22:14:47Z

/retest

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Jul 29, 2020

k8s-ci-robot requested review from ahg-g and draveness July 29, 2020 14:17

k8s-ci-robot assigned Huang-Wei Jul 29, 2020

Huang-Wei reviewed Jul 29, 2020

View reviewed changes

everpeace force-pushed the score-in-preemption-handle branch from cf08b16 to 01adfeb Compare July 31, 2020 10:45

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 31, 2020

Huang-Wei reviewed Jul 31, 2020

View reviewed changes

test/integration/scheduler/framework_test.go Outdated Show resolved Hide resolved

chendave reviewed Aug 1, 2020

View reviewed changes

everpeace force-pushed the score-in-preemption-handle branch from 01adfeb to 7b9abcb Compare August 3, 2020 01:23

everpeace requested a review from Huang-Wei August 3, 2020 02:18

Huang-Wei reviewed Aug 3, 2020

View reviewed changes

everpeace force-pushed the score-in-preemption-handle branch from 7b9abcb to d58058e Compare August 4, 2020 13:48

everpeace added 2 commits August 4, 2020 22:50

expose Run[Pre]ScorePlugins functions in PluginRunner interface

ef1fab7

Fix TestScorePlugin: numScore should be accessed with atomic.LoadInt32

d0421fa

everpeace force-pushed the score-in-preemption-handle branch from d58058e to d0421fa Compare August 4, 2020 13:50

everpeace commented Aug 4, 2020

View reviewed changes

everpeace requested a review from Huang-Wei August 4, 2020 16:00

Huang-Wei reviewed Aug 4, 2020

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 8, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 8, 2020

k8s-ci-robot added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 21, 2020

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-none Denotes a PR that doesn't merit a release note. labels Oct 9, 2020

k8s-ci-robot merged commit c72b30b into kubernetes:master Oct 10, 2020

k8s-ci-robot added this to the v1.20 milestone Oct 10, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

scheduler: expose Run[Pre]ScorePlugins functions to PreemptionHandle(through PluginRunner) #93534

scheduler: expose Run[Pre]ScorePlugins functions to PreemptionHandle(through PluginRunner) #93534

everpeace commented Jul 29, 2020 •

edited

k8s-ci-robot commented Jul 29, 2020

everpeace commented Jul 29, 2020

everpeace commented Jul 29, 2020

Huang-Wei left a comment

everpeace commented Jul 31, 2020 •

edited

everpeace commented Jul 31, 2020

everpeace commented Jul 31, 2020

chendave Aug 1, 2020

everpeace Aug 3, 2020

chendave Aug 3, 2020

Huang-Wei left a comment

everpeace left a comment

everpeace Aug 4, 2020

everpeace Aug 4, 2020 •

edited

Huang-Wei left a comment

ahg-g commented Aug 8, 2020

Huang-Wei commented Aug 8, 2020

k8s-ci-robot commented Aug 8, 2020

ahg-g commented Aug 21, 2020

everpeace commented Oct 9, 2020

fejta-bot commented Oct 9, 2020

fejta-bot commented Oct 9, 2020

Huang-Wei commented Oct 9, 2020

scheduler: expose Run[Pre]ScorePlugins functions to PreemptionHandle(through PluginRunner) #93534

scheduler: expose Run[Pre]ScorePlugins functions to PreemptionHandle(through PluginRunner) #93534

Conversation

everpeace commented Jul 29, 2020 • edited

k8s-ci-robot commented Jul 29, 2020

everpeace commented Jul 29, 2020

everpeace commented Jul 29, 2020

Huang-Wei left a comment

Choose a reason for hiding this comment

everpeace commented Jul 31, 2020 • edited

everpeace commented Jul 31, 2020

everpeace commented Jul 31, 2020

chendave Aug 1, 2020

Choose a reason for hiding this comment

everpeace Aug 3, 2020

Choose a reason for hiding this comment

chendave Aug 3, 2020

Choose a reason for hiding this comment

Huang-Wei left a comment

Choose a reason for hiding this comment

everpeace left a comment

Choose a reason for hiding this comment

everpeace Aug 4, 2020

Choose a reason for hiding this comment

everpeace Aug 4, 2020 • edited

Choose a reason for hiding this comment

Huang-Wei left a comment

Choose a reason for hiding this comment

ahg-g commented Aug 8, 2020

Huang-Wei commented Aug 8, 2020

k8s-ci-robot commented Aug 8, 2020

ahg-g commented Aug 21, 2020

everpeace commented Oct 9, 2020

fejta-bot commented Oct 9, 2020

fejta-bot commented Oct 9, 2020

Huang-Wei commented Oct 9, 2020

everpeace commented Jul 29, 2020 •

edited

everpeace commented Jul 31, 2020 •

edited

everpeace Aug 4, 2020 •

edited