diff --git a/cmd/kubelet/app/options/options.go b/cmd/kubelet/app/options/options.go index f084dfa2584..af5271ffeb4 100644 --- a/cmd/kubelet/app/options/options.go +++ b/cmd/kubelet/app/options/options.go @@ -30,6 +30,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" utilfeature "k8s.io/apiserver/pkg/util/feature" cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/logs" "k8s.io/kubelet/config/v1beta1" kubeletapis "k8s.io/kubelet/pkg/apis" "k8s.io/kubernetes/pkg/apis/core" @@ -533,9 +534,7 @@ func AddKubeletConfigFlags(mainfs *pflag.FlagSet, c *kubeletconfig.KubeletConfig fs.StringSliceVar(&c.EnforceNodeAllocatable, "enforce-node-allocatable", c.EnforceNodeAllocatable, "A comma separated list of levels of node allocatable enforcement to be enforced by kubelet. Acceptable options are 'none', 'pods', 'system-reserved', and 'kube-reserved'. If the latter two options are specified, '--system-reserved-cgroup' and '--kube-reserved-cgroup' must also be set, respectively. If 'none' is specified, no additional options should be set. See https://kubernetes.io/docs/tasks/administer-cluster/reserve-compute-resources/ for more details.") fs.StringVar(&c.SystemReservedCgroup, "system-reserved-cgroup", c.SystemReservedCgroup, "Absolute name of the top level cgroup that is used to manage non-kubernetes components for which compute resources were reserved via '--system-reserved' flag. Ex. '/system-reserved'. [default='']") fs.StringVar(&c.KubeReservedCgroup, "kube-reserved-cgroup", c.KubeReservedCgroup, "Absolute name of the top level cgroup that is used to manage kubernetes components for which compute resources were reserved via '--kube-reserved' flag. Ex. '/kube-reserved'. [default='']") - fs.StringVar(&c.Logging.Format, "logging-format", c.Logging.Format, "Sets the log format. Permitted formats: \"text\", \"json\". \nNon-default formats don't honor these flags: --add-dir-header, --alsologtostderr, --log-backtrace-at, --log-dir, --log-file, --log-file-max-size, --logtostderr, --skip-headers, --skip-log-headers, --stderrthreshold, --log-flush-frequency. \nNon-default choices are currently alpha and subject to change without warning.") - fs.BoolVar(&c.Logging.Sanitization, "experimental-logging-sanitization", c.Logging.Sanitization, `[Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). -Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production.`) + logs.BindLoggingFlags(&c.Logging, fs) // Graduated experimental flags, kept for backward compatibility fs.BoolVar(&c.KernelMemcgNotification, "experimental-kernel-memcg-notification", c.KernelMemcgNotification, "Use kernelMemcgNotification configuration, this flag will be removed in 1.23.") diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index bb52d49cf4c..475731d118a 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -434,9 +434,7 @@ func UnsecuredDependencies(s *options.KubeletServer, featureGate featuregate.Fea // Otherwise, the caller is assumed to have set up the Dependencies object and a default one will // not be generated. func Run(ctx context.Context, s *options.KubeletServer, kubeDeps *kubelet.Dependencies, featureGate featuregate.FeatureGate) error { - logOption := logs.NewOptions() - logOption.LogFormat = s.Logging.Format - logOption.LogSanitization = s.Logging.Sanitization + logOption := &logs.Options{Config: s.Logging} logOption.Apply() // To help debugging, immediately log version klog.InfoS("Kubelet version", "kubeletVersion", version.Get()) diff --git a/pkg/kubelet/apis/config/validation/validation.go b/pkg/kubelet/apis/config/validation/validation.go index 69b08a8229d..34de5dd02cf 100644 --- a/pkg/kubelet/apis/config/validation/validation.go +++ b/pkg/kubelet/apis/config/validation/validation.go @@ -23,6 +23,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" utilerrors "k8s.io/apimachinery/pkg/util/errors" utilvalidation "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/apimachinery/pkg/util/validation/field" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/component-base/logs" "k8s.io/component-base/metrics" @@ -200,11 +201,8 @@ func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration) error } allErrors = append(allErrors, metrics.ValidateShowHiddenMetricsVersion(kc.ShowHiddenMetricsForVersion)...) - logOption := logs.NewOptions() - if kc.Logging.Format != "" { - logOption.LogFormat = kc.Logging.Format + if errs := logs.ValidateLoggingConfiguration(&kc.Logging, field.NewPath("logging")); len(errs) > 0 { + allErrors = append(allErrors, errs.ToAggregate().Errors()...) } - allErrors = append(allErrors, logOption.Validate()...) - return utilerrors.NewAggregate(allErrors) } diff --git a/pkg/kubelet/apis/config/validation/validation_test.go b/pkg/kubelet/apis/config/validation/validation_test.go index f577be2ded0..d9a33e71d57 100644 --- a/pkg/kubelet/apis/config/validation/validation_test.go +++ b/pkg/kubelet/apis/config/validation/validation_test.go @@ -62,6 +62,9 @@ func TestValidateKubeletConfiguration(t *testing.T) { "CustomCPUCFSQuotaPeriod": true, "GracefulNodeShutdown": true, }, + Logging: componentbaseconfig.LoggingConfiguration{ + Format: "text", + }, } if allErrors := ValidateKubeletConfiguration(successCase1); allErrors != nil { t.Errorf("expect no errors, got %v", allErrors) @@ -102,6 +105,9 @@ func TestValidateKubeletConfiguration(t *testing.T) { FeatureGates: map[string]bool{ "CustomCPUCFSQuotaPeriod": true, }, + Logging: componentbaseconfig.LoggingConfiguration{ + Format: "text", + }, } if allErrors := ValidateKubeletConfiguration(successCase2); allErrors != nil { t.Errorf("expect no errors, got %v", allErrors) @@ -178,8 +184,11 @@ func TestValidateKubeletConfiguration(t *testing.T) { CPUCFSQuotaPeriod: metav1.Duration{Duration: 100 * time.Millisecond}, ShutdownGracePeriod: metav1.Duration{Duration: 30 * time.Second}, ShutdownGracePeriodCriticalPods: metav1.Duration{Duration: 60 * time.Second}, + Logging: componentbaseconfig.LoggingConfiguration{ + Format: "", + }, } - const numErrsErrorCase1 = 28 + const numErrsErrorCase1 = 29 if allErrors := ValidateKubeletConfiguration(errorCase1); len(allErrors.(utilerrors.Aggregate).Errors()) != numErrsErrorCase1 { t.Errorf("expect %d errors, got %v", numErrsErrorCase1, len(allErrors.(utilerrors.Aggregate).Errors())) } @@ -220,6 +229,9 @@ func TestValidateKubeletConfiguration(t *testing.T) { "CustomCPUCFSQuotaPeriod": true, "GracefulNodeShutdown": true, }, + Logging: componentbaseconfig.LoggingConfiguration{ + Format: "text", + }, } const numErrsErrorCase2 = 3 if allErrors := ValidateKubeletConfiguration(errorCase2); len(allErrors.(utilerrors.Aggregate).Errors()) != numErrsErrorCase2 { diff --git a/staging/src/k8s.io/component-base/logs/config.go b/staging/src/k8s.io/component-base/logs/config.go index ee05d823881..55d28a6cdbd 100644 --- a/staging/src/k8s.io/component-base/logs/config.go +++ b/staging/src/k8s.io/component-base/logs/config.go @@ -24,6 +24,7 @@ import ( "github.com/spf13/pflag" "k8s.io/klog/v2" + "k8s.io/component-base/config" json "k8s.io/component-base/logs/json" ) @@ -49,19 +50,17 @@ var supportedLogsFlags = map[string]struct{}{ } // BindLoggingFlags binds the Options struct fields to a flagset -func BindLoggingFlags(o *Options, fs *pflag.FlagSet) { +func BindLoggingFlags(c *config.LoggingConfiguration, fs *pflag.FlagSet) { 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, "logging-format", 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)) - + fs.StringVar(&c.Format, "logging-format", c.Format, 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)) // No new log formats should be added after generation is of flag options LogRegistry.Freeze() - fs.BoolVar(&o.LogSanitization, "experimental-logging-sanitization", o.LogSanitization, `[Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). + fs.BoolVar(&c.Sanitization, "experimental-logging-sanitization", c.Sanitization, `[Experimental] When enabled prevents logging of fields tagged as sensitive (passwords, keys, tokens). Runtime log sanitization may introduce significant computation overhead and therefore should not be enabled in production.`) } diff --git a/staging/src/k8s.io/component-base/logs/options.go b/staging/src/k8s.io/component-base/logs/options.go index be0420ca194..46e52a0a138 100644 --- a/staging/src/k8s.io/component-base/logs/options.go +++ b/staging/src/k8s.io/component-base/logs/options.go @@ -17,49 +17,50 @@ limitations under the License. package logs import ( - "github.com/go-logr/logr" "github.com/spf13/pflag" "k8s.io/klog/v2" + "k8s.io/component-base/config" + "k8s.io/component-base/config/v1alpha1" "k8s.io/component-base/logs/sanitization" ) // Options has klog format parameters type Options struct { - LogFormat string - LogSanitization bool + Config config.LoggingConfiguration } // NewOptions return new klog options func NewOptions() *Options { - return &Options{ - LogFormat: DefaultLogFormat, - } + c := v1alpha1.LoggingConfiguration{} + v1alpha1.RecommendedLoggingConfiguration(&c) + o := &Options{} + v1alpha1.Convert_v1alpha1_LoggingConfiguration_To_config_LoggingConfiguration(&c, &o.Config, nil) + return o } // Validate verifies if any unsupported flag is set // for non-default logging format func (o *Options) Validate() []error { - return ValidateLoggingConfiguration(o) + errs := ValidateLoggingConfiguration(&o.Config, nil) + if len(errs) != 0 { + return errs.ToAggregate().Errors() + } + return nil } // AddFlags add logging-format flag func (o *Options) AddFlags(fs *pflag.FlagSet) { - BindLoggingFlags(o, fs) + BindLoggingFlags(&o.Config, fs) } // Apply set klog logger from LogFormat type func (o *Options) Apply() { // if log format not exists, use nil loggr - loggr, _ := o.Get() + loggr, _ := LogRegistry.Get(o.Config.Format) klog.SetLogger(loggr) - if o.LogSanitization { + if o.Config.Sanitization { klog.SetLogFilter(&sanitization.SanitizingFilter{}) } } - -// Get logger with LogFormat field -func (o *Options) Get() (logr.Logger, error) { - return LogRegistry.Get(o.LogFormat) -} diff --git a/staging/src/k8s.io/component-base/logs/options_test.go b/staging/src/k8s.io/component-base/logs/options_test.go index 18a72d3dfb9..09b44bcef20 100644 --- a/staging/src/k8s.io/component-base/logs/options_test.go +++ b/staging/src/k8s.io/component-base/logs/options_test.go @@ -18,11 +18,13 @@ package logs import ( "bytes" - "fmt" "testing" "github.com/spf13/pflag" "github.com/stretchr/testify/assert" + + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/component-base/config" ) func TestFlags(t *testing.T) { @@ -48,7 +50,7 @@ func TestOptions(t *testing.T) { name string args []string want *Options - errs []error + errs field.ErrorList }{ { name: "Default log format", @@ -63,24 +65,35 @@ func TestOptions(t *testing.T) { name: "JSON log format", args: []string{"--logging-format=json"}, want: &Options{ - LogFormat: JSONLogFormat, + Config: config.LoggingConfiguration{ + Format: JSONLogFormat, + }, }, }, { name: "log sanitization", args: []string{"--experimental-logging-sanitization"}, want: &Options{ - LogFormat: DefaultLogFormat, - LogSanitization: true, + Config: config.LoggingConfiguration{ + Format: DefaultLogFormat, + Sanitization: true, + }, }, }, { name: "Unsupported log format", args: []string{"--logging-format=test"}, want: &Options{ - LogFormat: "test", + Config: config.LoggingConfiguration{ + Format: "test", + }, }, - errs: []error{fmt.Errorf("unsupported log format: test")}, + errs: field.ErrorList{&field.Error{ + Type: "FieldValueInvalid", + Field: "format", + BadValue: "test", + Detail: "Unsupported log format", + }}, }, } @@ -95,7 +108,8 @@ func TestOptions(t *testing.T) { } errs := o.Validate() if !assert.ElementsMatch(t, tc.errs, errs) { - t.Errorf("Wrong Validate() result for %q. expect %v, got %v", tc.name, tc.errs, errs) + t.Errorf("Wrong Validate() result for %q.\n expect:\t%+v\n got:\t%+v", tc.name, tc.errs, errs) + } }) } diff --git a/staging/src/k8s.io/component-base/logs/validate.go b/staging/src/k8s.io/component-base/logs/validate.go index 0fca18a0b40..86f1cecf34e 100644 --- a/staging/src/k8s.io/component-base/logs/validate.go +++ b/staging/src/k8s.io/component-base/logs/validate.go @@ -22,20 +22,23 @@ import ( "strings" "github.com/spf13/pflag" + + "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/component-base/config" ) -func ValidateLoggingConfiguration(o *Options) []error { - errs := []error{} - if o.LogFormat != DefaultLogFormat { +func ValidateLoggingConfiguration(c *config.LoggingConfiguration, fldPath *field.Path) field.ErrorList { + errs := field.ErrorList{} + if c.Format != DefaultLogFormat { allFlags := UnsupportedLoggingFlags(hyphensToUnderscores) for _, fname := range allFlags { if flagIsSet(fname, hyphensToUnderscores) { - errs = append(errs, fmt.Errorf("non-default logging format doesn't honor flag: %s", fname)) + errs = append(errs, field.Invalid(fldPath.Child("format"), c.Format, fmt.Sprintf("Non-default format doesn't honor flag: %s", fname))) } } } - if _, err := o.Get(); err != nil { - errs = append(errs, fmt.Errorf("unsupported log format: %s", o.LogFormat)) + if _, err := LogRegistry.Get(c.Format); err != nil { + errs = append(errs, field.Invalid(fldPath.Child("format"), c.Format, "Unsupported log format")) } return errs }