diff --git a/cmd/kubelet/app/options/BUILD b/cmd/kubelet/app/options/BUILD index 9b19054d13..079dfc02a3 100644 --- a/cmd/kubelet/app/options/BUILD +++ b/cmd/kubelet/app/options/BUILD @@ -3,6 +3,7 @@ package(default_visibility = ["//visibility:public"]) load( "@io_bazel_rules_go//go:def.bzl", "go_library", + "go_test", ) go_library( @@ -38,3 +39,14 @@ filegroup( srcs = [":package-srcs"], tags = ["automanaged"], ) + +go_test( + name = "go_default_test", + srcs = ["options_test.go"], + library = ":go_default_library", + deps = [ + "//vendor/github.com/spf13/pflag:go_default_library", + "//vendor/k8s.io/apimachinery/pkg/util/diff:go_default_library", + "//vendor/k8s.io/apiserver/pkg/util/flag:go_default_library", + ], +) diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index a227a41d80..577834f22e 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -22,6 +22,8 @@ import ( _ "net/http/pprof" "strings" + "github.com/spf13/pflag" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/apiserver/pkg/util/flag" utilflag "k8s.io/apiserver/pkg/util/flag" @@ -32,8 +34,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/v1alpha1" kubeletconfigvalidation "k8s.io/kubernetes/pkg/kubelet/apis/kubeletconfig/validation" utiltaints "k8s.io/kubernetes/pkg/util/taints" - - "github.com/spf13/pflag" ) const ( diff --git a/cmd/kubelet/app/options/options_test.go b/cmd/kubelet/app/options/options_test.go new file mode 100644 index 0000000000..a52d290620 --- /dev/null +++ b/cmd/kubelet/app/options/options_test.go @@ -0,0 +1,143 @@ +/* +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 options + +import ( + "fmt" + "reflect" + "testing" + + "github.com/spf13/pflag" + + "k8s.io/apimachinery/pkg/util/diff" + utilflag "k8s.io/apiserver/pkg/util/flag" +) + +func newKubeletServerOrDie() *KubeletServer { + s, err := NewKubeletServer() + if err != nil { + panic(err) + } + return s +} + +func cleanFlags(s *KubeletServer) { + s.KubeConfig = utilflag.NewStringFlag(s.KubeConfig.Value()) + s.DynamicConfigDir = utilflag.NewStringFlag(s.DynamicConfigDir.Value()) + s.InitConfigDir = utilflag.NewStringFlag(s.InitConfigDir.Value()) +} + +// TestRoundTrip ensures that flag values from the Kubelet can be serialized +// to arguments and then read back and have the same value. Also catches cases +// where the default value reported by the flag is not actually allowed to be +// specified. +func TestRoundTrip(t *testing.T) { + testCases := []struct { + name string + inputFlags func() *KubeletServer + outputFlags func() *KubeletServer + flagDefaulter func(*pflag.FlagSet) + skipDefault bool + err bool + expectArgs bool + }{ + { + name: "default flags are eliminated", + inputFlags: newKubeletServerOrDie, + outputFlags: newKubeletServerOrDie, + flagDefaulter: newKubeletServerOrDie().AddFlags, + err: false, + expectArgs: false, + }, + { + name: "default flag values round trip", + inputFlags: newKubeletServerOrDie, + outputFlags: newKubeletServerOrDie, + flagDefaulter: func(*pflag.FlagSet) {}, + err: false, + expectArgs: true, + }, + { + name: "nil address does not fail for optional argument", + inputFlags: func() *KubeletServer { + s := newKubeletServerOrDie() + s.HealthzBindAddress = "" + return s + }, + outputFlags: func() *KubeletServer { + s := newKubeletServerOrDie() + s.HealthzBindAddress = "" + return s + }, + flagDefaulter: func(*pflag.FlagSet) {}, + err: false, + expectArgs: true, + }, + } + for _, testCase := range testCases { + modifiedFlags := testCase.inputFlags() + args := asArgs(modifiedFlags.AddFlags, testCase.flagDefaulter) + if testCase.expectArgs != (len(args) > 0) { + t.Errorf("%s: unexpected args: %v", testCase.name, args) + continue + } + t.Logf("%s: args: %v", testCase.name, args) + flagSet := pflag.NewFlagSet("output", pflag.ContinueOnError) + outputFlags := testCase.outputFlags() + outputFlags.AddFlags(flagSet) + if err := flagSet.Parse(args); err != nil { + if !testCase.err { + t.Errorf("%s: unexpected flag error: %v", testCase.name, err) + } + continue + } + cleanFlags(outputFlags) + if !reflect.DeepEqual(modifiedFlags, outputFlags) { + t.Errorf("%s: flags did not round trip: %s", testCase.name, diff.ObjectReflectDiff(modifiedFlags, outputFlags)) + continue + } + } +} + +func asArgs(fn, defaultFn func(*pflag.FlagSet)) []string { + fs := pflag.NewFlagSet("extended", pflag.ContinueOnError) + fn(fs) + defaults := pflag.NewFlagSet("defaults", pflag.ContinueOnError) + defaultFn(defaults) + var args []string + fs.VisitAll(func(flag *pflag.Flag) { + s := flag.Value.String() + var defaultValue string + if defaultFlag := defaults.Lookup(flag.Name); defaultFlag != nil { + defaultValue = defaultFlag.Value.String() + if s == defaultValue { + return + } + } + if values, err := fs.GetStringSlice(flag.Name); err == nil { + for _, s := range values { + args = append(args, fmt.Sprintf("--%s=%s", flag.Name, s)) + } + } else { + if len(s) == 0 { + s = defaultValue + } + args = append(args, fmt.Sprintf("--%s=%s", flag.Name, s)) + } + }) + return args +} diff --git a/pkg/apis/componentconfig/helpers.go b/pkg/apis/componentconfig/helpers.go index 6f94f8516f..39fc082da0 100644 --- a/pkg/apis/componentconfig/helpers.go +++ b/pkg/apis/componentconfig/helpers.go @@ -35,6 +35,10 @@ type IPVar struct { } func (v IPVar) Set(s string) error { + if len(s) == 0 { + v.Val = nil + return nil + } if net.ParseIP(s) == nil { return fmt.Errorf("%q is not a valid IP address", s) } diff --git a/pkg/util/taints/taints.go b/pkg/util/taints/taints.go index 5957adfb27..a8f6434703 100644 --- a/pkg/util/taints/taints.go +++ b/pkg/util/taints/taints.go @@ -76,6 +76,10 @@ type taintsVar struct { } func (t taintsVar) Set(s string) error { + if len(s) == 0 { + *t.ptr = nil + return nil + } sts := strings.Split(s, ",") var taints []api.Taint for _, st := range sts { @@ -91,7 +95,7 @@ func (t taintsVar) Set(s string) error { func (t taintsVar) String() string { if len(*t.ptr) == 0 { - return "" + return "" } var taints []string for _, taint := range *t.ptr {