feat(scheduler): use api server to watch scheduled pods #82338

draveness · 2019-09-04T16:03:26Z

/kind feature
/sig scheduling
/priority important-longterm
/assign @Huang-Wei

What this PR does / why we need it:

Use api server to watch scheduled pods instead of listing them in a for loop.

Which issue(s) this PR fixes:

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:

Huang-Wei

Generally LGTM. Some comments below.

test/integration/scheduler_perf/scheduler_bench_test.go

test/integration/scheduler_perf/scheduler_test.go

k82cn · 2019-09-05T06:53:47Z

test/integration/scheduler_perf/scheduler_bench_test.go

+
+	scheduled := int32(0)
+	scheduledCh := make(chan struct{}, 1)
+	schedulerConfigArgs.PodInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{


I can not remember where do we start PodInformer routine :(, do you know that?

kubernetes/test/integration/util/util.go

Lines 79 to 92 in 64a6718

sched := scheduler.NewFromConfig(config)

scheduler.AddAllEventHandlers(sched,

v1.DefaultSchedulerName,

informerFactory.Core().V1().Nodes(),

informerFactory.Core().V1().Pods(),

informerFactory.Core().V1().PersistentVolumes(),

informerFactory.Core().V1().PersistentVolumeClaims(),

informerFactory.Core().V1().Services(),

informerFactory.Storage().V1().StorageClasses(),

informerFactory.Storage().V1beta1().CSINodes(),

)

informerFactory.Start(stopCh)

sched.Run()

It's here, please correct me if I'm wrong.

draveness · 2019-09-05T09:57:34Z

/retest Regards, Draven

…

On Sep 5, 2019, 5:54 PM +0800, Kubernetes Prow Robot ***@***.***>, wrote: @draveness: The following test failed, say /retest to rerun them all: Test name Commit Details Rerun command pull-kubernetes-node-e2e 0a3a95a link /test pull-kubernetes-node-e2e Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

draveness · 2019-09-05T14:24:53Z

/retest

draveness · 2019-09-05T15:00:52Z

/retest

Huang-Wei · 2019-09-06T02:45:25Z

/lgtm
/approve

/hold
in case @k82cn has other comments.

k8s-ci-robot · 2019-09-06T02:45:50Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: draveness, 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:

~~test/integration/scheduler_perf/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

draveness · 2019-09-11T01:57:23Z

/retest

draveness · 2019-09-11T03:29:30Z

/retest Regards, Draven

…

On Sep 11, 2019, 11:26 AM +0800, Kubernetes Prow Robot ***@***.***>, wrote: @draveness: The following test failed, say /retest to rerun them all: Test name Commit Details Rerun command pull-kubernetes-e2e-gce-100-performance 591a49b link /test pull-kubernetes-e2e-gce-100-performance Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

draveness · 2019-09-12T02:06:51Z

@Huang-Wei @k82cn All the comments have been addressed, PTAL

draveness · 2019-09-12T03:40:00Z

/retest Regards, Draven

…

On Sep 12, 2019, 11:00 AM +0800, Kubernetes Prow Robot ***@***.***>, wrote: @draveness: The following test failed, say /retest to rerun them all: Test name Commit Details Rerun command pull-kubernetes-kubemark-e2e-gce-big f09e8a4 link /test pull-kubernetes-kubemark-e2e-gce-big Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Huang-Wei · 2019-09-12T04:02:34Z

@draveness is anything new introduced since the preceding review? or it's just rebasing?

draveness · 2019-09-12T05:32:08Z

/retest Regards, Draven

…

On Sep 12, 2019, 12:07 PM +0800, Kubernetes Prow Robot ***@***.***>, wrote: @draveness: The following tests failed, say /retest to rerun them all: Test name Commit Details Rerun command pull-kubernetes-kubemark-e2e-gce-big f09e8a4 link /test pull-kubernetes-kubemark-e2e-gce-big pull-kubernetes-e2e-gce-100-performance f09e8a4 link /test pull-kubernetes-e2e-gce-100-performance Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

draveness · 2019-09-12T05:33:01Z

it’s just rebasing. :) Regards, Draven

…

On Sep 12, 2019, 12:07 PM +0800, Kubernetes Prow Robot ***@***.***>, wrote: @draveness: The following tests failed, say /retest to rerun them all: Test name Commit Details Rerun command pull-kubernetes-kubemark-e2e-gce-big f09e8a4 link /test pull-kubernetes-kubemark-e2e-gce-big pull-kubernetes-e2e-gce-100-performance f09e8a4 link /test pull-kubernetes-e2e-gce-100-performance Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR. 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. I understand the commands that are listed here. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Huang-Wei · 2019-09-14T05:08:27Z

test/integration/scheduler_perf/scheduler_test.go

@@ -151,39 +154,59 @@ func schedulePods(config *testConfig) int32 {
 			break
 		}
 	}
