fix scheduler client construction from configuration files #65507

liggitt · 2018-06-27T00:12:58Z

clients were being constructed from options prior to loading the specified configuration file. this is a regression from 1.10 behavior.

/sig scheduling
/kind bug
/priority critical-urgent
/milestone v1.11

fixes a regression in kube-scheduler to properly load client connection information from a `--config` file that references a kubeconfig file

liggitt · 2018-06-27T00:20:20Z

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

@@ -197,37 +197,37 @@ func (o *Options) Validate() []error {

 // Config return a scheduler config object
 func (o *Options) Config() (*schedulerappconfig.Config, error) {
+	c := &schedulerappconfig.Config{}
+	if err := o.ApplyTo(c); err != nil {


we apply the options to the config struct first (which handles loading config from a file), then use the c.ComponentConfig.... for the rest of the function

ravisantoshgudimetla · 2018-06-27T00:59:36Z

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

+
+func TestSchedulerOptions(t *testing.T) {
+	// temp dir
+	tmpDir, err := ioutil.TempDir("", "scheduler-options")


q: Is it safe to assume that tempdir always exists especially when running in containers?

Many unit tests already do this, so yes

ravisantoshgudimetla · 2018-06-27T01:00:01Z

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

+	}
+
+	// Insulate this test from picking up in-cluster config when run inside a pod
+	// We can't assume we have permissions to write to /var/run/secrets/... from a unit test to mock in-cluster config for testing


This is pretty clever!!

ravisantoshgudimetla

/lgtm

Thanks @liggitt.

liggitt · 2018-06-27T01:08:03Z

/retest

liggitt · 2018-06-27T01:53:27Z

/retest

k82cn · 2018-06-27T03:21:56Z

/approve

bsalamat

Thanks a lot, @liggitt! I only have minor comment about the test, but it is not important and given the urgency of this fix, I approve it.

/lgtm

bsalamat · 2018-06-27T04:27:45Z

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

+				} else if !strings.Contains(err.Error(), tc.expectedError) {
+					t.Errorf("expected %q, got %q", tc.expectedError, err.Error())
+				}
+				return


it would be better to continue instead of return here and below.

This is inside a t.Run function… returning continues to the next test case

Yes, you are right! I missed that it is inside t.Run.
Thanks again!

bsalamat · 2018-06-27T04:28:42Z

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

+	"k8s.io/kubernetes/pkg/apis/componentconfig"
+)
+
+func TestSchedulerOptions(t *testing.T) {


Impressive! Thanks for writing the test.

k8s-ci-robot · 2018-06-27T04:31:23Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bsalamat, k82cn, liggitt, ravisantoshgudimetla

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:

~~cmd/kube-scheduler/OWNERS~~ [bsalamat,k82cn]

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

k8s-github-robot · 2018-06-27T04:32:41Z

[MILESTONENOTIFIER] Milestone Pull Request: Up-to-date for process

@bsalamat @liggitt @ravisantoshgudimetla

Pull Request Labels

sig/scheduling: Pull Request will be escalated to these SIGs if needed.
priority/critical-urgent: Never automatically move pull request out of a release milestone; continually escalate to contributor and SIG through all available channels.
kind/bug: Fixes a bug discovered during the current release.

Help

k8s-github-robot · 2018-06-27T04:46:43Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-06-27T06:06:13Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here.

This reverts commit c67b56f. Upstream fix: kubernetes/kubernetes#65507

…7-upstream-release-1.11 Automatic merge from submit-queue. Automated cherry pick of #65507: fix scheduler client construction from configuration files Cherry pick of #65507 on release-1.11. #65507: fix scheduler client construction from configuration files

k8s-ci-robot added the sig/scheduling Categorizes an issue or PR as relevant to SIG Scheduling. label Jun 27, 2018

k8s-ci-robot added this to the v1.11 milestone Jun 27, 2018

k8s-ci-robot requested review from bsalamat and wojtek-t June 27, 2018 00:13

k8s-github-robot added the milestone/needs-approval label Jun 27, 2018

liggitt commented Jun 27, 2018

View reviewed changes

liggitt force-pushed the scheduler-config-client branch from c01906a to 3499352 Compare June 27, 2018 00:22

fix scheduler client construction from configuration files

14d31af

liggitt force-pushed the scheduler-config-client branch from 3499352 to 14d31af Compare June 27, 2018 00:37

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels Jun 27, 2018

liggitt mentioned this pull request Jun 27, 2018

Automated cherry pick of #65507: fix scheduler client construction from configuration files #65512

Merged

ravisantoshgudimetla reviewed Jun 27, 2018

View reviewed changes

k8s-ci-robot assigned ravisantoshgudimetla Jun 27, 2018

ravisantoshgudimetla approved these changes Jun 27, 2018

View reviewed changes

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 27, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 27, 2018

bsalamat approved these changes Jun 27, 2018

View reviewed changes

k8s-ci-robot assigned bsalamat Jun 27, 2018

bsalamat added the status/approved-for-milestone label Jun 27, 2018

k8s-github-robot removed the milestone/needs-approval label Jun 27, 2018

k8s-github-robot merged commit 2b18760 into kubernetes:master Jun 27, 2018

rfranzke added a commit to gardener/gardener that referenced this pull request Jun 27, 2018

Revert "kube-scheduler startup workaround for Kubernetes 1.11"

69242a8

This reverts commit c67b56f. Upstream fix: kubernetes/kubernetes#65507

liggitt deleted the scheduler-config-client branch June 28, 2018 20:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix scheduler client construction from configuration files #65507

fix scheduler client construction from configuration files #65507

liggitt commented Jun 27, 2018 •

edited

Loading

liggitt Jun 27, 2018 •

edited

Loading

ravisantoshgudimetla Jun 27, 2018

liggitt Jun 27, 2018

ravisantoshgudimetla Jun 27, 2018

ravisantoshgudimetla left a comment

liggitt commented Jun 27, 2018

liggitt commented Jun 27, 2018

k82cn commented Jun 27, 2018

bsalamat left a comment

bsalamat Jun 27, 2018

liggitt Jun 27, 2018

bsalamat Jun 27, 2018

bsalamat Jun 27, 2018

k8s-ci-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

fix scheduler client construction from configuration files #65507

fix scheduler client construction from configuration files #65507

Conversation

liggitt commented Jun 27, 2018 • edited Loading

liggitt Jun 27, 2018 • edited Loading

Choose a reason for hiding this comment

ravisantoshgudimetla Jun 27, 2018

Choose a reason for hiding this comment

liggitt Jun 27, 2018

Choose a reason for hiding this comment

ravisantoshgudimetla Jun 27, 2018

Choose a reason for hiding this comment

ravisantoshgudimetla left a comment

Choose a reason for hiding this comment

liggitt commented Jun 27, 2018

liggitt commented Jun 27, 2018

k82cn commented Jun 27, 2018

bsalamat left a comment

Choose a reason for hiding this comment

bsalamat Jun 27, 2018

Choose a reason for hiding this comment

liggitt Jun 27, 2018

Choose a reason for hiding this comment

bsalamat Jun 27, 2018

Choose a reason for hiding this comment

bsalamat Jun 27, 2018

Choose a reason for hiding this comment

k8s-ci-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

k8s-github-robot commented Jun 27, 2018

liggitt commented Jun 27, 2018 •

edited

Loading

liggitt Jun 27, 2018 •

edited

Loading