Merge pull request #119333 from liggitt/flushfrequencystring

Conditionally serialize flushFrequency as int

cmd/kube-proxy/app/server_test.go

@@ -222,7 +222,7 @@ nodePortAddresses:
 			},
 			Logging: logsapi.LoggingConfiguration{
 				Format:         "text",
-				FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+				FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 			},
 		}
 

pkg/kubelet/apis/config/v1beta1/defaults_test.go

@@ -21,6 +21,7 @@ import (
 	"time"
 
 	"github.com/google/go-cmp/cmp"
+
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -115,7 +116,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				VolumePluginDir:                           DefaultVolumePluginDir,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "text",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:        utilpointer.Bool(true),
 				EnableProfilingHandler:        utilpointer.Bool(true),
@@ -239,7 +240,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				KernelMemcgNotification:     false,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:          utilpointer.Bool(false),
 				ShutdownGracePeriod:             zeroDuration,
@@ -339,7 +340,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				VolumePluginDir:        DefaultVolumePluginDir,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "text",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:        utilpointer.Bool(false),
 				ReservedMemory:                []v1beta1.MemoryReservation{},
@@ -481,7 +482,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				KernelMemcgNotification:     true,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "json",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:          utilpointer.Bool(true),
 				ShutdownGracePeriod:             metav1.Duration{Duration: 60 * time.Second},
@@ -627,7 +628,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				KernelMemcgNotification:     true,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "json",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:          utilpointer.Bool(true),
 				ShutdownGracePeriod:             metav1.Duration{Duration: 60 * time.Second},
@@ -724,7 +725,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				VolumePluginDir:                           DefaultVolumePluginDir,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "text",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:        utilpointer.Bool(true),
 				EnableProfilingHandler:        utilpointer.Bool(true),
@@ -813,7 +814,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				VolumePluginDir:                           DefaultVolumePluginDir,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "text",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:        utilpointer.Bool(true),
 				EnableProfilingHandler:        utilpointer.Bool(true),
@@ -902,7 +903,7 @@ func TestSetDefaultsKubeletConfiguration(t *testing.T) {
 				VolumePluginDir:                           DefaultVolumePluginDir,
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "text",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 				EnableSystemLogHandler:        utilpointer.Bool(true),
 				EnableProfilingHandler:        utilpointer.Bool(true),

pkg/proxy/apis/config/v1alpha1/defaults_test.go

@@ -17,11 +17,13 @@ limitations under the License.
 package v1alpha1
 
 import (
-	"k8s.io/utils/pointer"
 	"testing"
 	"time"
 
+	"k8s.io/utils/pointer"
+
 	"github.com/google/go-cmp/cmp"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	componentbaseconfig "k8s.io/component-base/config/v1alpha1"
 	logsapi "k8s.io/component-base/logs/api/v1"
@@ -70,7 +72,7 @@ func TestDefaultsKubeProxyConfiguration(t *testing.T) {
 				ConfigSyncPeriod: metav1.Duration{Duration: 15 * time.Minute},
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "text",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 			},
 		},
@@ -110,7 +112,7 @@ func TestDefaultsKubeProxyConfiguration(t *testing.T) {
 				ConfigSyncPeriod: metav1.Duration{Duration: 15 * time.Minute},
 				Logging: logsapi.LoggingConfiguration{
 					Format:         "text",
-					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: 5 * time.Second},
+					FlushFrequency: logsapi.TimeOrMetaDuration{Duration: metav1.Duration{Duration: 5 * time.Second}, SerializeAsString: true},
 				},
 			},
 		},

staging/src/k8s.io/component-base/logs/api/v1/options.go

@@ -262,7 +262,7 @@ func apply(c *LoggingConfiguration, options *LoggingOptions, featureGate feature
 	if err := loggingFlags.Lookup("vmodule").Value.Set(VModuleConfigurationPflag(&c.VModule).String()); err != nil {
 		return fmt.Errorf("internal error while setting klog vmodule: %v", err)
 	}
-	klog.StartFlushDaemon(c.FlushFrequency.Duration)
+	klog.StartFlushDaemon(c.FlushFrequency.Duration.Duration)
 	klog.EnableContextualLogging(p.ContextualLoggingEnabled)
 	return nil
 }
