Merge pull request kubernetes-sigs#181 from jingyuanliang/dur

Switch away from home-made Duration type
jingyuanliang · Feb 14, 2025 · b7bbaea · b7bbaea
2 parents aff3cfe + 74b999c
commit b7bbaea
Show file tree

Hide file tree

Showing 2 changed files with 18 additions and 32 deletions.
diff --git a/cmd/ip-masq-agent/ip-masq-agent.go b/cmd/ip-masq-agent/ip-masq-agent.go
@@ -27,6 +27,7 @@ import (
 	"strings"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	utilyaml "k8s.io/apimachinery/pkg/util/yaml"
 	"k8s.io/component-base/logs"
 	"k8s.io/component-base/logs/logreduction"
@@ -66,29 +67,11 @@ var (
 
 // MasqConfig object
 type MasqConfig struct {
-	NonMasqueradeCIDRs []string `json:"nonMasqueradeCIDRs"`
-	CidrLimit          int      `json:"cidrLimit"`
-	MasqLinkLocal      bool     `json:"masqLinkLocal"`
-	MasqLinkLocalIPv6  bool     `json:"masqLinkLocalIPv6"`
-	ResyncInterval     Duration `json:"resyncInterval"`
-}
-
-// Duration - Go's JSON unmarshaler can't handle time.ParseDuration syntax when unmarshaling into time.Duration, so we do it here
-type Duration time.Duration
-
-// UnmarshalJSON ...
-func (d *Duration) UnmarshalJSON(json []byte) error {
-	if json[0] == '"' {
-		s := string(json[1 : len(json)-1])
-		t, err := time.ParseDuration(s)
-		if err != nil {
-			return err
-		}
-		*d = Duration(t)
-		return nil
-	}
-	s := string(json)
-	return fmt.Errorf("expected string value for unmarshal to field of type Duration, got %q", s)
+	NonMasqueradeCIDRs []string        `json:"nonMasqueradeCIDRs"`
+	CidrLimit          int             `json:"cidrLimit"`
+	MasqLinkLocal      bool            `json:"masqLinkLocal"`
+	MasqLinkLocalIPv6  bool            `json:"masqLinkLocalIPv6"`
+	ResyncInterval     metav1.Duration `json:"resyncInterval"`
 }
 
 // NewMasqConfig returns a MasqConfig with default values
@@ -113,7 +96,7 @@ func NewMasqConfig(masqAllReservedRanges bool) *MasqConfig {
 		CidrLimit:          64,
 		MasqLinkLocal:      false,
 		MasqLinkLocalIPv6:  false,
-		ResyncInterval:     Duration(60 * time.Second),
+		ResyncInterval:     metav1.Duration{60 * time.Second},
 	}
 }
 
@@ -176,7 +159,9 @@ func (m *MasqDaemon) Run() {
 	// Periodically resync to reconfigure or heal from any rule decay
 	for {
 		func() {
-			defer time.Sleep(time.Duration(m.config.ResyncInterval))
+			defer func() {
+				time.Sleep(m.config.ResyncInterval.Duration)
+			}()
 			// resync config
 			if err := m.osSyncConfig(); err != nil {
 				klog.Errorf("Error syncing configuration: %v", err)

diff --git a/cmd/ip-masq-agent/ip-masq-agent_test.go b/cmd/ip-masq-agent/ip-masq-agent_test.go
@@ -26,6 +26,7 @@ import (
 	"testing"
 	"time"
 
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/component-base/logs/logreduction"
 	"k8s.io/ip-masq-agent/cmd/ip-masq-agent/testing/fakefs"
 	"k8s.io/ip-masq-agent/pkg/interval"
@@ -123,7 +124,7 @@ func NewMasqConfigNoReservedRanges() *MasqConfig {
 		NonMasqueradeCIDRs: []string{"10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16"},
 		CidrLimit:          64,
 		MasqLinkLocal:      false,
-		ResyncInterval:     Duration(60 * time.Second),
+		ResyncInterval:     metav1.Duration{60 * time.Second},
 	}
 }
 
@@ -145,7 +146,7 @@ func NewMasqConfigWithReservedRanges() *MasqConfig {
 			"240.0.0.0/4"},
 		MasqLinkLocal:  false,
 		CidrLimit:      64,
-		ResyncInterval: Duration(60 * time.Second),
+		ResyncInterval: metav1.Duration{60 * time.Second},
 	}
 }
 
@@ -197,7 +198,7 @@ resyncInterval: 5s
 		NonMasqueradeCIDRs: []string{"172.16.0.0/12", "10.0.0.0/8"},
 		MasqLinkLocal:      true,
 		CidrLimit:          64,
-		ResyncInterval:     Duration(5 * time.Second)}},
+		ResyncInterval:     metav1.Duration{5 * time.Second}}},
 
 	{"valid yaml file, just nonMasqueradeCIDRs", fakefs.StringFS{File: `
 nonMasqueradeCIDRs:
@@ -222,7 +223,7 @@ resyncInterval: 5m
 		CidrLimit:          64,
 		NonMasqueradeCIDRs: NewMasqConfigNoReservedRanges().NonMasqueradeCIDRs,
 		MasqLinkLocal:      NewMasqConfigNoReservedRanges().MasqLinkLocal,
-		ResyncInterval:     Duration(5 * time.Minute)}},
+		ResyncInterval:     metav1.Duration{5 * time.Minute}}},
 
 	// invalid yaml
 	{"invalid yaml file", fakefs.StringFS{File: `*`}, fmt.Errorf("yaml: did not find expected alphabetic or numeric character"), NewMasqConfigNoReservedRanges()},
@@ -239,7 +240,7 @@ resyncInterval: 5m
 			CidrLimit:          64,
 			NonMasqueradeCIDRs: []string{"172.16.0.0/12", "10.0.0.0/8"},
 			MasqLinkLocal:      true,
-			ResyncInterval:     Duration(5 * time.Second)}},
+			ResyncInterval:     metav1.Duration{5 * time.Second}}},
 
 	{"valid json file, just nonMasqueradeCIDRs", fakefs.StringFS{File: `
 {
@@ -272,7 +273,7 @@ resyncInterval: 5m
 			CidrLimit:          64,
 			NonMasqueradeCIDRs: NewMasqConfigNoReservedRanges().NonMasqueradeCIDRs,
 			MasqLinkLocal:      NewMasqConfigNoReservedRanges().MasqLinkLocal,
-			ResyncInterval:     Duration(5 * time.Minute)}},
+			ResyncInterval:     metav1.Duration{5 * time.Minute}}},
 
 	// invalid json
 	{"invalid json file", fakefs.StringFS{File: `{*`}, fmt.Errorf("invalid character '*' looking for beginning of object key string"), NewMasqConfigNoReservedRanges()},
@@ -292,7 +293,7 @@ resyncInterval: 5m
 			CidrLimit:          64,
 			NonMasqueradeCIDRs: []string{"172.16.0.0/12", "10.0.0.0/8", "fc00::/7"},
 			MasqLinkLocal:      true,
-			ResyncInterval:     Duration(5 * time.Second)}},
+			ResyncInterval:     metav1.Duration{5 * time.Second}}},
 }
 
 // tests MasqDaemon.syncConfig