leader election lock based on scheduler name #42961

wanghaoran1988 · 2017-03-12T08:10:03Z

What this PR does / why we need it:
This pr changed the leader election lock based on scheduler name.
Which issue this PR fixes :
fixes #39032

Special notes for your reviewer:

Release note:

[scheduling]Fix a bug for multiple-schedulers that you cannot start a second scheduler without disabling leader-elect if the default scheduler has leader-elect enabled(default). We changed the leader election lock based on scheduler name.

k8s-ci-robot · 2017-03-12T08:10:04Z

Hi @wanghaoran1988. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with @k8s-bot ok to test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

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.

k8s-reviewable · 2017-03-12T08:10:08Z

This change is

k8s-github-robot · 2017-03-12T08:38:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

The following people have approved this PR: wanghaoran1988

Needs approval from an approver in each of these OWNERS Files:

plugin/cmd/kube-scheduler/OWNERS

We suggest the following people:
cc @davidopp
You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

wanghaoran1988 · 2017-03-12T12:41:32Z

/cc @k82cn @timothysc

k82cn · 2017-03-12T13:07:57Z

plugin/cmd/kube-scheduler/app/server.go

@@ -114,7 +114,7 @@ func Run(s *options.SchedulerServer) error {
 	rl := &resourcelock.EndpointsLock{
 		EndpointsMeta: metav1.ObjectMeta{
 			Namespace: "kube-system",
-			Name:      "kube-scheduler",
+			Name:      s.SchedulerName,


prefer to "kube-scheduler-" + s.SchedulerName

k82cn · 2017-03-12T13:08:07Z

@k8s-bot ok to test

timothysc · 2017-03-12T13:50:06Z

@wanghaoran1988 please address comment and re-bazel and we can get the change in.

wanghaoran1988 · 2017-03-12T15:48:09Z

@timothysc @k82cn Comments addressed, and I am already passed hack/verify-bazel.sh locally, not clear why it failed, could you point me what should I do?

ravilr · 2017-03-12T19:04:45Z

@timothysc IMO, this change warrants a release note. because, in HA setups with multiple instances of scheduler running and where upgrades are rolled out in a rolling manner, with this change after the upgrade, the new upgraded instance might acquire the lock while one of the old instances still holding the lock with old endpoint name. So, one should stop all old instances of scheduler before rolling out this change?

davidopp · 2017-03-12T21:50:02Z

cc/ @kubernetes/sig-scheduling-pr-reviews

timothysc · 2017-03-13T01:27:12Z

@wanghaoran1988 please add a release note.

@k8s-bot test this #IGNORE

wanghaoran1988 · 2017-03-13T05:27:36Z

@timothysc Release note added

k82cn · 2017-03-13T06:37:41Z

@wanghaoran1988 , please try /release-note command :).

timothysc · 2017-03-13T13:30:28Z

@k8s-bot test this #IGNORE

jayunit100 · 2017-03-13T14:19:25Z

thanks for this ! This might be why the scheduler_perf tests failed and went into lock frenzy when i tried to start them w/ a scheduling server

k82cn

please put generated codes in different commit.

k82cn · 2017-03-23T03:01:48Z

plugin/cmd/kube-scheduler/app/options/options.go

@@ -72,6 +72,8 @@ func (s *SchedulerServer) AddFlags(fs *pflag.FlagSet) {
 	fs.Float32Var(&s.KubeAPIQPS, "kube-api-qps", s.KubeAPIQPS, "QPS to use while talking with kubernetes apiserver")
 	fs.Int32Var(&s.KubeAPIBurst, "kube-api-burst", s.KubeAPIBurst, "Burst to use while talking with kubernetes apiserver")
 	fs.StringVar(&s.SchedulerName, "scheduler-name", s.SchedulerName, "Name of the scheduler, used to select which pods will be processed by this scheduler, based on pod's \"spec.SchedulerName\".")
+	fs.StringVar(&s.LockObjectNamespace, "lock-object-namespace", s.LockObjectNamespace, "Define the namespace of the lock object")
+	fs.StringVar(&s.LockObjectName, "lock-object-name", s.LockObjectName, "Define the name of the lock object")


The default value from cli should be DefaultLockObjectName .

@k82cn Addressed

k82cn · 2017-03-23T03:02:21Z

pkg/api/v1/types.go

@@ -4377,4 +4377,7 @@ const (
 	// When the --failure-domains scheduler flag is not specified,
 	// DefaultFailureDomains defines the set of label keys used when TopologyKey is empty in PreferredDuringScheduling anti-affinity.
 	DefaultFailureDomains string = metav1.LabelHostname + "," + metav1.LabelZoneFailureDomain + "," + metav1.LabelZoneRegion
+
+	// "kube-scheduler" is the default lock object name
+	DefaultLockObjectName = "kube-scheduler"


Where's DefaultLockObjectNamespace?

@k82cn I am using api.NamespaceSystem, correct me if I am wrong :)

prefer to DefaultLockObjectNamespace = api.NamespaceSystem

@k82cn Done

wanghaoran1988 · 2017-03-27T06:11:17Z

@k8s-bot gci gce e2e test this

liggitt · 2017-03-30T17:53:03Z

pkg/api/types.go

+	DefaultLockObjectNamespace string = NamespaceSystem
+
+	// "kube-scheduler" is the default lock object name
+	DefaultLockObjectName = "kube-scheduler"


if this is exported from the API package, it needs a name that indicates it is for the scheduler

these really seems like they belong in the component config package, with names that scope them to the scheduler

@liggitt Does SchedulerDefaultLockObjectNamespace, SchedulerDefaultLockObjectNamespace make sense to you ?

sure, something like that, and moved to the component config package

wanghaoran1988 · 2017-04-05T08:47:46Z

@k8s-bot non-cri e2e test this

timothysc · 2017-04-05T14:21:10Z

pkg/api/v1/types.go

@@ -218,6 +218,8 @@ const (
 	NamespaceDefault string = "default"
 	// NamespaceAll is the default argument to specify on a context when you want to list or filter resources across all namespaces
 	NamespaceAll string = ""
+	// NamespaceSystem is the system namespace where we place system components.
+	NamespaceSystem string = "kube-system"


Is this still needed now that you have SchedulerDefaultLockObjectNamespace ?

It looks like this is unused.

wanghaoran1988 · 2017-04-06T01:16:32Z

@k8s-bot non-cri e2e test this

wanghaoran1988 · 2017-04-06T13:21:12Z

@timothysc Updated, would you mind take a look again?

k8s-github-robot · 2017-04-06T15:26:14Z

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2017-04-06T16:12:10Z

Automatic merge from submit-queue

lovejoy · 2019-01-25T09:12:52Z

looks like we need to define scheduler-name & lock-object-name if we want to run multiple not-default scheduler

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Mar 12, 2017

k8s-github-robot assigned davidopp Mar 12, 2017

k8s-github-robot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. release-note-label-needed labels Mar 12, 2017

k82cn reviewed Mar 12, 2017

View reviewed changes

timothysc added sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. kind/bug Categorizes issue or PR as related to a bug. release-note-none Denotes a PR that doesn't merit a release note. and removed release-note-label-needed labels Mar 12, 2017

timothysc added this to the v1.6 milestone Mar 12, 2017

timothysc self-assigned this Mar 12, 2017

wanghaoran1988 force-pushed the fix_39032 branch from 33a5389 to 59bb6a7 Compare March 12, 2017 15:43

timothysc added release-note-label-needed and removed release-note-none Denotes a PR that doesn't merit a release note. labels Mar 13, 2017

k8s-github-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Mar 13, 2017

enisoc modified the milestones: v1.6.1, v1.6 Mar 23, 2017

k82cn reviewed Mar 23, 2017

View reviewed changes

wanghaoran1988 force-pushed the fix_39032 branch 3 times, most recently from 6345610 to c76aacb Compare March 27, 2017 05:08

timothysc modified the milestones: v1.7, v1.6.1 Mar 30, 2017

timothysc approved these changes Mar 30, 2017

View reviewed changes

timothysc added approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Mar 30, 2017

liggitt reviewed Mar 30, 2017

View reviewed changes

timothysc removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 30, 2017

wanghaoran1988 force-pushed the fix_39032 branch from c76aacb to c0190cf Compare April 5, 2017 07:28

timothysc reviewed Apr 5, 2017

View reviewed changes

wanghaoran1988 added 2 commits April 5, 2017 22:36

Multiple scheduler leader election support

fcc73d3

generated

251abaa

wanghaoran1988 force-pushed the fix_39032 branch from c0190cf to 251abaa Compare April 5, 2017 14:42

timothysc mentioned this pull request Apr 5, 2017

Scheduler can recieve its policy configuration from a ConfigMap #43892

Merged

timothysc added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 6, 2017

k8s-github-robot merged commit 54f3868 into kubernetes:master Apr 6, 2017

wanghaoran1988 deleted the fix_39032 branch November 22, 2017 05:58

yue9944882 mentioned this pull request Jul 24, 2019

Add resourceLockName options to leader election #80510

Closed

leader election lock based on scheduler name #42961

leader election lock based on scheduler name #42961

Conversation

wanghaoran1988 commented Mar 12, 2017 • edited Loading

k8s-ci-robot commented Mar 12, 2017

k8s-reviewable commented Mar 12, 2017

k8s-github-robot commented Mar 12, 2017

wanghaoran1988 commented Mar 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k82cn commented Mar 12, 2017

timothysc commented Mar 12, 2017

wanghaoran1988 commented Mar 12, 2017

ravilr commented Mar 12, 2017

davidopp commented Mar 12, 2017

timothysc commented Mar 13, 2017 • edited Loading

wanghaoran1988 commented Mar 13, 2017

k82cn commented Mar 13, 2017

timothysc commented Mar 13, 2017

jayunit100 commented Mar 13, 2017 • edited Loading

k82cn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

k82cn Mar 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wanghaoran1988 commented Mar 27, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wanghaoran1988 commented Apr 5, 2017

Choose a reason for hiding this comment

wanghaoran1988 commented Apr 6, 2017

wanghaoran1988 commented Apr 6, 2017

k8s-github-robot commented Apr 6, 2017

k8s-github-robot commented Apr 6, 2017

lovejoy commented Jan 25, 2019

wanghaoran1988 commented Mar 12, 2017 •

edited

Loading

timothysc commented Mar 13, 2017 •

edited

Loading

jayunit100 commented Mar 13, 2017 •

edited

Loading

k82cn Mar 23, 2017 •

edited

Loading