Merge pull request #65507 from liggitt/scheduler-config-client

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

fix scheduler client construction from configuration files

fixes #65483

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

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

cmd/kube-scheduler/app/options/BUILD

@@ -56,7 +56,10 @@ filegroup(
 
 go_test(
     name = "go_default_test",
-    srcs = ["insecure_serving_test.go"],
+    srcs = [
+        "insecure_serving_test.go",
+        "options_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//cmd/controller-manager/app/options:go_default_library",

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 {
+		return nil, err
+	}
+
 	// prepare kube clients.
-	client, leaderElectionClient, eventClient, err := createClients(o.ComponentConfig.ClientConnection, o.Master)
+	client, leaderElectionClient, eventClient, err := createClients(c.ComponentConfig.ClientConnection, o.Master)
 	if err != nil {
 		return nil, err
 	}
 
 	// Prepare event clients.
 	eventBroadcaster := record.NewBroadcaster()
-	recorder := eventBroadcaster.NewRecorder(legacyscheme.Scheme, corev1.EventSource{Component: o.ComponentConfig.SchedulerName})
+	recorder := eventBroadcaster.NewRecorder(legacyscheme.Scheme, corev1.EventSource{Component: c.ComponentConfig.SchedulerName})
 
 	// Set up leader election if enabled.
 	var leaderElectionConfig *leaderelection.LeaderElectionConfig
-	if o.ComponentConfig.LeaderElection.LeaderElect {
+	if c.ComponentConfig.LeaderElection.LeaderElect {
-		leaderElectionConfig, err = makeLeaderElectionConfig(o.ComponentConfig.LeaderElection, leaderElectionClient, recorder)
+		leaderElectionConfig, err = makeLeaderElectionConfig(c.ComponentConfig.LeaderElection, leaderElectionClient, recorder)
 		if err != nil {
 			return nil, err
 		}
 	}
 
-	c := &schedulerappconfig.Config{
+	c.Client = client
-		Client:          client,
+	c.InformerFactory = informers.NewSharedInformerFactory(client, 0)
-		InformerFactory: informers.NewSharedInformerFactory(client, 0),
+	c.PodInformer = factory.NewPodInformer(client, 0)
-		PodInformer:     factory.NewPodInformer(client, 0),
+	c.EventClient = eventClient
-		EventClient:     eventClient,
+	c.Recorder = recorder
-		Recorder:        recorder,
+	c.Broadcaster = eventBroadcaster
-		Broadcaster:     eventBroadcaster,
+	c.LeaderElection = leaderElectionConfig
-		LeaderElection:  leaderElectionConfig,
-	}
-	if err := o.ApplyTo(c); err != nil {
-		return nil, err
-	}
 
 	return c, nil
 }

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

@@ -0,0 +1,193 @@
+/*
+Copyright 2018 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package options
+
+import (
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"k8s.io/kubernetes/pkg/apis/componentconfig"
+)
+
+func TestSchedulerOptions(t *testing.T) {
+	// temp dir
+	tmpDir, err := ioutil.TempDir("", "scheduler-options")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir)
+
+	// record the username requests were made with
+	username := ""
+	// https server
+	server := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		username, _, _ = req.BasicAuth()
+		if username == "" {
+			username = "none, tls"
+		}
+		w.WriteHeader(200)
+		w.Write([]byte(`ok`))
+	}))
+	defer server.Close()
+	// http server
+	insecureserver := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		username, _, _ = req.BasicAuth()
+		if username == "" {
+			username = "none, http"
+		}
+		w.WriteHeader(200)
+		w.Write([]byte(`ok`))
+	}))
+	defer insecureserver.Close()
+
+	// config file and kubeconfig
+	configFile := filepath.Join(tmpDir, "scheduler.yaml")
+	configKubeconfig := filepath.Join(tmpDir, "config.kubeconfig")
+	if err := ioutil.WriteFile(configFile, []byte(fmt.Sprintf(`
+apiVersion: componentconfig/v1alpha1
+kind: KubeSchedulerConfiguration
+clientConnection:
+  kubeconfig: "%s"
+leaderElection:
+  leaderElect: true`, configKubeconfig)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+	if err := ioutil.WriteFile(configKubeconfig, []byte(fmt.Sprintf(`
+apiVersion: v1
+kind: Config
+clusters:
+- cluster:
+    insecure-skip-tls-verify: true
+    server: %s
+  name: default
+contexts:
+- context:
+    cluster: default
+    user: default
+  name: default
+current-context: default
+users:
+- name: default
+  user:
+    username: config
+`, server.URL)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+
+	// flag-specified kubeconfig
+	flagKubeconfig := filepath.Join(tmpDir, "flag.kubeconfig")
+	if err := ioutil.WriteFile(flagKubeconfig, []byte(fmt.Sprintf(`
+apiVersion: v1
+kind: Config
+clusters:
+- cluster:
+    insecure-skip-tls-verify: true
+    server: %s
+  name: default
+contexts:
+- context:
+    cluster: default
+    user: default
+  name: default
+current-context: default
+users:
+- name: default
+  user:
+    username: flag
+`, server.URL)), os.FileMode(0600)); err != nil {
+		t.Fatal(err)
+	}
+
+	// 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
+	originalHost := os.Getenv("KUBERNETES_SERVICE_HOST")
+	if len(originalHost) > 0 {
+		os.Setenv("KUBERNETES_SERVICE_HOST", "")
+		defer os.Setenv("KUBERNETES_SERVICE_HOST", originalHost)
+	}
+
+	testcases := []struct {
+		name             string
+		options          *Options
+		expectedUsername string
+		expectedError    string
+	}{
+		{
+			name:             "config file",
+			options:          &Options{ConfigFile: configFile},
+			expectedUsername: "config",
+		},
+		{
+			name: "kubeconfig flag",
+			options: &Options{
+				ComponentConfig: componentconfig.KubeSchedulerConfiguration{
+					ClientConnection: componentconfig.ClientConnectionConfiguration{
+						KubeConfigFile: flagKubeconfig}}},
+			expectedUsername: "flag",
+		},
+		{
+			name:             "overridden master",
+			options:          &Options{Master: insecureserver.URL},
+			expectedUsername: "none, http",
+		},
+		{
+			name:          "no config",
+			options:       &Options{},
+			expectedError: "no configuration has been provided",
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			// create the config
+			config, err := tc.options.Config()
+
+			// handle errors
+			if err != nil {
+				if tc.expectedError == "" {
+					t.Error(err)
+				} else if !strings.Contains(err.Error(), tc.expectedError) {
+					t.Errorf("expected %q, got %q", tc.expectedError, err.Error())
+				}
+				return
+			}
+
+			// ensure we have a client
+			if config.Client == nil {
+				t.Error("unexpected nil client")
+				return
+			}
+
+			// test the client talks to the endpoint we expect with the credentials we expect
+			username = ""
+			_, err = config.Client.Discovery().RESTClient().Get().AbsPath("/").DoRaw()
+			if err != nil {
+				t.Error(err)
+				return
+			}
+			if username != tc.expectedUsername {
+				t.Errorf("expected server call with user %s, got %s", tc.expectedUsername, username)
+			}
+		})
+	}
+}