Merge pull request #51380 from mtaufen/dkcfg-test-file-load

Automatic merge from submit-queue (batch tested with PRs 49861, 50933, 51380, 50688, 51305)

Test loading Kubelet config from a file

**What this PR does / why we need it**:
Adds a test for loading kubelet config from a file, part of improving https://github.com/kubernetes/kubernetes/issues/50217

**Release note**:
```release-note
NONE
```

pkg/kubelet/kubeletconfig/configfiles/BUILD

@@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"])
 load(
     "@io_bazel_rules_go//go:def.bzl",
     "go_library",
+    "go_test",
 )
 
 go_library(
@@ -29,3 +30,19 @@ filegroup(
     srcs = [":package-srcs"],
     tags = ["automanaged"],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["configfiles_test.go"],
+    library = ":go_default_library",
+    deps = [
+        "//pkg/kubelet/apis/kubeletconfig:go_default_library",
+        "//pkg/kubelet/apis/kubeletconfig/scheme:go_default_library",
+        "//pkg/kubelet/apis/kubeletconfig/v1alpha1:go_default_library",
+        "//pkg/kubelet/kubeletconfig/util/files:go_default_library",
+        "//pkg/kubelet/kubeletconfig/util/filesystem:go_default_library",
+        "//pkg/kubelet/kubeletconfig/util/test:go_default_library",
+        "//vendor/github.com/davecgh/go-spew/spew:go_default_library",
+        "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library",
+    ],
+)

pkg/kubelet/kubeletconfig/configfiles/configfiles.go

@@ -27,6 +27,8 @@ import (
 	utilfs "k8s.io/kubernetes/pkg/kubelet/kubeletconfig/util/filesystem"
 )
 
+const kubeletFile = "kubelet"
+
 // Loader loads configuration from a storage layer
 type Loader interface {
 	// Load loads and returns the KubeletConfiguration from the storage layer, or an error if a configuration could not be loaded
@@ -43,8 +45,8 @@ type fsLoader struct {
 	configDir string
 }
 
-// NewFSLoader returns a Loader that loads a KubeletConfiguration from the files in `configDir`
+// NewFsLoader returns a Loader that loads a KubeletConfiguration from the files in `configDir`
-func NewFSLoader(fs utilfs.Filesystem, configDir string) (Loader, error) {
+func NewFsLoader(fs utilfs.Filesystem, configDir string) (Loader, error) {
 	_, kubeletCodecs, err := kubeletscheme.NewSchemeAndCodecs()
 	if err != nil {
 		return nil, err
@@ -58,10 +60,8 @@ func NewFSLoader(fs utilfs.Filesystem, configDir string) (Loader, error) {
 }
 
 func (loader *fsLoader) Load() (*kubeletconfig.KubeletConfiguration, error) {
-	errfmt := fmt.Sprintf("failed to load Kubelet config files from %q, error: ", loader.configDir) + "%v"
-
 	// require the config be in a file called "kubelet"
-	path := filepath.Join(loader.configDir, "kubelet")
+	path := filepath.Join(loader.configDir, kubeletFile)
 	data, err := loader.fs.ReadFile(path)
 	if err != nil {
 		return nil, fmt.Errorf("failed to read init config file %q, error: %v", path, err)
@@ -69,7 +69,7 @@ func (loader *fsLoader) Load() (*kubeletconfig.KubeletConfiguration, error) {
 
 	// no configuration is an error, some parameters are required
 	if len(data) == 0 {
-		return nil, fmt.Errorf(errfmt, fmt.Errorf("config file was empty, but some parameters are required"))
+		return nil, fmt.Errorf("init config file %q was empty, but some parameters are required", path)
 	}
 
 	return utilcodec.DecodeKubeletConfiguration(loader.kubeletCodecs, data)

pkg/kubelet/kubeletconfig/configfiles/configfiles_test.go

@@ -0,0 +1,115 @@
+/*
+Copyright 2017 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 configfiles
+
+import (
+	"fmt"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/davecgh/go-spew/spew"
+
+	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	"k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig"
+	kubeletscheme "k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/scheme"
+	kubeletconfigv1alpha1 "k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/v1alpha1"
+	utilfiles "k8s.io/kubernetes/pkg/kubelet/kubeletconfig/util/files"
+	utilfs "k8s.io/kubernetes/pkg/kubelet/kubeletconfig/util/filesystem"
+	utiltest "k8s.io/kubernetes/pkg/kubelet/kubeletconfig/util/test"
+)
+
+func addFile(fs utilfs.Filesystem, path string, file string) error {
+	if err := utilfiles.EnsureDir(fs, filepath.Dir(path)); err != nil {
+		return err
+	}
+	if err := utilfiles.ReplaceFile(fs, path, []byte(file)); err != nil {
+		return err
+	}
+	return nil
+}
+
+func TestLoad(t *testing.T) {
+	kubeletScheme, _, err := kubeletscheme.NewSchemeAndCodecs()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// get the built-in default configuration
+	external := &kubeletconfigv1alpha1.KubeletConfiguration{}
+	kubeletScheme.Default(external)
+	defaultConfig := &kubeletconfig.KubeletConfiguration{}
+	err = kubeletScheme.Convert(external, defaultConfig, nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	cases := []struct {
+		desc   string
+		file   string
+		expect *kubeletconfig.KubeletConfiguration
+		err    string
+	}{
+		{"empty data", ``, nil, "was empty"},
+		// invalid format
+		{"invalid yaml", `*`, nil, "failed to decode"},
+		{"invalid json", `{*`, nil, "failed to decode"},
+		// invalid object
+		{"missing kind", `{"apiVersion":"kubeletconfig/v1alpha1"}`, nil, "failed to decode"},
+		{"missing version", `{"kind":"KubeletConfiguration"}`, nil, "failed to decode"},
+		{"unregistered kind", `{"kind":"BogusKind","apiVersion":"kubeletconfig/v1alpha1"}`, nil, "failed to decode"},
+		{"unregistered version", `{"kind":"KubeletConfiguration","apiVersion":"bogusversion"}`, nil, "failed to decode"},
+		// empty object with correct kind and version should result in the defaults for that kind and version
+		{"default from yaml", `kind: KubeletConfiguration
+apiVersion: kubeletconfig/v1alpha1`, defaultConfig, ""},
+		{"default from json", `{"kind":"KubeletConfiguration","apiVersion":"kubeletconfig/v1alpha1"}`, defaultConfig, ""},
+	}
+
+	fs := utilfs.NewFakeFs()
+	for i := range cases {
+		dir := fmt.Sprintf("/%d", i)
+		if err := addFile(fs, filepath.Join(dir, kubeletFile), cases[i].file); err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		loader, err := NewFsLoader(fs, dir)
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+		kc, err := loader.Load()
+		if utiltest.SkipRest(t, cases[i].desc, err, cases[i].err) {
+			continue
+		}
+		// we expect the parsed configuration to match what we described in the ConfigMap
+		if !apiequality.Semantic.DeepEqual(cases[i].expect, kc) {
+			t.Errorf("case %q, expect config %s but got %s", cases[i].desc, spew.Sdump(cases[i].expect), spew.Sdump(kc))
+		}
+	}
+
+	// finally test for a missing file
+	desc := "missing kubelet file"
+	contains := "failed to read"
+	loader, err := NewFsLoader(fs, "/fake")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	_, err = loader.Load()
+	if err == nil {
+		t.Errorf("case %q, expect error to contain %q but got nil error", desc, contains)
+	} else if !strings.Contains(err.Error(), contains) {
+		t.Errorf("case %q, expect error to contain %q but got %q", desc, contains, err.Error())
+	}
+}

pkg/kubelet/kubeletconfig/controller.go

@@ -87,7 +87,7 @@ func NewController(initConfigDir string,
 
 	var initLoader configfiles.Loader
 	if len(initConfigDir) > 0 {
-		initLoader, err = configfiles.NewFSLoader(fs, initConfigDir)
+		initLoader, err = configfiles.NewFsLoader(fs, initConfigDir)
 		if err != nil {
 			return nil, err
 		}