From edff7403861162805781db6b138cdfa7e8e3d6d4 Mon Sep 17 00:00:00 2001 From: chenyw1990 Date: Sat, 20 Feb 2021 14:52:37 +0800 Subject: [PATCH] fix json log format panic, change the flag names in flagIsSet --- .../app/options/globalflags_test.go | 5 ++- .../apis/config/validation/validation_test.go | 45 +++++++++++++++++++ .../cli/globalflag/globalflags.go | 13 ++---- .../cli/globalflag/globalflags_test.go | 5 ++- .../src/k8s.io/component-base/logs/options.go | 33 +++++++++++--- 5 files changed, 83 insertions(+), 18 deletions(-) diff --git a/cmd/kube-apiserver/app/options/globalflags_test.go b/cmd/kube-apiserver/app/options/globalflags_test.go index 7510efc74e4..f4f6dac6c3e 100644 --- a/cmd/kube-apiserver/app/options/globalflags_test.go +++ b/cmd/kube-apiserver/app/options/globalflags_test.go @@ -36,6 +36,7 @@ func TestAddCustomGlobalFlags(t *testing.T) { // flag set. This allows us to test against all global flags from // flags.CommandLine. nfs := namedFlagSets.FlagSet("test") + nfs.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) globalflag.AddGlobalFlags(nfs, "test-cmd") AddCustomGlobalFlags(nfs) @@ -46,11 +47,11 @@ func TestAddCustomGlobalFlags(t *testing.T) { // Get all flags from flags.CommandLine, except flag `test.*`. wantedFlag := []string{"help"} - pflag.CommandLine.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) pflag.CommandLine.AddGoFlagSet(flag.CommandLine) + normalizeFunc := nfs.GetNormalizeFunc() pflag.VisitAll(func(flag *pflag.Flag) { if !strings.Contains(flag.Name, "test.") { - wantedFlag = append(wantedFlag, flag.Name) + wantedFlag = append(wantedFlag, string(normalizeFunc(nfs, flag.Name))) } }) sort.Strings(wantedFlag) diff --git a/pkg/kubelet/apis/config/validation/validation_test.go b/pkg/kubelet/apis/config/validation/validation_test.go index 641b4154b35..f577be2ded0 100644 --- a/pkg/kubelet/apis/config/validation/validation_test.go +++ b/pkg/kubelet/apis/config/validation/validation_test.go @@ -22,6 +22,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" + componentbaseconfig "k8s.io/component-base/config" kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config" ) @@ -106,6 +107,50 @@ func TestValidateKubeletConfiguration(t *testing.T) { t.Errorf("expect no errors, got %v", allErrors) } + successCase3 := &kubeletconfig.KubeletConfiguration{ + CgroupsPerQOS: true, + EnforceNodeAllocatable: []string{"pods"}, + SystemReservedCgroup: "", + KubeReservedCgroup: "", + SystemCgroups: "", + CgroupRoot: "", + EventBurst: 10, + EventRecordQPS: 5, + HealthzPort: 10248, + ImageGCHighThresholdPercent: 85, + ImageGCLowThresholdPercent: 80, + IPTablesDropBit: 15, + IPTablesMasqueradeBit: 14, + KubeAPIBurst: 10, + KubeAPIQPS: 5, + MaxOpenFiles: 1000000, + MaxPods: 110, + OOMScoreAdj: -999, + PodsPerCore: 100, + Port: 65535, + ReadOnlyPort: 0, + RegistryBurst: 10, + RegistryPullQPS: 5, + HairpinMode: kubeletconfig.PromiscuousBridge, + NodeLeaseDurationSeconds: 1, + CPUCFSQuotaPeriod: metav1.Duration{Duration: 50 * time.Millisecond}, + ReservedSystemCPUs: "0-3", + TopologyManagerScope: kubeletconfig.ContainerTopologyManagerScope, + TopologyManagerPolicy: kubeletconfig.NoneTopologyManagerPolicy, + ShutdownGracePeriod: metav1.Duration{Duration: 10 * time.Minute}, + ShutdownGracePeriodCriticalPods: metav1.Duration{Duration: 0}, + FeatureGates: map[string]bool{ + "CustomCPUCFSQuotaPeriod": true, + "GracefulNodeShutdown": true, + }, + Logging: componentbaseconfig.LoggingConfiguration{ + Format: "json", + }, + } + if allErrors := ValidateKubeletConfiguration(successCase3); allErrors != nil { + t.Errorf("expect no errors, got %v", allErrors) + } + errorCase1 := &kubeletconfig.KubeletConfiguration{ CgroupsPerQOS: false, EnforceNodeAllocatable: []string{"pods", "system-reserved", "kube-reserved", "illegal-key"}, diff --git a/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go b/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go index acdf8b688f2..7a5640be743 100644 --- a/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go +++ b/staging/src/k8s.io/component-base/cli/globalflag/globalflags.go @@ -20,7 +20,6 @@ import ( "flag" "fmt" "os" - "strings" "github.com/spf13/pflag" "k8s.io/component-base/logs" @@ -41,23 +40,19 @@ func AddGlobalFlags(fs *pflag.FlagSet, name string) { func addKlogFlags(fs *pflag.FlagSet) { local := flag.NewFlagSet(os.Args[0], flag.ExitOnError) klog.InitFlags(local) + normalizeFunc := fs.GetNormalizeFunc() local.VisitAll(func(fl *flag.Flag) { - fl.Name = normalize(fl.Name) + fl.Name = string(normalizeFunc(fs, fl.Name)) fs.AddGoFlag(fl) }) } -// normalize replaces underscores with hyphens -// we should always use hyphens instead of underscores when registering component flags -func normalize(s string) string { - return strings.Replace(s, "_", "-", -1) -} - // Register adds a flag to local that targets the Value associated with the Flag named globalName in flag.CommandLine. func Register(local *pflag.FlagSet, globalName string) { if f := flag.CommandLine.Lookup(globalName); f != nil { pflagFlag := pflag.PFlagFromGoFlag(f) - pflagFlag.Name = normalize(pflagFlag.Name) + normalizeFunc := local.GetNormalizeFunc() + pflagFlag.Name = string(normalizeFunc(local, pflagFlag.Name)) local.AddFlag(pflagFlag) } else { panic(fmt.Sprintf("failed to find flag in global flagset (flag): %s", globalName)) diff --git a/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go b/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go index 2a97a450238..27780c76864 100644 --- a/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go +++ b/staging/src/k8s.io/component-base/cli/globalflag/globalflags_test.go @@ -24,12 +24,14 @@ import ( "testing" "github.com/spf13/pflag" + cliflag "k8s.io/component-base/cli/flag" ) func TestAddGlobalFlags(t *testing.T) { namedFlagSets := &cliflag.NamedFlagSets{} nfs := namedFlagSets.FlagSet("global") + nfs.SetNormalizeFunc(cliflag.WordSepNormalizeFunc) AddGlobalFlags(nfs, "test-cmd") actualFlag := []string{} @@ -40,9 +42,10 @@ func TestAddGlobalFlags(t *testing.T) { // Get all flags from flags.CommandLine, except flag `test.*`. wantedFlag := []string{"help"} pflag.CommandLine.AddGoFlagSet(flag.CommandLine) + normalizeFunc := nfs.GetNormalizeFunc() pflag.VisitAll(func(flag *pflag.Flag) { if !strings.Contains(flag.Name, "test.") { - wantedFlag = append(wantedFlag, normalize(flag.Name)) + wantedFlag = append(wantedFlag, string(normalizeFunc(nfs, flag.Name))) } }) sort.Strings(wantedFlag) diff --git a/staging/src/k8s.io/component-base/logs/options.go b/staging/src/k8s.io/component-base/logs/options.go index 1698e5232f0..1b53a6b2cde 100644 --- a/staging/src/k8s.io/component-base/logs/options.go +++ b/staging/src/k8s.io/component-base/logs/options.go @@ -57,9 +57,9 @@ func NewOptions() *Options { func (o *Options) Validate() []error { errs := []error{} if o.LogFormat != defaultLogFormat { - allFlags := unsupportedLoggingFlags() + allFlags := unsupportedLoggingFlags(hyphensToUnderscores) for _, fname := range allFlags { - if flagIsSet(fname) { + if flagIsSet(fname, hyphensToUnderscores) { errs = append(errs, fmt.Errorf("non-default logging format doesn't honor flag: %s", fname)) } } @@ -70,11 +70,23 @@ func (o *Options) Validate() []error { return errs } -func flagIsSet(name string) bool { +// hyphensToUnderscores replaces hyphens with underscores +// we should always use underscores instead of hyphens when validate flags +func hyphensToUnderscores(s string) string { + return strings.Replace(s, "-", "_", -1) +} + +func flagIsSet(name string, normalizeFunc func(name string) string) bool { f := flag.Lookup(name) if f != nil { return f.DefValue != f.Value.String() } + if normalizeFunc != nil { + f = flag.Lookup(normalizeFunc(name)) + if f != nil { + return f.DefValue != f.Value.String() + } + } pf := pflag.Lookup(name) if pf != nil { return pf.DefValue != pf.Value.String() @@ -84,7 +96,12 @@ func flagIsSet(name string) bool { // AddFlags add logging-format flag func (o *Options) AddFlags(fs *pflag.FlagSet) { - unsupportedFlags := fmt.Sprintf("--%s", strings.Join(unsupportedLoggingFlags(), ", --")) + normalizeFunc := func(name string) string { + f := fs.GetNormalizeFunc() + return string(f(fs, name)) + } + + unsupportedFlags := fmt.Sprintf("--%s", strings.Join(unsupportedLoggingFlags(normalizeFunc), ", --")) formats := fmt.Sprintf(`"%s"`, strings.Join(logRegistry.List(), `", "`)) fs.StringVar(&o.LogFormat, logFormatFlagName, defaultLogFormat, fmt.Sprintf("Sets the log format. Permitted formats: %s.\nNon-default formats don't honor these flags: %s.\nNon-default choices are currently alpha and subject to change without warning.", formats, unsupportedFlags)) @@ -109,7 +126,7 @@ func (o *Options) Get() (logr.Logger, error) { return logRegistry.Get(o.LogFormat) } -func unsupportedLoggingFlags() []string { +func unsupportedLoggingFlags(normalizeFunc func(name string) string) []string { allFlags := []string{} // k8s.io/klog flags @@ -117,7 +134,11 @@ func unsupportedLoggingFlags() []string { klog.InitFlags(fs) fs.VisitAll(func(flag *flag.Flag) { if _, found := supportedLogsFlags[flag.Name]; !found { - allFlags = append(allFlags, strings.Replace(flag.Name, "_", "-", -1)) + name := flag.Name + if normalizeFunc != nil { + name = normalizeFunc(name) + } + allFlags = append(allFlags, name) } })