Merge pull request kubernetes#99257 from chenyw1990/fixedLogFormatJsonPanic

fix json log format panic, change the flag name in flagIsSet

Author: k8s-ci-robot

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)

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"},

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))

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)

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,15 +126,19 @@ 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
 	fs := &flag.FlagSet{}
 	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)
 		}
 	})