Merge pull request #99257 from chenyw1990/fixedLogFormatJsonPanic

fix json log format panic, change the flag name in flagIsSet
This commit is contained in:
Kubernetes Prow Robot 2021-03-09 04:27:29 -08:00 committed by GitHub
commit 4d969ac90d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 83 additions and 18 deletions

View File

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

View File

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

View File

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

View File

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

View File

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