+
+	scheduled := int32(0)
+	ctx, cancelFunc := context.WithCancel(context.Background())


A nit: cancel is a more convenal name.

draveness · 2019-09-14T09:20:52Z

/retest

test/integration/scheduler_perf/scheduler_bench_test.go

k82cn · 2019-09-16T08:56:02Z

LGTM overall :)

draveness · 2019-09-16T19:37:29Z

Can I get a lgtm on this PR? Thanks :) Regards, Draven

…

On Sep 16, 2019, 4:59 PM +0800, Klaus Ma ***@***.***>, wrote: LGTM overall :) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Huang-Wei · 2019-09-16T20:30:23Z

@draveness Sure. Before /lgtm, can you resolve #82338 (comment)?

draveness · 2019-09-17T03:25:58Z

Thanks, Please take another look Regards, Draven

…

On Sep 17, 2019, 4:32 AM +0800, Wei Huang ***@***.***>, wrote: @draveness Sure. Before /lgtm, can you resolve #82338 (comment)? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

Huang-Wei · 2019-09-17T16:15:02Z

/lgtm

Thanks, @draveness .

draveness · 2019-09-18T02:54:16Z

/hold cancel

k8s-ci-robot added the release-note-none Denotes a PR that doesn't merit a release note. label Sep 4, 2019

k8s-ci-robot assigned Huang-Wei Sep 4, 2019

k8s-ci-robot requested review from Huang-Wei and k82cn September 4, 2019 16:03

draveness mentioned this pull request Sep 4, 2019

Enhancement for scheduler perf #81719

Closed

5 tasks

Huang-Wei reviewed Sep 5, 2019

View reviewed changes

k82cn reviewed Sep 5, 2019

View reviewed changes

draveness force-pushed the feature/use-apiserver-in-scheduler-benchmarks branch from 421215c to 0a3a95a Compare September 5, 2019 09:29

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Sep 6, 2019

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 6, 2019

draveness force-pushed the feature/use-apiserver-in-scheduler-benchmarks branch from 0a3a95a to 22cf232 Compare September 11, 2019 00:54

k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 11, 2019

draveness force-pushed the feature/use-apiserver-in-scheduler-benchmarks branch from 22cf232 to 591a49b Compare September 11, 2019 01:23

draveness requested a review from Huang-Wei September 11, 2019 08:59

draveness force-pushed the feature/use-apiserver-in-scheduler-benchmarks branch from 591a49b to b9f7828 Compare September 12, 2019 02:03

draveness force-pushed the feature/use-apiserver-in-scheduler-benchmarks branch from b9f7828 to f09e8a4 Compare September 12, 2019 02:05

Huang-Wei reviewed Sep 14, 2019

View reviewed changes

draveness force-pushed the feature/use-apiserver-in-scheduler-benchmarks branch from f09e8a4 to ad1cf1b Compare September 14, 2019 05:14

