From d55a01645be0c552be13b40ee20bd76e017f81de Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 18 Jan 2022 16:50:55 +0100 Subject: [PATCH] component-base/logs: add feature gates It is useful to have the ability to control whether alpha or beta features are enabled. We can group features under LoggingAlphaOptions and LoggingBetaOptions because the configuration is designed so that each feature individually must be enabled via its own option. Currently, the JSON format itself is beta (graduated in 1.23) but additional options for it were only added in 1.23 and thus are still alpha: $ go run ./staging/src/k8s.io/component-base/logs/example/cmd/logger.go --logging-format=json --log-json-split-stream --log-json-info-buffer-size 1M --feature-gates LoggingBetaOptions=false [format: Forbidden: Log format json is BETA and disabled, see LoggingBetaOptions feature, options.json.splitStream: Forbidden: Feature LoggingAlphaOptions is disabled, options.json.infoBufferSize: Forbidden: Feature LoggingAlphaOptions is disabled] $ go run ./staging/src/k8s.io/component-base/logs/example/cmd/logger.go --logging-format=json --log-json-split-stream --log-json-info-buffer-size 1M [options.json.splitStream: Forbidden: Feature LoggingAlphaOptions is disabled, options.json.infoBufferSize: Forbidden: Feature LoggingAlphaOptions is disabled] This is the same approach that was taken for CPUManagerPolicyAlphaOptions and CPUManagerPolicyBetaOptions. --- cmd/kube-apiserver/app/server.go | 2 +- .../app/controllermanager.go | 2 +- cmd/kube-scheduler/app/server.go | 3 +- cmd/kubelet/app/server.go | 2 +- pkg/features/kube_features.go | 2 + .../apis/config/validation/validation.go | 2 +- .../featuregate/feature_gate.go | 14 ++++ .../component-base/logs/api/v1/features.go | 57 ++++++++++++++++ .../logs/api/v1/features_test.go | 47 ++++++++++++++ .../component-base/logs/api/v1/options.go | 51 ++++++++++++--- .../logs/api/v1/options_test.go | 12 ++-- .../component-base/logs/api/v1/registry.go | 7 ++ .../component-base/logs/api/v1/types.go | 11 ++-- .../logs/api/v1/validate_test.go | 35 +++++++++- .../component-base/logs/example/cmd/logger.go | 9 ++- .../k8s.io/component-base/logs/json/json.go | 5 ++ .../logs/json/register/register_test.go | 65 +++++++++++++++++-- 17 files changed, 296 insertions(+), 30 deletions(-) create mode 100644 staging/src/k8s.io/component-base/logs/api/v1/features.go create mode 100644 staging/src/k8s.io/component-base/logs/api/v1/features_test.go diff --git a/cmd/kube-apiserver/app/server.go b/cmd/kube-apiserver/app/server.go index 2e3f9b8ad76a..df878a1abf25 100644 --- a/cmd/kube-apiserver/app/server.go +++ b/cmd/kube-apiserver/app/server.go @@ -104,7 +104,7 @@ cluster's shared state through which all other components interact.`, // Activate logging as soon as possible, after that // show flags with the final logging configuration. - if err := s.Logs.ValidateAndApply(); err != nil { + if err := s.Logs.ValidateAndApply(utilfeature.DefaultFeatureGate); err != nil { return err } cliflag.PrintFlags(fs) diff --git a/cmd/kube-controller-manager/app/controllermanager.go b/cmd/kube-controller-manager/app/controllermanager.go index e74aa6e2122b..51f084af2b1f 100644 --- a/cmd/kube-controller-manager/app/controllermanager.go +++ b/cmd/kube-controller-manager/app/controllermanager.go @@ -118,7 +118,7 @@ controller, and serviceaccounts controller.`, // Activate logging as soon as possible, after that // show flags with the final logging configuration. - if err := s.Logs.ValidateAndApply(); err != nil { + if err := s.Logs.ValidateAndApply(utilfeature.DefaultFeatureGate); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) os.Exit(1) } diff --git a/cmd/kube-scheduler/app/server.go b/cmd/kube-scheduler/app/server.go index b5bcb6be9f72..dbb68228c1b0 100644 --- a/cmd/kube-scheduler/app/server.go +++ b/cmd/kube-scheduler/app/server.go @@ -36,6 +36,7 @@ import ( "k8s.io/apiserver/pkg/server/healthz" "k8s.io/apiserver/pkg/server/mux" "k8s.io/apiserver/pkg/server/routes" + utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/client-go/informers" "k8s.io/client-go/kubernetes/scheme" "k8s.io/client-go/tools/events" @@ -113,7 +114,7 @@ func runCommand(cmd *cobra.Command, opts *options.Options, registryOptions ...Op // Activate logging as soon as possible, after that // show flags with the final logging configuration. - if err := opts.Logs.ValidateAndApply(); err != nil { + if err := opts.Logs.ValidateAndApply(utilfeature.DefaultFeatureGate); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) os.Exit(1) } diff --git a/cmd/kubelet/app/server.go b/cmd/kubelet/app/server.go index e302e2935ea2..93592c7548b6 100644 --- a/cmd/kubelet/app/server.go +++ b/cmd/kubelet/app/server.go @@ -225,7 +225,7 @@ HTTP server: The kubelet can also listen for HTTP and respond to a simple API // Config and flags parsed, now we can initialize logging. logs.InitLogs() - if err := kubeletConfig.Logging.ValidateAndApply(); err != nil { + if err := kubeletConfig.Logging.ValidateAndApply(utilfeature.DefaultFeatureGate); err != nil { klog.ErrorS(err, "Failed to initialize logging") os.Exit(1) } diff --git a/pkg/features/kube_features.go b/pkg/features/kube_features.go index 8f00c6e0ef54..1a642325e97b 100644 --- a/pkg/features/kube_features.go +++ b/pkg/features/kube_features.go @@ -21,6 +21,7 @@ import ( genericfeatures "k8s.io/apiserver/pkg/features" utilfeature "k8s.io/apiserver/pkg/util/feature" "k8s.io/component-base/featuregate" + logsapi "k8s.io/component-base/logs/api/v1" ) const ( @@ -844,6 +845,7 @@ const ( func init() { runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(defaultKubernetesFeatureGates)) + runtime.Must(utilfeature.DefaultMutableFeatureGate.Add(logsapi.FeatureGates)) } // defaultKubernetesFeatureGates consists of all known Kubernetes-specific feature keys. diff --git a/pkg/kubelet/apis/config/validation/validation.go b/pkg/kubelet/apis/config/validation/validation.go index 8edd932abc16..f1716e25de3b 100644 --- a/pkg/kubelet/apis/config/validation/validation.go +++ b/pkg/kubelet/apis/config/validation/validation.go @@ -236,7 +236,7 @@ func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration) error } allErrors = append(allErrors, metrics.ValidateShowHiddenMetricsVersion(kc.ShowHiddenMetricsForVersion)...) - if errs := kc.Logging.ValidateAsField(field.NewPath("logging")); len(errs) > 0 { + if errs := kc.Logging.ValidateAsField(localFeatureGate, field.NewPath("logging")); len(errs) > 0 { allErrors = append(allErrors, errs.ToAggregate().Errors()...) } diff --git a/staging/src/k8s.io/component-base/featuregate/feature_gate.go b/staging/src/k8s.io/component-base/featuregate/feature_gate.go index c7166d80b849..00e2e2bb7e8c 100644 --- a/staging/src/k8s.io/component-base/featuregate/feature_gate.go +++ b/staging/src/k8s.io/component-base/featuregate/feature_gate.go @@ -373,3 +373,17 @@ func (f *featureGate) DeepCopy() MutableFeatureGate { closed: f.closed, } } + +// Enabled is a wrapper around FeatureGate.Enabled. If the feature gate is nil +// or the key is not known, it returns the default value, otherwise +// the result of FeatureGate.Enabled. +func EnabledOrDefault(f FeatureGate, key Feature, defaultEnabled bool) bool { + if f != nil { + for knownKey := range f.DeepCopy().GetAll() { + if knownKey == key { + return f.Enabled(key) + } + } + } + return defaultEnabled +} diff --git a/staging/src/k8s.io/component-base/logs/api/v1/features.go b/staging/src/k8s.io/component-base/logs/api/v1/features.go new file mode 100644 index 000000000000..e0611d7f4345 --- /dev/null +++ b/staging/src/k8s.io/component-base/logs/api/v1/features.go @@ -0,0 +1,57 @@ +/* +Copyright 2022 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 v1 + +import ( + "k8s.io/component-base/featuregate" +) + +const ( + // owner: @pohly + // alpha: v1.24 + // beta: see LoggingBetaOptions + // + // Allow fine-tuning of experimental, alpha-quality logging options. + // + // Per https://groups.google.com/g/kubernetes-sig-architecture/c/Nxsc7pfe5rw/m/vF2djJh0BAAJ + // we want to avoid a proliferation of feature gates. This feature gate: + // - will guard *a group* of logging options whose quality level is alpha. + // - will never graduate to beta or stable. + LoggingAlphaOptions featuregate.Feature = "LoggingAlphaOptions" + + // owner: @pohly + // alpha: see LoggingAlphaOptions + // beta: v1.24 + // + // Allow fine-tuning of experimental, beta-quality logging options. + // + // Per https://groups.google.com/g/kubernetes-sig-architecture/c/Nxsc7pfe5rw/m/vF2djJh0BAAJ + // we want to avoid a proliferation of feature gates. This feature gate: + // - will guard *a group* of logging options whose quality level is beta. + // - is thus *introduced* as beta + // - will never graduate to stable. + LoggingBetaOptions featuregate.Feature = "LoggingBetaOptions" +) + +// FeatureGates defines the feature gates checked by +// LoggingConfiguration.ValidateAndApply. They must be added to a MutableFeatureGate +// by the user of the package, otherwise that function uses the defaults +// defined here. +var FeatureGates = map[featuregate.Feature]featuregate.FeatureSpec{ + LoggingAlphaOptions: {Default: false, PreRelease: featuregate.Alpha}, + LoggingBetaOptions: {Default: true, PreRelease: featuregate.Beta}, +} diff --git a/staging/src/k8s.io/component-base/logs/api/v1/features_test.go b/staging/src/k8s.io/component-base/logs/api/v1/features_test.go new file mode 100644 index 000000000000..29b578e89b2d --- /dev/null +++ b/staging/src/k8s.io/component-base/logs/api/v1/features_test.go @@ -0,0 +1,47 @@ +/* +Copyright 2022 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 v1 + +import ( + "k8s.io/component-base/featuregate" +) + +var ( + // pre-defined feature gates with the features from this package in a + // certain state (default, all enabled, all disabled). + defaultFeatureGate, enabledFeatureGate, disabledFeatureGate featuregate.FeatureGate +) + +func init() { + mutable := featuregate.NewFeatureGate() + if err := mutable.Add(FeatureGates); err != nil { + panic(err) + } + defaultFeatureGate = mutable + enabled := mutable.DeepCopy() + disabled := mutable.DeepCopy() + for feature := range mutable.GetAll() { + if err := enabled.SetFromMap(map[string]bool{string(feature): true}); err != nil { + panic(err) + } + if err := disabled.SetFromMap(map[string]bool{string(feature): false}); err != nil { + panic(err) + } + } + enabledFeatureGate = enabled + disabledFeatureGate = disabled +} diff --git a/staging/src/k8s.io/component-base/logs/api/v1/options.go b/staging/src/k8s.io/component-base/logs/api/v1/options.go index d2b5345c6a25..6a0859bae1b9 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/options.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/options.go @@ -32,6 +32,7 @@ import ( utilerrors "k8s.io/apimachinery/pkg/util/errors" "k8s.io/apimachinery/pkg/util/validation/field" cliflag "k8s.io/component-base/cli/flag" + "k8s.io/component-base/featuregate" ) var ( @@ -58,8 +59,8 @@ func NewLoggingConfiguration() *LoggingConfiguration { // This should be invoked as early as possible because then the rest of the program // startup (including validation of other options) will already run with the final // logging configuration. -func (c *LoggingConfiguration) ValidateAndApply() error { - errs := c.validate() +func (c *LoggingConfiguration) ValidateAndApply(featureGate featuregate.FeatureGate) error { + errs := c.validate(featureGate) if len(errs) > 0 { return utilerrors.NewAggregate(errs) } @@ -70,8 +71,8 @@ func (c *LoggingConfiguration) ValidateAndApply() error { // validate verifies if any unsupported flag is set for non-default logging // format. It's meant to be used when LoggingConfiguration is used as a // stand-alone struct with command line flags. -func (c *LoggingConfiguration) validate() []error { - errs := c.ValidateAsField(nil) +func (c *LoggingConfiguration) validate(featureGate featuregate.FeatureGate) []error { + errs := c.ValidateAsField(featureGate, nil) if len(errs) != 0 { return errs.ToAggregate().Errors() } @@ -80,7 +81,9 @@ func (c *LoggingConfiguration) validate() []error { // ValidateAsField is a variant of Validate that is meant to be used when // LoggingConfiguration is embedded inside a larger configuration struct. -func (c *LoggingConfiguration) ValidateAsField(fldPath *field.Path) field.ErrorList { +// If featureGate is nil or doesn't have the FeatureGates of this package, +// the defaults for the features are used. +func (c *LoggingConfiguration) ValidateAsField(featureGate featuregate.FeatureGate, fldPath *field.Path) field.ErrorList { errs := field.ErrorList{} if c.Format != DefaultLogFormat { // WordSepNormalizeFunc is just a guess. Commands should use it, @@ -92,8 +95,19 @@ func (c *LoggingConfiguration) ValidateAsField(fldPath *field.Path) field.ErrorL } } } - if _, err := logRegistry.get(c.Format); err != nil { + factory, err := logRegistry.get(c.Format) + if err != nil { errs = append(errs, field.Invalid(fldPath.Child("format"), c.Format, "Unsupported log format")) + } else if factory != nil { + feature := factory.Feature() + if feature != "" && !featuregate.EnabledOrDefault(featureGate, feature, FeatureGates[feature].Default) { + // Look up alpha/beta from actual feature. Should exist, but the API doesn't guarantee it. + status := "experimental" + if featureSpec, ok := featureGate.DeepCopy().GetAll()[feature]; ok { + status = string(featureSpec.PreRelease) + } + errs = append(errs, field.Forbidden(fldPath.Child("format"), fmt.Sprintf("Log format %s is %s and disabled, see %s feature", c.Format, status, feature))) + } } // The type in our struct is uint32, but klog only accepts positive int32. @@ -116,7 +130,26 @@ func (c *LoggingConfiguration) ValidateAsField(fldPath *field.Path) field.ErrorL } } - // Currently nothing to validate for c.Options. + errs = append(errs, c.validateFormatOptions(featureGate, fldPath.Child("options"))...) + return errs +} + +func (c *LoggingConfiguration) validateFormatOptions(featureGate featuregate.FeatureGate, fldPath *field.Path) field.ErrorList { + errs := field.ErrorList{} + errs = append(errs, c.validateJSONOptions(featureGate, fldPath.Child("json"))...) + return errs +} + +func (c *LoggingConfiguration) validateJSONOptions(featureGate featuregate.FeatureGate, fldPath *field.Path) field.ErrorList { + errs := field.ErrorList{} + if !featuregate.EnabledOrDefault(featureGate, LoggingAlphaOptions, FeatureGates[LoggingAlphaOptions].Default) { + if c.Options.JSON.SplitStream { + errs = append(errs, field.Forbidden(fldPath.Child("splitStream"), fmt.Sprintf("Feature %s is disabled", LoggingAlphaOptions))) + } + if c.Options.JSON.InfoBufferSize.Value() != 0 { + errs = append(errs, field.Forbidden(fldPath.Child("infoBufferSize"), fmt.Sprintf("Feature %s is disabled", LoggingAlphaOptions))) + } + } return errs } @@ -138,8 +171,8 @@ func (c *LoggingConfiguration) AddFlags(fs *pflag.FlagSet) { // JSON options. We only register them if "json" is a valid format. The // config file API however always has them. if _, err := logRegistry.get("json"); err == nil { - fs.BoolVar(&c.Options.JSON.SplitStream, "log-json-split-stream", false, "[Experimental] In JSON format, write error messages to stderr and info messages to stdout. The default is to write a single stream to stdout.") - fs.Var(&c.Options.JSON.InfoBufferSize, "log-json-info-buffer-size", "[Experimental] In JSON format with split output streams, the info messages can be buffered for a while to increase performance. The default value of zero bytes disables buffering. The size can be specified as number of bytes (512), multiples of 1000 (1K), multiples of 1024 (2Ki), or powers of those (3M, 4G, 5Mi, 6Gi).") + fs.BoolVar(&c.Options.JSON.SplitStream, "log-json-split-stream", false, "[Alpha] In JSON format, write error messages to stderr and info messages to stdout. The default is to write a single stream to stdout.") + fs.Var(&c.Options.JSON.InfoBufferSize, "log-json-info-buffer-size", "[Alpha] In JSON format with split output streams, the info messages can be buffered for a while to increase performance. The default value of zero bytes disables buffering. The size can be specified as number of bytes (512), multiples of 1000 (1K), multiples of 1024 (2Ki), or powers of those (3M, 4G, 5Mi, 6Gi).") } } diff --git a/staging/src/k8s.io/component-base/logs/api/v1/options_test.go b/staging/src/k8s.io/component-base/logs/api/v1/options_test.go index 73fb08f99fec..88800c00dd2c 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/options_test.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/options_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/component-base/featuregate" "k8s.io/klog/v2" ) @@ -49,10 +50,11 @@ func TestFlags(t *testing.T) { func TestOptions(t *testing.T) { newOptions := NewLoggingConfiguration() testcases := []struct { - name string - args []string - want *LoggingConfiguration - errs field.ErrorList + name string + args []string + featureGate featuregate.FeatureGate + want *LoggingConfiguration + errs field.ErrorList }{ { name: "Default log format", @@ -89,7 +91,7 @@ func TestOptions(t *testing.T) { if !assert.Equal(t, tc.want, c) { t.Errorf("Wrong Validate() result for %q. expect %v, got %v", tc.name, tc.want, c) } - errs := c.ValidateAndApply() + errs := c.ValidateAndApply(defaultFeatureGate) defer klog.StopFlushDaemon() if !assert.ElementsMatch(t, 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/api/v1/registry.go b/staging/src/k8s.io/component-base/logs/api/v1/registry.go index 31a8995c0b5a..90047b73e21d 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/registry.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/registry.go @@ -21,6 +21,8 @@ import ( "sort" "github.com/go-logr/logr" + + "k8s.io/component-base/featuregate" ) var logRegistry = newLogFormatRegistry() @@ -34,6 +36,11 @@ type logFormatRegistry struct { // LogFormatFactory provides support for a certain additional, // non-default log format. type LogFormatFactory interface { + // Feature returns the name of the feature that controls + // support for the log format, if there is one. Must be + // one of the features from FeatureGates. + Feature() featuregate.Feature + // Create returns a logger with the requested configuration. // Returning a flush function for the logger is optional. // If provided, the caller must ensure that it is called diff --git a/staging/src/k8s.io/component-base/logs/api/v1/types.go b/staging/src/k8s.io/component-base/logs/api/v1/types.go index 0a80fadd8a03..24fd1bbc6445 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/types.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/types.go @@ -35,6 +35,9 @@ const ( JSONLogFormat = "json" ) +// The alpha or beta level of structs is the highest stability level of any field +// inside it. Feature gates will get checked during LoggingConfiguration.ValidateAndApply. + // LoggingConfiguration contains logging options. type LoggingConfiguration struct { // Format Flag specifies the structure of log messages. @@ -52,7 +55,7 @@ type LoggingConfiguration struct { // VModule overrides the verbosity threshold for individual files. // Only supported for "text" log format. VModule VModuleConfiguration `json:"vmodule,omitempty"` - // [Experimental] Options holds additional parameters that are specific + // [Alpha] Options holds additional parameters that are specific // to the different logging formats. Only the options for the selected // format get used, but all of them get validated. Options FormatOptions `json:"options,omitempty"` @@ -60,17 +63,17 @@ type LoggingConfiguration struct { // FormatOptions contains options for the different logging formats. type FormatOptions struct { - // [Experimental] JSON contains options for logging format "json". + // [Alpha] JSON contains options for logging format "json". JSON JSONOptions `json:"json,omitempty"` } // JSONOptions contains options for logging format "json". type JSONOptions struct { - // [Experimental] SplitStream redirects error messages to stderr while + // [Alpha] SplitStream redirects error messages to stderr while // info messages go to stdout, with buffering. The default is to write // both to stdout, without buffering. SplitStream bool `json:"splitStream,omitempty"` - // [Experimental] InfoBufferSize sets the size of the info stream when + // [Alpha] InfoBufferSize sets the size of the info stream when // using split streams. The default is zero, which disables buffering. InfoBufferSize resource.QuantityValue `json:"infoBufferSize,omitempty"` } diff --git a/staging/src/k8s.io/component-base/logs/api/v1/validate_test.go b/staging/src/k8s.io/component-base/logs/api/v1/validate_test.go index 0879b4107d3c..86c4267740a9 100644 --- a/staging/src/k8s.io/component-base/logs/api/v1/validate_test.go +++ b/staging/src/k8s.io/component-base/logs/api/v1/validate_test.go @@ -21,11 +21,26 @@ import ( "testing" "github.com/stretchr/testify/assert" + "k8s.io/apimachinery/pkg/api/resource" + "k8s.io/component-base/featuregate" ) func TestValidation(t *testing.T) { + jsonOptionsEnabled := LoggingConfiguration{ + Format: "text", + Options: FormatOptions{ + JSON: JSONOptions{ + SplitStream: true, + InfoBufferSize: resource.QuantityValue{ + Quantity: *resource.NewQuantity(1024, resource.DecimalSI), + }, + }, + }, + } + jsonErrors := `[options.json.splitStream: Forbidden: Feature LoggingAlphaOptions is disabled, options.json.infoBufferSize: Forbidden: Feature LoggingAlphaOptions is disabled]` testcases := map[string]struct { config LoggingConfiguration + featureGate featuregate.FeatureGate expectErrors string }{ "okay": { @@ -105,11 +120,29 @@ func TestValidation(t *testing.T) { }, expectErrors: `[format: Invalid value: "json": Unsupported log format, vmodule: Forbidden: Only supported for text log format]`, }, + "JSON used, default gates": { + config: jsonOptionsEnabled, + featureGate: defaultFeatureGate, + expectErrors: jsonErrors, + }, + "JSON used, disabled gates": { + config: jsonOptionsEnabled, + featureGate: disabledFeatureGate, + expectErrors: jsonErrors, + }, + "JSON used, enabled gates": { + config: jsonOptionsEnabled, + featureGate: enabledFeatureGate, + }, } for name, test := range testcases { t.Run(name, func(t *testing.T) { - errs := test.config.ValidateAsField(nil) + featureGate := test.featureGate + if featureGate == nil { + featureGate = defaultFeatureGate + } + errs := test.config.ValidateAsField(featureGate, nil) if len(errs) == 0 { if test.expectErrors != "" { t.Fatalf("did not get expected error(s): %s", test.expectErrors) diff --git a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go index adc00569d5ff..366ff29c1a43 100644 --- a/staging/src/k8s.io/component-base/logs/example/cmd/logger.go +++ b/staging/src/k8s.io/component-base/logs/example/cmd/logger.go @@ -24,7 +24,9 @@ import ( "github.com/spf13/cobra" "k8s.io/component-base/cli" + "k8s.io/component-base/featuregate" "k8s.io/component-base/logs" + logsapi "k8s.io/component-base/logs/api/v1" "k8s.io/klog/v2" _ "k8s.io/component-base/logs/json/register" @@ -37,10 +39,14 @@ func main() { } func NewLoggerCommand() *cobra.Command { + featureGates := featuregate.NewFeatureGate() + if err := featureGates.Add(logsapi.FeatureGates); err != nil { + panic(err) + } o := logs.NewOptions() cmd := &cobra.Command{ Run: func(cmd *cobra.Command, args []string) { - if err := o.ValidateAndApply(); err != nil { + if err := o.ValidateAndApply(featureGates); err != nil { fmt.Fprintf(os.Stderr, "%v\n", err) os.Exit(1) } @@ -48,6 +54,7 @@ func NewLoggerCommand() *cobra.Command { }, } o.AddFlags(cmd.Flags()) + featureGates.AddFlag(cmd.Flags()) return cmd } diff --git a/staging/src/k8s.io/component-base/logs/json/json.go b/staging/src/k8s.io/component-base/logs/json/json.go index bcef2a73256a..aad92c02438b 100644 --- a/staging/src/k8s.io/component-base/logs/json/json.go +++ b/staging/src/k8s.io/component-base/logs/json/json.go @@ -26,6 +26,7 @@ import ( "go.uber.org/zap" "go.uber.org/zap/zapcore" + "k8s.io/component-base/featuregate" logsapi "k8s.io/component-base/logs/api/v1" ) @@ -82,6 +83,10 @@ type Factory struct{} var _ logsapi.LogFormatFactory = Factory{} +func (f Factory) Feature() featuregate.Feature { + return logsapi.LoggingBetaOptions +} + func (f Factory) Create(options logsapi.FormatOptions) (logr.Logger, func()) { // We intentionally avoid all os.File.Sync calls. Output is unbuffered, // therefore we don't need to flush, and calling the underlying fsync diff --git a/staging/src/k8s.io/component-base/logs/json/register/register_test.go b/staging/src/k8s.io/component-base/logs/json/register/register_test.go index 431c4eb75768..1bd2ab6e730f 100644 --- a/staging/src/k8s.io/component-base/logs/json/register/register_test.go +++ b/staging/src/k8s.io/component-base/logs/json/register/register_test.go @@ -24,6 +24,7 @@ import ( "github.com/stretchr/testify/assert" "k8s.io/apimachinery/pkg/util/validation/field" + "k8s.io/component-base/featuregate" logsapi "k8s.io/component-base/logs/api/v1" ) @@ -41,13 +42,63 @@ func TestJSONFlag(t *testing.T) { } func TestJSONFormatRegister(t *testing.T) { + defaultGates := featuregate.NewFeatureGate() + if err := defaultGates.Add(logsapi.FeatureGates); err != nil { + panic(err) + } + allEnabled := defaultGates.DeepCopy() + allDisabled := defaultGates.DeepCopy() + for feature := range defaultGates.GetAll() { + if err := allEnabled.SetFromMap(map[string]bool{string(feature): true}); err != nil { + panic(err) + } + if err := allDisabled.SetFromMap(map[string]bool{string(feature): false}); err != nil { + panic(err) + } + } newOptions := logsapi.NewLoggingConfiguration() testcases := []struct { - name string - args []string - want *logsapi.LoggingConfiguration - errs field.ErrorList + name string + args []string + featureGate featuregate.FeatureGate + want *logsapi.LoggingConfiguration + errs field.ErrorList }{ + { + name: "JSON log format, default gates", + args: []string{"--logging-format=json"}, + want: func() *logsapi.LoggingConfiguration { + c := newOptions.DeepCopy() + c.Format = logsapi.JSONLogFormat + return c + }(), + }, + { + name: "JSON log format, disabled gates", + args: []string{"--logging-format=json"}, + featureGate: allDisabled, + want: func() *logsapi.LoggingConfiguration { + c := newOptions.DeepCopy() + c.Format = logsapi.JSONLogFormat + return c + }(), + errs: field.ErrorList{&field.Error{ + Type: "FieldValueForbidden", + Field: "format", + BadValue: "", + Detail: "Log format json is BETA and disabled, see LoggingBetaOptions feature", + }}, + }, + { + name: "JSON log format, enabled gates", + args: []string{"--logging-format=json"}, + featureGate: allEnabled, + want: func() *logsapi.LoggingConfiguration { + c := newOptions.DeepCopy() + c.Format = logsapi.JSONLogFormat + return c + }(), + }, { name: "JSON log format", args: []string{"--logging-format=json"}, @@ -83,7 +134,11 @@ func TestJSONFormatRegister(t *testing.T) { if !assert.Equal(t, tc.want, c) { t.Errorf("Wrong Validate() result for %q. expect %v, got %v", tc.name, tc.want, c) } - errs := c.ValidateAsField(nil) + featureGate := tc.featureGate + if featureGate == nil { + featureGate = defaultGates + } + errs := c.ValidateAsField(featureGate, nil) if !assert.ElementsMatch(t, tc.errs, errs) { t.Errorf("Wrong Validate() result for %q.\n expect:\t%+v\n got:\t%+v", tc.name, tc.errs, errs)