@@ -342,7 +342,7 @@ func addFlags(c *LoggingConfiguration, fs flagSet) {
 	// No new log formats should be added after generation is of flag options
 	logRegistry.freeze()
 
-	fs.DurationVar(&c.FlushFrequency.Duration, LogFlushFreqFlagName, c.FlushFrequency.Duration, "Maximum number of seconds between log flushes")
+	fs.DurationVar(&c.FlushFrequency.Duration.Duration, LogFlushFreqFlagName, c.FlushFrequency.Duration.Duration, "Maximum number of seconds between log flushes")
 	fs.VarP(VerbosityLevelPflag(&c.Verbosity), "v", "v", "number for the log level verbosity")
 	fs.Var(VModuleConfigurationPflag(&c.VModule), "vmodule", "comma-separated list of pattern=N settings for file-filtered logging (only works for text log format)")
 
@@ -364,8 +364,9 @@ func SetRecommendedLoggingConfiguration(c *LoggingConfiguration) {
 	if c.Format == "" {
 		c.Format = "text"
 	}
-	if c.FlushFrequency.Duration == 0 {
+	if c.FlushFrequency.Duration.Duration == 0 {
-		c.FlushFrequency.Duration = LogFlushFreqDefault
+		c.FlushFrequency.Duration.Duration = LogFlushFreqDefault
+		c.FlushFrequency.SerializeAsString = true
 	}
 	var empty resource.QuantityValue
 	if c.Options.JSON.InfoBufferSize == empty {

staging/src/k8s.io/component-base/logs/api/v1/types.go

@@ -63,18 +63,30 @@ type LoggingConfiguration struct {
 
 // TimeOrMetaDuration is present only for backwards compatibility for the
 // flushFrequency field, and new fields should use metav1.Duration.
-type TimeOrMetaDuration metav1.Duration
+type TimeOrMetaDuration struct {
+	// Duration holds the duration
+	Duration metav1.Duration
+	// SerializeAsString controls whether the value is serialized as a string or an integer
+	SerializeAsString bool `json:"-"`
+}
 
 func (t TimeOrMetaDuration) MarshalJSON() ([]byte, error) {
-	return (metav1.Duration(t)).MarshalJSON()
+	if t.SerializeAsString {
+		return t.Duration.MarshalJSON()
+	} else {
+		// Marshal as integer for backwards compatibility
+		return json.Marshal(t.Duration.Duration)
+	}
 }
 
 func (t *TimeOrMetaDuration) UnmarshalJSON(b []byte) error {
 	if len(b) > 0 && b[0] == '"' {
 		// string values unmarshal as metav1.Duration
-		return json.Unmarshal(b, (*metav1.Duration)(t))
+		t.SerializeAsString = true
+		return json.Unmarshal(b, &t.Duration)
 	}
-	if err := json.Unmarshal(b, &t.Duration); err != nil {
+	t.SerializeAsString = false
+	if err := json.Unmarshal(b, &t.Duration.Duration); err != nil {
 		return fmt.Errorf("invalid duration %q: %w", string(b), err)
 	}
 	return nil

staging/src/k8s.io/component-base/logs/api/v1/types_test.go

@@ -25,8 +25,10 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/assert"
-	"k8s.io/apimachinery/pkg/api/resource"
 	"sigs.k8s.io/json"
+
+	"k8s.io/apimachinery/pkg/api/resource"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func TestVModule(t *testing.T) {
@@ -169,7 +171,7 @@ func TestCompatibility(t *testing.T) {
 			expectAllFields: true,
 			expectConfig: LoggingConfiguration{
 				Format:         JSONLogFormat,
-				FlushFrequency: TimeOrMetaDuration{Duration: time.Nanosecond},
+				FlushFrequency: TimeOrMetaDuration{Duration: metav1.Duration{Duration: time.Nanosecond}},
 				Verbosity:      VerbosityLevel(5),
 				VModule: VModuleConfiguration{
 					{
@@ -239,6 +241,10 @@ func notZeroRecursive(t *testing.T, i interface{}, path string) bool {
 				// Cannot access value.
 				continue
 			}
+			if typeOfI.Field(i).Tag.Get("json") == "-" {
+				// unserialized field
+				continue
+			}
 			if !notZeroRecursive(t, value.Field(i).Interface(), path+"."+typeOfI.Field(i).Name) {
 				valid = false
 			}
@@ -277,12 +283,12 @@ func TestTimeOrMetaDuration_UnmarshalJSON(t *testing.T) {
 			name:   "string values unmarshal as metav1.Duration",
 			tomd:   &TimeOrMetaDuration{},
 			arg:    "1s",
-			wanted: "1s",
+			wanted: `"1s"`,
 		}, {
 			name:   "int values unmarshal as metav1.Duration",
 			tomd:   &TimeOrMetaDuration{},
 			arg:    1000000000,
-			wanted: "1s",
+			wanted: `1000000000`,
 		}, {
 			name:   "invalid value return error",
 			tomd:   &TimeOrMetaDuration{},
@@ -298,8 +304,12 @@ func TestTimeOrMetaDuration_UnmarshalJSON(t *testing.T) {
 			}
 
 			if err := tt.tomd.UnmarshalJSON(b); err == nil {
-				if tt.wanted != tt.tomd.String() {
+				data, err := tt.tomd.MarshalJSON()
-					t.Errorf("unexpected wanted for %s, wanted: %v, got: %v", tt.name, tt.wanted, tt.tomd.String())
+				if err != nil {
+					t.Fatal(err)
+				}
+				if tt.wanted != string(data) {
+					t.Errorf("unexpected wanted for %s, wanted: %v, got: %v", tt.name, tt.wanted, string(data))
 				}
 			} else {
 				if err.Error() != tt.wanted {

staging/src/k8s.io/component-base/logs/api/v1/zz_generated.deepcopy.go

@@ -81,6 +81,7 @@ func (in *LoggingConfiguration) DeepCopy() *LoggingConfiguration {
 // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
 func (in *TimeOrMetaDuration) DeepCopyInto(out *TimeOrMetaDuration) {
 	*out = *in
+	out.Duration = in.Duration
 	return
 }