Huang-Wei reviewed Sep 16, 2019

View reviewed changes

test/integration/scheduler_perf/scheduler_bench_test.go Show resolved Hide resolved

feat(scheduler): use api server to watch scheduled pods

54e0f47

draveness force-pushed the feature/use-apiserver-in-scheduler-benchmarks branch from ad1cf1b to 54e0f47 Compare September 17, 2019 03:25

draveness requested a review from Huang-Wei September 17, 2019 03:26

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 17, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 18, 2019

k8s-ci-robot merged commit 57d8750 into kubernetes:master Sep 18, 2019

k8s-ci-robot added this to the v1.17 milestone Sep 18, 2019

draveness deleted the feature/use-apiserver-in-scheduler-benchmarks branch October 15, 2019 04:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(scheduler): use api server to watch scheduled pods #82338

feat(scheduler): use api server to watch scheduled pods #82338

draveness commented Sep 4, 2019 •

edited

Huang-Wei left a comment

k82cn Sep 5, 2019

draveness Sep 5, 2019

draveness commented Sep 5, 2019 via email

draveness commented Sep 5, 2019

draveness commented Sep 5, 2019

Huang-Wei commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

draveness commented Sep 11, 2019

draveness commented Sep 11, 2019 via email

draveness commented Sep 12, 2019

draveness commented Sep 12, 2019 via email

Huang-Wei commented Sep 12, 2019

draveness commented Sep 12, 2019 via email

draveness commented Sep 12, 2019 via email

Huang-Wei Sep 14, 2019

draveness Sep 14, 2019

draveness commented Sep 14, 2019

k82cn commented Sep 16, 2019

draveness commented Sep 16, 2019 via email

Huang-Wei commented Sep 16, 2019

draveness commented Sep 17, 2019 via email

Huang-Wei commented Sep 17, 2019

draveness commented Sep 18, 2019

	sched := scheduler.NewFromConfig(config)
	scheduler.AddAllEventHandlers(sched,
	v1.DefaultSchedulerName,
	informerFactory.Core().V1().Nodes(),
	informerFactory.Core().V1().Pods(),
	informerFactory.Core().V1().PersistentVolumes(),
	informerFactory.Core().V1().PersistentVolumeClaims(),
	informerFactory.Core().V1().Services(),
	informerFactory.Storage().V1().StorageClasses(),
	informerFactory.Storage().V1beta1().CSINodes(),
	)

	informerFactory.Start(stopCh)
	sched.Run()

feat(scheduler): use api server to watch scheduled pods #82338

feat(scheduler): use api server to watch scheduled pods #82338

Conversation

draveness commented Sep 4, 2019 • edited

Huang-Wei left a comment

Choose a reason for hiding this comment

k82cn Sep 5, 2019

Choose a reason for hiding this comment

draveness Sep 5, 2019

Choose a reason for hiding this comment

draveness commented Sep 5, 2019 via email

draveness commented Sep 5, 2019

draveness commented Sep 5, 2019

Huang-Wei commented Sep 6, 2019

k8s-ci-robot commented Sep 6, 2019

draveness commented Sep 11, 2019

draveness commented Sep 11, 2019 via email

draveness commented Sep 12, 2019

draveness commented Sep 12, 2019 via email

Huang-Wei commented Sep 12, 2019

draveness commented Sep 12, 2019 via email

draveness commented Sep 12, 2019 via email

Huang-Wei Sep 14, 2019

Choose a reason for hiding this comment

draveness Sep 14, 2019

Choose a reason for hiding this comment

draveness commented Sep 14, 2019

k82cn commented Sep 16, 2019

draveness commented Sep 16, 2019 via email

Huang-Wei commented Sep 16, 2019

draveness commented Sep 17, 2019 via email

Huang-Wei commented Sep 17, 2019

draveness commented Sep 18, 2019

draveness commented Sep 4, 2019 •

edited